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 :
>
> 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

[Paolo Bonzini]

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

[Wanpeng Li]

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

[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
--
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

[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.

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