On Mon, 15 Nov 2010, Alexander Graf wrote:
>
> On 21.10.2010, at 19:36, Anthony.Perard@xxxxxxxxxx wrote:
>
> > From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >
> > This option gives the ability to switch one "accelerator" like kvm, xen
> > or the default one tcg. We can specify more than one accelerator by
> > separate them by a comma. QEMU will try each one and use the first whose
> > works.
> >
> > So,
> >
> > -accel xen,kvm,tcg
> >
> > which would try Xen support first, then KVM and finaly tcg if none of
> > the other works.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> > qemu-options.hx | 10 ++++++
> > vl.c | 86
> > ++++++++++++++++++++++++++++++++++++++++++++++++-------
> > 2 files changed, 85 insertions(+), 11 deletions(-)
> >
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 718d47a..ba8385b 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -1925,6 +1925,16 @@ Enable KVM full virtualization support. This option
> > is only available
> > if KVM support is enabled when compiling.
> > ETEXI
> >
> > +DEF("accel", HAS_ARG, QEMU_OPTION_accel, \
> > + "-accel accel use an accelerator (kvm,xen,tcg), default is tcg\n",
> > QEMU_ARCH_ALL)
> > +STEXI
> > +@item -accel @var{accel}[,@var{accel}[,...]]
> > +@findex -accel
> > +This is use to enable an accelerator, in kvm,xen,tcg.
> > +By default, it use only tcg. If there a more than one accelerator
> > +specified, the next one is used if the first don't work.
> > +ETEXI
> > +
> > DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
> > "-xen-domid id specify xen guest domain id\n", QEMU_ARCH_ALL)
> > DEF("xen-create", 0, QEMU_OPTION_xen_create,
> > diff --git a/vl.c b/vl.c
> > index df414ef..40a26ee 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1750,6 +1750,74 @@ static int debugcon_parse(const char *devname)
> > return 0;
> > }
> >
> > +static struct {
> > + const char *opt_name;
> > + const char *name;
> > + int (*available)(void);
> > + int (*init)(int smp_cpus);
> > + int *allowed;
> > +} accel_list[] = {
> > + { "tcg", "tcg", NULL, NULL, NULL },
> > + { "kvm", "KVM", kvm_available, kvm_init, &kvm_allowed },
>
> Thinking about this a bit more ...
>
> kvm_available is a function pointer that gets #defined to (0) when we don't
> have KVM available. I can imagine some compiler might throw a warning on us
> for this one day.
>
> Is there a valid reason not to do
>
> static inline int kvm_enabled()
> {
> #ifdef CONFIG_KVM
> return kvm_allowed;
> #else
> return 0;
> #endif
> }
>
> That should compile into the exact same code but be valid for function
> pointers.
I will do this change, as well as for the two others patches.
> > +};
> > +
> > +static int accel_parse_init(const char *opts)
> > +{
> > + const char *p = opts;
> > + char buf[10];
> > + int i, ret;
> > + bool accel_initalised = 0;
> > + bool init_failed = 0;
> > +
> > + while (!accel_initalised && *p != '\0') {
> > + if (*p == ',') {
> > + p++;
> > + }
> > + p = get_opt_name(buf, sizeof (buf), p, ',');
> > + for (i = 0; i < ARRAY_SIZE(accel_list); i++) {
> > + if (strcmp(accel_list[i].opt_name, buf) == 0) {
> > + if (accel_list[i].init) {
>
> If you'd make the function pointers mandatory, you could also get rid of a
> lot of checks here. Just introduce a new small stub helper for tcg_init and
> tcg_allowed and always call the pointers.
Yes, this will make the code more readable.
> I take back my Acked-by. Sorry, I guess I should have read things in more
> detail first O_o. I still believe that this patch can go in before the
> others, but I'd at least like to see some comments on the (0) pointer thing
> :).
And I will send the patch out of the patch series.
Thanks,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home