Refactor/Fix: Restoring snp_vlek_load api to prior release and fixing functionality#316
Conversation
DGonzalezVillal
requested review from
larrydewey and
tylerfanelli
as code owners
tylerfanelli
left a comment
tylerfanelli
left a comment
There was a problem hiding this comment.
In the commit message, s/strucutre/structure.
Restoring snp_vlek_load function to avoid breaking api. It takes the usual slice with the hashstick instead of the new structure. The new structure remains, since it can be used to validate the hashstick bytes. Signed-off-by: DGonzalezVillal <Diego.GonzalezVillalobos@amd.com>
DGonzalezVillal
force-pushed
the
vlek-load
branch
from
5cb1b38 to
d55ff35
Compare
WrappedVlekHashstick now owns the VLEK byte data ([u8; 432]) instead of borrowing it via a reference. This change ensures that the structure is self-contained and safely passed to the kernel, which expects a pointer to a buffer that includes the full hashstick data. Referencing the buffer (even with careful lifetime management) was not working reliably and added complexity. Owning the data is simpler, safer, and easier to maintain. SnpVlekLoad.len is now correctly set to the size of the SnpVlekLoad struct, not the size of the hashstick. According to the SEV-SNP spec, this field represents the size of the command buffer, not the size of the payload. Signed-off-by: DGonzalezVillal <Diego.GonzalezVillalobos@amd.com>
DGonzalezVillal
force-pushed
the
vlek-load
branch
from
d55ff35 to
21e8dd1
Compare
DGonzalezVillal
changed the title
|
I was finally able to get my hands on a VLEK and a VLEK hashstick, so I started testing VLEK loading and realized that our current implementation was broken. I’ve updated this PR with changes that reflect what I found to work. First, regarding the FFI WrappedVlekStructure: we were passing a reference to this struct in the command buffer, but the kernel expects the struct to own the data. I tried to adjust the existing implementation and play around with lifetimes, but ultimately found that making the struct own the data—rather than referencing it—was the simplest and most reliable solution. Next, in the SnpVlekLoad structure, the length parameter was incorrectly set to the length of the hashstick. In reality, this value should be set to the length of the structure itself (16 bytes). With these changes, I was able to successfully load a hashstick on a host. The original API can stay the same, so it's not changed for users. Also worth noting: the upstream kernel currently has a bug I identified, which prevents the feature from working correctly even though it is technically enabled. It will require a patch for proper functionality. Let me know your comments or suggestions. |
|
LGTM. We're technically breaking API, but I feel that the VLEK hashstick feature is so new that some instability is expected, correct? |
larrydewey
left a comment
larrydewey
left a comment
There was a problem hiding this comment.
This looks good, but I do think we should adhere to semver and change the version before releasing.
3 participants
Restoring snp_vlek_load function to avoid breaking api. It takes the usual slice with the hashstick instead of the new strucutre. The new structure remains, since it can be used to validate the hashstick bytes.