[PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation

Pankaj Gupta posted 31 patches 5 months, 4 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Cornelia Huck <cohuck@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
[PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation
Posted by Pankaj Gupta 5 months, 4 weeks ago
From: Michael Roth <michael.roth@amd.com>

SEV-SNP firmware allows a special guest page to be populated with a
table of guest CPUID values so that they can be validated through
firmware before being loaded into encrypted guest memory where they can
be used in place of hypervisor-provided values[1].

As part of SEV-SNP guest initialization, use this interface to validate
the CPUID entries reported by KVM_GET_CPUID2 prior to initial guest
start and populate the CPUID page reserved by OVMF with the resulting
encrypted data.

[1] SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6

Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Pankaj Gupta <pankaj.gupta@amd.com>
---
 target/i386/sev.c | 164 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 162 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 504f641038..4388ffe867 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -214,6 +214,36 @@ static const char *const sev_fw_errlist[] = {
 
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
 
+/* <linux/kvm.h> doesn't expose this, so re-use the max from kvm.c */
+#define KVM_MAX_CPUID_ENTRIES 100
+
+typedef struct KvmCpuidInfo {
+    struct kvm_cpuid2 cpuid;
+    struct kvm_cpuid_entry2 entries[KVM_MAX_CPUID_ENTRIES];
+} KvmCpuidInfo;
+
+#define SNP_CPUID_FUNCTION_MAXCOUNT 64
+#define SNP_CPUID_FUNCTION_UNKNOWN 0xFFFFFFFF
+
+typedef struct {
+    uint32_t eax_in;
+    uint32_t ecx_in;
+    uint64_t xcr0_in;
+    uint64_t xss_in;
+    uint32_t eax;
+    uint32_t ebx;
+    uint32_t ecx;
+    uint32_t edx;
+    uint64_t reserved;
+} __attribute__((packed)) SnpCpuidFunc;
+
+typedef struct {
+    uint32_t count;
+    uint32_t reserved1;
+    uint64_t reserved2;
+    SnpCpuidFunc entries[SNP_CPUID_FUNCTION_MAXCOUNT];
+} __attribute__((packed)) SnpCpuidInfo;
+
 static int
 sev_ioctl(int fd, int cmd, void *data, int *error)
 {
@@ -801,6 +831,35 @@ out:
     return ret;
 }
 
+static void
+sev_snp_cpuid_report_mismatches(SnpCpuidInfo *old,
+                                SnpCpuidInfo *new)
+{
+    size_t i;
+
+    if (old->count != new->count) {
+        error_report("SEV-SNP: CPUID validation failed due to count mismatch,"
+                     "provided: %d, expected: %d", old->count, new->count);
+        return;
+    }
+
+    for (i = 0; i < old->count; i++) {
+        SnpCpuidFunc *old_func, *new_func;
+
+        old_func = &old->entries[i];
+        new_func = &new->entries[i];
+
+        if (memcmp(old_func, new_func, sizeof(SnpCpuidFunc))) {
+            error_report("SEV-SNP: CPUID validation failed for function 0x%x, index: 0x%x"
+                         "provided: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, edx: 0x%08x"
+                         "expected: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, edx: 0x%08x",
+                         old_func->eax_in, old_func->ecx_in,
+                         old_func->eax, old_func->ebx, old_func->ecx, old_func->edx,
+                         new_func->eax, new_func->ebx, new_func->ecx, new_func->edx);
+        }
+    }
+}
+
 static const char *
 snp_page_type_to_str(int type)
 {
@@ -819,6 +878,7 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
                       SevLaunchUpdateData *data)
 {
     int ret, fw_error;
+    SnpCpuidInfo snp_cpuid_info;
     struct kvm_sev_snp_launch_update update = {0};
 
     if (!data->hva || !data->len) {
@@ -828,6 +888,11 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
         return 1;
     }
 
+    if (data->type == KVM_SEV_SNP_PAGE_TYPE_CPUID) {
+        /* Save a copy for comparison in case the LAUNCH_UPDATE fails */
+        memcpy(&snp_cpuid_info, data->hva, sizeof(snp_cpuid_info));
+    }
+
     update.uaddr = (__u64)(unsigned long)data->hva;
     update.gfn_start = data->gpa >> TARGET_PAGE_BITS;
     update.len = data->len;
@@ -855,6 +920,11 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
         if (ret && ret != -EAGAIN) {
             error_report("SNP_LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
                          ret, fw_error, fw_error_to_str(fw_error));
+
+            if (data->type == KVM_SEV_SNP_PAGE_TYPE_CPUID) {
+                sev_snp_cpuid_report_mismatches(&snp_cpuid_info, data->hva);
+                error_report("SEV-SNP: failed update CPUID page");
+            }
             break;
         }
     }
@@ -1017,7 +1087,8 @@ sev_launch_finish(SevCommonState *sev_common)
 }
 
 static int
-snp_launch_update_data(uint64_t gpa, void *hva, uint32_t len, int type)
+snp_launch_update_data(uint64_t gpa, void *hva,
+                       uint32_t len, int type)
 {
     SevLaunchUpdateData *data;
 
@@ -1032,6 +1103,90 @@ snp_launch_update_data(uint64_t gpa, void *hva, uint32_t len, int type)
     return 0;
 }
 
+static int
+sev_snp_cpuid_info_fill(SnpCpuidInfo *snp_cpuid_info,
+                        const KvmCpuidInfo *kvm_cpuid_info)
+{
+    size_t i;
+
+    if (kvm_cpuid_info->cpuid.nent > SNP_CPUID_FUNCTION_MAXCOUNT) {
+        error_report("SEV-SNP: CPUID entry count (%d) exceeds max (%d)",
+                     kvm_cpuid_info->cpuid.nent, SNP_CPUID_FUNCTION_MAXCOUNT);
+        return -1;
+    }
+
+    memset(snp_cpuid_info, 0, sizeof(*snp_cpuid_info));
+
+    for (i = 0; i < kvm_cpuid_info->cpuid.nent; i++) {
+        const struct kvm_cpuid_entry2 *kvm_cpuid_entry;
+        SnpCpuidFunc *snp_cpuid_entry;
+
+        kvm_cpuid_entry = &kvm_cpuid_info->entries[i];
+        snp_cpuid_entry = &snp_cpuid_info->entries[i];
+
+        snp_cpuid_entry->eax_in = kvm_cpuid_entry->function;
+        if (kvm_cpuid_entry->flags == KVM_CPUID_FLAG_SIGNIFCANT_INDEX) {
+            snp_cpuid_entry->ecx_in = kvm_cpuid_entry->index;
+        }
+        snp_cpuid_entry->eax = kvm_cpuid_entry->eax;
+        snp_cpuid_entry->ebx = kvm_cpuid_entry->ebx;
+        snp_cpuid_entry->ecx = kvm_cpuid_entry->ecx;
+        snp_cpuid_entry->edx = kvm_cpuid_entry->edx;
+
+        /*
+         * Guest kernels will calculate EBX themselves using the 0xD
+         * subfunctions corresponding to the individual XSAVE areas, so only
+         * encode the base XSAVE size in the initial leaves, corresponding
+         * to the initial XCR0=1 state.
+         */
+        if (snp_cpuid_entry->eax_in == 0xD &&
+            (snp_cpuid_entry->ecx_in == 0x0 || snp_cpuid_entry->ecx_in == 0x1)) {
+            snp_cpuid_entry->ebx = 0x240;
+            snp_cpuid_entry->xcr0_in = 1;
+            snp_cpuid_entry->xss_in = 0;
+        }
+    }
+
+    snp_cpuid_info->count = i;
+
+    return 0;
+}
+
+static int
+snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, uint32_t cpuid_len)
+{
+    KvmCpuidInfo kvm_cpuid_info = {0};
+    SnpCpuidInfo snp_cpuid_info;
+    CPUState *cs = first_cpu;
+    int ret;
+    uint32_t i = 0;
+
+    assert(sizeof(snp_cpuid_info) <= cpuid_len);
+
+    /* get the cpuid list from KVM */
+    do {
+        kvm_cpuid_info.cpuid.nent = ++i;
+        ret = kvm_vcpu_ioctl(cs, KVM_GET_CPUID2, &kvm_cpuid_info);
+    } while (ret == -E2BIG);
+
+    if (ret) {
+        error_report("SEV-SNP: unable to query CPUID values for CPU: '%s'",
+                     strerror(-ret));
+        return 1;
+    }
+
+    ret = sev_snp_cpuid_info_fill(&snp_cpuid_info, &kvm_cpuid_info);
+    if (ret) {
+        error_report("SEV-SNP: failed to generate CPUID table information");
+        return 1;
+    }
+
+    memcpy(hva, &snp_cpuid_info, sizeof(snp_cpuid_info));
+
+    return snp_launch_update_data(cpuid_addr, hva, cpuid_len,
+                                  KVM_SEV_SNP_PAGE_TYPE_CPUID);
+}
+
 static int
 snp_metadata_desc_to_page_type(int desc_type)
 {
@@ -1066,7 +1221,12 @@ snp_populate_metadata_pages(SevSnpGuestState *sev_snp,
             exit(1);
         }
 
-        ret = snp_launch_update_data(desc->base, hva, desc->len, type);
+        if (type == KVM_SEV_SNP_PAGE_TYPE_CPUID) {
+            ret = snp_launch_update_cpuid(desc->base, hva, desc->len);
+        } else {
+            ret = snp_launch_update_data(desc->base, hva, desc->len, type);
+        }
+
         if (ret) {
             error_report("%s: Failed to add metadata page gpa 0x%x+%x type %d",
                          __func__, desc->base, desc->len, desc->type);
-- 
2.34.1
Re: [PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation
Posted by Xiaoyao Li 4 months, 3 weeks ago
On 5/30/2024 7:16 PM, Pankaj Gupta wrote:
> From: Michael Roth <michael.roth@amd.com>
> 
> SEV-SNP firmware allows a special guest page to be populated with a
> table of guest CPUID values so that they can be validated through
> firmware before being loaded into encrypted guest memory where they can
> be used in place of hypervisor-provided values[1].
> 
> As part of SEV-SNP guest initialization, use this interface to validate
> the CPUID entries reported by KVM_GET_CPUID2 prior to initial guest
> start and populate the CPUID page reserved by OVMF with the resulting
> encrypted data.

How is KVM CPUIDs (leaf 0x40000001) validated?

I suppose not all KVM_FEATURE_XXX are supported for SNP guest. And SNP 
firmware doesn't validate such CPUID range. So how does them get validated?

> [1] SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6
>
Re: [PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation
Posted by Michael Roth 4 months, 3 weeks ago
On Tue, Jul 02, 2024 at 11:07:18AM +0800, Xiaoyao Li wrote:
> On 5/30/2024 7:16 PM, Pankaj Gupta wrote:
> > From: Michael Roth <michael.roth@amd.com>
> > 
> > SEV-SNP firmware allows a special guest page to be populated with a
> > table of guest CPUID values so that they can be validated through
> > firmware before being loaded into encrypted guest memory where they can
> > be used in place of hypervisor-provided values[1].
> > 
> > As part of SEV-SNP guest initialization, use this interface to validate
> > the CPUID entries reported by KVM_GET_CPUID2 prior to initial guest
> > start and populate the CPUID page reserved by OVMF with the resulting
> > encrypted data.
> 
> How is KVM CPUIDs (leaf 0x40000001) validated?
> 
> I suppose not all KVM_FEATURE_XXX are supported for SNP guest. And SNP
> firmware doesn't validate such CPUID range. So how does them get validated?

This rules for CPUID enforcement are documented in the PPR for each AMD
CPU model in Chapter 2, section "CPUID Policy Enforcement". For the
situation you mentioned, it's stated there that:

  The PSP enforces the following policy:
  - If the CPUID function is not in the standard range (Fn00000000 through
    Fn0000FFFF) or the extended range
    (Fn8000_0000 through Fn8000_FFFF), the function output check is
    UnChecked.
  - If the CPUID function is in the standard or extended range and the
    function is not listed in SEV-SNP CPUID
    Policy table, then the output check is Strict and required to be 0. Note
    that if the CPUID function does not depend
    on ECX and/or XCR0, then the PSP policy ignores those inputs,
    respectively.
  - Otherwise, the check is defined according to the values listed in
    SEV-SNP CPUID Policy table.

So there are specific ranges that are checked, mainly ones where there
is potential for guests to misbehave if they are being lied to. But
hypervisor-ranges are paravirtual in a sense so there's no assumptions
being made about what the underlying hardware is doing, so the checks
are needed as much in those cases.

-Mike

> 
> > [1] SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6
> > 
>
Re: [PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation
Posted by Xiaoyao Li 4 months, 3 weeks ago
On 7/4/2024 8:34 AM, Michael Roth wrote:
> On Tue, Jul 02, 2024 at 11:07:18AM +0800, Xiaoyao Li wrote:
>> On 5/30/2024 7:16 PM, Pankaj Gupta wrote:
>>> From: Michael Roth <michael.roth@amd.com>
>>>
>>> SEV-SNP firmware allows a special guest page to be populated with a
>>> table of guest CPUID values so that they can be validated through
>>> firmware before being loaded into encrypted guest memory where they can
>>> be used in place of hypervisor-provided values[1].
>>>
>>> As part of SEV-SNP guest initialization, use this interface to validate
>>> the CPUID entries reported by KVM_GET_CPUID2 prior to initial guest
>>> start and populate the CPUID page reserved by OVMF with the resulting
>>> encrypted data.
>>
>> How is KVM CPUIDs (leaf 0x40000001) validated?
>>
>> I suppose not all KVM_FEATURE_XXX are supported for SNP guest. And SNP
>> firmware doesn't validate such CPUID range. So how does them get validated?
> 
> This rules for CPUID enforcement are documented in the PPR for each AMD
> CPU model in Chapter 2, section "CPUID Policy Enforcement". For the
> situation you mentioned, it's stated there that:
> 
>    The PSP enforces the following policy:
>    - If the CPUID function is not in the standard range (Fn00000000 through
>      Fn0000FFFF) or the extended range
>      (Fn8000_0000 through Fn8000_FFFF), the function output check is
>      UnChecked.
>    - If the CPUID function is in the standard or extended range and the
>      function is not listed in SEV-SNP CPUID
>      Policy table, then the output check is Strict and required to be 0. Note
>      that if the CPUID function does not depend
>      on ECX and/or XCR0, then the PSP policy ignores those inputs,
>      respectively.
>    - Otherwise, the check is defined according to the values listed in
>      SEV-SNP CPUID Policy table.
> 
> So there are specific ranges that are checked, mainly ones where there
> is potential for guests to misbehave if they are being lied to. But
> hypervisor-ranges are paravirtual in a sense so there's no assumptions
> being made about what the underlying hardware is doing, so the checks
> are needed as much in those cases.

I'm a little confused. Per your reference above, hypervisor-ranges is 
unchecked because it's not in the standard range nor the extended range.

And your last sentence said "so the checks are needed as much in those 
cases". So how does hypervisor-ranges get checked?

> -Mike
> 
>>
>>> [1] SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6
>>>
>>
Re: [PATCH v4 20/31] i386/sev: Add support for SNP CPUID validation
Posted by Paolo Bonzini 4 months, 3 weeks ago
On Thu, Jul 4, 2024 at 6:10 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> > So there are specific ranges that are checked, mainly ones where there
> > is potential for guests to misbehave if they are being lied to. But
> > hypervisor-ranges are paravirtual in a sense so there's no assumptions
> > being made about what the underlying hardware is doing, so the checks
> > are needed as much in those cases.
>
> I'm a little confused. Per your reference above, hypervisor-ranges is
> unchecked because it's not in the standard range nor the extended range.
>
> And your last sentence said "so the checks are needed as much in those
> cases". So how does hypervisor-ranges get checked?

I think "not" is missing in the sentence.

Paolo