| Message ID | 20240801235333.357075-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SEV: allow KVM_SEV_GET_ATTESTATION_REPORT for SNP guests | expand |
On Fri, Aug 02, 2024 at 01:53:33AM +0200, Paolo Bonzini wrote: > Even though KVM_SEV_GET_ATTESTATION_REPORT is not one of the commands > that were added for SEV-SNP guests, it can be applied to them. Filtering Is the command actually succeeding for an SNP-enabled guest? When I test this, I get a fw_err code of 1 (INVALID_PLATFORM_STATE), and after speaking with some firmware folks that seems to be the expected behavior. There's also some other things that aren't going to work as expected, e.g. KVM uses sev->handle as the handle for the guest it wants to fetch the attestation report for, but in the case of SNP, sev->handle will be uninitialized since that only happens via KVM_SEV_LAUNCH_UPDATE_DATA, which isn't usable for SNP guests. As I understand it, the only firmware commands allowed for SNP guests are those listed in the SNP firmware ABI, section "Command Reference", and in any instance where a legacy command from the legacy SEV/SEV-ES firmware ABI is also applicable for SNP, the legacy command will be defined again in the "Command Reference" section of the SNP spec. E.g., GET_ID is specifically documented in both the SEV/SEV-ES firmware ABI, as well as the SNP firmware ABI spec. But ATTESTATION (and the similar LAUNCH_MEASURE) are only mentioned in the SEV/SEV-ES Firmware ABI, so I think it makes sense that KVM also only allows them for SEV/SEV-ES. -Mike > it out, for example, makes the QEMU command query-sev-attestation-report > fail. > > Cc: Michael Roth <michael.roth@amd.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/kvm/svm/sev.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 5c125e4c1096..17307257d632 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -2587,7 +2587,9 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) > * Once KVM_SEV_INIT2 initializes a KVM instance as an SNP guest, only > * allow the use of SNP-specific commands. > */ > - if (sev_snp_guest(kvm) && sev_cmd.id < KVM_SEV_SNP_LAUNCH_START) { > + if (sev_snp_guest(kvm) && > + sev_cmd.id < KVM_SEV_SNP_LAUNCH_START && > + sev_cmd.id != KVM_SEV_GET_ATTESTATION_REPORT) { > r = -EPERM; > goto out; > } > -- > 2.45.2 >
On Fri, Aug 2, 2024 at 10:41 PM Michael Roth <michael.roth@amd.com> wrote: > On Fri, Aug 02, 2024 at 01:53:33AM +0200, Paolo Bonzini wrote: > > Even though KVM_SEV_GET_ATTESTATION_REPORT is not one of the commands > > that were added for SEV-SNP guests, it can be applied to them. Filtering > > Is the command actually succeeding for an SNP-enabled guest? When I > test this, I get a fw_err code of 1 (INVALID_PLATFORM_STATE), and > after speaking with some firmware folks that seems to be the expected > behavior. So is there no equivalent of QEMU's query-sev-attestation-report for SEV-SNP? (And is there any user of query-sev-attestation-report for non-SNP?) Paolo > There's also some other things that aren't going to work as expected, > e.g. KVM uses sev->handle as the handle for the guest it wants to fetch > the attestation report for, but in the case of SNP, sev->handle will be > uninitialized since that only happens via KVM_SEV_LAUNCH_UPDATE_DATA, > which isn't usable for SNP guests. > > As I understand it, the only firmware commands allowed for SNP guests are > those listed in the SNP firmware ABI, section "Command Reference", and > in any instance where a legacy command from the legacy SEV/SEV-ES firmware > ABI is also applicable for SNP, the legacy command will be defined again > in the "Command Reference" section of the SNP spec. E.g., GET_ID is > specifically documented in both the SEV/SEV-ES firmware ABI, as well as > the SNP firmware ABI spec. But ATTESTATION (and the similar LAUNCH_MEASURE) > are only mentioned in the SEV/SEV-ES Firmware ABI, so I think it makes > sense that KVM also only allows them for SEV/SEV-ES.
On Mon, Aug 05, 2024 at 04:32:16PM +0200, Paolo Bonzini wrote: > On Fri, Aug 2, 2024 at 10:41 PM Michael Roth <michael.roth@amd.com> wrote: > > On Fri, Aug 02, 2024 at 01:53:33AM +0200, Paolo Bonzini wrote: > > > Even though KVM_SEV_GET_ATTESTATION_REPORT is not one of the commands > > > that were added for SEV-SNP guests, it can be applied to them. Filtering > > > > Is the command actually succeeding for an SNP-enabled guest? When I > > test this, I get a fw_err code of 1 (INVALID_PLATFORM_STATE), and > > after speaking with some firmware folks that seems to be the expected > > behavior. > > So is there no equivalent of QEMU's query-sev-attestation-report for > SEV-SNP? No, but all the attestation support is via the guest request interface. It would be possible for KVM to provide the measurement by logging the digest values > (And is there any user of query-sev-attestation-report for > non-SNP?) No, this would have always returned error, either via KVM, or via firmware failure. But maybe QEMU should do the error handling a bit more directly in this case. I can send a patch for QEMU 9.1 that results in an error when issued for an SNP guest. -Mike > > Paolo > > > There's also some other things that aren't going to work as expected, > > e.g. KVM uses sev->handle as the handle for the guest it wants to fetch > > the attestation report for, but in the case of SNP, sev->handle will be > > uninitialized since that only happens via KVM_SEV_LAUNCH_UPDATE_DATA, > > which isn't usable for SNP guests. > > > > As I understand it, the only firmware commands allowed for SNP guests are > > those listed in the SNP firmware ABI, section "Command Reference", and > > in any instance where a legacy command from the legacy SEV/SEV-ES firmware > > ABI is also applicable for SNP, the legacy command will be defined again > > in the "Command Reference" section of the SNP spec. E.g., GET_ID is > > specifically documented in both the SEV/SEV-ES firmware ABI, as well as > > the SNP firmware ABI spec. But ATTESTATION (and the similar LAUNCH_MEASURE) > > are only mentioned in the SEV/SEV-ES Firmware ABI, so I think it makes > > sense that KVM also only allows them for SEV/SEV-ES. >
On Mon, Aug 5, 2024 at 5:39 PM Michael Roth <michael.roth@amd.com> wrote: > > (And is there any user of query-sev-attestation-report for > > non-SNP?) > > No, this would have always returned error, either via KVM, or via > firmware failure. I mean for *non-SNP*. If no one ever used it, we can deprecate the command. Paolo
On Mon, Aug 05, 2024 at 06:15:51PM +0200, Paolo Bonzini wrote: > On Mon, Aug 5, 2024 at 5:39 PM Michael Roth <michael.roth@amd.com> wrote: > > > (And is there any user of query-sev-attestation-report for > > > non-SNP?) > > > > No, this would have always returned error, either via KVM, or via > > firmware failure. > > I mean for *non-SNP*. If no one ever used it, we can deprecate the command. This would have been the main architectured way to fetch an attestation report from firmware for non-SNP cases, so I think it's likely being used to some degree. Libvirt itself does not seem to use it directly, however (opting for the less verbose query-sev-launch-measure), but it seems likely there would be SEV deployments that do. libvirt might find other bits like policy/major/minor/etc. to be redundant since it also controls the guest config, but maybe 3rd party tools prefer to known those values as provided by firmware so they can ensure they match expected values. -Mike > > Paolo >
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 5c125e4c1096..17307257d632 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2587,7 +2587,9 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp) * Once KVM_SEV_INIT2 initializes a KVM instance as an SNP guest, only * allow the use of SNP-specific commands. */ - if (sev_snp_guest(kvm) && sev_cmd.id < KVM_SEV_SNP_LAUNCH_START) { + if (sev_snp_guest(kvm) && + sev_cmd.id < KVM_SEV_SNP_LAUNCH_START && + sev_cmd.id != KVM_SEV_GET_ATTESTATION_REPORT) { r = -EPERM; goto out; }
Even though KVM_SEV_GET_ATTESTATION_REPORT is not one of the commands that were added for SEV-SNP guests, it can be applied to them. Filtering it out, for example, makes the QEMU command query-sev-attestation-report fail. Cc: Michael Roth <michael.roth@amd.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/svm/sev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)