Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

[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 <wanpeng.li@hotmail.com>
> ---
>  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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

[Wanpeng Li]

2015-11-19 19:05 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> 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 <wanpeng.li@hotmail.com>
>> ---
>>  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.

Ok.

>
>> +     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".

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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

[Paolo Bonzini]

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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

[Wanpeng Li]

2015-11-19 19:05 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
> 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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

[Paolo Bonzini]

On 25/11/2015 13:45, Wanpeng Li wrote:
> 2015-11-19 19:05 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>
>> 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