[PATCH] KVM-USER: Check kvm extensions at runtime

[PATCH] KVM-USER: Check kvm extensions at runtime

[Gregory Haskins]

Newer userspace may run on an older kernel.  Therefore we need a way to 
check the capabilities of the kernel so that we can downgrade userspace
dynamically if necessary

Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
---

 qemu/qemu-kvm.c |   19 +++++++++++++++++++
 user/kvmctl.c   |   14 ++++++++++++++
 user/kvmctl.h   |    5 +++++
 3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 59e79bf..6096f21 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -697,6 +697,25 @@ int kvm_qemu_init()
 int kvm_qemu_create_context(void)
 {
     int i;
+    uint64_t ext;
+
+    if (kvm_check_extension(kvm_context, &ext) < 0) {
+	kvm_qemu_destroy();
+	return -1;
+    }
+
+    if (!(ext & KVM_EXTENSION_LAPIC) && kvm_apic_level) {
+	/*
+	 * Opps... the kernel doesnt support apic-emulation even though
+	 * the user wants it.  We must turn this off and warn the user
+	 */
+	kvm_apic_level = 0;
+
+	/* FIXME: We should log this officially */
+	printf("WARNING: older kernel does not support apic " \
+	       "emulation.  Falling back to userspace emulation.  Upgrade " \
+	       "your kernel/modules\n");
+    }
 
     if (kvm_create(kvm_context, phys_ram_size, kvm_apic_level,
 		   (void**)&phys_ram_base) < 0) {
diff --git a/user/kvmctl.c b/user/kvmctl.c
index ea86426..fcabc1a 100644
--- a/user/kvmctl.c
+++ b/user/kvmctl.c
@@ -889,3 +889,17 @@ int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, const sigset_t *sigset)
 	free(sigmask);
 	return r;
 }
+
+int kvm_check_extension(kvm_context_t kvm, uint64_t *ext)
+{
+	*ext = 0;
+
+	int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_APIC_MSG);
+	if (r < 0)
+		return r;
+
+	if (r)
+		*ext |= KVM_EXTENSION_LAPIC;
+
+	return 0;
+}
diff --git a/user/kvmctl.h b/user/kvmctl.h
index 07cbc08..5371d71 100644
--- a/user/kvmctl.h
+++ b/user/kvmctl.h
@@ -384,4 +384,9 @@ int kvm_dirty_pages_log_enable_all(kvm_context_t kvm);
  */
 int kvm_dirty_pages_log_reset(kvm_context_t kvm);
 
+#define KVM_EXTENSION_LAPIC (1<<0)
+
+int kvm_check_extension(kvm_context_t kvm, uint64_t *ext);
+
+
 #endif


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] KVM-USER: Check kvm extensions at runtime

[Avi Kivity]

Gregory Haskins wrote:
> Newer userspace may run on an older kernel.  Therefore we need a way to 
> check the capabilities of the kernel so that we can downgrade userspace
> dynamically if necessary
>
> Signed-off-by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
> ---
>
>  qemu/qemu-kvm.c |   19 +++++++++++++++++++
>  user/kvmctl.c   |   14 ++++++++++++++
>  user/kvmctl.h   |    5 +++++
>  3 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
> index 59e79bf..6096f21 100644
> --- a/qemu/qemu-kvm.c
> +++ b/qemu/qemu-kvm.c
> @@ -697,6 +697,25 @@ int kvm_qemu_init()
>  int kvm_qemu_create_context(void)
>  {
>      int i;
> +    uint64_t ext;
> +
> +    if (kvm_check_extension(kvm_context, &ext) < 0) {
> +	kvm_qemu_destroy();
> +	return -1;
> +    }
> +
> +    if (!(ext & KVM_EXTENSION_LAPIC) && kvm_apic_level) {
> +	/*
> +	 * Opps... the kernel doesnt support apic-emulation even though
> +	 * the user wants it.  We must turn this off and warn the user
> +	 */
> +	kvm_apic_level = 0;
> +
> +	/* FIXME: We should log this officially */
> +	printf("WARNING: older kernel does not support apic " \
> +	       "emulation.  Falling back to userspace emulation.  Upgrade " \
> +	       "your kernel/modules\n");
> +    }
>   

The user may not be in a position to upgrade the kernel.  Think distro 
kernel and userspace.  A warning is a bit severe.

>  
>      if (kvm_create(kvm_context, phys_ram_size, kvm_apic_level,
>  		   (void**)&phys_ram_base) < 0) {
> diff --git a/user/kvmctl.c b/user/kvmctl.c
> index ea86426..fcabc1a 100644
> --- a/user/kvmctl.c
> +++ b/user/kvmctl.c
> @@ -889,3 +889,17 @@ int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, const sigset_t *sigset)
>  	free(sigmask);
>  	return r;
>  }
> +
> +int kvm_check_extension(kvm_context_t kvm, uint64_t *ext)
> +{
> +	*ext = 0;
> +
> +	int r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_APIC_MSG);
> +	if (r < 0)
> +		return r;
>   

-errno for new code, please.

> +
> +	if (r)
> +		*ext |= KVM_EXTENSION_LAPIC;
> +
> +	return 0;
> +}
>   

How about a struct with a field per extension instead of bitmasks?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] KVM-USER: Check kvm extensions at runtime

[Gregory Haskins]

>>> On Wed, May 16, 2007 at  7:47 AM, in message <464AEF5B.3010104-atKUWr5tajBWk0Htik3J/w@public.gmane.org>,
Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote: 
> Gregory Haskins wrote:
>> Newer userspace may run on an older kernel.  Therefore we need a way to 
>> check the capabilities of the kernel so that we can downgrade userspace
>> dynamically if necessary
>>
>> Signed- off- by: Gregory Haskins <ghaskins-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>
>> ---
>>
>>  qemu/qemu- kvm.c |   19 +++++++++++++++++++
>>  user/kvmctl.c   |   14 ++++++++++++++
>>  user/kvmctl.h   |    5 +++++
>>  3 files changed, 38 insertions(+), 0 deletions(- )
>>
>> diff -- git a/qemu/qemu- kvm.c b/qemu/qemu- kvm.c
>> index 59e79bf..6096f21 100644
>> ---  a/qemu/qemu- kvm.c
>> +++ b/qemu/qemu- kvm.c
>> @@ - 697,6 +697,25 @@ int kvm_qemu_init()
>>  int kvm_qemu_create_context(void)
>>  {
>>      int i;
>> +    uint64_t ext;
>> +
>> +    if (kvm_check_extension(kvm_context, &ext) < 0) {
>> +	kvm_qemu_destroy();
>> +	return - 1;
>> +    }
>> +
>> +    if (!(ext & KVM_EXTENSION_LAPIC) && kvm_apic_level) {
>> +	/*
>> +	 * Opps... the kernel doesnt support apic- emulation even though
>> +	 * the user wants it.  We must turn this off and warn the user
>> +	 */
>> +	kvm_apic_level = 0;
>> +
>> +	/* FIXME: We should log this officially */
>> +	printf("WARNING: older kernel does not support apic " \
>> +	       "emulation.  Falling back to userspace emulation.  Upgrade " \
>> +	       "your kernel/modules\n");
>> +    }
>>   
> 
> The user may not be in a position to upgrade the kernel.  Think distro 
> kernel and userspace.  A warning is a bit severe.

Ack.  Should I drop the printf all together?  Or just get rid of the "WARNING" and "please upgrade" part?.  Also, is there a better way to log in this thing (instead of to stdout?)

All the rest of your comments: Ack.

> 
>>  
>>      if (kvm_create(kvm_context, phys_ram_size, kvm_apic_level,
>>  		   (void**)&phys_ram_base) < 0) {
>> diff -- git a/user/kvmctl.c b/user/kvmctl.c
>> index ea86426..fcabc1a 100644
>> ---  a/user/kvmctl.c
>> +++ b/user/kvmctl.c
>> @@ - 889,3 +889,17 @@ int kvm_set_signal_mask(kvm_context_t kvm, int vcpu, 
> const sigset_t *sigset)
>>  	free(sigmask);
>>  	return r;
>>  }
>> +
>> +int kvm_check_extension(kvm_context_t kvm, uint64_t *ext)
>> +{
>> +	*ext = 0;
>> +
>> +	int r = ioctl(kvm- >fd, KVM_CHECK_EXTENSION, KVM_APIC_MSG);
>> +	if (r < 0)
>> +		return r;
>>   
> 
> - errno for new code, please.
> 
>> +
>> +	if (r)
>> +		*ext |= KVM_EXTENSION_LAPIC;
>> +
>> +	return 0;
>> +}
>>   
> 
> How about a struct with a field per extension instead of bitmasks?




-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

Re: [PATCH] KVM-USER: Check kvm extensions at runtime

[Avi Kivity]

Gregory Haskins wrote:
>>> diff -- git a/qemu/qemu- kvm.c b/qemu/qemu- kvm.c
>>> index 59e79bf..6096f21 100644
>>> ---  a/qemu/qemu- kvm.c
>>> +++ b/qemu/qemu- kvm.c
>>> @@ - 697,6 +697,25 @@ int kvm_qemu_init()
>>>  int kvm_qemu_create_context(void)
>>>  {
>>>      int i;
>>> +    uint64_t ext;
>>> +
>>> +    if (kvm_check_extension(kvm_context, &ext) < 0) {
>>> +	kvm_qemu_destroy();
>>> +	return - 1;
>>> +    }
>>> +
>>> +    if (!(ext & KVM_EXTENSION_LAPIC) && kvm_apic_level) {
>>> +	/*
>>> +	 * Opps... the kernel doesnt support apic- emulation even though
>>> +	 * the user wants it.  We must turn this off and warn the user
>>> +	 */
>>> +	kvm_apic_level = 0;
>>> +
>>> +	/* FIXME: We should log this officially */
>>> +	printf("WARNING: older kernel does not support apic " \
>>> +	       "emulation.  Falling back to userspace emulation.  Upgrade " \
>>> +	       "your kernel/modules\n");
>>> +    }
>>>   
>>>       
>> The user may not be in a position to upgrade the kernel.  Think distro 
>> kernel and userspace.  A warning is a bit severe.
>>     
>
> Ack.  Should I drop the printf all together?  Or just get rid of the "WARNING" and "please upgrade" part?.  Also, is there a better way to log in this thing (instead of to stdout?)
>   

Maybe only display it if the user specified the apic level option on the 
qemu command line.  It's a bit messy, but will be useful once the bug 
reports start to flow in.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/