Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option

[Ian Campbell]

On Wed, 2011-05-25 at 11:54 +0100, Anthony PERARD wrote:
> On Wed, 25 May 2011, Ian Campbell wrote:
> 
> > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote:
> > > On 2011-05-24 18:06, Ian Campbell wrote:
> > > > On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote:
> > > >> From: Jan Kiszka <address@hidden>
> > > >>
> > > >> -machine somehow suggests that it selects the machine, but it doesn't.
> > > >> Fix that before this command is set in stone.
> > > >>
> > > >> Actually, -machine should supersede -M and allow to introduce arbitrary
> > > >> per-machine options to the command line. That will change the internal
> > > >> realization again, but we will be able to keep the user interface
> > > >> stable.
> > > >>
> > > >> CC: Anthony PERARD <address@hidden>
> > > >> Signed-off-by: Jan Kiszka <address@hidden>
> > > >
> > > > "-machine xenfv" doesn't work for me with this patch, it gives:
> > > >         Program received signal SIGSEGV, Segmentation fault.
> > > >         hypercall_buffer_cache_lock (xch=0x0) at xc_hcall_buf.c:36
> > > >         36      xc_hcall_buf.c: No such file or directory.
> > > >                 in xc_hcall_buf.c
> > > >         (gdb) bt
> > > >         #0  hypercall_buffer_cache_lock (xch=0x0) at xc_hcall_buf.c:36
> > > >         #1  0xb7d53f1d in hypercall_buffer_cache_alloc (xch=0x0, 
> > > > b=0xbffff77c, nr_pages=1) at xc_hcall_buf.c:52
> > > >         #2  xc__hypercall_buffer_alloc_pages (xch=0x0, b=0xbffff77c, 
> > > > nr_pages=1) at xc_hcall_buf.c:128
> > > >         #3  0xb7d54028 in xc__hypercall_buffer_alloc (xch=0x0, 
> > > > b=0xbffff77c, size=16) at xc_hcall_buf.c:162
> > > >         #4  0xb7d42719 in xc_get_hvm_param (handle=0x0, dom=248, 
> > > > param=5, value=0xbffff810) at xc_domain.c:1078
> > > >         #5  0x08252777 in xen_hvm_init () at 
> > > > /home/ianc/devel/qemu.git/xen-all.c:803
> > > >         #6  0x082921e9 in pc_xen_hvm_init (ram_size=536870912, 
> > > > boot_device=0xbffffabb "cda", kernel_filename=0x0, 
> > > > kernel_cmdline=0x82d648f "", initrd_filename=0x0, cpu_model=0x0) at 
> > > > /home/ianc/devel/qemu.git/hw/pc_piix.c:246
> > > >         #7  0x081e3f0e in main (argc=26, argv=0xbffffba4, 
> > > > envp=0xbffffc10) at /home/ianc/devel/qemu.git/vl.c:3162
> > > >
> > > > I suspect this is because xen_init() (which sets xch) is never called,
> > > > perhaps because it causes the accelerator to always be tcg? If I use
> > > > "-machine xenfv,accel=xen" then it works as expected, -M continues to
> > > > work too AFAICT.
> > > >
> > > > It would be nice to retain the behaviour of defaulting to accel=xen for
> > > > machine=xenfv. (to be honest I can't quite see where this behaviour came
> > > > from, nor what about this patch changes it...)
> > >
> > > Well, first of all I think this revealed a Xen bug because it crashes
> > > when you try to run xenfv with an inappropriate accelerator, no? What is
> > > the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv
> > > -machine accel=tcg?
> >
> > Unsurprisingly it crashed...
> >
> > I'm not sure if this is a bug in the xen side of simply a case of
> > providing enough rope. I didn't really follow the threads which resulted
> > in the accel stuff all that closely so I'm not sure what the intention
> > was, it seems to me that the accel option is invalid with certain
> > machine types though.
> 
> This options was here to initialise the xen stuff early in the QEMU
> initilisation, and the purpose was to replace --enable-xen (and
> --enable-kvm) at the same occasion.
> 
> But since most of the xen stuff have been put in xen_hvm_init, called
> by machine{xenfv}->init(), it's could make sense to call xen_init from
> xen_hvm_init, or just check if it have been called.

I posted a patch to do that check elsewhere in this thread, I did wonder
about moving the call too but I'll leave that up to you.

> > I suppose in theory -machine xenpv,accel=kvm might result in xenner or
> > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?).
> > For -machine xenfv I don't expect anything other than accel=xen makes
> > much sense.
> 
> But a -machine pc,accel=xen could result in a xenfv machine. We already
> use the pc machine for xenfv with few difference. So a -machine
> pc,accel=xen:kvm could use the hypervisor present on the machine :)
> even if it's not realy useful in many case.

That seems reasonable, I guess.

Ian.

-- 
Ian Campbell
Current Noise: She Said Destroy - No Zen

Tell me, O Octopus, I begs,
Is those things arms, or is they legs?
I marvel at thee, Octopus;
If I were thou, I'd call me us.
                -- Ogden Nash