From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758243AbbKSLFb (ORCPT ); Thu, 19 Nov 2015 06:05:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59252 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752892AbbKSLFa (ORCPT ); Thu, 19 Nov 2015 06:05:30 -0500 Subject: Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest To: Wanpeng li References: Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org From: Paolo Bonzini X-Enigmail-Draft-Status: N1110 Message-ID: <564DACF5.9090908@redhat.com> Date: Thu, 19 Nov 2015 12:05:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/11/2015 11:45, Wanpeng li wrote: > Intel cpu doesn't support syscall/sysret in non 64-bit mode which > is different from AMD. Expose syscall/sysret to intel 32-bit guest > just makes no sense and leads to #UD which will confuse the users. > > This patch disable expose syscall/sysret to intel 32-bit guest. > > Signed-off-by: Wanpeng li > --- > arch/x86/kvm/cpuid.c | 16 ++++++++++++++++ > arch/x86/kvm/x86.c | 1 + > 2 files changed, 17 insertions(+) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 6525e92..f8ddca6 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -62,6 +62,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu) > { > struct kvm_cpuid_entry2 *best; > struct kvm_lapic *apic = vcpu->arch.apic; > + bool vendor_intel = false; > > best = kvm_find_cpuid_entry(vcpu, 1, 0); > if (!best) > @@ -114,6 +115,21 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu) > vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu); > > kvm_pmu_refresh(vcpu); > + > + /* Don't expose syscall/sysret to intel non 64-bit mode guest */ > + best = kvm_find_cpuid_entry(vcpu, 0x00000000, 0x00000000); > + if (best && best->ebx == 0x756e6547 && best->ecx == 0x6c65746e && > + best->edx == 0x49656e69) > + vendor_intel = true; You can use X86EMUL_CPUID_VENDOR_GenuineIntel_ebx/ecx/edx instead here. > + best = kvm_find_cpuid_entry(vcpu, 0x80000001, 0); > + if (best && vendor_intel) { > + if (!is_long_mode(vcpu)) > + best->edx &= ~F(SYSCALL); > + else > + best->edx |= F(SYSCALL); This is not correct. As far as I know, the SYSCALL bit is always present in CPUID, even if the machine is running in 32-bit mode; CPUID documentation (SDM Volume 2) explicitly documents bit 11 as "Bit 11: SYSCALL/SYSRET available in 64-bit mode". So there are two possibilities here: 1) Clear F(SYSCALL) in kvm_update_cpuid, like you are doing here but only if F(LM) is already clear (in addition to the vendor being Intel). 2) do nothing, declare this a userspace bug. Linux knows that only AMD machines support SYSCALL in 32-bit mode (there is a separate feature bit X86_FEATURE_SYSCALL32 that is only set by arch/x86/kernel/cpu/amd.c). Are you fixing an issue with Windows? If so, with what QEMU command line? But if it's not an issue with a real guest, doing nothing would be my favorite option... In any case, set_efer doesn't need to call kvm_update_cpuid. Thanks, Paolo