SVSM: require SNP restricted injection

[msft-jlange]

To maintain security of the SVSM, require that Restricted Injection is present in the SEV features in order to boot.

[roy-hopkins]

I tested this on the latest IGVM QEMU and it results in a panic on SVSM startup due to the SEV features not being propagated from the IGVM file via QEMU. I'll look at fixing this in the IGVM QEMU.

[Zildj1an]

Requiring restricted injection is a good idea and it doesn't affect the guest itself! 👍

[roy-hopkins]

By default, QEMU does not enable restricted injection. It can be enabled via a parameter on the command line. At the moment this is true for both the IGVM and non-IGVM versions of QEMU.

Maybe you could add a commit to this PR that updates the documentation:

diff --git a/Documentation/INSTALL.md b/Documentation/INSTALL.md
index 477b4d1..35931a0 100644
--- a/Documentation/INSTALL.md
+++ b/Documentation/INSTALL.md
@@ -225,7 +225,7 @@ guest:
   -cpu EPYC-v4 \
   -machine q35,confidential-guest-support=sev0,memory-backend=ram1,kvm-type=protected \
   -object memory-backend-memfd-private,id=ram1,size=8G,share=true \
-  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,svsm=on \
+  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,svsm=on,init-flags=1 \
 ```
 
 This selects the ```EPYC-v4``` CPU type which will pass the CPUID validation
@@ -233,7 +233,9 @@ done by the AMD security processor. It also allocates memory
 from the ```memory-backend-memfd-private``` backend, which is a requirement to
 run SEV-SNP guests. An ```sev-snp-guest``` object needs to be defined to
 enable SEV-SNP protection for the guest. The 'svsm=on' parameter makes
-QEMU reserve a small amount of guest memory for the SVSM.
+QEMU reserve a small amount of guest memory for the SVSM. The `init-flags=1`
+parameter enables the SEV-SNP restricted injection feature which is required
+by COCONUT-SVSM.
 
 Further, the OVMF binaries and the SVSM binary need to be passed to
 QEMU. This happens via pflash drives:
@@ -255,7 +257,7 @@ $ sudo $HOME/bin/qemu-svsm/bin/qemu-system-x86_64 \
   -cpu EPYC-v4 \
   -machine q35,confidential-guest-support=sev0,memory-backend=ram1,kvm-type=protected \
   -object memory-backend-memfd-private,id=ram1,size=8G,share=true \
-  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,svsm=on \
+  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,svsm=on,init-flags=1 \
   -smp 8 \
   -no-reboot \
   -drive if=pflash,format=raw,unit=0,file=/path/to/firmware/OVMF_CODE.fd,readonly=on \

[msft-jlange]

By default, QEMU does not enable restricted injection. It can be enabled via a parameter on the command line. At the moment this is true for both the IGVM and non-IGVM versions of QEMU.

Maybe you could add a commit to this PR that updates the documentation:

I think a better strategy would be to modify QEMU to honor the SEV features specified in the IGVM file embedded in the VMSA. I recognize that this may take some doing, but is really the only robust way to ensure that the IGVM file gets the behaviors it expects at launch. This will, of course, become non-negotiable once we reach a point that the IGVM file includes an ID BLOCK that specifies the expected measurement of the SVSM image, which will contain the VMSA and its expected SEV features.

If this is too much to bite off in the short term, then it will be better for this PR to wait until that work is done. If there is strong desire to merge this PR before that work can be accomplished, then it would be much better for somebody else to pick up this PR and complete it - somebody who is able to actually work with QEMU and validate the workflow, which is something I am not going to be in a position to do.

[roy-hopkins]

I think a better strategy would be to modify QEMU to honor the SEV features specified in the IGVM file embedded in the VMSA.

Yes, and I've been working this week on exactly this. It will be fairly easy to set restricted injection to match the IGVM file in QEMU but due to the way this works in QEMU/kvm other features will require further work.

However, the current versions of QEMU we host in coconut currently require the extra command-line option to be set to enable restricted injection. If this PR is merged then users will have no option but to add that extra flag otherwise the SVSM will panic on startup, hence the suggestion for the documentation change in this PR.

But this is a draft PR, and if it is not going to be merged until we've moved to a QEMU that supports setting this via IGVM then we can leave the documentation unchanged.

[msft-jlange]

I think a better strategy would be to modify QEMU to honor the SEV features specified in the IGVM file embedded in the VMSA.

Yes, and I've been working this week on exactly this. It will be fairly easy to set restricted injection to match the IGVM file in QEMU but due to the way this works in QEMU/kvm other features will require further work.

However, the current versions of QEMU we host in coconut currently require the extra command-line option to be set to enable restricted injection. If this PR is merged then users will have no option but to add that extra flag otherwise the SVSM will panic on startup, hence the suggestion for the documentation change in this PR.

But this is a draft PR, and if it is not going to be merged until we've moved to a QEMU that supports setting this via IGVM then we can leave the documentation unchanged.

I think we are in agreement. This PR will remain a draft PR until your work to support VMSA-in-IGVM is complete. If there is pressure to move forward with requiring restricted injection before that, then we can revisit and talk about how to change the workflow (and document the changes).

[msft-jlange]

Both restricted injection and boot from IGVM are part of the SVSM patch set for KVM so this should be safe to merge now.

[Freax13]

Both restricted injection and boot from IGVM are part of the SVSM patch set for KVM so this should be safe to merge now.

I'm assuming you're referring to coconut-svsm/linux#7, right? That PR hasn't been merged yet and IMO we should wait for that. Otherwise, if users follow the installation guide in this repo, they'll end up with a broken setup.

[msft-jlange]

I'm assuming you're referring to coconut-svsm/linux#7, right? That PR hasn't been merged yet and IMO we should wait for that. Otherwise, if users follow the installation guide in this repo, they'll end up a broken setup.

@roy-hopkins has assured me that the existing SVSM patch set is sufficient to make this work correctly. Roy, did I misunderstand?

[roy-hopkins]

I'm assuming you're referring to coconut-svsm/linux#7, right? That PR hasn't been merged yet and IMO we should wait for that. Otherwise, if users follow the installation guide in this repo, they'll end up a broken setup.

@roy-hopkins has assured me that the existing SVSM patch set is sufficient to make this work correctly. Roy, did I misunderstand?

My error - I thought the COCONUT QEMU/KVM had been updated since this PR was opened, but the previous update to 6.10 was never merged and used as the default kernel. The comment I wrote above about updating documentation still stands for the current situation, but it makes more sense to wait for coconut-svsm/linux#7 to be merged. I'm working on that now.

[msft-jlange]

My error - I thought the COCONUT QEMU/KVM had been updated since this PR was opened, but the previous update to 6.10 was never merged and used as the default kernel. The comment I wrote above about updating documentation still stands for the current situation, but it makes more sense to wait for coconut-svsm/linux#7 to be merged. I'm working on that now.

Thanks for clarifying. I'll move this PR back to draft state and await your report on when it's safe to merge.