Skip to contentSkip to conversation

KVM: Fix MAXPHYADDR calculation when cpuid does not support it

PATCH Applied Avi Kivity 3 participants 11 messages Apr 11, 2010 – Apr 12, 2010

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Avi Kivity

author
#0 lore raw

PATCH 1/1 KVM: Fix MAXPHYADDR calculation when cpuid does not support it

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 <[email protected]>

Signed-off-by Avi Kivity <[email protected]>
---
 arch/x86/kvm/x86.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +4 -0
index fd5c3d3..b2460cd 100644
@@ -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;
 }
Signature · 1 line
1.7.0.4
author
#1 lore raw
Quoted 7 lines · 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<[email protected]>

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

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw
In reply to Avi Kivity
Quoted 19 lines · 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 <[email protected]>

Signed-off-by Avi Kivity <[email protected]>
---
 arch/x86/kvm/x86.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +0 -0
index fd5c3d3..b2460cd 100644
@@ -4165,9 +4165,13 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;

arch/x86/kvm/x86.c int cpuid_maxphyaddr(struct kvm_vcpu *vcpu) · line 4177

Quoted 10 lines
arch/x86/kvm/x86.c +4 -0
+	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.

author
#3 lore raw
Quoted 20 lines · Pekka Enberg
Quoted 19 lines · 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 <[email protected]>

Signed-off-by Avi Kivity <[email protected]>
---
 arch/x86/kvm/x86.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/x86/kvm/x86.c +0 -0
index fd5c3d3..b2460cd 100644
@@ -4165,9 +4165,13 @@ int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 {
     struct kvm_cpuid_entry2 *best;
Quoted 12 lines

arch/x86/kvm/x86.c int cpuid_maxphyaddr(struct kvm_vcpu *vcpu) · line 4177

Quoted 10 lines
arch/x86/kvm/x86.c +4 -0
+    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?

Signature · 1 line
error compiling committee.c: too many arguments to function
#4 lore raw
Quoted 3 lines · Avi Kivity
Quoted 1 line

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

#5 lore raw
Quoted 6 lines · Pekka Enberg
Quoted 3 lines · Avi Kivity
Quoted 1 line

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

author
#6 lore raw
Quoted 9 lines · Pekka Enberg
Quoted 6 lines · Pekka Enberg
Quoted 3 lines · Avi Kivity
Quoted 1 line

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
#7 lore raw
Quoted 5 lines · Avi Kivity
Quoted 4 lines · Pekka Enberg
Quoted 3 lines · Pekka Enberg
Quoted 2 lines · Avi Kivity
Quoted 1 line

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

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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

author
#8 lore raw
Quoted 8 lines · Pekka Enberg
Quoted 7 lines · Avi Kivity
Quoted 6 lines · Pekka Enberg
Quoted 5 lines · Pekka Enberg
Quoted 3 lines · Avi Kivity
Quoted 1 line

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?

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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.

Signature · 1 line
error compiling committee.c: too many arguments to function
Acked-by
#9 lore raw
Quoted 3 lines · Avi Kivity
Quoted 1 line

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 <[email protected]>