From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Wang Subject: Re: [PATCH RFC] x86/kvm: expose the CPUID of SPEC_CTRL and STIBP to guests Date: Tue, 09 Jan 2018 17:16:27 +0800 Message-ID: <5A54886B.7030104@intel.com> References: <2b777b98-3ee1-4d47-9aa3-8c127de927b7@default> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: jmattson@google.com, pbonzini@redhat.com, kvm@vger.kernel.org To: Liran Alon Return-path: Received: from mga14.intel.com ([192.55.52.115]:41635 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751008AbeAIJOG (ORCPT ); Tue, 9 Jan 2018 04:14:06 -0500 In-Reply-To: <2b777b98-3ee1-4d47-9aa3-8c127de927b7@default> Sender: kvm-owner@vger.kernel.org List-ID: On 01/09/2018 04:43 PM, Liran Alon wrote: > ----- wei.w.wang@intel.com wrote: > >> This patch shows an alternative approach to the one posted here: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mail-2Darchive.com_linux-2Dkernel-40vger.kernel.org_msg1580364.html&d=DwIBAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=Jk6Q8nNzkQ6LJ6g42qARkg6ryIDGQr-yKXPNGZbpTx0&m=o_uh0-IfLdLMwf2MaOOQVKtZbPlJ-fDVmglRj277cwk&s=j1oGOoYysZhUuGuuvuwdTU_OO7bv1ysIyvDtAlc6C-c&e= >> >> The advantages are >> 1) Simpler; >> 2) More reasonable because this is used to fill the hardware security >> hole, for all the x86 cpus that physically support the two CPUIDs, >> which means the hole already exists physically. All the VMs should >> use this feature no matter what CPU model they are using. So, > I'm not sure I 100% agree with this. > There should be a way for the userspace agent to disable these CPUIDs if wanted. > You don't want to lose the ability to expose a mimic of a real physical CPU-model of core2duo that > doesn't have these CPUIDs. A good solution can be that these features will be exposed by default to guests > if available on hardware but can still be explicitly not-exposed if userspace agent wishes so. I think the case we are handling here is different: It shouldn't be treated as a regular feature (e.g. xsaves) that a user can choose to use or not. It is a security hole (or say a bug). When we fixed a bug, we don't give users an option to select to trigger the bug, right? > The only weird side-effect of this is that live-migration between different physical hosts running with > exact same QEMU cmdline will result in different CPUID values exposed to guest. I think live migration itself doesn't do the CPUID check, so adding the QEMU side hardcode part doesn't help. That is, even using that patch 7/7, I think it wouldn't make a difference. This is the same case when we migrate a VM from a skylake physical machine to an older machine, the difference of supported CPUID won't even generate a warning when the destination side QEMU gets booted, right? >> exposing >> the two CPUIDs as long as they are physically supported by the >> hardware, >> and this doesn't require the QEMU side hardcode as usual. >> >> When the related feature bits are added to the kernel, and we can >> simply >> change it to: >> best->edx |= F(SPEC_CTRL) | F(PRED_CMD); >> >> Signed-off-by: Wei Wang >> --- >> arch/x86/kvm/cpuid.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >> index 0099e10..c33d3d4 100644 >> --- a/arch/x86/kvm/cpuid.c >> +++ b/arch/x86/kvm/cpuid.c >> @@ -70,6 +70,8 @@ u64 kvm_supported_xcr0(void) >> /* These are scattered features in cpufeatures.h. */ >> #define KVM_CPUID_BIT_AVX512_4VNNIW 2 >> #define KVM_CPUID_BIT_AVX512_4FMAPS 3 >> +#define KVM_CPUID_BIT_SPEC_CTRL 26 >> +#define KVM_CPUID_BIT_STIBP 27 >> #define KF(x) bit(KVM_CPUID_BIT_##x) >> >> int kvm_update_cpuid(struct kvm_vcpu *vcpu) >> @@ -109,6 +111,9 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu) >> } >> } >> >> + if (cpuid_edx(0x7) & (KF(SPEC_CTRL) | KF(STIBP))) > You should put this inside the "if (best) {...}" block above. Right, thanks. Best, Wei