linux-kernel - Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature flags with host mask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YBLvvpeEORjVd2IP@google.com>
Date:   Thu, 28 Jan 2021 09:09:18 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, Borislav Petkov <bp@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH v2 05/14] KVM: x86: Override reported SME/SEV feature
 flags with host mask

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@....com>
> > Cc: Tom Lendacky <thomas.lendacky@....com>
> > Signed-off-by: Sean Christopherson <seanjc@...gle.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.