Skip to content

Conversation

@rizlik
Copy link
Contributor

@rizlik rizlik commented Aug 11, 2025

No description provided.

@rizlik rizlik requested review from bigbrett and billphipps August 11, 2025 18:15
@bigbrett bigbrett requested a review from Copilot August 12, 2025 15:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for custom DMA memory copy callbacks and introduces configuration guards to selectively disable CMAC DMA and key DMA functionality. This provides more flexibility in DMA operations while allowing systems to exclude specific DMA features when not needed.

  • Adds custom DMA memory copy callback infrastructure with new configuration option WOLFHSM_CFG_DMA_CUSTOM_CLIENT_COPY
  • Introduces guards WOLFHSM_CFG_NO_CMAC_DMA and WOLFHSM_CFG_NO_KEY_DMA to disable specific DMA operations
  • Updates DMA copy functions to use custom callbacks when available, falling back to memcpy when not configured

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
wolfhsm/wh_settings.h Adds documentation for the three new configuration options
wolfhsm/wh_server.h Defines new callback types, updates context structures, and adds registration function declaration
test/wh_test_crypto.c Adds guard to conditionally compile key DMA tests
src/wh_server_dma.c Implements custom callback registration and integrates callbacks into copy operations
src/wh_server_crypto.c Adds guard around CMAC DMA handling
src/wh_client_cryptocb.c Adds guard around CMAC DMA client-side handling

Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits and some edits for clarity. I also bring up a global architecture question for discussion.

@rizlik
Copy link
Contributor Author

rizlik commented Aug 13, 2025

A few nits and some edits for clarity. I also bring up a global architecture question for discussion.

Thanks for the review, fixed nits and reverted unneeded macros, let me know if you want me to do some git history cleaning before going forward.

billphipps
billphipps previously approved these changes Aug 18, 2025
Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a few naming nits that I'll bring in a future PR and I'll tag with Brett about making this the default and removing the CFG value. Looks good!

@billphipps billphipps dismissed bigbrett’s stale review August 18, 2025 13:43

Let's work a few of the other questions after the PIC32CZ changes are merged and tested as that exercises this interface.

@billphipps billphipps merged commit cff6c02 into wolfSSL:main Aug 18, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants