Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest
On 25/11/2015 13:45, Wanpeng Li wrote: > 2015-11-19 19:05 GMT+08:00 Paolo Bonzini : >> >> 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). > > It seems that F(LM) is always set in the case of qemu-system-x86_64 w/ > 32-bit guest, vmware also exposes LM bit to 32 bit guest(however it > doesn't expose syscall/sysret) Does it expose the SYSCALL bit? Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest
2015-11-19 19:05 GMT+08:00 Paolo Bonzini : > > 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). It seems that F(LM) is always set in the case of qemu-system-x86_64 w/ 32-bit guest, vmware also exposes LM bit to 32 bit guest(however it doesn't expose syscall/sysret), so maybe we can't depend on F(LM). Regards, Wanpeng Li -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest
2015-11-19 19:05 GMT+08:00 Paolo Bonzini : > > > 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, 0x, 0x); >> + 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. Ok. > >> + best = kvm_find_cpuid_entry(vcpu, 0x8001, 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". No, I try a 32-bit linux host kernel, cpuid tool shows that SYSCALL bit is not set. > > 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). Good point. :-) > > 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 No. > 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. Btw, I also try a 32-bit linux guest in vmware on my macbook, it also doesn't expose syscall/sysret. Regards, Wanpeng Li -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest
On 19/11/2015 13:01, Wanpeng Li wrote: > > 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". > > No, I try a 32-bit linux host kernel, cpuid tool shows that SYSCALL > bit is not set. Ok, let me try... Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest
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, 0x, 0x); > + 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, 0x8001, 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html