| Message ID | 20210114003708.3798992-6-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SVM: Misc SEV cleanups | expand |
On 1/13/21 6:36 PM, Sean Christopherson wrote: > Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX, > and use it to override the supported CPUID flags reported to userspace. > Masking the reported CPUID flags avoids over-reporting KVM support, e.g. > without the mask a SEV-SNP capable CPU may incorrectly advertise SNP > support to userspace. > > Cc: Brijesh Singh <brijesh.singh@amd.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/cpuid.c | 2 ++ > arch/x86/kvm/cpuid.h | 1 + > 2 files changed, 3 insertions(+) thanks Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 13036cf0b912..b7618cdd06b5 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > case 0x8000001F: > if (!boot_cpu_has(X86_FEATURE_SEV)) > entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > + else > + cpuid_entry_override(entry, CPUID_8000_001F_EAX); > break; > /*Add support for Centaur's CPUID instruction*/ > case 0xC0000000: > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index dc921d76e42e..8b6fc9bde248 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = { > [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX}, > [CPUID_7_EDX] = { 7, 0, CPUID_EDX}, > [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, > + [CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX}, > }; > > /*
On 14/01/21 01:36, Sean Christopherson wrote: > Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX, > and use it to override the supported CPUID flags reported to userspace. > Masking the reported CPUID flags avoids over-reporting KVM support, e.g. > without the mask a SEV-SNP capable CPU may incorrectly advertise SNP > support to userspace. > > Cc: Brijesh Singh <brijesh.singh@amd.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/cpuid.c | 2 ++ > arch/x86/kvm/cpuid.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 13036cf0b912..b7618cdd06b5 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > case 0x8000001F: > if (!boot_cpu_has(X86_FEATURE_SEV)) > entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > + else > + cpuid_entry_override(entry, CPUID_8000_001F_EAX); > break; > /*Add support for Centaur's CPUID instruction*/ > case 0xC0000000: > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index dc921d76e42e..8b6fc9bde248 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = { > [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX}, > [CPUID_7_EDX] = { 7, 0, CPUID_EDX}, > [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, > + [CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX}, > }; > > /* > I don't understand, wouldn't this also need a kvm_cpu_cap_mask call somewhere else? As it is, it doesn't do anything. Paolo
On Thu, Jan 28, 2021, Paolo Bonzini wrote: > On 14/01/21 01:36, Sean Christopherson wrote: > > Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX, > > and use it to override the supported CPUID flags reported to userspace. > > Masking the reported CPUID flags avoids over-reporting KVM support, e.g. > > without the mask a SEV-SNP capable CPU may incorrectly advertise SNP > > support to userspace. > > > > Cc: Brijesh Singh <brijesh.singh@amd.com> > > Cc: Tom Lendacky <thomas.lendacky@amd.com> > > Signed-off-by: Sean Christopherson <seanjc@google.com> > > --- > > arch/x86/kvm/cpuid.c | 2 ++ > > arch/x86/kvm/cpuid.h | 1 + > > 2 files changed, 3 insertions(+) > > > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > index 13036cf0b912..b7618cdd06b5 100644 > > --- a/arch/x86/kvm/cpuid.c > > +++ b/arch/x86/kvm/cpuid.c > > @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > > case 0x8000001F: > > if (!boot_cpu_has(X86_FEATURE_SEV)) > > entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > > + else > > + cpuid_entry_override(entry, CPUID_8000_001F_EAX); > > break; > > /*Add support for Centaur's CPUID instruction*/ > > case 0xC0000000: > > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > > index dc921d76e42e..8b6fc9bde248 100644 > > --- a/arch/x86/kvm/cpuid.h > > +++ b/arch/x86/kvm/cpuid.h > > @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = { > > [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX}, > > [CPUID_7_EDX] = { 7, 0, CPUID_EDX}, > > [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, > > + [CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX}, > > }; > > /* > > > > I don't understand, wouldn't this also need a kvm_cpu_cap_mask call > somewhere else? As it is, it doesn't do anything. Ugh, yes, apparently I thought the kernel would magically clear bits it doesn't care about. Looking at this again, I think the kvm_cpu_cap_mask() invocation should always mask off X86_FEATURE_SME. SME cannot be virtualized, and AFAIK it's not emulated by KVM. This would fix an oddity where SME would be advertised if SEV is also supported. Boris has queue the kernel change to tip/x86/cpu, I'll spin v4 against that.
On 28/01/21 18:09, Sean Christopherson wrote: > On Thu, Jan 28, 2021, Paolo Bonzini wrote: >> On 14/01/21 01:36, Sean Christopherson wrote: >>> Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX, >>> and use it to override the supported CPUID flags reported to userspace. >>> Masking the reported CPUID flags avoids over-reporting KVM support, e.g. >>> without the mask a SEV-SNP capable CPU may incorrectly advertise SNP >>> support to userspace. >>> >>> Cc: Brijesh Singh <brijesh.singh@amd.com> >>> Cc: Tom Lendacky <thomas.lendacky@amd.com> >>> Signed-off-by: Sean Christopherson <seanjc@google.com> >>> --- >>> arch/x86/kvm/cpuid.c | 2 ++ >>> arch/x86/kvm/cpuid.h | 1 + >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >>> index 13036cf0b912..b7618cdd06b5 100644 >>> --- a/arch/x86/kvm/cpuid.c >>> +++ b/arch/x86/kvm/cpuid.c >>> @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) >>> case 0x8000001F: >>> if (!boot_cpu_has(X86_FEATURE_SEV)) >>> entry->eax = entry->ebx = entry->ecx = entry->edx = 0; >>> + else >>> + cpuid_entry_override(entry, CPUID_8000_001F_EAX); >>> break; >>> /*Add support for Centaur's CPUID instruction*/ >>> case 0xC0000000: >>> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h >>> index dc921d76e42e..8b6fc9bde248 100644 >>> --- a/arch/x86/kvm/cpuid.h >>> +++ b/arch/x86/kvm/cpuid.h >>> @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = { >>> [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX}, >>> [CPUID_7_EDX] = { 7, 0, CPUID_EDX}, >>> [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, >>> + [CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX}, >>> }; >>> /* >>> >> >> I don't understand, wouldn't this also need a kvm_cpu_cap_mask call >> somewhere else? As it is, it doesn't do anything. > > Ugh, yes, apparently I thought the kernel would magically clear bits it doesn't > care about. > > Looking at this again, I think the kvm_cpu_cap_mask() invocation should always > mask off X86_FEATURE_SME. SME cannot be virtualized, and AFAIK it's not > emulated by KVM. This would fix an oddity where SME would be advertised if SEV > is also supported. > > Boris has queue the kernel change to tip/x86/cpu, I'll spin v4 against that. You can send it after the 5.12 merge window. Paolo
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 13036cf0b912..b7618cdd06b5 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -855,6 +855,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) case 0x8000001F: if (!boot_cpu_has(X86_FEATURE_SEV)) entry->eax = entry->ebx = entry->ecx = entry->edx = 0; + else + cpuid_entry_override(entry, CPUID_8000_001F_EAX); break; /*Add support for Centaur's CPUID instruction*/ case 0xC0000000: diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h index dc921d76e42e..8b6fc9bde248 100644 --- a/arch/x86/kvm/cpuid.h +++ b/arch/x86/kvm/cpuid.h @@ -63,6 +63,7 @@ static const struct cpuid_reg reverse_cpuid[] = { [CPUID_8000_0007_EBX] = {0x80000007, 0, CPUID_EBX}, [CPUID_7_EDX] = { 7, 0, CPUID_EDX}, [CPUID_7_1_EAX] = { 7, 1, CPUID_EAX}, + [CPUID_8000_001F_EAX] = {0x8000001f, 1, CPUID_EAX}, }; /*
Add a reverse-CPUID entry for the memory encryption word, 0x8000001F.EAX, and use it to override the supported CPUID flags reported to userspace. Masking the reported CPUID flags avoids over-reporting KVM support, e.g. without the mask a SEV-SNP capable CPU may incorrectly advertise SNP support to userspace. Cc: Brijesh Singh <brijesh.singh@amd.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/cpuid.c | 2 ++ arch/x86/kvm/cpuid.h | 1 + 2 files changed, 3 insertions(+)