* [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
@ 2012-04-30 11:45 Gleb Natapov
2012-04-30 16:37 ` Davidlohr Bueso
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Gleb Natapov @ 2012-04-30 11:45 UTC (permalink / raw)
To: kvm; +Cc: avi, mtosatti, mst
This couid range does not exist on real HW and Intel spec says that
"Information returned for highest basic information leaf" will be
returned. Not very well defined.
Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 99c4bbe..a7a7a94 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -178,14 +178,16 @@ static inline int kvm_para_available(void)
unsigned int eax, ebx, ecx, edx;
char signature[13];
- cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
- memcpy(signature + 0, &ebx, 4);
- memcpy(signature + 4, &ecx, 4);
- memcpy(signature + 8, &edx, 4);
- signature[12] = 0;
-
- if (strcmp(signature, "KVMKVMKVM") == 0)
- return 1;
+ if (cpu_has_hypervisor) {
+ cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
+ memcpy(signature + 0, &ebx, 4);
+ memcpy(signature + 4, &ecx, 4);
+ memcpy(signature + 8, &edx, 4);
+ signature[12] = 0;
+
+ if (strcmp(signature, "KVMKVMKVM") == 0)
+ return 1;
+ }
return 0;
}
--
Gleb.
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-04-30 11:45 [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf Gleb Natapov
@ 2012-04-30 16:37 ` Davidlohr Bueso
2012-05-01 6:52 ` Gleb Natapov
2012-05-01 12:50 ` Avi Kivity
2012-05-06 13:00 ` Avi Kivity
2 siblings, 1 reply; 9+ messages in thread
From: Davidlohr Bueso @ 2012-04-30 16:37 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, avi, mtosatti, mst
On Mon, 2012-04-30 at 14:45 +0300, Gleb Natapov wrote:
> This couid range does not exist on real HW and Intel spec says that
> "Information returned for highest basic information leaf" will be
> returned. Not very well defined.
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> index 99c4bbe..a7a7a94 100644
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -178,14 +178,16 @@ static inline int kvm_para_available(void)
> unsigned int eax, ebx, ecx, edx;
> char signature[13];
>
> - cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> - memcpy(signature + 0, &ebx, 4);
> - memcpy(signature + 4, &ecx, 4);
> - memcpy(signature + 8, &edx, 4);
> - signature[12] = 0;
> -
> - if (strcmp(signature, "KVMKVMKVM") == 0)
> - return 1;
> + if (cpu_has_hypervisor) {
> + cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> + memcpy(signature + 0, &ebx, 4);
> + memcpy(signature + 4, &ecx, 4);
> + memcpy(signature + 8, &edx, 4);
> + signature[12] = 0;
> +
> + if (strcmp(signature, "KVMKVMKVM") == 0)
> + return 1;
> + }
>
> return 0;
> }
Wouldn't this be better?
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -170,6 +170,9 @@ static inline int kvm_para_available(void)
unsigned int eax, ebx, ecx, edx;
char signature[13];
+ if (!cpu_has_hypervisor)
+ return 0;
+
cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
memcpy(signature + 0, &ebx, 4);
memcpy(signature + 4, &ecx, 4);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-04-30 16:37 ` Davidlohr Bueso
@ 2012-05-01 6:52 ` Gleb Natapov
0 siblings, 0 replies; 9+ messages in thread
From: Gleb Natapov @ 2012-05-01 6:52 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: kvm, avi, mtosatti, mst
On Mon, Apr 30, 2012 at 06:37:12PM +0200, Davidlohr Bueso wrote:
> On Mon, 2012-04-30 at 14:45 +0300, Gleb Natapov wrote:
> > This couid range does not exist on real HW and Intel spec says that
> > "Information returned for highest basic information leaf" will be
> > returned. Not very well defined.
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
> > index 99c4bbe..a7a7a94 100644
> > --- a/arch/x86/include/asm/kvm_para.h
> > +++ b/arch/x86/include/asm/kvm_para.h
> > @@ -178,14 +178,16 @@ static inline int kvm_para_available(void)
> > unsigned int eax, ebx, ecx, edx;
> > char signature[13];
> >
> > - cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> > - memcpy(signature + 0, &ebx, 4);
> > - memcpy(signature + 4, &ecx, 4);
> > - memcpy(signature + 8, &edx, 4);
> > - signature[12] = 0;
> > -
> > - if (strcmp(signature, "KVMKVMKVM") == 0)
> > - return 1;
> > + if (cpu_has_hypervisor) {
> > + cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> > + memcpy(signature + 0, &ebx, 4);
> > + memcpy(signature + 4, &ecx, 4);
> > + memcpy(signature + 8, &edx, 4);
> > + signature[12] = 0;
> > +
> > + if (strcmp(signature, "KVMKVMKVM") == 0)
> > + return 1;
> > + }
> >
> > return 0;
> > }
>
> Wouldn't this be better?
>
If I thought it would I would have wrote it like that :) I prefer less exit
points from a function if possible.
> --- a/arch/x86/include/asm/kvm_para.h
> +++ b/arch/x86/include/asm/kvm_para.h
> @@ -170,6 +170,9 @@ static inline int kvm_para_available(void)
> unsigned int eax, ebx, ecx, edx;
> char signature[13];
>
> + if (!cpu_has_hypervisor)
> + return 0;
> +
> cpuid(KVM_CPUID_SIGNATURE, &eax, &ebx, &ecx, &edx);
> memcpy(signature + 0, &ebx, 4);
> memcpy(signature + 4, &ecx, 4);
>
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-04-30 11:45 [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf Gleb Natapov
2012-04-30 16:37 ` Davidlohr Bueso
@ 2012-05-01 12:50 ` Avi Kivity
2012-05-01 12:55 ` Gleb Natapov
2012-05-06 13:00 ` Avi Kivity
2 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2012-05-01 12:50 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 04/30/2012 02:45 PM, Gleb Natapov wrote:
> This couid range does not exist on real HW and Intel spec says that
> "Information returned for highest basic information leaf" will be
> returned. Not very well defined.
>
Correct in principle. But IIRC some old qemus supported the kvm cpuid
range without setting the hypervisor bit.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-05-01 12:50 ` Avi Kivity
@ 2012-05-01 12:55 ` Gleb Natapov
2012-05-01 12:59 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2012-05-01 12:55 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, mtosatti, mst
On Tue, May 01, 2012 at 03:50:44PM +0300, Avi Kivity wrote:
> On 04/30/2012 02:45 PM, Gleb Natapov wrote:
> > This couid range does not exist on real HW and Intel spec says that
> > "Information returned for highest basic information leaf" will be
> > returned. Not very well defined.
> >
>
> Correct in principle. But IIRC some old qemus supported the kvm cpuid
> range without setting the hypervisor bit.
>
How old?
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-05-01 12:55 ` Gleb Natapov
@ 2012-05-01 12:59 ` Avi Kivity
2012-05-02 12:09 ` Gleb Natapov
0 siblings, 1 reply; 9+ messages in thread
From: Avi Kivity @ 2012-05-01 12:59 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 05/01/2012 03:55 PM, Gleb Natapov wrote:
> On Tue, May 01, 2012 at 03:50:44PM +0300, Avi Kivity wrote:
> > On 04/30/2012 02:45 PM, Gleb Natapov wrote:
> > > This couid range does not exist on real HW and Intel spec says that
> > > "Information returned for highest basic information leaf" will be
> > > returned. Not very well defined.
> > >
> >
> > Correct in principle. But IIRC some old qemus supported the kvm cpuid
> > range without setting the hypervisor bit.
> >
> How old?
commit e19967a0e00c7af4efd2f2db12a7d5d7f0b387f7
Author: Avi Kivity <avi@qumranet.com>
Date: Tue Apr 22 11:44:19 2008 +0300
Set "hypervisor present" cpuid bit
This tells Microsoft guests that they are running under a hypervisor.
Signed-off-by: Avi Kivity <avi@qumranet.com
$ git describe --contains e19967a0e00c
kvm-80rc2~489
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-05-01 12:59 ` Avi Kivity
@ 2012-05-02 12:09 ` Gleb Natapov
2012-05-06 12:55 ` Avi Kivity
0 siblings, 1 reply; 9+ messages in thread
From: Gleb Natapov @ 2012-05-02 12:09 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, mtosatti, mst
On Tue, May 01, 2012 at 03:59:33PM +0300, Avi Kivity wrote:
> On 05/01/2012 03:55 PM, Gleb Natapov wrote:
> > On Tue, May 01, 2012 at 03:50:44PM +0300, Avi Kivity wrote:
> > > On 04/30/2012 02:45 PM, Gleb Natapov wrote:
> > > > This couid range does not exist on real HW and Intel spec says that
> > > > "Information returned for highest basic information leaf" will be
> > > > returned. Not very well defined.
> > > >
> > >
> > > Correct in principle. But IIRC some old qemus supported the kvm cpuid
> > > range without setting the hypervisor bit.
> > >
> > How old?
>
> commit e19967a0e00c7af4efd2f2db12a7d5d7f0b387f7
> Author: Avi Kivity <avi@qumranet.com>
> Date: Tue Apr 22 11:44:19 2008 +0300
>
> Set "hypervisor present" cpuid bit
>
> This tells Microsoft guests that they are running under a hypervisor.
>
> Signed-off-by: Avi Kivity <avi@qumranet.com
>
> $ git describe --contains e19967a0e00c
> kvm-80rc2~489
>
Since latest Windowses will not run on such old version correctly (may
use tsc for QPC) may be dropping support for Linuxes past 3.4 is a
possibility too?
--
Gleb.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-05-02 12:09 ` Gleb Natapov
@ 2012-05-06 12:55 ` Avi Kivity
0 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2012-05-06 12:55 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 05/02/2012 03:09 PM, Gleb Natapov wrote:
> > $ git describe --contains e19967a0e00c
> > kvm-80rc2~489
> >
> Since latest Windowses will not run on such old version correctly (may
> use tsc for QPC) may be dropping support for Linuxes past 3.4 is a
> possibility too?
>
>
These versions are *way* too old to support.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf.
2012-04-30 11:45 [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf Gleb Natapov
2012-04-30 16:37 ` Davidlohr Bueso
2012-05-01 12:50 ` Avi Kivity
@ 2012-05-06 13:00 ` Avi Kivity
2 siblings, 0 replies; 9+ messages in thread
From: Avi Kivity @ 2012-05-06 13:00 UTC (permalink / raw)
To: Gleb Natapov; +Cc: kvm, mtosatti, mst
On 04/30/2012 02:45 PM, Gleb Natapov wrote:
> This couid range does not exist on real HW and Intel spec says that
> "Information returned for highest basic information leaf" will be
> returned. Not very well defined.
>
>
Applied, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-05-06 13:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-30 11:45 [PATCH] kvm_para_available() should check hypervisor bit before accessing hypervisor cpuid leaf Gleb Natapov
2012-04-30 16:37 ` Davidlohr Bueso
2012-05-01 6:52 ` Gleb Natapov
2012-05-01 12:50 ` Avi Kivity
2012-05-01 12:55 ` Gleb Natapov
2012-05-01 12:59 ` Avi Kivity
2012-05-02 12:09 ` Gleb Natapov
2012-05-06 12:55 ` Avi Kivity
2012-05-06 13:00 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox