Patchwork Remove hardcoded xen-platform device initialization

login
register
mail settings
Submitter Paul Durrant
Date June 13, 2013, 9:50 a.m.
Message ID <1371117054-5694-1-git-send-email-paul.durrant@citrix.com>
Download mbox | patch
Permalink /patch/251023/
State New
Headers show

Comments

Paul Durrant - June 13, 2013, 9:50 a.m.
The xen-platform device should be initialized by the Xen toolstack by
passing the appropriate -device argument on the command line.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    3 ---
 1 file changed, 3 deletions(-)
Stefano Stabellini - June 13, 2013, 5:33 p.m.
On Thu, 13 Jun 2013, Paul Durrant wrote:
> The xen-platform device should be initialized by the Xen toolstack by
> passing the appropriate -device argument on the command line.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

This patch is problematic because we can't know for sure the version of
upstream QEMU that is going to be used with Xen.
If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
guests won't be able to use PV drivers.



>  hw/i386/pc_piix.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d618570..e25012d 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -174,9 +174,6 @@ static void pc_init1(MemoryRegion *system_memory,
>      pc_register_ferr_irq(gsi[13]);
>  
>      pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> -    if (xen_enabled()) {
> -        pci_create_simple(pci_bus, -1, "xen-platform");
> -    }
>  
>      /* init basic PC hardware */
>      pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled());
Ian Campbell - June 13, 2013, 5:44 p.m.
On Thu, 2013-06-13 at 18:33 +0100, Stefano Stabellini wrote:
> On Thu, 13 Jun 2013, Paul Durrant wrote:
> > The xen-platform device should be initialized by the Xen toolstack by
> > passing the appropriate -device argument on the command line.
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> This patch is problematic because we can't know for sure the version of
> upstream QEMU that is going to be used with Xen.
> If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
> guests won't be able to use PV drivers.

Is the right answer a lever to disable, rather than enable, it?

A workaround for the situation you envisage is to use the
device_model_args config option, not ideal though.

> 
> 
> 
> >  hw/i386/pc_piix.c |    3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d618570..e25012d 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -174,9 +174,6 @@ static void pc_init1(MemoryRegion *system_memory,
> >      pc_register_ferr_irq(gsi[13]);
> >  
> >      pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
> > -    if (xen_enabled()) {
> > -        pci_create_simple(pci_bus, -1, "xen-platform");
> > -    }
> >  
> >      /* init basic PC hardware */
> >      pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled());
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Paolo Bonzini - June 13, 2013, 6:02 p.m.
Il 13/06/2013 13:44, Ian Campbell ha scritto:
> On Thu, 2013-06-13 at 18:33 +0100, Stefano Stabellini wrote:
>> On Thu, 13 Jun 2013, Paul Durrant wrote:
>>> The xen-platform device should be initialized by the Xen toolstack by
>>> passing the appropriate -device argument on the command line.
>>>
>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>
>> This patch is problematic because we can't know for sure the version of
>> upstream QEMU that is going to be used with Xen.
>> If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
>> guests won't be able to use PV drivers.
> 
> Is the right answer a lever to disable, rather than enable, it?
> 
> A workaround for the situation you envisage is to use the
> device_model_args config option, not ideal though.

I think the right solution for this is to move towards using the normal
"-M pc" machine.  libxl can simply use "-M pc -machine accel=xen -device
xen-platform-pv"; older versions that use the xenfv machine will still work.

And if you do this, you will also get the benefit of versioned machine
types.

Paolo
Paul Durrant - June 14, 2013, 8:50 a.m.
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 13 June 2013 18:33
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Subject: Re: [Qemu-devel] [PATCH] Remove hardcoded xen-platform device
> initialization
> 
> On Thu, 13 Jun 2013, Paul Durrant wrote:
> > The xen-platform device should be initialized by the Xen toolstack by
> > passing the appropriate -device argument on the command line.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> This patch is problematic because we can't know for sure the version of
> upstream QEMU that is going to be used with Xen.
> If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
> guests won't be able to use PV drivers.
> 

Is there not a compatibility matrix? The hardcoded init. is just blatantly the wrong thing to be doing and it needs to go.

Could my accompanying toolstack patch not be backported to the next 4.2 release as mitigation?

  Paul
Paul Durrant - June 14, 2013, 8:56 a.m.
> -----Original Message-----

> From: Ian Campbell

> Sent: 13 June 2013 18:44

> To: Stefano Stabellini

> Cc: Paul Durrant; qemu-devel@nongnu.org; xen-devel@lists.xen.org

> Subject: Re: [Xen-devel] [Qemu-devel] [PATCH] Remove hardcoded xen-

> platform device initialization

> 

> On Thu, 2013-06-13 at 18:33 +0100, Stefano Stabellini wrote:

> > On Thu, 13 Jun 2013, Paul Durrant wrote:

> > > The xen-platform device should be initialized by the Xen toolstack by

> > > passing the appropriate -device argument on the command line.

> > >

> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

> >

> > This patch is problematic because we can't know for sure the version of

> > upstream QEMU that is going to be used with Xen.

> > If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,

> > guests won't be able to use PV drivers.

> 

> Is the right answer a lever to disable, rather than enable, it?

> 


I didn't want to add yes another xen specific command line argument to QEMU; it feels wrong when there's already a perfectly good way of specifying an arbitrary device on the command line, but I suppose I could add a xl version argument or somesuch and gate the device creation on xl version >= 4.3.0? Calling out the compatibility of different versions of xl with different versions of QEMU in some document seems like a better way to go though otherwise this sort of issue is probably going to come up again in future.

  Paul

> A workaround for the situation you envisage is to use the

> device_model_args config option, not ideal though.

> 

> >

> >

> >

> > >  hw/i386/pc_piix.c |    3 ---

> > >  1 file changed, 3 deletions(-)

> > >

> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c

> > > index d618570..e25012d 100644

> > > --- a/hw/i386/pc_piix.c

> > > +++ b/hw/i386/pc_piix.c

> > > @@ -174,9 +174,6 @@ static void pc_init1(MemoryRegion

> *system_memory,

> > >      pc_register_ferr_irq(gsi[13]);

> > >

> > >      pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);

> > > -    if (xen_enabled()) {

> > > -        pci_create_simple(pci_bus, -1, "xen-platform");

> > > -    }

> > >

> > >      /* init basic PC hardware */

> > >      pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy,

> xen_enabled());

> >

> >

> > _______________________________________________

> > Xen-devel mailing list

> > Xen-devel@lists.xen.org

> > http://lists.xen.org/xen-devel

>
Paul Durrant - June 14, 2013, 9 a.m.
> -----Original Message-----

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo

> Bonzini

> Sent: 13 June 2013 19:03

> To: Ian Campbell

> Cc: Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org; xen-

> devel@lists.xen.org

> Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device

> initialization

> 

> Il 13/06/2013 13:44, Ian Campbell ha scritto:

> > On Thu, 2013-06-13 at 18:33 +0100, Stefano Stabellini wrote:

> >> On Thu, 13 Jun 2013, Paul Durrant wrote:

> >>> The xen-platform device should be initialized by the Xen toolstack by

> >>> passing the appropriate -device argument on the command line.

> >>>

> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

> >>

> >> This patch is problematic because we can't know for sure the version of

> >> upstream QEMU that is going to be used with Xen.

> >> If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,

> >> guests won't be able to use PV drivers.

> >

> > Is the right answer a lever to disable, rather than enable, it?

> >

> > A workaround for the situation you envisage is to use the

> > device_model_args config option, not ideal though.

> 

> I think the right solution for this is to move towards using the normal

> "-M pc" machine.  libxl can simply use "-M pc -machine accel=xen -device

> xen-platform-pv"; older versions that use the xenfv machine will still work.

> 

> And if you do this, you will also get the benefit of versioned machine

> types.

> 


Thanks. I'll have a look at that.

  Paul
Paul Durrant - June 14, 2013, 10:38 a.m.
> -----Original Message-----

> From: qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org

> [mailto:qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org] On

> Behalf Of Paul Durrant

> Sent: 14 June 2013 10:01

> To: Paolo Bonzini; Ian Campbell

> Cc: xen-devel@lists.xen.org; qemu-devel@nongnu.org; Stefano Stabellini

> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-

> platform device initialization

> 

> > -----Original Message-----

> > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo

> > Bonzini

> > Sent: 13 June 2013 19:03

> > To: Ian Campbell

> > Cc: Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org; xen-

> > devel@lists.xen.org

> > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device

> > initialization

> >

> > Il 13/06/2013 13:44, Ian Campbell ha scritto:

> > > On Thu, 2013-06-13 at 18:33 +0100, Stefano Stabellini wrote:

> > >> On Thu, 13 Jun 2013, Paul Durrant wrote:

> > >>> The xen-platform device should be initialized by the Xen toolstack by

> > >>> passing the appropriate -device argument on the command line.

> > >>>

> > >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

> > >>

> > >> This patch is problematic because we can't know for sure the version of

> > >> upstream QEMU that is going to be used with Xen.

> > >> If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,

> > >> guests won't be able to use PV drivers.

> > >

> > > Is the right answer a lever to disable, rather than enable, it?

> > >

> > > A workaround for the situation you envisage is to use the

> > > device_model_args config option, not ideal though.

> >

> > I think the right solution for this is to move towards using the normal

> > "-M pc" machine.  libxl can simply use "-M pc -machine accel=xen -device

> > xen-platform-pv"; older versions that use the xenfv machine will still work.

> >

> > And if you do this, you will also get the benefit of versioned machine

> > types.

> >

> 

> Thanks. I'll have a look at that.

> 


It's a little more complicated than I thought. There are two machine types, xenpv and xenfv (i.e. HVM), and they share the accel=xen option. Thus QEMU attaches to the VM in the machine code rather than the accelerator init code. Using -M pc is therefore not an option, unless we perhaps have separate accel options. Either way this doesn't address the backwards compatibility issue. I think QEMU is going to need to know which version of the Xen toolstack invoked it unless we specify a compatibility matrix.

  Paul
Paolo Bonzini - June 14, 2013, 1:50 p.m.
Il 14/06/2013 06:38, Paul Durrant ha scritto:
>>>> I think the right solution for this is to move towards using
>>>> the normal "-M pc" machine.  libxl can simply use "-M pc
>>>> -machine accel=xen -device xen-platform-pv"; older versions
>>>> that use the xenfv machine will still work.
>>>> 
>>>> And if you do this, you will also get the benefit of
>>>> versioned machine types.
>>>> 
>> 
>> Thanks. I'll have a look at that.
>
> It's a little more complicated than I thought. There are two machine
> types, xenpv and xenfv (i.e. HVM), and they share the accel=xen
> option.

Yes, I was talking of xenfv only.

> Thus QEMU attaches to the VM in the machine code rather than
> the accelerator init code. Using -M pc is therefore not an option,
> unless we perhaps have separate accel options.

You're talking about xen_hvm_init, right?  IIRC there is a hypercall
that lets you know if a domain is PV or FV so you could move large parts
of it to accelerator init.  What's left can be done in "if
(xen_enabled())" (especially those parts that have matching TCG/KVM code
in normal "-M pc" initialization, and have that code currently disabled
for Xen: unifying the two or at least doing an if/else would be nicer).

> Either way this
> doesn't address the backwards compatibility issue. I think QEMU is
> going to need to know which version of the Xen toolstack invoked it
> unless we specify a compatibility matrix.

Yes, "xenfv" needs to stay as legacy for compatibility purposes.  But if
you move the toolchain and QEMU towards using "-M pc" (requiring new
QEMU for new toolchains that do) it would be a very nice cleanup.  It is
also needed if you ever want to support Q35/PCIe.

Paolo
Paolo Bonzini - June 14, 2013, 1:52 p.m.
Il 14/06/2013 04:50, Paul Durrant ha scritto:
>> > This patch is problematic because we can't know for sure the version of
>> > upstream QEMU that is going to be used with Xen.
>> > If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
>> > guests won't be able to use PV drivers.
>> > 
> Is there not a compatibility matrix? The hardcoded init. is just blatantly the wrong thing to be doing and it needs to go.
> 
> Could my accompanying toolstack patch not be backported to the next 4.2 release as mitigation?

I think Ian is right.  You should revert the toolstack patch and not
apply this one for now.  Then aim at using "-M pc" in 4.4 (and require
1.6 or 1.7), so that there is no need for a compatibility matrix.

Paolo
Paul Durrant - June 14, 2013, 1:57 p.m.
> -----Original Message-----
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: 14 June 2013 14:53
> To: Paul Durrant
> Cc: Stefano Stabellini; qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Subject: Re: [PATCH] Remove hardcoded xen-platform device initialization
> 
> Il 14/06/2013 04:50, Paul Durrant ha scritto:
> >> > This patch is problematic because we can't know for sure the version of
> >> > upstream QEMU that is going to be used with Xen.
> >> > If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2,
> >> > guests won't be able to use PV drivers.
> >> >
> > Is there not a compatibility matrix? The hardcoded init. is just blatantly the
> wrong thing to be doing and it needs to go.
> >
> > Could my accompanying toolstack patch not be backported to the next 4.2
> release as mitigation?
> 
> I think Ian is right.  You should revert the toolstack patch and not
> apply this one for now.  Then aim at using "-M pc" in 4.4 (and require
> 1.6 or 1.7), so that there is no need for a compatibility matrix.
> 

As I said in another mail on the thread, I don't think we'll ever be able to use -M pc, so I'm planning to create a xenfv-2.0 machine type and leave xenfv with the hardcoded device creation semantic. Then I'll add code to libxl to parse qemu's response to -M help and choose xenfv-2.0 if it exists (and specify the -device argument accordingly).

  Paul
Paul Durrant - June 14, 2013, 2:11 p.m.
> -----Original Message-----

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo

> Bonzini

> Sent: 14 June 2013 14:51

> To: Paul Durrant

> Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-

> devel@lists.xen.org

> Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device

> initialization

> 

> Il 14/06/2013 06:38, Paul Durrant ha scritto:

> >>>> I think the right solution for this is to move towards using

> >>>> the normal "-M pc" machine.  libxl can simply use "-M pc

> >>>> -machine accel=xen -device xen-platform-pv"; older versions

> >>>> that use the xenfv machine will still work.

> >>>>

> >>>> And if you do this, you will also get the benefit of

> >>>> versioned machine types.

> >>>>

> >>

> >> Thanks. I'll have a look at that.

> >

> > It's a little more complicated than I thought. There are two machine

> > types, xenpv and xenfv (i.e. HVM), and they share the accel=xen

> > option.

> 

> Yes, I was talking of xenfv only.

> 

> > Thus QEMU attaches to the VM in the machine code rather than

> > the accelerator init code. Using -M pc is therefore not an option,

> > unless we perhaps have separate accel options.

> 

> You're talking about xen_hvm_init, right?  IIRC there is a hypercall

> that lets you know if a domain is PV or FV so you could move large parts

> of it to accelerator init.  What's left can be done in "if

> (xen_enabled())" (especially those parts that have matching TCG/KVM code

> in normal "-M pc" initialization, and have that code currently disabled

> for Xen: unifying the two or at least doing an if/else would be nicer).

> 

> > Either way this

> > doesn't address the backwards compatibility issue. I think QEMU is

> > going to need to know which version of the Xen toolstack invoked it

> > unless we specify a compatibility matrix.

> 

> Yes, "xenfv" needs to stay as legacy for compatibility purposes.  But if

> you move the toolchain and QEMU towards using "-M pc" (requiring new

> QEMU for new toolchains that do) it would be a very nice cleanup.  It is

> also needed if you ever want to support Q35/PCIe.

> 


I think we're still going to need -M xenpv, I think; it's quite distinct from pc. I guess we could use -M pc for HVM and gate the accel code as you suggest but, if that's the way we're going, it would seem more logical just to ditch the accel code for xenpv completely (assuming we can do all we need from the machine init) and then use -M pc -accel=xen for HVM guests going forward. But that does rather screw up my autodiscovery plans because I would not know, for a given qemu binary, which machine type to use. If I create a new xenfv-2.0 machine type though I *can* do auto discovery... in which case do we need the -accel=xen option at all?

  Paul
Paolo Bonzini - June 14, 2013, 2:57 p.m.
Il 14/06/2013 10:11, Paul Durrant ha scritto:
> I think we're still going to need -M xenpv, I think; it's quite
> distinct from pc.

Of course!  Even more: "-M xenpv" should be reused on ARM.

> I guess we could use -M pc for HVM and gate the
> accel code as you suggest but, if that's the way we're going, it
> would seem more logical just to ditch the accel code for xenpv
> completely (assuming we can do all we need from the machine init) and
> then use -M pc -accel=xen for HVM guests going forward.

There is common code between pv and fv, and that one definitely belongs
in xen_init.  Most fv-only code probably should be in pc_init.  The rest
should move to xen_init though, because it would apply just as well for
example to Q35.  It's a bit ugly to have fv-only code there, but it's
better than having a Xen-specific machine type.  Xen/KVM/TCG should be
as similar as possible at the QEMU level, any difference should be
handled in the toolstack.

> But that does
> rather screw up my autodiscovery plans because I would not know, for
> a given qemu binary, which machine type to use.

There's no need for that.  4.4 can just use "-M pc" unconditionally,
<=4.3 will just use "-M xenfv" unconditionally.

> If I create a new
> xenfv-2.0 machine type though I *can* do auto discovery... in which
> case do we need the -accel=xen option at all?

Yes.  Please try not do things differently from other accelerators.

Paolo
Paul Durrant - June 14, 2013, 3:10 p.m.
> -----Original Message-----

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo

> Bonzini

> Sent: 14 June 2013 15:58

> To: Paul Durrant

> Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-

> devel@lists.xen.org

> Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device

> initialization

> 

> Il 14/06/2013 10:11, Paul Durrant ha scritto:

> > I think we're still going to need -M xenpv, I think; it's quite

> > distinct from pc.

> 

> Of course!  Even more: "-M xenpv" should be reused on ARM.

> 

> > I guess we could use -M pc for HVM and gate the

> > accel code as you suggest but, if that's the way we're going, it

> > would seem more logical just to ditch the accel code for xenpv

> > completely (assuming we can do all we need from the machine init) and

> > then use -M pc -accel=xen for HVM guests going forward.

> 

> There is common code between pv and fv, and that one definitely belongs

> in xen_init.  Most fv-only code probably should be in pc_init.  The rest

> should move to xen_init though, because it would apply just as well for

> example to Q35.  It's a bit ugly to have fv-only code there, but it's

> better than having a Xen-specific machine type.  Xen/KVM/TCG should be

> as similar as possible at the QEMU level, any difference should be

> handled in the toolstack.

> 

> > But that does

> > rather screw up my autodiscovery plans because I would not know, for

> > a given qemu binary, which machine type to use.

> 

> There's no need for that.  4.4 can just use "-M pc" unconditionally,

> <=4.3 will just use "-M xenfv" unconditionally.

> 

> > If I create a new

> > xenfv-2.0 machine type though I *can* do auto discovery... in which

> > case do we need the -accel=xen option at all?

> 

> Yes.  Please try not do things differently from other accelerators.

> 


Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.

  Paul
Stefano Stabellini - June 18, 2013, 6:56 p.m.
On Fri, 14 Jun 2013, Paul Durrant wrote:
> > -----Original Message-----
> > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> > Bonzini
> > Sent: 14 June 2013 15:58
> > To: Paul Durrant
> > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-
> > devel@lists.xen.org
> > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> > initialization
> > 
> > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > I think we're still going to need -M xenpv, I think; it's quite
> > > distinct from pc.
> > 
> > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > 
> > > I guess we could use -M pc for HVM and gate the
> > > accel code as you suggest but, if that's the way we're going, it
> > > would seem more logical just to ditch the accel code for xenpv
> > > completely (assuming we can do all we need from the machine init) and
> > > then use -M pc -accel=xen for HVM guests going forward.
> > 
> > There is common code between pv and fv, and that one definitely belongs
> > in xen_init.  Most fv-only code probably should be in pc_init.  The rest
> > should move to xen_init though, because it would apply just as well for
> > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > better than having a Xen-specific machine type.  Xen/KVM/TCG should be
> > as similar as possible at the QEMU level, any difference should be
> > handled in the toolstack.
> > 
> > > But that does
> > > rather screw up my autodiscovery plans because I would not know, for
> > > a given qemu binary, which machine type to use.
> > 
> > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > <=4.3 will just use "-M xenfv" unconditionally.
> > 
> > > If I create a new
> > > xenfv-2.0 machine type though I *can* do auto discovery... in which
> > > case do we need the -accel=xen option at all?
> > 
> > Yes.  Please try not do things differently from other accelerators.
> > 
> 
> Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.

xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
use -M pc for HVM guests and retain -M xenpv for pv guests.

However it seems to me that we also need a way in libxl to find out
whether QEMU is new enough for us to be able to use -M pc.
We can't just assume that users will be able to figure out the magic
rune they need to write in the VM config file to solve their VM crash at
boot problem.

We could spawn an instance of QEMU just to figure out the QEMU version
but we certainly cannot do that every time we start a new VM.
Once we figure out the QEMU version the first time we could write it to
xenstore so that the next time we don't have to go through the same
process again.
Paolo Bonzini - June 18, 2013, 7:12 p.m.
Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
>> > 
>> > Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.
> xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> use -M pc for HVM guests and retain -M xenpv for pv guests.
> 
> However it seems to me that we also need a way in libxl to find out
> whether QEMU is new enough for us to be able to use -M pc.
> We can't just assume that users will be able to figure out the magic
> rune they need to write in the VM config file to solve their VM crash at
> boot problem.
> 
> We could spawn an instance of QEMU just to figure out the QEMU version
> but we certainly cannot do that every time we start a new VM.
> Once we figure out the QEMU version the first time we could write it to
> xenstore so that the next time we don't have to go through the same
> process again.

Can you just assume that 4.4 requires QEMU 1.6 or newer?

Paolo
Stefano Stabellini - June 18, 2013, 7:35 p.m.
On Tue, 18 Jun 2013, Paolo Bonzini wrote:
> Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
> >> > 
> >> > Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.
> > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > 
> > However it seems to me that we also need a way in libxl to find out
> > whether QEMU is new enough for us to be able to use -M pc.
> > We can't just assume that users will be able to figure out the magic
> > rune they need to write in the VM config file to solve their VM crash at
> > boot problem.
> > 
> > We could spawn an instance of QEMU just to figure out the QEMU version
> > but we certainly cannot do that every time we start a new VM.
> > Once we figure out the QEMU version the first time we could write it to
> > xenstore so that the next time we don't have to go through the same
> > process again.
> 
> Can you just assume that 4.4 requires QEMU 1.6 or newer?

I would rather not make that assumption because we cannot control what
distro are going to package. I wouldn't want a distro to ship with Xen
HVM guests broken because they choose the wrong QEMU version. Of course
we could put that in the release notes, but there are lots of distros
out there and I am pretty sure that at least one of them is not going to
read them.
Andreas Färber - June 18, 2013, 9:38 p.m.
Am 18.06.2013 21:35, schrieb Stefano Stabellini:
> On Tue, 18 Jun 2013, Paolo Bonzini wrote:
>> Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
>>> xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
>>> use -M pc for HVM guests and retain -M xenpv for pv guests.
>>>
>>> However it seems to me that we also need a way in libxl to find out
>>> whether QEMU is new enough for us to be able to use -M pc.
>>> We can't just assume that users will be able to figure out the magic
>>> rune they need to write in the VM config file to solve their VM crash at
>>> boot problem.
>>>
>>> We could spawn an instance of QEMU just to figure out the QEMU version
>>> but we certainly cannot do that every time we start a new VM.
>>> Once we figure out the QEMU version the first time we could write it to
>>> xenstore so that the next time we don't have to go through the same
>>> process again.
>>
>> Can you just assume that 4.4 requires QEMU 1.6 or newer?
> 
> I would rather not make that assumption because we cannot control what
> distro are going to package. I wouldn't want a distro to ship with Xen
> HVM guests broken because they choose the wrong QEMU version. Of course
> we could put that in the release notes, but there are lots of distros
> out there and I am pretty sure that at least one of them is not going to
> read them.

You could check for existence of the pc-i440fx-1.6 machine and infer
that it is at least v1.6 (might break in some distant future of course
and for current git commits until your changes get merged).

Andreas
Ian Campbell - June 19, 2013, 8:11 a.m.
On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
> On Fri, 14 Jun 2013, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> > > Bonzini
> > > Sent: 14 June 2013 15:58
> > > To: Paul Durrant
> > > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-
> > > devel@lists.xen.org
> > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> > > initialization
> > > 
> > > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > > I think we're still going to need -M xenpv, I think; it's quite
> > > > distinct from pc.
> > > 
> > > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > > 
> > > > I guess we could use -M pc for HVM and gate the
> > > > accel code as you suggest but, if that's the way we're going, it
> > > > would seem more logical just to ditch the accel code for xenpv
> > > > completely (assuming we can do all we need from the machine init) and
> > > > then use -M pc -accel=xen for HVM guests going forward.
> > > 
> > > There is common code between pv and fv, and that one definitely belongs
> > > in xen_init.  Most fv-only code probably should be in pc_init.  The rest
> > > should move to xen_init though, because it would apply just as well for
> > > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > > better than having a Xen-specific machine type.  Xen/KVM/TCG should be
> > > as similar as possible at the QEMU level, any difference should be
> > > handled in the toolstack.
> > > 
> > > > But that does
> > > > rather screw up my autodiscovery plans because I would not know, for
> > > > a given qemu binary, which machine type to use.
> > > 
> > > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > > <=4.3 will just use "-M xenfv" unconditionally.
> > > 
> > > > If I create a new
> > > > xenfv-2.0 machine type though I *can* do auto discovery... in which
> > > > case do we need the -accel=xen option at all?
> > > 
> > > Yes.  Please try not do things differently from other accelerators.
> > > 
> > 
> > Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.
> 
> xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> use -M pc for HVM guests and retain -M xenpv for pv guests.
> 
> However it seems to me that we also need a way in libxl to find out
> whether QEMU is new enough for us to be able to use -M pc.
> We can't just assume that users will be able to figure out the magic
> rune they need to write in the VM config file to solve their VM crash at
> boot problem.

What crash at boot problem?

> We could spawn an instance of QEMU just to figure out the QEMU version
> but we certainly cannot do that every time we start a new VM.
> Once we figure out the QEMU version the first time we could write it to
> xenstore so that the next time we don't have to go through the same
> process again.

Due to the device_model_override we might need to make this per-path.
You'd also likely need to store mtime or something in case qemu gets
upgraded, although perhaps that is getting unnecessarily picky...

Ian.
Paul Durrant - June 19, 2013, 8:27 a.m.
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 18 June 2013 20:35
> To: Paolo Bonzini
> Cc: Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org; xen-
> devel@lists.xen.org; Ian Campbell
> Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> initialization
> 
> On Tue, 18 Jun 2013, Paolo Bonzini wrote:
> > Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
> > >> >
> > >> > Ok. I guess we can have the ability to override the machine type in the
> VM config, so you could still kick off an older qemu with a newer libxl - but it
> sounds like the auto-discovery idea is a no-go then.
> > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > >
> > > However it seems to me that we also need a way in libxl to find out
> > > whether QEMU is new enough for us to be able to use -M pc.
> > > We can't just assume that users will be able to figure out the magic
> > > rune they need to write in the VM config file to solve their VM crash at
> > > boot problem.
> > >
> > > We could spawn an instance of QEMU just to figure out the QEMU
> version
> > > but we certainly cannot do that every time we start a new VM.
> > > Once we figure out the QEMU version the first time we could write it to
> > > xenstore so that the next time we don't have to go through the same
> > > process again.
> >
> > Can you just assume that 4.4 requires QEMU 1.6 or newer?
> 
> I would rather not make that assumption because we cannot control what
> distro are going to package. I wouldn't want a distro to ship with Xen
> HVM guests broken because they choose the wrong QEMU version. Of
> course
> we could put that in the release notes, but there are lots of distros
> out there and I am pretty sure that at least one of them is not going to
> read them.

Can't we just leave the default set at xenfv (as it is in my patch) until we're happy that most distros are carrying a new enough qemu?

  Paul
Ian Campbell - June 19, 2013, 8:29 a.m.
On Tue, 2013-06-18 at 23:38 +0200, Andreas Färber wrote:
> Am 18.06.2013 21:35, schrieb Stefano Stabellini:
> > On Tue, 18 Jun 2013, Paolo Bonzini wrote:
> >> Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
> >>> xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> >>> use -M pc for HVM guests and retain -M xenpv for pv guests.
> >>>
> >>> However it seems to me that we also need a way in libxl to find out
> >>> whether QEMU is new enough for us to be able to use -M pc.
> >>> We can't just assume that users will be able to figure out the magic
> >>> rune they need to write in the VM config file to solve their VM crash at
> >>> boot problem.
> >>>
> >>> We could spawn an instance of QEMU just to figure out the QEMU version
> >>> but we certainly cannot do that every time we start a new VM.
> >>> Once we figure out the QEMU version the first time we could write it to
> >>> xenstore so that the next time we don't have to go through the same
> >>> process again.
> >>
> >> Can you just assume that 4.4 requires QEMU 1.6 or newer?
> > 
> > I would rather not make that assumption because we cannot control what
> > distro are going to package. I wouldn't want a distro to ship with Xen
> > HVM guests broken because they choose the wrong QEMU version. Of course
> > we could put that in the release notes, but there are lots of distros
> > out there and I am pretty sure that at least one of them is not going to
> > read them.
> 
> You could check for existence of the pc-i440fx-1.6 machine and infer
> that it is at least v1.6 (might break in some distant future of course
> and for current git commits until your changes get merged).

Actually, this raises an interesting point. AIUI "pc" is simply and
alias for the most recent "pc-X.Y" and "pc-X.Y" is present to allow for
qemu "upgrading" the set of emulated hardware, as in it represents
changing the set of emulated peripherals, not just fixing bugs in the
emulation etc, is that right?

I think it would be preferable for us to request a specific platform
(pc-i440fx-1.6 if that's the one) and a conscious decision to support
newer platforms (and can test it etc).

Ian.
Paul Durrant - June 19, 2013, 8:37 a.m.
> -----Original Message-----

> From: Ian Campbell

> Sent: 19 June 2013 09:29

> To: Andreas Färber

> Cc: Stefano Stabellini; Paolo Bonzini; Paul Durrant; xen-devel@lists.xen.org;

> qemu-devel@nongnu.org

> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-

> platform device initialization

> 

> On Tue, 2013-06-18 at 23:38 +0200, Andreas Färber wrote:

> > Am 18.06.2013 21:35, schrieb Stefano Stabellini:

> > > On Tue, 18 Jun 2013, Paolo Bonzini wrote:

> > >> Il 18/06/2013 20:56, Stefano Stabellini ha scritto:

> > >>> xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just

> > >>> use -M pc for HVM guests and retain -M xenpv for pv guests.

> > >>>

> > >>> However it seems to me that we also need a way in libxl to find out

> > >>> whether QEMU is new enough for us to be able to use -M pc.

> > >>> We can't just assume that users will be able to figure out the magic

> > >>> rune they need to write in the VM config file to solve their VM crash at

> > >>> boot problem.

> > >>>

> > >>> We could spawn an instance of QEMU just to figure out the QEMU

> version

> > >>> but we certainly cannot do that every time we start a new VM.

> > >>> Once we figure out the QEMU version the first time we could write it

> to

> > >>> xenstore so that the next time we don't have to go through the same

> > >>> process again.

> > >>

> > >> Can you just assume that 4.4 requires QEMU 1.6 or newer?

> > >

> > > I would rather not make that assumption because we cannot control

> what

> > > distro are going to package. I wouldn't want a distro to ship with Xen

> > > HVM guests broken because they choose the wrong QEMU version. Of

> course

> > > we could put that in the release notes, but there are lots of distros

> > > out there and I am pretty sure that at least one of them is not going to

> > > read them.

> >

> > You could check for existence of the pc-i440fx-1.6 machine and infer

> > that it is at least v1.6 (might break in some distant future of course

> > and for current git commits until your changes get merged).

> 

> Actually, this raises an interesting point. AIUI "pc" is simply and

> alias for the most recent "pc-X.Y" and "pc-X.Y" is present to allow for

> qemu "upgrading" the set of emulated hardware, as in it represents

> changing the set of emulated peripherals, not just fixing bugs in the

> emulation etc, is that right?

> 

> I think it would be preferable for us to request a specific platform

> (pc-i440fx-1.6 if that's the one) and a conscious decision to support

> newer platforms (and can test it etc).

> 


If that is the case then the xenfv code is also incorrect, since it does not call through to a specific version of 'pc'.

  Paul
Paolo Bonzini - June 19, 2013, 8:41 a.m.
Il 19/06/2013 10:29, Ian Campbell ha scritto:
>> > You could check for existence of the pc-i440fx-1.6 machine and infer
>> > that it is at least v1.6 (might break in some distant future of course
>> > and for current git commits until your changes get merged).
> Actually, this raises an interesting point. AIUI "pc" is simply and
> alias for the most recent "pc-X.Y" and "pc-X.Y" is present to allow for
> qemu "upgrading" the set of emulated hardware, as in it represents
> changing the set of emulated peripherals, not just fixing bugs in the
> emulation etc, is that right?

Usually it represents adding _features_ to the emulation.  There are
some cases where the set of emulated peripherals change (e.g. pvpanic
added in 1.5), but it's the exception rather than the rule.  There are
also some cases of bug-compatibility, but again they're not the most
common use of versioned machine types.

You do not know how older guests react to those new features, and you
want to prevent moving guests to older versions that lack some features.
 For these reasons, libvirt always sticks to the alias target that was
found at creation time.

Example 1: you defined a machine last year with machine type "pc".
libvirt actually stored it with machine type "pc-1.2".  Today you run it
and hardware/features do not change.

Example 2: you defined a machine today with machine type "pc".  libvirt
actually stored it with machine type "pc-1.6".  You run it on a machine
with an old QEMU, the machine doesn't start.

Example 3: you ask libvirt to edit the definition of the machine of
example 1.  You change the machine type back to "pc".  libvirt stores it
with machine type "pc-1.6".  Hardware may change compared to the
previous runs of the VM, but it will otherwise remain stable.

Example 4: you use vi to edit the definition of the machine of example
1/3.  You change the machine type to "pc".  You lose any guarantee that
hardware does not change.  You should not do this.

Example 5: you use "virsh create" to start a VM based on an XML file,
rather than "virsh define"+"virsh start" as in examples 1-2.  You lose
any guarantee that hardware does not change.  Not frowned upon as much
as example 4, since the VM is supposed to be transient.

> I think it would be preferable for us to request a specific platform
> (pc-i440fx-1.6 if that's the one) and a conscious decision to support
> newer platforms (and can test it etc).

That can be a good idea, since Xen support is a bit less tested than KVM
with bleeding-edge QEMU.  You can query the machine types from the Xen
toolchain, and auto-remap "pc" to a machine type that is never newer
than what you consider to be well-tested.

It would require two QEMU invocations per "xl create".  However, most of
the startup time of QEMU is loading dynamic libraries.  A good deal of
that time amortizes well over two invocations of QEMU.  Hence, you can
use the first invocation to query the machine type (using "-nodefaults
-nographic" etc. and a QMP connection), and the second to actually start
the VM.

Paolo
Ian Campbell - June 19, 2013, 8:56 a.m.
On Wed, 2013-06-19 at 10:41 +0200, Paolo Bonzini wrote:
> Il 19/06/2013 10:29, Ian Campbell ha scritto:
> >> > You could check for existence of the pc-i440fx-1.6 machine and infer
> >> > that it is at least v1.6 (might break in some distant future of course
> >> > and for current git commits until your changes get merged).
> > Actually, this raises an interesting point. AIUI "pc" is simply and
> > alias for the most recent "pc-X.Y" and "pc-X.Y" is present to allow for
> > qemu "upgrading" the set of emulated hardware, as in it represents
> > changing the set of emulated peripherals, not just fixing bugs in the
> > emulation etc, is that right?
> 
> Usually it represents adding _features_ to the emulation.  There are
> some cases where the set of emulated peripherals change (e.g. pvpanic
> added in 1.5), but it's the exception rather than the rule.  There are
> also some cases of bug-compatibility, but again they're not the most
> common use of versioned machine types.
> 
> You do not know how older guests react to those new features, and you
> want to prevent moving guests to older versions that lack some features.
>  For these reasons, libvirt always sticks to the alias target that was
> found at creation time.

I had a grep around libvirt wondering how it handled this and didn't
find it. The approach you describe makes perfect sense when you have a
persistent config. The case with xl is more like your example 5:

> Example 5: you use "virsh create" to start a VM based on an XML file,
> rather than "virsh define"+"virsh start" as in examples 1-2.  You lose
> any guarantee that hardware does not change.  Not frowned upon as much
> as example 4, since the VM is supposed to be transient.

For something like xapi we'd likely want to support some sort of model
similar to libvirt, so whatever we do at the libxl layer needs to
consider both approaches.

> It would require two QEMU invocations per "xl create".  However, most of
> the startup time of QEMU is loading dynamic libraries.  A good deal of
> that time amortizes well over two invocations of QEMU.

Not to mention that on a system already running a domain or two there is
a good chance that at least the relevant bits of QEMU are already in
RAM.

In fact, I wonder if we could just query the qemu which is running to
provide dom0 itself with qdisk services, at least in the case where the
guest is configured to use the same version of qemu.

Ian.
Paul Durrant - June 19, 2013, 9:11 a.m.
> -----Original Message-----

> From: Ian Campbell

> Sent: 19 June 2013 09:56

> To: Paolo Bonzini

> Cc: Andreas Färber; Stefano Stabellini; Paul Durrant; xen-

> devel@lists.xen.org; qemu-devel@nongnu.org

> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-

> platform device initialization

> 

> On Wed, 2013-06-19 at 10:41 +0200, Paolo Bonzini wrote:

> > Il 19/06/2013 10:29, Ian Campbell ha scritto:

> > >> > You could check for existence of the pc-i440fx-1.6 machine and infer

> > >> > that it is at least v1.6 (might break in some distant future of course

> > >> > and for current git commits until your changes get merged).

> > > Actually, this raises an interesting point. AIUI "pc" is simply and

> > > alias for the most recent "pc-X.Y" and "pc-X.Y" is present to allow for

> > > qemu "upgrading" the set of emulated hardware, as in it represents

> > > changing the set of emulated peripherals, not just fixing bugs in the

> > > emulation etc, is that right?

> >

> > Usually it represents adding _features_ to the emulation.  There are

> > some cases where the set of emulated peripherals change (e.g. pvpanic

> > added in 1.5), but it's the exception rather than the rule.  There are

> > also some cases of bug-compatibility, but again they're not the most

> > common use of versioned machine types.

> >

> > You do not know how older guests react to those new features, and you

> > want to prevent moving guests to older versions that lack some features.

> >  For these reasons, libvirt always sticks to the alias target that was

> > found at creation time.

> 

> I had a grep around libvirt wondering how it handled this and didn't

> find it. The approach you describe makes perfect sense when you have a

> persistent config. The case with xl is more like your example 5:

> 

> > Example 5: you use "virsh create" to start a VM based on an XML file,

> > rather than "virsh define"+"virsh start" as in examples 1-2.  You lose

> > any guarantee that hardware does not change.  Not frowned upon as much

> > as example 4, since the VM is supposed to be transient.

> 

> For something like xapi we'd likely want to support some sort of model

> similar to libvirt, so whatever we do at the libxl layer needs to

> consider both approaches.

> 

> > It would require two QEMU invocations per "xl create".  However, most of

> > the startup time of QEMU is loading dynamic libraries.  A good deal of

> > that time amortizes well over two invocations of QEMU.

> 

> Not to mention that on a system already running a domain or two there is

> a good chance that at least the relevant bits of QEMU are already in

> RAM.

> 

> In fact, I wonder if we could just query the qemu which is running to

> provide dom0 itself with qdisk services, at least in the case where the

> guest is configured to use the same version of qemu.

> 


...in which case can we agree to accept an undocumented override ability in the cfg file. If we're going to introduce some form of auto-selection then we should at least allow developers to bypass it if they need to. I'm happy to get rid the the enumeration, and go for a freeform string which gets passed as the -machine optval if it's specified.

  Paul
Paolo Bonzini - June 19, 2013, 12:51 p.m.
Il 19/06/2013 10:56, Ian Campbell ha scritto:
> The case with xl is more like your example 5:

Ok, that's what I remembered. :)

> > Example 5: you use "virsh create" to start a VM based on an XML file,
> > rather than "virsh define"+"virsh start" as in examples 1-2.  You lose
> > any guarantee that hardware does not change.  Not frowned upon as much
> > as example 4, since the VM is supposed to be transient.
> 
> For something like xapi we'd likely want to support some sort of model
> similar to libvirt, so whatever we do at the libxl layer needs to
> consider both approaches.

I think xapi can itself query qemu if this is the case (in this respect,
libvirt is somewhat in the middle between xl and xapi, depending on
whether you use transient or persistent domains).  Or perhaps you can
add an xl command to query qemu and provide capabilities.

xl can default to some "blessed" old-enough machine if it is available
or just the newest "pc" if the blessed machine is unavailable.  And of
course it can be overridden by the guest config.

Paolo
Stefano Stabellini - June 19, 2013, 1:47 p.m.
On Wed, 19 Jun 2013, Ian Campbell wrote:
> In fact, I wonder if we could just query the qemu which is running to
> provide dom0 itself with qdisk services, at least in the case where the
> guest is configured to use the same version of qemu.

Great idea, that should work.
Stefano Stabellini - June 19, 2013, 1:53 p.m.
On Wed, 19 Jun 2013, Ian Campbell wrote:
> On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
> > On Fri, 14 Jun 2013, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo
> > > > Bonzini
> > > > Sent: 14 June 2013 15:58
> > > > To: Paul Durrant
> > > > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-
> > > > devel@lists.xen.org
> > > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> > > > initialization
> > > > 
> > > > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > > > I think we're still going to need -M xenpv, I think; it's quite
> > > > > distinct from pc.
> > > > 
> > > > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > > > 
> > > > > I guess we could use -M pc for HVM and gate the
> > > > > accel code as you suggest but, if that's the way we're going, it
> > > > > would seem more logical just to ditch the accel code for xenpv
> > > > > completely (assuming we can do all we need from the machine init) and
> > > > > then use -M pc -accel=xen for HVM guests going forward.
> > > > 
> > > > There is common code between pv and fv, and that one definitely belongs
> > > > in xen_init.  Most fv-only code probably should be in pc_init.  The rest
> > > > should move to xen_init though, because it would apply just as well for
> > > > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > > > better than having a Xen-specific machine type.  Xen/KVM/TCG should be
> > > > as similar as possible at the QEMU level, any difference should be
> > > > handled in the toolstack.
> > > > 
> > > > > But that does
> > > > > rather screw up my autodiscovery plans because I would not know, for
> > > > > a given qemu binary, which machine type to use.
> > > > 
> > > > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > > > <=4.3 will just use "-M xenfv" unconditionally.
> > > > 
> > > > > If I create a new
> > > > > xenfv-2.0 machine type though I *can* do auto discovery... in which
> > > > > case do we need the -accel=xen option at all?
> > > > 
> > > > Yes.  Please try not do things differently from other accelerators.
> > > > 
> > > 
> > > Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then.
> > 
> > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > 
> > However it seems to me that we also need a way in libxl to find out
> > whether QEMU is new enough for us to be able to use -M pc.
> > We can't just assume that users will be able to figure out the magic
> > rune they need to write in the VM config file to solve their VM crash at
> > boot problem.
> 
> What crash at boot problem?

If you start QEMU as device model on Xen with the wrong machine option
(for example -M pc on an old QEMU), QEMU would probably just abort
during initialization.


> > We could spawn an instance of QEMU just to figure out the QEMU version
> > but we certainly cannot do that every time we start a new VM.
> > Once we figure out the QEMU version the first time we could write it to
> > xenstore so that the next time we don't have to go through the same
> > process again.
> 
> Due to the device_model_override we might need to make this per-path.
> You'd also likely need to store mtime or something in case qemu gets
> upgraded, although perhaps that is getting unnecessarily picky...
 
I think of device_model_override as an option for developers. People
that use device_model_override can also override the QEMUMachine
version.
I am more worried about your average user that gets a default broken
configuration on her favourite distro.
Stefano Stabellini - June 19, 2013, 1:55 p.m.
On Wed, 19 Jun 2013, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: 18 June 2013 20:35
> > To: Paolo Bonzini
> > Cc: Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org; xen-
> > devel@lists.xen.org; Ian Campbell
> > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> > initialization
> > 
> > On Tue, 18 Jun 2013, Paolo Bonzini wrote:
> > > Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
> > > >> >
> > > >> > Ok. I guess we can have the ability to override the machine type in the
> > VM config, so you could still kick off an older qemu with a newer libxl - but it
> > sounds like the auto-discovery idea is a no-go then.
> > > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > > >
> > > > However it seems to me that we also need a way in libxl to find out
> > > > whether QEMU is new enough for us to be able to use -M pc.
> > > > We can't just assume that users will be able to figure out the magic
> > > > rune they need to write in the VM config file to solve their VM crash at
> > > > boot problem.
> > > >
> > > > We could spawn an instance of QEMU just to figure out the QEMU
> > version
> > > > but we certainly cannot do that every time we start a new VM.
> > > > Once we figure out the QEMU version the first time we could write it to
> > > > xenstore so that the next time we don't have to go through the same
> > > > process again.
> > >
> > > Can you just assume that 4.4 requires QEMU 1.6 or newer?
> > 
> > I would rather not make that assumption because we cannot control what
> > distro are going to package. I wouldn't want a distro to ship with Xen
> > HVM guests broken because they choose the wrong QEMU version. Of
> > course
> > we could put that in the release notes, but there are lots of distros
> > out there and I am pretty sure that at least one of them is not going to
> > read them.
> 
> Can't we just leave the default set at xenfv (as it is in my patch) until we're happy that most distros are carrying a new enough qemu?

As Ian pointed out, we already start a QEMU instance at host boot time
to serve qdisk requests. We might as well use it to retrieve the QEMU
version and select the machine version based on that.
Paul Durrant - June 19, 2013, 2 p.m.
> -----Original Message-----
> From: qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org
> [mailto:qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org] On
> Behalf Of Stefano Stabellini
> Sent: 19 June 2013 14:53
> To: Ian Campbell
> Cc: Paolo Bonzini; Paul Durrant; xen-devel@lists.xen.org; qemu-
> devel@nongnu.org; Stefano Stabellini
> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
> platform device initialization
> 
> On Wed, 19 Jun 2013, Ian Campbell wrote:
> > On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
> > > On Fri, 14 Jun 2013, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of
> Paolo
> > > > > Bonzini
> > > > > Sent: 14 June 2013 15:58
> > > > > To: Paul Durrant
> > > > > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-
> > > > > devel@lists.xen.org
> > > > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform
> device
> > > > > initialization
> > > > >
> > > > > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > > > > I think we're still going to need -M xenpv, I think; it's quite
> > > > > > distinct from pc.
> > > > >
> > > > > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > > > >
> > > > > > I guess we could use -M pc for HVM and gate the
> > > > > > accel code as you suggest but, if that's the way we're going, it
> > > > > > would seem more logical just to ditch the accel code for xenpv
> > > > > > completely (assuming we can do all we need from the machine init)
> and
> > > > > > then use -M pc -accel=xen for HVM guests going forward.
> > > > >
> > > > > There is common code between pv and fv, and that one definitely
> belongs
> > > > > in xen_init.  Most fv-only code probably should be in pc_init.  The rest
> > > > > should move to xen_init though, because it would apply just as well
> for
> > > > > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > > > > better than having a Xen-specific machine type.  Xen/KVM/TCG
> should be
> > > > > as similar as possible at the QEMU level, any difference should be
> > > > > handled in the toolstack.
> > > > >
> > > > > > But that does
> > > > > > rather screw up my autodiscovery plans because I would not know,
> for
> > > > > > a given qemu binary, which machine type to use.
> > > > >
> > > > > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > > > > <=4.3 will just use "-M xenfv" unconditionally.
> > > > >
> > > > > > If I create a new
> > > > > > xenfv-2.0 machine type though I *can* do auto discovery... in which
> > > > > > case do we need the -accel=xen option at all?
> > > > >
> > > > > Yes.  Please try not do things differently from other accelerators.
> > > > >
> > > >
> > > > Ok. I guess we can have the ability to override the machine type in the
> VM config, so you could still kick off an older qemu with a newer libxl - but it
> sounds like the auto-discovery idea is a no-go then.
> > >
> > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > >
> > > However it seems to me that we also need a way in libxl to find out
> > > whether QEMU is new enough for us to be able to use -M pc.
> > > We can't just assume that users will be able to figure out the magic
> > > rune they need to write in the VM config file to solve their VM crash at
> > > boot problem.
> >
> > What crash at boot problem?
> 
> If you start QEMU as device model on Xen with the wrong machine option
> (for example -M pc on an old QEMU), QEMU would probably just abort
> during initialization.
> 
> 
> > > We could spawn an instance of QEMU just to figure out the QEMU
> version
> > > but we certainly cannot do that every time we start a new VM.
> > > Once we figure out the QEMU version the first time we could write it to
> > > xenstore so that the next time we don't have to go through the same
> > > process again.
> >
> > Due to the device_model_override we might need to make this per-path.
> > You'd also likely need to store mtime or something in case qemu gets
> > upgraded, although perhaps that is getting unnecessarily picky...
> 
> I think of device_model_override as an option for developers. People
> that use device_model_override can also override the QEMUMachine
> version.

Are you suggesting we allow a freeform -machine option in libxl, or are you suggesting they point device_model_override at a script which drops the -M argument and inserts their new choice before invoking qemu?

  Paul

> I am more worried about your average user that gets a default broken
> configuration on her favourite distro.
Ian Campbell - June 19, 2013, 3:09 p.m.
On Wed, 2013-06-19 at 14:55 +0100, Stefano Stabellini wrote:
> On Wed, 19 Jun 2013, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > Sent: 18 June 2013 20:35
> > > To: Paolo Bonzini
> > > Cc: Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org; xen-
> > > devel@lists.xen.org; Ian Campbell
> > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device
> > > initialization
> > > 
> > > On Tue, 18 Jun 2013, Paolo Bonzini wrote:
> > > > Il 18/06/2013 20:56, Stefano Stabellini ha scritto:
> > > > >> >
> > > > >> > Ok. I guess we can have the ability to override the machine type in the
> > > VM config, so you could still kick off an older qemu with a newer libxl - but it
> > > sounds like the auto-discovery idea is a no-go then.
> > > > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > > > >
> > > > > However it seems to me that we also need a way in libxl to find out
> > > > > whether QEMU is new enough for us to be able to use -M pc.
> > > > > We can't just assume that users will be able to figure out the magic
> > > > > rune they need to write in the VM config file to solve their VM crash at
> > > > > boot problem.
> > > > >
> > > > > We could spawn an instance of QEMU just to figure out the QEMU
> > > version
> > > > > but we certainly cannot do that every time we start a new VM.
> > > > > Once we figure out the QEMU version the first time we could write it to
> > > > > xenstore so that the next time we don't have to go through the same
> > > > > process again.
> > > >
> > > > Can you just assume that 4.4 requires QEMU 1.6 or newer?
> > > 
> > > I would rather not make that assumption because we cannot control what
> > > distro are going to package. I wouldn't want a distro to ship with Xen
> > > HVM guests broken because they choose the wrong QEMU version. Of
> > > course
> > > we could put that in the release notes, but there are lots of distros
> > > out there and I am pretty sure that at least one of them is not going to
> > > read them.
> > 
> > Can't we just leave the default set at xenfv (as it is in my patch) until we're happy that most distros are carrying a new enough qemu?
> 
> As Ian pointed out, we already start a QEMU instance at host boot time
> to serve qdisk requests. We might as well use it to retrieve the QEMU
> version and select the machine version based on that.

Don't forget the case where the user has explicitly requested a
different qemu version to what dom0 is using... In that case we have to
start the one they wanted. Should be a minority situation though.

Ian.
Stefano Stabellini - June 19, 2013, 4:27 p.m.
On Wed, 19 Jun 2013, Paul Durrant wrote:
> > -----Original Message-----
> > From: qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org
> > [mailto:qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org] On
> > Behalf Of Stefano Stabellini
> > Sent: 19 June 2013 14:53
> > To: Ian Campbell
> > Cc: Paolo Bonzini; Paul Durrant; xen-devel@lists.xen.org; qemu-
> > devel@nongnu.org; Stefano Stabellini
> > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
> > platform device initialization
> > 
> > On Wed, 19 Jun 2013, Ian Campbell wrote:
> > > On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
> > > > On Fri, 14 Jun 2013, Paul Durrant wrote:
> > > > > > -----Original Message-----
> > > > > > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of
> > Paolo
> > > > > > Bonzini
> > > > > > Sent: 14 June 2013 15:58
> > > > > > To: Paul Durrant
> > > > > > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen-
> > > > > > devel@lists.xen.org
> > > > > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform
> > device
> > > > > > initialization
> > > > > >
> > > > > > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > > > > > I think we're still going to need -M xenpv, I think; it's quite
> > > > > > > distinct from pc.
> > > > > >
> > > > > > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > > > > >
> > > > > > > I guess we could use -M pc for HVM and gate the
> > > > > > > accel code as you suggest but, if that's the way we're going, it
> > > > > > > would seem more logical just to ditch the accel code for xenpv
> > > > > > > completely (assuming we can do all we need from the machine init)
> > and
> > > > > > > then use -M pc -accel=xen for HVM guests going forward.
> > > > > >
> > > > > > There is common code between pv and fv, and that one definitely
> > belongs
> > > > > > in xen_init.  Most fv-only code probably should be in pc_init.  The rest
> > > > > > should move to xen_init though, because it would apply just as well
> > for
> > > > > > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > > > > > better than having a Xen-specific machine type.  Xen/KVM/TCG
> > should be
> > > > > > as similar as possible at the QEMU level, any difference should be
> > > > > > handled in the toolstack.
> > > > > >
> > > > > > > But that does
> > > > > > > rather screw up my autodiscovery plans because I would not know,
> > for
> > > > > > > a given qemu binary, which machine type to use.
> > > > > >
> > > > > > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > > > > > <=4.3 will just use "-M xenfv" unconditionally.
> > > > > >
> > > > > > > If I create a new
> > > > > > > xenfv-2.0 machine type though I *can* do auto discovery... in which
> > > > > > > case do we need the -accel=xen option at all?
> > > > > >
> > > > > > Yes.  Please try not do things differently from other accelerators.
> > > > > >
> > > > >
> > > > > Ok. I guess we can have the ability to override the machine type in the
> > VM config, so you could still kick off an older qemu with a newer libxl - but it
> > sounds like the auto-discovery idea is a no-go then.
> > > >
> > > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > > >
> > > > However it seems to me that we also need a way in libxl to find out
> > > > whether QEMU is new enough for us to be able to use -M pc.
> > > > We can't just assume that users will be able to figure out the magic
> > > > rune they need to write in the VM config file to solve their VM crash at
> > > > boot problem.
> > >
> > > What crash at boot problem?
> > 
> > If you start QEMU as device model on Xen with the wrong machine option
> > (for example -M pc on an old QEMU), QEMU would probably just abort
> > during initialization.
> > 
> > 
> > > > We could spawn an instance of QEMU just to figure out the QEMU
> > version
> > > > but we certainly cannot do that every time we start a new VM.
> > > > Once we figure out the QEMU version the first time we could write it to
> > > > xenstore so that the next time we don't have to go through the same
> > > > process again.
> > >
> > > Due to the device_model_override we might need to make this per-path.
> > > You'd also likely need to store mtime or something in case qemu gets
> > > upgraded, although perhaps that is getting unnecessarily picky...
> > 
> > I think of device_model_override as an option for developers. People
> > that use device_model_override can also override the QEMUMachine
> > version.
> 
> Are you suggesting we allow a freeform -machine option in libxl, or are you suggesting they point device_model_override at a script which drops the -M argument and inserts their new choice before invoking qemu?

I am suggesting that we could have a qemu_machine_override option in
QEMU, or maybe a device_models_args_override that not only adds any user
supplied arguments to QEMU (like the current device_model_args) but also
replace the default arguments completely.
Paul Durrant - June 20, 2013, 7:32 a.m.
> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 19 June 2013 17:28
> To: Paul Durrant
> Cc: Stefano Stabellini; Ian Campbell; Paolo Bonzini; xen-devel@lists.xen.org;
> qemu-devel@nongnu.org
> Subject: RE: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
> platform device initialization
> 
> On Wed, 19 Jun 2013, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org
> > > [mailto:qemu-devel-bounces+paul.durrant=citrix.com@nongnu.org] On
> > > Behalf Of Stefano Stabellini
> > > Sent: 19 June 2013 14:53
> > > To: Ian Campbell
> > > Cc: Paolo Bonzini; Paul Durrant; xen-devel@lists.xen.org; qemu-
> > > devel@nongnu.org; Stefano Stabellini
> > > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
> > > platform device initialization
> > >
> > > On Wed, 19 Jun 2013, Ian Campbell wrote:
> > > > On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
> > > > > On Fri, 14 Jun 2013, Paul Durrant wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf
> Of
> > > Paolo
> > > > > > > Bonzini
> > > > > > > Sent: 14 June 2013 15:58
> > > > > > > To: Paul Durrant
> > > > > > > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org;
> xen-
> > > > > > > devel@lists.xen.org
> > > > > > > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-
> platform
> > > device
> > > > > > > initialization
> > > > > > >
> > > > > > > Il 14/06/2013 10:11, Paul Durrant ha scritto:
> > > > > > > > I think we're still going to need -M xenpv, I think; it's quite
> > > > > > > > distinct from pc.
> > > > > > >
> > > > > > > Of course!  Even more: "-M xenpv" should be reused on ARM.
> > > > > > >
> > > > > > > > I guess we could use -M pc for HVM and gate the
> > > > > > > > accel code as you suggest but, if that's the way we're going, it
> > > > > > > > would seem more logical just to ditch the accel code for xenpv
> > > > > > > > completely (assuming we can do all we need from the machine
> init)
> > > and
> > > > > > > > then use -M pc -accel=xen for HVM guests going forward.
> > > > > > >
> > > > > > > There is common code between pv and fv, and that one definitely
> > > belongs
> > > > > > > in xen_init.  Most fv-only code probably should be in pc_init.  The
> rest
> > > > > > > should move to xen_init though, because it would apply just as
> well
> > > for
> > > > > > > example to Q35.  It's a bit ugly to have fv-only code there, but it's
> > > > > > > better than having a Xen-specific machine type.  Xen/KVM/TCG
> > > should be
> > > > > > > as similar as possible at the QEMU level, any difference should be
> > > > > > > handled in the toolstack.
> > > > > > >
> > > > > > > > But that does
> > > > > > > > rather screw up my autodiscovery plans because I would not
> know,
> > > for
> > > > > > > > a given qemu binary, which machine type to use.
> > > > > > >
> > > > > > > There's no need for that.  4.4 can just use "-M pc" unconditionally,
> > > > > > > <=4.3 will just use "-M xenfv" unconditionally.
> > > > > > >
> > > > > > > > If I create a new
> > > > > > > > xenfv-2.0 machine type though I *can* do auto discovery... in
> which
> > > > > > > > case do we need the -accel=xen option at all?
> > > > > > >
> > > > > > > Yes.  Please try not do things differently from other accelerators.
> > > > > > >
> > > > > >
> > > > > > Ok. I guess we can have the ability to override the machine type in
> the
> > > VM config, so you could still kick off an older qemu with a newer libxl -
> but it
> > > sounds like the auto-discovery idea is a no-go then.
> > > > >
> > > > > xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to just
> > > > > use -M pc for HVM guests and retain -M xenpv for pv guests.
> > > > >
> > > > > However it seems to me that we also need a way in libxl to find out
> > > > > whether QEMU is new enough for us to be able to use -M pc.
> > > > > We can't just assume that users will be able to figure out the magic
> > > > > rune they need to write in the VM config file to solve their VM crash
> at
> > > > > boot problem.
> > > >
> > > > What crash at boot problem?
> > >
> > > If you start QEMU as device model on Xen with the wrong machine
> option
> > > (for example -M pc on an old QEMU), QEMU would probably just abort
> > > during initialization.
> > >
> > >
> > > > > We could spawn an instance of QEMU just to figure out the QEMU
> > > version
> > > > > but we certainly cannot do that every time we start a new VM.
> > > > > Once we figure out the QEMU version the first time we could write it
> to
> > > > > xenstore so that the next time we don't have to go through the same
> > > > > process again.
> > > >
> > > > Due to the device_model_override we might need to make this per-
> path.
> > > > You'd also likely need to store mtime or something in case qemu gets
> > > > upgraded, although perhaps that is getting unnecessarily picky...
> > >
> > > I think of device_model_override as an option for developers. People
> > > that use device_model_override can also override the QEMUMachine
> > > version.
> >
> > Are you suggesting we allow a freeform -machine option in libxl, or are you
> suggesting they point device_model_override at a script which drops the -M
> argument and inserts their new choice before invoking qemu?
> 
> I am suggesting that we could have a qemu_machine_override option in
> QEMU, or maybe a device_models_args_override that not only adds any
> user
> supplied arguments to QEMU (like the current device_model_args) but also
> replace the default arguments completely.

But I intended the device_model_machine option exactly as such an override and you NACKed it. Was that because I limited the choice be using an enumeration rather than a freeform string?

  Paul

Patch

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..e25012d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -174,9 +174,6 @@  static void pc_init1(MemoryRegion *system_memory,
     pc_register_ferr_irq(gsi[13]);
 
     pc_vga_init(isa_bus, pci_enabled ? pci_bus : NULL);
-    if (xen_enabled()) {
-        pci_create_simple(pci_bus, -1, "xen-platform");
-    }
 
     /* init basic PC hardware */
     pc_basic_device_init(isa_bus, gsi, &rtc_state, &floppy, xen_enabled());