[PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Avi Kivity]

MAXPHYADDR is derived from cpuid 0x80000008, but when that isn't present, we
get some random value.

Fix by checking first that cpuid 0x80000008 is supported.

Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd5c3d3..b2460cd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4165,9 +4165,13 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 
+	best = kvm_find_cpuid_entry(vcpu, 0x80000000, 0);
+	if (!best || best->eax < 0x80000008)
+		goto not_found;
 	best = kvm_find_cpuid_entry(vcpu, 0x80000008, 0);
 	if (best)
 		return best->eax & 0xff;
+not_found:
 	return 36;
 }
 
-- 
1.7.0.4

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Avi Kivity]

On 04/11/2010 03:33 PM, Avi Kivity wrote:
> MAXPHYADDR is derived from cpuid 0x80000008, but when that isn't present, we
> get some random value.
>
> Fix by checking first that cpuid 0x80000008 is supported.
>
> Pekka Enberg<penberg@cs.helsinki.fi>
>    

^ += Reported-by: (looking forward to Tested-by: too)

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Pekka Enberg]

Avi Kivity wrote:
> MAXPHYADDR is derived from cpuid 0x80000008, but when that isn't present, we
> get some random value.
> 
> Fix by checking first that cpuid 0x80000008 is supported.
> 
> Pekka Enberg <penberg@cs.helsinki.fi>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/x86.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fd5c3d3..b2460cd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4165,9 +4165,13 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpuid_entry2 *best;
>  
> +	best = kvm_find_cpuid_entry(vcpu, 0x80000000, 0);
> +	if (!best || best->eax < 0x80000008)
> +		goto not_found;
>  	best = kvm_find_cpuid_entry(vcpu, 0x80000008, 0);
>  	if (best)
>  		return best->eax & 0xff;
> +not_found:
>  	return 36;
>  }
>  

Hmm, doesn't seem to work here. I still that triple fault in guest.

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Avi Kivity]

On 04/11/2010 04:32 PM, Pekka Enberg wrote:
> Avi Kivity wrote:
>> MAXPHYADDR is derived from cpuid 0x80000008, but when that isn't 
>> present, we
>> get some random value.
>>
>> Fix by checking first that cpuid 0x80000008 is supported.
>>
>> Pekka Enberg <penberg@cs.helsinki.fi>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>> ---
>>  arch/x86/kvm/x86.c |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index fd5c3d3..b2460cd 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4165,9 +4165,13 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
>>  {
>>      struct kvm_cpuid_entry2 *best;
>>
>> +    best = kvm_find_cpuid_entry(vcpu, 0x80000000, 0);
>> +    if (!best || best->eax < 0x80000008)
>> +        goto not_found;
>>      best = kvm_find_cpuid_entry(vcpu, 0x80000008, 0);
>>      if (best)
>>          return best->eax & 0xff;
>> +not_found:
>>      return 36;
>>  }
>>
>
> Hmm, doesn't seem to work here. I still that triple fault in guest.

Can you add a printk to see what value is returned and why?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Pekka Enberg]

Avi Kivity wrote:
>> Hmm, doesn't seem to work here. I still that triple fault in guest.
> 
> Can you add a printk to see what value is returned and why?

Argh, it's a off-by one bug in my userspace tool... So the CPU really 
does support 0x80000008 and I'm just an idiot. :-)

			Pekka

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Pekka Enberg]

Pekka Enberg wrote:
> Avi Kivity wrote:
>>> Hmm, doesn't seem to work here. I still that triple fault in guest.
>>
>> Can you add a printk to see what value is returned and why?
> 
> Argh, it's a off-by one bug in my userspace tool... So the CPU really 
> does support 0x80000008 and I'm just an idiot. :-)

So to explain this a bit more: I'm setting 0x80000000 to 0x80000008 but 
failing to install an entry for 0x80000008. I don't quite understand why 
kvm_find_cpuid_entry() doesn't return NULL in this case?

			Pekka

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Avi Kivity]

On 04/11/2010 04:45 PM, Pekka Enberg wrote:
> Pekka Enberg wrote:
>> Avi Kivity wrote:
>>>> Hmm, doesn't seem to work here. I still that triple fault in guest.
>>>
>>> Can you add a printk to see what value is returned and why?
>>
>> Argh, it's a off-by one bug in my userspace tool... So the CPU really 
>> does support 0x80000008 and I'm just an idiot. :-)
>
> So to explain this a bit more: I'm setting 0x80000000 to 0x80000008 
> but failing to install an entry for 0x80000008. I don't quite 
> understand why kvm_find_cpuid_entry() doesn't return NULL in this case?

cpuid is defined to return the highest entry if it doesn't find a match.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Pekka Enberg]

Avi Kivity wrote:
> On 04/11/2010 04:45 PM, Pekka Enberg wrote:
>> Pekka Enberg wrote:
>>> Avi Kivity wrote:
>>>>> Hmm, doesn't seem to work here. I still that triple fault in guest.
>>>>
>>>> Can you add a printk to see what value is returned and why?
>>>
>>> Argh, it's a off-by one bug in my userspace tool... So the CPU really 
>>> does support 0x80000008 and I'm just an idiot. :-)
>>
>> So to explain this a bit more: I'm setting 0x80000000 to 0x80000008 
>> but failing to install an entry for 0x80000008. I don't quite 
>> understand why kvm_find_cpuid_entry() doesn't return NULL in this case?
> 
> cpuid is defined to return the highest entry if it doesn't find a match.

OK, then it's a bug of my own doing and we don't need to do anything in 
the kernel.

			Pekka

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Avi Kivity]

On 04/11/2010 04:53 PM, Pekka Enberg wrote:
> Avi Kivity wrote:
>> On 04/11/2010 04:45 PM, Pekka Enberg wrote:
>>> Pekka Enberg wrote:
>>>> Avi Kivity wrote:
>>>>>> Hmm, doesn't seem to work here. I still that triple fault in guest.
>>>>>
>>>>> Can you add a printk to see what value is returned and why?
>>>>
>>>> Argh, it's a off-by one bug in my userspace tool... So the CPU 
>>>> really does support 0x80000008 and I'm just an idiot. :-)
>>>
>>> So to explain this a bit more: I'm setting 0x80000000 to 0x80000008 
>>> but failing to install an entry for 0x80000008. I don't quite 
>>> understand why kvm_find_cpuid_entry() doesn't return NULL in this case?
>>
>> cpuid is defined to return the highest entry if it doesn't find a match.
>
> OK, then it's a bug of my own doing and we don't need to do anything 
> in the kernel.

I think the patch is nevertheless correct, not sure why it worked so far.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Pekka Enberg]

Avi Kivity wrote:
>> OK, then it's a bug of my own doing and we don't need to do anything 
>> in the kernel.
> 
> I think the patch is nevertheless correct, not sure why it worked so far.

Yes, agreed.  I'm guessing most 64-bit CPUs support 0x80000008 and qemu 
does the right thing so the bug doesn't trigger in practice.

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

Re: [PATCH] KVM: Fix MAXPHYADDR calculation when cpuid does not support it

[Marcelo Tosatti]

On Sun, Apr 11, 2010 at 03:33:32PM +0300, Avi Kivity wrote:
> MAXPHYADDR is derived from cpuid 0x80000008, but when that isn't present, we
> get some random value.
> 
> Fix by checking first that cpuid 0x80000008 is supported.
> 
> Pekka Enberg <penberg@cs.helsinki.fi>
> Signed-off-by: Avi Kivity <avi@redhat.com>

Applied, thanks.