Implement vmsa build subcommand

[tylerfanelli]

3 subcommands to be added based off of danpb's virt-dom-sev-vmsa-tool.py

build: Build a VMSA buffer based off of given inputs
update: Update an already-existing VMSA buffer.
show: Pretty-print a VMSA buffer's contents for human-readable viewing.

Build subcommand still a WIP at the moment.

[tylerfanelli]

@crobinso While testing build and update, I'll rebase your work on the show subcommand on my local vmsa branch.

[crobinso]

Thanks! But I will submit the vmsa show command separately, once the infrastructure pieces land.

IMO we should just concentrate on infrastructure and vmsa build for first PR. We get that in, then update and show can be submitted separately on top of that.

What's on your todo list before this is out of draft state? I'd like to see tests FWIW. test cases will help make sure we don't regress when we move the Vmsa definitions to sev crate

[tylerfanelli]

Thanks! But I will submit the vmsa show command separately, once the infrastructure pieces land.

Sure, that works too.

IMO we should just concentrate on infrastructure and vmsa build for first PR. We get that in, then update and show can be submitted separately on top of that.

What's on your todo list before this is out of draft state? I'd like to see tests FWIW. test cases will help make sure we don't regress when we move the Vmsa definitions to sev crate

Ensuring that the VMSA buffer is packed correctly when writing to the file is top priority right now (currently, the buffer isn't correctly packed to 4KB when writing). I'm using the sevctl-test package to ensure the behavior is expected.

I'll push my changes to sevctl-test with tests for the VMSA commands when I have everything correct.

[berrange]

I think this build command needs to come with test coverage, since this is complex and subtle which si very easy to get wrong. Essentially we want to run the code and compare its output to known good output, for a variety of scenarios. I don't think we want to checking in a bunch of binary VMSA blobs though. Rather we should provide the 'show' command in this PR too, and store the output of 'show' for various known good VMSA blobs, and test against that.

[tylerfanelli]

I think this build command needs to come with test coverage, since this is complex and subtle which si very easy to get wrong. Essentially we want to run the code and compare its output to known good output, for a variety of scenarios. I don't think we want to checking in a bunch of binary VMSA blobs though. Rather we should provide the 'show' command in this PR too, and store the output of 'show' for various known good VMSA blobs, and test against that.

Yea, I'm testing with a fork of sevctl-test, and when everything is ready I will also push my changes to that repo as well.

[berrange]

Testing with the 'build' command the output doesn't match the checksum of the VMSA generated by my original python script.

The actual VMSA register values are all the same, but the padding to round it up to 4096 bytes contains random data. The padding needs to be all-zeroes.

The 'show' command, or rather the code for loading VMSA blobs should validate that the padding is all zeros too, to prevent silent mistakes slipping past.

[tylerfanelli]

Testing with the 'build' command the output doesn't match the checksum of the VMSA generated by my original python script.

The actual VMSA register values are all the same, but the padding to round it up to 4096 bytes contains random data. The padding needs to be all-zeroes.

@berrange: I wasn't aware of that. Will fix now.
Just for my own curiosity, is there a specific reason for this?

The 'show' command, or rather the code for loading VMSA blobs should validate that the padding is all zeros too, to prevent silent mistakes slipping past.

Sure. Will add a check to make sure all padded bytes are zero.

[berrange]

The 'vmsa build' command is segfaulting for me now i'm afraid.

[berrange]

@berrange: I wasn't aware of that. Will fix now. Just for my own curiosity, is there a specific reason for this?

IIUC, the VM launch measurement will contain a cryptographic hash over the entire 4k page of data, even though the actual register content only spans a subset of that range. So that padding upto 4k has to contain predictable data, so you can reproduce the measurement later.

[tylerfanelli]

@berrange

Files are equal now:

[tfanelli@virtlab1011 sevctl]$ cmp --silent sevctl_vmsa0.bin virt_vmsa_tool_vmsa0.bin && echo 'identical' || echo 'not identical'
identical

[tylerfanelli]

@berrange @crobinso

I'll be adding the update command now. When you have the time, can you review build and show? I can also add some tests that I used if that's helpful.

[crobinso]

TLDR: drop patch 3, squash build bits from patch 4 into patch 2, resubmit, and let's merge it

This successfully recreates the two working vmsa0.bin and vmsa1.bin binaries I created with danpb's script. I think that's good enough to commit the build command.

Please drop the 'show' changes though, I will submit those separately. It was basically a hack to prove that it's working, with format similar to danpb's script so I could manually verify the results are correct. I don't think that list of println statements is the way to go and maybe we should just use {:#?} formatting which is future proof with any VMSA structure changes. But that can be discussed in the future PR.

You've mentioned sevctl-test but IMO this should (also?) be covered with unittests. It will be easier to verify all the combinations with some unittest infrastructure. I will add that with my vmsa show submission.