| Message ID | 20240530111643.1091816-21-pankaj.gupta@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) support | expand |
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 >
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 > > >
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 >>> >>
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
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);