Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAX32665 and MAX32666 SDK TPU/MAA Bug #1089

Closed
night1rider opened this issue Jul 23, 2024 · 5 comments · Fixed by #1104
Closed

MAX32665 and MAX32666 SDK TPU/MAA Bug #1089

night1rider opened this issue Jul 23, 2024 · 5 comments · Fixed by #1104

Comments

@night1rider
Copy link

night1rider commented Jul 23, 2024

Hello,

I am working on using the TPU on the MAX32665 and MAX32666 to do hardware acceleration in wolfSSL. I found an issue potentially with the process of setting different math modes for the MAA.

I think it has to do with this line:
#define MXC_SETFIELD(reg, mask, setting) (reg = ((reg) & ~(mask)) | ((setting) & (mask)))

I suspect it is not preforming the bit masking correct for the values give for the MAA controls. You can test this by trying your MAA example that is in the master branch and switching it to do a multiple operation. You will find it actually does a square operation.

With the current bit masking it will not use values correctly and cut off the last bit of the math control values do not get shifted left by 1 bit so this values will not work as the math operation control register in in bit position [3:1]:

        MXC_TPU_MAA_EXP      0b000
        MXC_TPU_MAA_SQ       0b001
        MXC_TPU_MAA_MUL      0b010
        MXC_TPU_MAA_SQMUL    0b011
        MXC_TPU_MAA_ADD      0b100
        MXC_TPU_MAA_SUB      0b101

To currently use the MAA I have to define control values as such:

        #define WC_MXC_TPU_MAA_EXP      0b0000
        #define WC_MXC_TPU_MAA_SQ       0b0010
        #define WC_MXC_TPU_MAA_MUL      0b0100
        #define WC_MXC_TPU_MAA_SQMUL    0b0110
        #define WC_MXC_TPU_MAA_ADD      0b1000
        #define WC_MXC_TPU_MAA_SUB      0b1010

and edit this line in the SDK:
if (clc >= 0x6) {
to:
if (clc >= 0b1111) {

To properly use the MAA hardware. This will end up setting the correct bits to use the current bit masking controls.

@chazste
Copy link

chazste commented Aug 2, 2024

I do not believe that the issue is the macro, rather the incorrect use of the macro

MXC_SETFIELD(tpu->maa_ctrl, MXC_F_TPU_REVA_MAA_CTRL_CLC, clc);

Should instead be:

MXC_SETFIELD(tpu->maa_ctrl, MXC_F_TPU_REVA_MAA_CTRL_CLC, (clc << MXC_F_TPU_REVA_MAA_CTRL_CLC_POS));

@Jake-Carter
Copy link
Contributor

Thank you @night1rider, I appreciate your efforts adding part support to WolfSSL.

@chazste I agree - looks like there was also the same issue for MXC_TPU_RevA_Cipher_KeySelect.

Just opened a PR with the fix.

@night1rider
Copy link
Author

night1rider commented Aug 5, 2024

@chazste Hello, I tried this suggestion and made sure I use the appropriate macros/enums and values but it seems this does not fix this issue. When running wolfssl tests with my temp fix the calculations will pass, however with this fix it will preform calculations of some kind, but the expected calculations are not correct and the wolfssl tests will fail now.

@night1rider
Copy link
Author

@chazste Your solution actually works, I had tested with:
MXC_SETFIELD(tpu->maa_ctrl, MXC_F_TPU_REVA_MAA_CTRL_CLC, clc << MXC_F_TPU_REVA_MAA_CTRL_CLC);
instead of
MXC_SETFIELD(tpu->maa_ctrl, MXC_F_TPU_REVA_MAA_CTRL_CLC, clc << MXC_F_TPU_REVA_MAA_CTRL_CLC_POS);

Due to a typo in the initial pr fix.

Thanks!

@chazste
Copy link

chazste commented Aug 6, 2024

@night1rider thank you for checking again. I'm glad that worked.

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 a pull request may close this issue.

3 participants