[PATCH] Qemu: Add sanity check for vcpu config

[PATCH] Qemu: Add sanity check for vcpu config

[Liu, Jinsong]

[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]

>From 4d483620c863f626543fdc407c2951ceef49623e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
Date: Sun, 21 Mar 2010 22:48:04 +0800
Subject: [PATCH] Qemu: Add sanity check for vcpu config

Currently Xen/Qemu support max 128 vcpus. To avoid mis-setting
at config file, this patch add sanity check for vcpu config.
1. maxvcpus and vcpus should no more than HVM_MAX_VCPUS (128)
2. vcpus should no more than maxvcpus.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 vl.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 30debd7..d69d90c 100644
--- a/vl.c
+++ b/vl.c
@@ -4706,6 +4706,34 @@ static void termsig_setup(void)
 
 #endif
 
+/* 32bit Hamming weight */
+unsigned int hweight32(unsigned int w)
+{
+	unsigned int res = w - ((w >> 1) & 0x55555555);
+	res = (res & 0x33333333) + ((res >> 2) & 0x33333333);
+	res = (res + (res >> 4)) & 0x0F0F0F0F;
+	res = res + (res >> 8);
+	return (res + (res >> 16)) & 0x000000FF;
+}
+
+static void vcpu_sanity_check(void)
+{
+    int i, vcpu_avail_weight = 0;
+
+    for (i=0; i<((HVM_MAX_VCPUS + 31)/32); i++)
+        vcpu_avail_weight += hweight32(vcpu_avail[i]);
+
+    if (vcpus > HVM_MAX_VCPUS) {
+        fprintf(stderr, "maxvcpus and vcpus should not more than %d\n",
+                         HVM_MAX_VCPUS);
+        exit(EXIT_FAILURE);
+    }
+    if (vcpu_avail_weight > vcpus) {
+        fprintf(stderr, "vcpus should not more than maxvcpus\n");
+        exit(EXIT_FAILURE);
+    }
+}
+
 #define STEP   8   /* 8 characters fill uint32_t bitmap */
 #define SPACE  8   /* space for non-hex characters in vcpu str */
 #define MAX_VCPU_STR_LEN    ((HVM_MAX_VCPUS + 3)/4 + SPACE)
@@ -5569,6 +5597,8 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    vcpu_sanity_check();
+
     if (nographic) {
        if (serial_device_index == 0)
            serial_devices[0] = "stdio";
-- 
1.5.6.4

[-- Attachment #2: qemu-vcpu-sanity-check.patch --]
[-- Type: application/octet-stream, Size: 2028 bytes --]

From 4d483620c863f626543fdc407c2951ceef49623e Mon Sep 17 00:00:00 2001
From: Liu, Jinsong <jinsong.liu@intel.com>
Date: Sun, 21 Mar 2010 22:48:04 +0800
Subject: [PATCH] Qemu: Add sanity check for vcpu config

Currently Xen/Qemu support max 128 vcpus. To avoid mis-setting
at config file, this patch add sanity check for vcpu config.
1. maxvcpus and vcpus should no more than HVM_MAX_VCPUS (128)
2. vcpus should no more than maxvcpus.

Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
---
 vl.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 30debd7..d69d90c 100644
--- a/vl.c
+++ b/vl.c
@@ -4706,6 +4706,34 @@ static void termsig_setup(void)
 
 #endif
 
+/* 32bit Hamming weight */
+unsigned int hweight32(unsigned int w)
+{
+	unsigned int res = w - ((w >> 1) & 0x55555555);
+	res = (res & 0x33333333) + ((res >> 2) & 0x33333333);
+	res = (res + (res >> 4)) & 0x0F0F0F0F;
+	res = res + (res >> 8);
+	return (res + (res >> 16)) & 0x000000FF;
+}
+
+static void vcpu_sanity_check(void)
+{
+    int i, vcpu_avail_weight = 0;
+
+    for (i=0; i<((HVM_MAX_VCPUS + 31)/32); i++)
+        vcpu_avail_weight += hweight32(vcpu_avail[i]);
+
+    if (vcpus > HVM_MAX_VCPUS) {
+        fprintf(stderr, "maxvcpus and vcpus should not more than %d\n",
+                         HVM_MAX_VCPUS);
+        exit(EXIT_FAILURE);
+    }
+    if (vcpu_avail_weight > vcpus) {
+        fprintf(stderr, "vcpus should not more than maxvcpus\n");
+        exit(EXIT_FAILURE);
+    }
+}
+
 #define STEP   8   /* 8 characters fill uint32_t bitmap */
 #define SPACE  8   /* space for non-hex characters in vcpu str */
 #define MAX_VCPU_STR_LEN    ((HVM_MAX_VCPUS + 3)/4 + SPACE)
@@ -5569,6 +5597,8 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    vcpu_sanity_check();
+
     if (nographic) {
        if (serial_device_index == 0)
            serial_devices[0] = "stdio";
-- 
1.5.6.4


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] Qemu: Add sanity check for vcpu config

[Keir Fraser]

Wouldn't xend be a more sensible place to do the check? It can certainly
result in a better error message, I would have thought.

 -- Keir

On 21/03/2010 15:09, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:

> From 4d483620c863f626543fdc407c2951ceef49623e Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong <jinsong.liu@intel.com>
> Date: Sun, 21 Mar 2010 22:48:04 +0800
> Subject: [PATCH] Qemu: Add sanity check for vcpu config
> 
> Currently Xen/Qemu support max 128 vcpus. To avoid mis-setting
> at config file, this patch add sanity check for vcpu config.
> 1. maxvcpus and vcpus should no more than HVM_MAX_VCPUS (128)
> 2. vcpus should no more than maxvcpus.
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
> ---
>  vl.c |   30 ++++++++++++++++++++++++++++++
>  1 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 30debd7..d69d90c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4706,6 +4706,34 @@ static void termsig_setup(void)
>  
>  #endif
>  
> +/* 32bit Hamming weight */
> +unsigned int hweight32(unsigned int w)
> +{
> + unsigned int res = w - ((w >> 1) & 0x55555555);
> + res = (res & 0x33333333) + ((res >> 2) & 0x33333333);
> + res = (res + (res >> 4)) & 0x0F0F0F0F;
> + res = res + (res >> 8);
> + return (res + (res >> 16)) & 0x000000FF;
> +}
> +
> +static void vcpu_sanity_check(void)
> +{
> +    int i, vcpu_avail_weight = 0;
> +
> +    for (i=0; i<((HVM_MAX_VCPUS + 31)/32); i++)
> +        vcpu_avail_weight += hweight32(vcpu_avail[i]);
> +
> +    if (vcpus > HVM_MAX_VCPUS) {
> +        fprintf(stderr, "maxvcpus and vcpus should not more than %d\n",
> +                         HVM_MAX_VCPUS);
> +        exit(EXIT_FAILURE);
> +    }
> +    if (vcpu_avail_weight > vcpus) {
> +        fprintf(stderr, "vcpus should not more than maxvcpus\n");
> +        exit(EXIT_FAILURE);
> +    }
> +}
> +
>  #define STEP   8   /* 8 characters fill uint32_t bitmap */
>  #define SPACE  8   /* space for non-hex characters in vcpu str */
>  #define MAX_VCPU_STR_LEN    ((HVM_MAX_VCPUS + 3)/4 + SPACE)
> @@ -5569,6 +5597,8 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    vcpu_sanity_check();
> +
>      if (nographic) {
>         if (serial_device_index == 0)
>             serial_devices[0] = "stdio";

Re: [PATCH] Qemu: Add sanity check for vcpu config

[Ian Jackson]

Keir Fraser writes ("[Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config"):
> Wouldn't xend be a more sensible place to do the check? It can certainly
> result in a better error message, I would have thought.

Quite so.  If you add feature to xend, it should be changed in libxl
too.

Ian.

RE: Re: [PATCH] Qemu: Add sanity check for vcpu config

[Liu, Jinsong]

Keir,

Have you decided where should vcpu sanity check be located?
We add vcpu sanity check at qemu because:
1. xen/qemu itself has knowledge like HVM_MAX_VCPUS, xend doesn't have such knowledge;
2. there are many ways to transfer config/cmdline para to qemu, xm/xend is one of them which used at Xen. On other system like KVM, there is no xm/xend at all. So naturely qemu is a better place to do sanity check than xend;
3. even if we add sanity check at xend, qemu still need do sanity check. After all, qemu cannot totally trust its input;

Thanks,
Jinsong

Ian Jackson wrote:
> Keir Fraser writes ("[Xen-devel] Re: [PATCH] Qemu: Add sanity check
> for vcpu config"): 
>> Wouldn't xend be a more sensible place to do the check? It can
>> certainly result in a better error message, I would have thought.
> 
> Quite so.  If you add feature to xend, it should be changed in libxl
> too.
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: Re: [PATCH] Qemu: Add sanity check for vcpu config

[Keir Fraser]

I'm not dead against a sanity check in qemu.

 -- Keir

On 23/03/2010 02:17, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:

> Keir,
> 
> Have you decided where should vcpu sanity check be located?
> We add vcpu sanity check at qemu because:
> 1. xen/qemu itself has knowledge like HVM_MAX_VCPUS, xend doesn't have such
> knowledge;
> 2. there are many ways to transfer config/cmdline para to qemu, xm/xend is one
> of them which used at Xen. On other system like KVM, there is no xm/xend at
> all. So naturely qemu is a better place to do sanity check than xend;
> 3. even if we add sanity check at xend, qemu still need do sanity check. After
> all, qemu cannot totally trust its input;
> 
> Thanks,
> Jinsong
> 
> Ian Jackson wrote:
>> Keir Fraser writes ("[Xen-devel] Re: [PATCH] Qemu: Add sanity check
>> for vcpu config"):
>>> Wouldn't xend be a more sensible place to do the check? It can
>>> certainly result in a better error message, I would have thought.
>> 
>> Quite so.  If you add feature to xend, it should be changed in libxl
>> too.
>> 
>> Ian.
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
> 

RE: Re: [PATCH] Qemu: Add sanity check for vcpu config

[Liu, Jinsong]

Yes, I know :)
I just speaking out my thinking.

Thanks,
Jinsong

Keir Fraser wrote:
> I'm not dead against a sanity check in qemu.
> 
>  -- Keir
> 
> On 23/03/2010 02:17, "Liu, Jinsong" <jinsong.liu@intel.com> wrote:
> 
>> Keir,
>> 
>> Have you decided where should vcpu sanity check be located?
>> We add vcpu sanity check at qemu because:
>> 1. xen/qemu itself has knowledge like HVM_MAX_VCPUS, xend doesn't
>> have such knowledge; 
>> 2. there are many ways to transfer config/cmdline para to qemu,
>> xm/xend is one of them which used at Xen. On other system like KVM,
>> there is no xm/xend at all. So naturely qemu is a better place to do
>> sanity check than xend; 
>> 3. even if we add sanity check at xend, qemu still need do sanity
>> check. After all, qemu cannot totally trust its input;
>> 
>> Thanks,
>> Jinsong
>> 
>> Ian Jackson wrote:
>>> Keir Fraser writes ("[Xen-devel] Re: [PATCH] Qemu: Add sanity check
>>> for vcpu config"):
>>>> Wouldn't xend be a more sensible place to do the check? It can
>>>> certainly result in a better error message, I would have thought.
>>> 
>>> Quite so.  If you add feature to xend, it should be changed in
>>> libxl too. 
>>> 
>>> Ian.
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xensource.com
>>> http://lists.xensource.com/xen-devel

RE: Re: [PATCH] Qemu: Add sanity check for vcpu config

[Ian Jackson]

Liu, Jinsong writes ("RE: [Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config"):
> 3. even if we add sanity check at xend, qemu still need do sanity
> check. After all, qemu cannot totally trust its input;

This is a very good argument.  I will apply your patch.

Ian.