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

Is element order reversed for vaes* instructions in rovisinc Spike branch? #268

Closed
JamesKenneyImperas opened this issue Jan 8, 2023 · 3 comments

Comments

@JamesKenneyImperas
Copy link

JamesKenneyImperas commented Jan 8, 2023

Following on from issue #266, I'm wondering whether the element order is reversed for vaes* instructions in the rivosinc Spike reference implementation. Can you please check whether the specification and model are correct? Thanks.

Apart from the trivial vaesz.vs, I'm not able to get any vaes* instruction to match what I think the specification is saying. I looked at one of the easier instructions, vaeskf2.vi. For this the specification says:

      let w[3] : bits(32) = if (rnd[0]==1) then
        RoundKeyB[3] XOR aes_subword_fwd(CurrentRoundKey[0])
      else
        RoundKeyB[3] XOR aes_subword_fwd(aes_rotword(CurrentRoundKey[0])) XOR aes_decode_rcon(rnd>>1);
      w[2] : bits(32) = w[3] XOR RoundKeyB[2]
      w[1] : bits(32) = w[2] XOR RoundKeyB[1]
      w[0] : bits(32) = w[1] XOR RoundKeyB[0]
      set_velem(vd, EGW=128, i, w[3:0]);

But the implementation in Spike seems to be:

      let w[0] : bits(32) = if (rnd[0]!=1) then       // sense reversed (issue #267) 
        RoundKeyB[0] XOR aes_subword_fwd(CurrentRoundKey[3])
      else
        RoundKeyB[0] XOR aes_subword_fwd(aes_rotword(CurrentRoundKey[3])) XOR aes_decode_rcon(rnd>>1);
      w[1] : bits(32) = w[0] XOR RoundKeyB[1]
      w[2] : bits(32) = w[1] XOR RoundKeyB[2]
      w[3] : bits(32) = w[2] XOR RoundKeyB[3]
      set_velem(vd, EGW=128, i, w[3:0]);

Is it possible the element order is reversed for all vaes* instructions?

Thanks.

@JamesKenneyImperas
Copy link
Author

Just to confirm: to match Spike behaviour, definitions of vaeskf1.vi, vaeskf2.vi, aes_shift_rows_fwd and aes_shift_rows_inv in the specification need to be modified.

@kdockser
Copy link
Collaborator

Thanks for letting us know about this James.
The Spike code is correct and I am in the process of fixing the pseudo-code in the specification. As you said, the word order was reversed in the key-round generation pseudo code. The shift_rows function pseudo-code has been replaced.

@kdockser
Copy link
Collaborator

Updated in spec.

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

No branches or pull requests

2 participants