Liu, Jinsong
2010-Mar-21 15:09 UTC
[Xen-devel] [PATCH] Qemu: Add sanity check for vcpu config
>From 4d483620c863f626543fdc407c2951ceef49623e Mon Sep 17 00:00:00 2001From: 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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-21 17:15 UTC
[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. -- 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";_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Mar-22 17:21 UTC
[Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config
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
Liu, Jinsong
2010-Mar-23 02:17 UTC
RE: [Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Mar-23 07:22 UTC
Re: [Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config
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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Liu, Jinsong
2010-Mar-23 08:07 UTC
RE: [Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Mar-24 11:20 UTC
RE: [Xen-devel] Re: [PATCH] Qemu: Add sanity check for vcpu config
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.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel