[Qemu-devel] [PATCH] xen: make xen-platform a default device

[Qemu-devel] [PATCH] xen: make xen-platform a default device

[Gerd Hoffmann]

Patch hooks up the xen platform device to the default device code we
have in qemu.  Two effects:

  (1) The device will not be created in case -nodefaults is specified
      on the command line.
  (2) Autocreating the device is also turned off in case xen-platform
      is added manually via -device.

With the patch applied you can move the xen-platform device to some
other place with a simple 'qemu -device xen-platform,addr=$slot'.

Tested-by: Tiejun Chen <tiejun.chen@intel.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/i386/pc_piix.c    | 2 +-
 include/hw/xen/xen.h | 1 +
 vl.c                 | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eaf3e61..f987d03 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
     pc_init_pci(args);
 
     bus = pci_find_primary_bus();
-    if (bus != NULL) {
+    if (bus != NULL && default_xenplatform) {
         pci_create_simple(bus, -1, "xen-platform");
     }
 }
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 85fda3d..b350413 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -20,6 +20,7 @@ enum xen_mode {
 
 extern uint32_t xen_domid;
 extern enum xen_mode xen_mode;
+extern int default_xenplatform;
 
 extern bool xen_allowed;
 
diff --git a/vl.c b/vl.c
index 709d8cd..673148e 100644
--- a/vl.c
+++ b/vl.c
@@ -226,6 +226,7 @@ static int default_floppy = 1;
 static int default_cdrom = 1;
 static int default_sdcard = 1;
 static int default_vga = 1;
+int default_xenplatform = 1;
 
 static struct {
     const char *driver;
@@ -247,6 +248,7 @@ static struct {
     { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
     { .driver = "vmware-svga",          .flag = &default_vga       },
     { .driver = "qxl-vga",              .flag = &default_vga       },
+    { .driver = "xen-platform",         .flag = &default_xenplatform },
 };
 
 static QemuOptsList qemu_rtc_opts = {
@@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
         default_monitor = 0;
         default_net = 0;
         default_vga = 0;
+        default_xenplatform = 0;
     }
 
     if (is_daemonized()) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Michael S. Tsirkin]

On Thu, May 22, 2014 at 09:20:50AM +0200, Gerd Hoffmann wrote:
> Patch hooks up the xen platform device to the default device code we
> have in qemu.  Two effects:
> 
>   (1) The device will not be created in case -nodefaults is specified
>       on the command line.
>   (2) Autocreating the device is also turned off in case xen-platform
>       is added manually via -device.
> 
> With the patch applied you can move the xen-platform device to some
> other place with a simple 'qemu -device xen-platform,addr=$slot'.
> 
> Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Applied, thanks!

> ---
>  hw/i386/pc_piix.c    | 2 +-
>  include/hw/xen/xen.h | 1 +
>  vl.c                 | 3 +++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index eaf3e61..f987d03 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>      pc_init_pci(args);
>  
>      bus = pci_find_primary_bus();
> -    if (bus != NULL) {
> +    if (bus != NULL && default_xenplatform) {
>          pci_create_simple(bus, -1, "xen-platform");
>      }
>  }
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 85fda3d..b350413 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -20,6 +20,7 @@ enum xen_mode {
>  
>  extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
> +extern int default_xenplatform;
>  
>  extern bool xen_allowed;
>  
> diff --git a/vl.c b/vl.c
> index 709d8cd..673148e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -226,6 +226,7 @@ static int default_floppy = 1;
>  static int default_cdrom = 1;
>  static int default_sdcard = 1;
>  static int default_vga = 1;
> +int default_xenplatform = 1;
>  
>  static struct {
>      const char *driver;
> @@ -247,6 +248,7 @@ static struct {
>      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
>      { .driver = "vmware-svga",          .flag = &default_vga       },
>      { .driver = "qxl-vga",              .flag = &default_vga       },
> +    { .driver = "xen-platform",         .flag = &default_xenplatform },
>  };
>  
>  static QemuOptsList qemu_rtc_opts = {
> @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
>          default_monitor = 0;
>          default_net = 0;
>          default_vga = 0;
> +        default_xenplatform = 0;
>      }
>  
>      if (is_daemonized()) {
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Chen, Tiejun]

> -----Original Message-----
> From: qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org
> [mailto:qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org] On Behalf Of
> Michael S. Tsirkin
> Sent: Thursday, May 22, 2014 3:22 PM
> To: Gerd Hoffmann
> Cc: qemu-devel@nongnu.org; Anthony Liguori
> Subject: Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device
> 
> On Thu, May 22, 2014 at 09:20:50AM +0200, Gerd Hoffmann wrote:
> > Patch hooks up the xen platform device to the default device code we
> > have in qemu.  Two effects:
> >
> >   (1) The device will not be created in case -nodefaults is specified
> >       on the command line.
> >   (2) Autocreating the device is also turned off in case xen-platform
> >       is added manually via -device.
> >
> > With the patch applied you can move the xen-platform device to some
> > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> >
> > Tested-by: Tiejun Chen <tiejun.chen@intel.com>

Sorry, I'm not sure if this works well as Gerd expect.

I already reply something online, please take a look at that before apply. 

Thanks
Tiejun

> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Applied, thanks!
> 
> > ---
> >  hw/i386/pc_piix.c    | 2 +-
> >  include/hw/xen/xen.h | 1 +
> >  vl.c                 | 3 +++
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index
> > eaf3e61..f987d03 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs
> *args)
> >      pc_init_pci(args);
> >
> >      bus = pci_find_primary_bus();
> > -    if (bus != NULL) {
> > +    if (bus != NULL && default_xenplatform) {
> >          pci_create_simple(bus, -1, "xen-platform");
> >      }
> >  }
> > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index
> > 85fda3d..b350413 100644
> > --- a/include/hw/xen/xen.h
> > +++ b/include/hw/xen/xen.h
> > @@ -20,6 +20,7 @@ enum xen_mode {
> >
> >  extern uint32_t xen_domid;
> >  extern enum xen_mode xen_mode;
> > +extern int default_xenplatform;
> >
> >  extern bool xen_allowed;
> >
> > diff --git a/vl.c b/vl.c
> > index 709d8cd..673148e 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -226,6 +226,7 @@ static int default_floppy = 1;  static int
> > default_cdrom = 1;  static int default_sdcard = 1;  static int
> > default_vga = 1;
> > +int default_xenplatform = 1;
> >
> >  static struct {
> >      const char *driver;
> > @@ -247,6 +248,7 @@ static struct {
> >      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
> >      { .driver = "vmware-svga",          .flag = &default_vga       },
> >      { .driver = "qxl-vga",              .flag = &default_vga       },
> > +    { .driver = "xen-platform",         .flag = &default_xenplatform },
> >  };
> >
> >  static QemuOptsList qemu_rtc_opts = { @@ -4101,6 +4103,7 @@ int
> > main(int argc, char **argv, char **envp)
> >          default_monitor = 0;
> >          default_net = 0;
> >          default_vga = 0;
> > +        default_xenplatform = 0;
> >      }
> >
> >      if (is_daemonized()) {
> > --
> > 1.8.3.1

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Michael S. Tsirkin]

On Thu, May 22, 2014 at 10:57:54AM +0000, Chen, Tiejun wrote:
> > -----Original Message-----
> > From: qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org
> > [mailto:qemu-devel-bounces+tiejun.chen=intel.com@nongnu.org] On Behalf Of
> > Michael S. Tsirkin
> > Sent: Thursday, May 22, 2014 3:22 PM
> > To: Gerd Hoffmann
> > Cc: qemu-devel@nongnu.org; Anthony Liguori
> > Subject: Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device
> > 
> > On Thu, May 22, 2014 at 09:20:50AM +0200, Gerd Hoffmann wrote:
> > > Patch hooks up the xen platform device to the default device code we
> > > have in qemu.  Two effects:
> > >
> > >   (1) The device will not be created in case -nodefaults is specified
> > >       on the command line.
> > >   (2) Autocreating the device is also turned off in case xen-platform
> > >       is added manually via -device.
> > >
> > > With the patch applied you can move the xen-platform device to some
> > > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > >
> > > Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> 
> Sorry, I'm not sure if this works well as Gerd expect.
> 
> I already reply something online, please take a look at that before apply. 
> 
> Thanks
> Tiejun

No worries, I'll wait.

> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > Applied, thanks!
> > 
> > > ---
> > >  hw/i386/pc_piix.c    | 2 +-
> > >  include/hw/xen/xen.h | 1 +
> > >  vl.c                 | 3 +++
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index
> > > eaf3e61..f987d03 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs
> > *args)
> > >      pc_init_pci(args);
> > >
> > >      bus = pci_find_primary_bus();
> > > -    if (bus != NULL) {
> > > +    if (bus != NULL && default_xenplatform) {
> > >          pci_create_simple(bus, -1, "xen-platform");
> > >      }
> > >  }
> > > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h index
> > > 85fda3d..b350413 100644
> > > --- a/include/hw/xen/xen.h
> > > +++ b/include/hw/xen/xen.h
> > > @@ -20,6 +20,7 @@ enum xen_mode {
> > >
> > >  extern uint32_t xen_domid;
> > >  extern enum xen_mode xen_mode;
> > > +extern int default_xenplatform;
> > >
> > >  extern bool xen_allowed;
> > >
> > > diff --git a/vl.c b/vl.c
> > > index 709d8cd..673148e 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -226,6 +226,7 @@ static int default_floppy = 1;  static int
> > > default_cdrom = 1;  static int default_sdcard = 1;  static int
> > > default_vga = 1;
> > > +int default_xenplatform = 1;
> > >
> > >  static struct {
> > >      const char *driver;
> > > @@ -247,6 +248,7 @@ static struct {
> > >      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
> > >      { .driver = "vmware-svga",          .flag = &default_vga       },
> > >      { .driver = "qxl-vga",              .flag = &default_vga       },
> > > +    { .driver = "xen-platform",         .flag = &default_xenplatform },
> > >  };
> > >
> > >  static QemuOptsList qemu_rtc_opts = { @@ -4101,6 +4103,7 @@ int
> > > main(int argc, char **argv, char **envp)
> > >          default_monitor = 0;
> > >          default_net = 0;
> > >          default_vga = 0;
> > > +        default_xenplatform = 0;
> > >      }
> > >
> > >      if (is_daemonized()) {
> > > --
> > > 1.8.3.1

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Thu, 22 May 2014, Gerd Hoffmann wrote:
> Patch hooks up the xen platform device to the default device code we
> have in qemu.  Two effects:
> 
>   (1) The device will not be created in case -nodefaults is specified
>       on the command line.
>   (2) Autocreating the device is also turned off in case xen-platform
>       is added manually via -device.
> 
> With the patch applied you can move the xen-platform device to some
> other place with a simple 'qemu -device xen-platform,addr=$slot'.
> 
> Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Given that libxl always passes -nodefaults to QEMU, this patch is going
to effectively disable xen_platform_pci for all Xen users. It is not a
good idea. With the patch applied a Xen user would have no way to enable
xen_platform_pci except for passing some magic command line runes via
device_model_args_hvm.


>  hw/i386/pc_piix.c    | 2 +-
>  include/hw/xen/xen.h | 1 +
>  vl.c                 | 3 +++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index eaf3e61..f987d03 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>      pc_init_pci(args);
>  
>      bus = pci_find_primary_bus();
> -    if (bus != NULL) {
> +    if (bus != NULL && default_xenplatform) {
>          pci_create_simple(bus, -1, "xen-platform");
>      }
>  }
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 85fda3d..b350413 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -20,6 +20,7 @@ enum xen_mode {
>  
>  extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
> +extern int default_xenplatform;
>  
>  extern bool xen_allowed;
>  
> diff --git a/vl.c b/vl.c
> index 709d8cd..673148e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -226,6 +226,7 @@ static int default_floppy = 1;
>  static int default_cdrom = 1;
>  static int default_sdcard = 1;
>  static int default_vga = 1;
> +int default_xenplatform = 1;
>  
>  static struct {
>      const char *driver;
> @@ -247,6 +248,7 @@ static struct {
>      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
>      { .driver = "vmware-svga",          .flag = &default_vga       },
>      { .driver = "qxl-vga",              .flag = &default_vga       },
> +    { .driver = "xen-platform",         .flag = &default_xenplatform },
>  };
>  
>  static QemuOptsList qemu_rtc_opts = {
> @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
>          default_monitor = 0;
>          default_net = 0;
>          default_vga = 0;
> +        default_xenplatform = 0;
>      }
>  
>      if (is_daemonized()) {
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Michael S. Tsirkin]

On Thu, May 22, 2014 at 01:11:28PM +0100, Stefano Stabellini wrote:
> On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > Patch hooks up the xen platform device to the default device code we
> > have in qemu.  Two effects:
> > 
> >   (1) The device will not be created in case -nodefaults is specified
> >       on the command line.
> >   (2) Autocreating the device is also turned off in case xen-platform
> >       is added manually via -device.
> > 
> > With the patch applied you can move the xen-platform device to some
> > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > 
> > Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Given that libxl always passes -nodefaults to QEMU, this patch is going
> to effectively disable xen_platform_pci for all Xen users. It is not a
> good idea. With the patch applied a Xen user would have no way to enable
> xen_platform_pci except for passing some magic command line runes via
> device_model_args_hvm.
> 

Yes, it's an unfortunate use of the interface.
How about a new machine type for xenfv - that's the only one
that's affected, right?
It's time xen started versioning qemu hardware anyway.

> >  hw/i386/pc_piix.c    | 2 +-
> >  include/hw/xen/xen.h | 1 +
> >  vl.c                 | 3 +++
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index eaf3e61..f987d03 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
> >      pc_init_pci(args);
> >  
> >      bus = pci_find_primary_bus();
> > -    if (bus != NULL) {
> > +    if (bus != NULL && default_xenplatform) {
> >          pci_create_simple(bus, -1, "xen-platform");
> >      }
> >  }
> > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> > index 85fda3d..b350413 100644
> > --- a/include/hw/xen/xen.h
> > +++ b/include/hw/xen/xen.h
> > @@ -20,6 +20,7 @@ enum xen_mode {
> >  
> >  extern uint32_t xen_domid;
> >  extern enum xen_mode xen_mode;
> > +extern int default_xenplatform;
> >  
> >  extern bool xen_allowed;
> >  
> > diff --git a/vl.c b/vl.c
> > index 709d8cd..673148e 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -226,6 +226,7 @@ static int default_floppy = 1;
> >  static int default_cdrom = 1;
> >  static int default_sdcard = 1;
> >  static int default_vga = 1;
> > +int default_xenplatform = 1;
> >  
> >  static struct {
> >      const char *driver;
> > @@ -247,6 +248,7 @@ static struct {
> >      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
> >      { .driver = "vmware-svga",          .flag = &default_vga       },
> >      { .driver = "qxl-vga",              .flag = &default_vga       },
> > +    { .driver = "xen-platform",         .flag = &default_xenplatform },
> >  };
> >  
> >  static QemuOptsList qemu_rtc_opts = {
> > @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
> >          default_monitor = 0;
> >          default_net = 0;
> >          default_vga = 0;
> > +        default_xenplatform = 0;
> >      }
> >  
> >      if (is_daemonized()) {
> > -- 
> > 1.8.3.1
> > 
> > 

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Gerd Hoffmann]

On Do, 2014-05-22 at 15:35 +0300, Michael S. Tsirkin wrote:
> On Thu, May 22, 2014 at 01:11:28PM +0100, Stefano Stabellini wrote:
> > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > Patch hooks up the xen platform device to the default device code we
> > > have in qemu.  Two effects:
> > > 
> > >   (1) The device will not be created in case -nodefaults is specified
> > >       on the command line.
> > >   (2) Autocreating the device is also turned off in case xen-platform
> > >       is added manually via -device.
> > > 
> > > With the patch applied you can move the xen-platform device to some
> > > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > > 
> > > Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > to effectively disable xen_platform_pci for all Xen users. It is not a
> > good idea. With the patch applied a Xen user would have no way to enable
> > xen_platform_pci except for passing some magic command line runes via
> > device_model_args_hvm.
> > 
> 
> Yes, it's an unfortunate use of the interface.
> How about a new machine type for xenfv - that's the only one
> that's affected, right?
> It's time xen started versioning qemu hardware anyway.

That would do the trick for sure.  Question is what versioning scheme.
Might be useful for xen machine types to version after xen releases,
i.e. xenfv-4.4 would would be supposed to be compatible with the xen-4.4
toolstack etc.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Michael S. Tsirkin]

On Thu, May 22, 2014 at 02:53:19PM +0200, Gerd Hoffmann wrote:
> On Do, 2014-05-22 at 15:35 +0300, Michael S. Tsirkin wrote:
> > On Thu, May 22, 2014 at 01:11:28PM +0100, Stefano Stabellini wrote:
> > > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > Patch hooks up the xen platform device to the default device code we
> > > > have in qemu.  Two effects:
> > > > 
> > > >   (1) The device will not be created in case -nodefaults is specified
> > > >       on the command line.
> > > >   (2) Autocreating the device is also turned off in case xen-platform
> > > >       is added manually via -device.
> > > > 
> > > > With the patch applied you can move the xen-platform device to some
> > > > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > > > 
> > > > Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > > 
> > > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > good idea. With the patch applied a Xen user would have no way to enable
> > > xen_platform_pci except for passing some magic command line runes via
> > > device_model_args_hvm.
> > > 
> > 
> > Yes, it's an unfortunate use of the interface.
> > How about a new machine type for xenfv - that's the only one
> > that's affected, right?
> > It's time xen started versioning qemu hardware anyway.
> 
> That would do the trick for sure.  Question is what versioning scheme.
> Might be useful for xen machine types to version after xen releases,
> i.e. xenfv-4.4 would would be supposed to be compatible with the xen-4.4
> toolstack etc.
> 
> cheers,
>   Gerd
> 

It's much easier for us to version it with qemu machine version,
like we do for pc, if that can work at all.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Gerd Hoffmann]

  Hi,

> Given that libxl always passes -nodefaults to QEMU, this patch is going
> to effectively disable xen_platform_pci for all Xen users. It is not a
> good idea. With the patch applied a Xen user would have no way to enable
> xen_platform_pci except for passing some magic command line runes via
> device_model_args_hvm.

Indeed.

> > @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
> >          default_monitor = 0;
> >          default_net = 0;
> >          default_vga = 0;
> > +        default_xenplatform = 0;
> >      }

With that chunk removed -nodefaults will have no effect for the xen
platform device, but explicitly moving it somewhere else via -device
xen-platform,addr=$slot should still work.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Thu, 22 May 2014, Gerd Hoffmann wrote:
>   Hi,
> 
> > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > to effectively disable xen_platform_pci for all Xen users. It is not a
> > good idea. With the patch applied a Xen user would have no way to enable
> > xen_platform_pci except for passing some magic command line runes via
> > device_model_args_hvm.
> 
> Indeed.
> 
> > > @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
> > >          default_monitor = 0;
> > >          default_net = 0;
> > >          default_vga = 0;
> > > +        default_xenplatform = 0;
> > >      }
> 
> With that chunk removed -nodefaults will have no effect for the xen
> platform device, but explicitly moving it somewhere else via -device
> xen-platform,addr=$slot should still work.

That could work.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paolo Bonzini]

Il 22/05/2014 14:11, Stefano Stabellini ha scritto:
> On Thu, 22 May 2014, Gerd Hoffmann wrote:
>> Patch hooks up the xen platform device to the default device code we
>> have in qemu.  Two effects:
>>
>>   (1) The device will not be created in case -nodefaults is specified
>>       on the command line.
>>   (2) Autocreating the device is also turned off in case xen-platform
>>       is added manually via -device.
>>
>> With the patch applied you can move the xen-platform device to some
>> other place with a simple 'qemu -device xen-platform,addr=$slot'.
>>
>> Tested-by: Tiejun Chen <tiejun.chen@intel.com>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>
> Given that libxl always passes -nodefaults to QEMU, this patch is going
> to effectively disable xen_platform_pci for all Xen users. It is not a
> good idea. With the patch applied a Xen user would have no way to enable
> xen_platform_pci except for passing some magic command line runes via
> device_model_args_hvm.

In fact this code only runs for "-M xenfv".  If you use "-M pc", the 
xen-platform device has to be added manually.  Perhaps it would be 
worthwhile to do the opposite, i.e. add the xen-platform device to "-M 
pc" if not using -nodefaults.

Paolo

>
>>  hw/i386/pc_piix.c    | 2 +-
>>  include/hw/xen/xen.h | 1 +
>>  vl.c                 | 3 +++
>>  3 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index eaf3e61..f987d03 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>>      pc_init_pci(args);
>>
>>      bus = pci_find_primary_bus();
>> -    if (bus != NULL) {
>> +    if (bus != NULL && default_xenplatform) {
>>          pci_create_simple(bus, -1, "xen-platform");
>>      }
>>  }
>> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
>> index 85fda3d..b350413 100644
>> --- a/include/hw/xen/xen.h
>> +++ b/include/hw/xen/xen.h
>> @@ -20,6 +20,7 @@ enum xen_mode {
>>
>>  extern uint32_t xen_domid;
>>  extern enum xen_mode xen_mode;
>> +extern int default_xenplatform;
>>
>>  extern bool xen_allowed;
>>
>> diff --git a/vl.c b/vl.c
>> index 709d8cd..673148e 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -226,6 +226,7 @@ static int default_floppy = 1;
>>  static int default_cdrom = 1;
>>  static int default_sdcard = 1;
>>  static int default_vga = 1;
>> +int default_xenplatform = 1;
>>
>>  static struct {
>>      const char *driver;
>> @@ -247,6 +248,7 @@ static struct {
>>      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
>>      { .driver = "vmware-svga",          .flag = &default_vga       },
>>      { .driver = "qxl-vga",              .flag = &default_vga       },
>> +    { .driver = "xen-platform",         .flag = &default_xenplatform },
>>  };
>>
>>  static QemuOptsList qemu_rtc_opts = {
>> @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
>>          default_monitor = 0;
>>          default_net = 0;
>>          default_vga = 0;
>> +        default_xenplatform = 0;
>>      }
>>
>>      if (is_daemonized()) {
>> --
>> 1.8.3.1
>>
>>
>
>

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Gerd Hoffmann]

> > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > to effectively disable xen_platform_pci for all Xen users. It is not a
> > good idea. With the patch applied a Xen user would have no way to enable
> > xen_platform_pci except for passing some magic command line runes via
> > device_model_args_hvm.
> 
> In fact this code only runs for "-M xenfv".  If you use "-M pc", the 
> xen-platform device has to be added manually.  Perhaps it would be 
> worthwhile to do the opposite, i.e. add the xen-platform device to "-M 
> pc" if not using -nodefaults.

/me looks at the code.  Yes, all the differences between xenfv and pc
machine types are guarded by if (xen_enabled()) these days, except for
adding the platform device.

So using the pc machine type should just work on xen, and give you a
machine without the platform device.  So it can be added via -device, at
any slot, if needed.  No need to patch qemu at all.  Adding or not
adding xen-platform can easily handled by libxl then, depending on the
xen_platform_pci switch in the config file.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > good idea. With the patch applied a Xen user would have no way to enable
> > > xen_platform_pci except for passing some magic command line runes via
> > > device_model_args_hvm.
> > 
> > In fact this code only runs for "-M xenfv".  If you use "-M pc", the 
> > xen-platform device has to be added manually.  Perhaps it would be 
> > worthwhile to do the opposite, i.e. add the xen-platform device to "-M 
> > pc" if not using -nodefaults.
> 
> /me looks at the code.  Yes, all the differences between xenfv and pc
> machine types are guarded by if (xen_enabled()) these days, except for
> adding the platform device.
> 
> So using the pc machine type should just work on xen, and give you a
> machine without the platform device.  So it can be added via -device, at
> any slot, if needed.  No need to patch qemu at all.  Adding or not
> adding xen-platform can easily handled by libxl then, depending on the
> xen_platform_pci switch in the config file.

I agree. Changing libxl to always use -M pc and using -device to add
xen-platform when needed sounds like the best option.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paul Durrant]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 22 May 2014 14:57
> To: Gerd Hoffmann
> Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org;
> Anthony Liguori; mst@redhat.com
> Subject: Re: [PATCH] xen: make xen-platform a default device
> 
> On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > > good idea. With the patch applied a Xen user would have no way to
> enable
> > > > xen_platform_pci except for passing some magic command line runes
> via
> > > > device_model_args_hvm.
> > >
> > > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> > > xen-platform device has to be added manually.  Perhaps it would be
> > > worthwhile to do the opposite, i.e. add the xen-platform device to "-M
> > > pc" if not using -nodefaults.
> >
> > /me looks at the code.  Yes, all the differences between xenfv and pc
> > machine types are guarded by if (xen_enabled()) these days, except for
> > adding the platform device.
> >
> > So using the pc machine type should just work on xen, and give you a
> > machine without the platform device.  So it can be added via -device, at
> > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > adding xen-platform can easily handled by libxl then, depending on the
> > xen_platform_pci switch in the config file.
> 
> I agree. Changing libxl to always use -M pc and using -device to add
> xen-platform when needed sounds like the best option.

Really? You opposed this before:

http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html

stating that you wanted a mechanism to query the running version of QEMU before choosing the machine type.

  Paul

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Fri, 23 May 2014, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: 22 May 2014 14:57
> > To: Gerd Hoffmann
> > Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-devel@nongnu.org;
> > Anthony Liguori; mst@redhat.com
> > Subject: Re: [PATCH] xen: make xen-platform a default device
> > 
> > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > > > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > > > good idea. With the patch applied a Xen user would have no way to
> > enable
> > > > > xen_platform_pci except for passing some magic command line runes
> > via
> > > > > device_model_args_hvm.
> > > >
> > > > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> > > > xen-platform device has to be added manually.  Perhaps it would be
> > > > worthwhile to do the opposite, i.e. add the xen-platform device to "-M
> > > > pc" if not using -nodefaults.
> > >
> > > /me looks at the code.  Yes, all the differences between xenfv and pc
> > > machine types are guarded by if (xen_enabled()) these days, except for
> > > adding the platform device.
> > >
> > > So using the pc machine type should just work on xen, and give you a
> > > machine without the platform device.  So it can be added via -device, at
> > > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > > adding xen-platform can easily handled by libxl then, depending on the
> > > xen_platform_pci switch in the config file.
> > 
> > I agree. Changing libxl to always use -M pc and using -device to add
> > xen-platform when needed sounds like the best option.
> 
> Really? You opposed this before:
> 
> http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html
> 
> stating that you wanted a mechanism to query the running version of QEMU before choosing the machine type.

Many QEMU releases have been made since then. I feel more comfortable
with using -M pc, especially given the alternatives that have been
proposed. Of course having a mechanism to query the running version of
QEMU would still be the safest solution.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paul Durrant]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 23 May 2014 11:11
> To: Paul Durrant
> Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> devel@nongnu.org; Anthony Liguori; mst@redhat.com
> Subject: RE: [PATCH] xen: make xen-platform a default device
> 
> On Fri, 23 May 2014, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > Sent: 22 May 2014 14:57
> > > To: Gerd Hoffmann
> > > Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-
> devel@nongnu.org;
> > > Anthony Liguori; mst@redhat.com
> > > Subject: Re: [PATCH] xen: make xen-platform a default device
> > >
> > > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > > > Given that libxl always passes -nodefaults to QEMU, this patch is
> going
> > > > > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > > > > good idea. With the patch applied a Xen user would have no way to
> > > enable
> > > > > > xen_platform_pci except for passing some magic command line
> runes
> > > via
> > > > > > device_model_args_hvm.
> > > > >
> > > > > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> > > > > xen-platform device has to be added manually.  Perhaps it would be
> > > > > worthwhile to do the opposite, i.e. add the xen-platform device to "-
> M
> > > > > pc" if not using -nodefaults.
> > > >
> > > > /me looks at the code.  Yes, all the differences between xenfv and pc
> > > > machine types are guarded by if (xen_enabled()) these days, except
> for
> > > > adding the platform device.
> > > >
> > > > So using the pc machine type should just work on xen, and give you a
> > > > machine without the platform device.  So it can be added via -device, at
> > > > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > > > adding xen-platform can easily handled by libxl then, depending on the
> > > > xen_platform_pci switch in the config file.
> > >
> > > I agree. Changing libxl to always use -M pc and using -device to add
> > > xen-platform when needed sounds like the best option.
> >
> > Really? You opposed this before:
> >
> > http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html
> >
> > stating that you wanted a mechanism to query the running version of
> QEMU before choosing the machine type.
> 
> Many QEMU releases have been made since then. I feel more comfortable
> with using -M pc, especially given the alternatives that have been
> proposed. Of course having a mechanism to query the running version of
> QEMU would still be the safest solution.

But I thought the problem was that, should someone change what 'pc' means, then libxl would invoke the 'wrong' machine type on a domain restore. I know that xenfv is essentially 'pc' at the moment but would could fix on a particular version if compatibility becomes an issue. So, rather than choosing 'pc' now, should we not choose 'pc-i440fx-1.6' which is the current default?

  Paul

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Fri, 23 May 2014, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > Sent: 23 May 2014 11:11
> > To: Paul Durrant
> > Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> > devel@nongnu.org; Anthony Liguori; mst@redhat.com
> > Subject: RE: [PATCH] xen: make xen-platform a default device
> > 
> > On Fri, 23 May 2014, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > > Sent: 22 May 2014 14:57
> > > > To: Gerd Hoffmann
> > > > Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-
> > devel@nongnu.org;
> > > > Anthony Liguori; mst@redhat.com
> > > > Subject: Re: [PATCH] xen: make xen-platform a default device
> > > >
> > > > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > > > > Given that libxl always passes -nodefaults to QEMU, this patch is
> > going
> > > > > > > to effectively disable xen_platform_pci for all Xen users. It is not a
> > > > > > > good idea. With the patch applied a Xen user would have no way to
> > > > enable
> > > > > > > xen_platform_pci except for passing some magic command line
> > runes
> > > > via
> > > > > > > device_model_args_hvm.
> > > > > >
> > > > > > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> > > > > > xen-platform device has to be added manually.  Perhaps it would be
> > > > > > worthwhile to do the opposite, i.e. add the xen-platform device to "-
> > M
> > > > > > pc" if not using -nodefaults.
> > > > >
> > > > > /me looks at the code.  Yes, all the differences between xenfv and pc
> > > > > machine types are guarded by if (xen_enabled()) these days, except
> > for
> > > > > adding the platform device.
> > > > >
> > > > > So using the pc machine type should just work on xen, and give you a
> > > > > machine without the platform device.  So it can be added via -device, at
> > > > > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > > > > adding xen-platform can easily handled by libxl then, depending on the
> > > > > xen_platform_pci switch in the config file.
> > > >
> > > > I agree. Changing libxl to always use -M pc and using -device to add
> > > > xen-platform when needed sounds like the best option.
> > >
> > > Really? You opposed this before:
> > >
> > > http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html
> > >
> > > stating that you wanted a mechanism to query the running version of
> > QEMU before choosing the machine type.
> > 
> > Many QEMU releases have been made since then. I feel more comfortable
> > with using -M pc, especially given the alternatives that have been
> > proposed. Of course having a mechanism to query the running version of
> > QEMU would still be the safest solution.
> 
> But I thought the problem was that, should someone change what 'pc' means, then libxl would invoke the 'wrong' machine type on a domain restore. I know that xenfv is essentially 'pc' at the moment but would could fix on a particular version if compatibility becomes an issue. So, rather than choosing 'pc' now, should we not choose 'pc-i440fx-1.6' which is the current default?

One issue is that -M pc didn't always work with Xen. Now it does and we
are already relying on it in libxl since
2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
releases from 1.6 onward work well with Xen and -M pc. Older QEMU
releases are considered ancient and unmaintained. This is what I was
referring to in my last reply. I really meant "we should move away from
xenfv and use a pc.* machine that does not create xen-platform by
default".

As you say the other issue is the version of the pc machine, especially
in relation to save/restore. However since:

commit 2bc047635b51abd41c917aa2b813211ee0de2c38
Author: Anthony PERARD <anthony.perard@citrix.com>
Date:   Wed Nov 27 18:21:34 2013 +0000

    libxl: Handle xen_platform_pci=0 case with qemu-xen.

we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
should change that too and backport the patch to 4.4. pc-i440fx-1.6
seems like a good choice to me.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paul Durrant]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> Sent: 23 May 2014 12:37
> To: Paul Durrant
> Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> devel@nongnu.org; Anthony Liguori; mst@redhat.com; Anthony Perard
> Subject: RE: [PATCH] xen: make xen-platform a default device
> 
> On Fri, 23 May 2014, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > Sent: 23 May 2014 11:11
> > > To: Paul Durrant
> > > Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> > > devel@nongnu.org; Anthony Liguori; mst@redhat.com
> > > Subject: RE: [PATCH] xen: make xen-platform a default device
> > >
> > > On Fri, 23 May 2014, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com]
> > > > > Sent: 22 May 2014 14:57
> > > > > To: Gerd Hoffmann
> > > > > Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-
> > > devel@nongnu.org;
> > > > > Anthony Liguori; mst@redhat.com
> > > > > Subject: Re: [PATCH] xen: make xen-platform a default device
> > > > >
> > > > > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > > > > > Given that libxl always passes -nodefaults to QEMU, this patch is
> > > going
> > > > > > > > to effectively disable xen_platform_pci for all Xen users. It is not
> a
> > > > > > > > good idea. With the patch applied a Xen user would have no way
> to
> > > > > enable
> > > > > > > > xen_platform_pci except for passing some magic command line
> > > runes
> > > > > via
> > > > > > > > device_model_args_hvm.
> > > > > > >
> > > > > > > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> > > > > > > xen-platform device has to be added manually.  Perhaps it would
> be
> > > > > > > worthwhile to do the opposite, i.e. add the xen-platform device
> to "-
> > > M
> > > > > > > pc" if not using -nodefaults.
> > > > > >
> > > > > > /me looks at the code.  Yes, all the differences between xenfv and
> pc
> > > > > > machine types are guarded by if (xen_enabled()) these days,
> except
> > > for
> > > > > > adding the platform device.
> > > > > >
> > > > > > So using the pc machine type should just work on xen, and give you
> a
> > > > > > machine without the platform device.  So it can be added via -
> device, at
> > > > > > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > > > > > adding xen-platform can easily handled by libxl then, depending on
> the
> > > > > > xen_platform_pci switch in the config file.
> > > > >
> > > > > I agree. Changing libxl to always use -M pc and using -device to add
> > > > > xen-platform when needed sounds like the best option.
> > > >
> > > > Really? You opposed this before:
> > > >
> > > > http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html
> > > >
> > > > stating that you wanted a mechanism to query the running version of
> > > QEMU before choosing the machine type.
> > >
> > > Many QEMU releases have been made since then. I feel more
> comfortable
> > > with using -M pc, especially given the alternatives that have been
> > > proposed. Of course having a mechanism to query the running version of
> > > QEMU would still be the safest solution.
> >
> > But I thought the problem was that, should someone change what 'pc'
> means, then libxl would invoke the 'wrong' machine type on a domain
> restore. I know that xenfv is essentially 'pc' at the moment but would could
> fix on a particular version if compatibility becomes an issue. So, rather than
> choosing 'pc' now, should we not choose 'pc-i440fx-1.6' which is the current
> default?
> 
> One issue is that -M pc didn't always work with Xen. Now it does and we
> are already relying on it in libxl since
> 2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
> releases from 1.6 onward work well with Xen and -M pc. Older QEMU
> releases are considered ancient and unmaintained. This is what I was
> referring to in my last reply. I really meant "we should move away from
> xenfv and use a pc.* machine that does not create xen-platform by
> default".
> 
> As you say the other issue is the version of the pc machine, especially
> in relation to save/restore. However since:
> 
> commit 2bc047635b51abd41c917aa2b813211ee0de2c38
> Author: Anthony PERARD <anthony.perard@citrix.com>
> Date:   Wed Nov 27 18:21:34 2013 +0000
> 
>     libxl: Handle xen_platform_pci=0 case with qemu-xen.
> 
> we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
> should change that too and backport the patch to 4.4. pc-i440fx-1.6
> seems like a good choice to me.

Yep - that sounds good.

  Paul

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Thu, 22 May 2014, Paolo Bonzini wrote:
> Il 22/05/2014 14:11, Stefano Stabellini ha scritto:
> > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > Patch hooks up the xen platform device to the default device code we
> > > have in qemu.  Two effects:
> > > 
> > >   (1) The device will not be created in case -nodefaults is specified
> > >       on the command line.
> > >   (2) Autocreating the device is also turned off in case xen-platform
> > >       is added manually via -device.
> > > 
> > > With the patch applied you can move the xen-platform device to some
> > > other place with a simple 'qemu -device xen-platform,addr=$slot'.
> > > 
> > > Tested-by: Tiejun Chen <tiejun.chen@intel.com>
> > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > Given that libxl always passes -nodefaults to QEMU, this patch is going
> > to effectively disable xen_platform_pci for all Xen users. It is not a
> > good idea. With the patch applied a Xen user would have no way to enable
> > xen_platform_pci except for passing some magic command line runes via
> > device_model_args_hvm.
> 
> In fact this code only runs for "-M xenfv".  If you use "-M pc", the
> xen-platform device has to be added manually.  Perhaps it would be worthwhile
> to do the opposite, i.e. add the xen-platform device to "-M pc" if not using
> -nodefaults.

Unfortunately that would not work too. libxl exploits the fact that -M
pc does not have a xen-platform device by default:

    case LIBXL_DOMAIN_TYPE_HVM:
        if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
            /* Switching here to the machine "pc" which does not add
             * the xen-platform device instead of the default "xenfv" machine.
             */
            flexarray_append(dm_args, "pc,accel=xen");
        } else {
            flexarray_append(dm_args, "xenfv");
        }

We could/should change libxl to always use -M pc but I would still
rather avoid changing the existing interface between libxl and QEMU.
Versioning xenfv or using a new pc machine is OK though.


> > >  hw/i386/pc_piix.c    | 2 +-
> > >  include/hw/xen/xen.h | 1 +
> > >  vl.c                 | 3 +++
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > index eaf3e61..f987d03 100644
> > > --- a/hw/i386/pc_piix.c
> > > +++ b/hw/i386/pc_piix.c
> > > @@ -385,7 +385,7 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
> > >      pc_init_pci(args);
> > > 
> > >      bus = pci_find_primary_bus();
> > > -    if (bus != NULL) {
> > > +    if (bus != NULL && default_xenplatform) {
> > >          pci_create_simple(bus, -1, "xen-platform");
> > >      }
> > >  }
> > > diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> > > index 85fda3d..b350413 100644
> > > --- a/include/hw/xen/xen.h
> > > +++ b/include/hw/xen/xen.h
> > > @@ -20,6 +20,7 @@ enum xen_mode {
> > > 
> > >  extern uint32_t xen_domid;
> > >  extern enum xen_mode xen_mode;
> > > +extern int default_xenplatform;
> > > 
> > >  extern bool xen_allowed;
> > > 
> > > diff --git a/vl.c b/vl.c
> > > index 709d8cd..673148e 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -226,6 +226,7 @@ static int default_floppy = 1;
> > >  static int default_cdrom = 1;
> > >  static int default_sdcard = 1;
> > >  static int default_vga = 1;
> > > +int default_xenplatform = 1;
> > > 
> > >  static struct {
> > >      const char *driver;
> > > @@ -247,6 +248,7 @@ static struct {
> > >      { .driver = "isa-cirrus-vga",       .flag = &default_vga       },
> > >      { .driver = "vmware-svga",          .flag = &default_vga       },
> > >      { .driver = "qxl-vga",              .flag = &default_vga       },
> > > +    { .driver = "xen-platform",         .flag = &default_xenplatform },
> > >  };
> > > 
> > >  static QemuOptsList qemu_rtc_opts = {
> > > @@ -4101,6 +4103,7 @@ int main(int argc, char **argv, char **envp)
> > >          default_monitor = 0;
> > >          default_net = 0;
> > >          default_vga = 0;
> > > +        default_xenplatform = 0;
> > >      }
> > > 
> > >      if (is_daemonized()) {
> > > --
> > > 1.8.3.1
> > > 
> > > 
> > 
> > 
> 

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paolo Bonzini]

Il 22/05/2014 15:55, Stefano Stabellini ha scritto:
>> > In fact this code only runs for "-M xenfv".  If you use "-M pc", the
>> > xen-platform device has to be added manually.  Perhaps it would be worthwhile
>> > to do the opposite, i.e. add the xen-platform device to "-M pc" if not using
>> > -nodefaults.
> Unfortunately that would not work too. libxl exploits the fact that -M
> pc does not have a xen-platform device by default:

It wouldn't affect libxl, since it uses -nodefaults too (thus the 
xen-platform device would not be auto-added for -M pc).

Paolo

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Fabio Fantoni]

> One issue is that -M pc didn't always work with Xen. Now it does and we
> are already relying on it in libxl since
> 2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
> releases from 1.6 onward work well with Xen and -M pc. Older QEMU
> releases are considered ancient and unmaintained. This is what I was
> referring to in my last reply. I really meant "we should move away from
> xenfv and use a pc.* machine that does not create xen-platform by
> default".
>
> As you say the other issue is the version of the pc machine, especially
> in relation to save/restore. However since:
>
> commit 2bc047635b51abd41c917aa2b813211ee0de2c38
> Author: Anthony PERARD <address@hidden>
> Date:   Wed Nov 27 18:21:34 2013 +0000
>
>      libxl: Handle xen_platform_pci=0 case with qemu-xen.
>
> we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
> should change that too and backport the patch to 4.4. pc-i440fx-1.6
> seems like a good choice to me.

Use "-M pc" as default seems a good idea.
Use specific version maybe too.
This way the base hardware should stay the same even if updating qemu, 
is right? If yes, this should also solve possible problem like windows 
reactivation request for different hardware, right?
How about create also xl parameter to select the "pc" model?

Thanks for any reply and sorry for my bad english.

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Stefano Stabellini]

On Fri, 23 May 2014, Fabio Fantoni wrote:
> > One issue is that -M pc didn't always work with Xen. Now it does and we
> > are already relying on it in libxl since
> > 2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
> > releases from 1.6 onward work well with Xen and -M pc. Older QEMU
> > releases are considered ancient and unmaintained. This is what I was
> > referring to in my last reply. I really meant "we should move away from
> > xenfv and use a pc.* machine that does not create xen-platform by
> > default".
> > 
> > As you say the other issue is the version of the pc machine, especially
> > in relation to save/restore. However since:
> > 
> > commit 2bc047635b51abd41c917aa2b813211ee0de2c38
> > Author: Anthony PERARD <address@hidden>
> > Date:   Wed Nov 27 18:21:34 2013 +0000
> > 
> >      libxl: Handle xen_platform_pci=0 case with qemu-xen.
> > 
> > we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
> > should change that too and backport the patch to 4.4. pc-i440fx-1.6
> > seems like a good choice to me.
> 
> Use "-M pc" as default seems a good idea.
> Use specific version maybe too.
> This way the base hardware should stay the same even if updating qemu, is
> right?

Yep


> If yes, this should also solve possible problem like windows
> reactivation request for different hardware, right?

Right


> How about create also xl parameter to select the "pc" model?

Having a VM config parameter for it is OK. In the past I argued against
relying on such a parameter to solve all compatibility issues with QEMU
because I would like libxl to be able to select the right QEMU machine
automatically, without user intervention.  But the option could still be
useful for debugging. 

Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Fabio Fantoni]

Il 23/05/2014 16:30, Stefano Stabellini ha scritto:
> On Fri, 23 May 2014, Fabio Fantoni wrote:
>>> One issue is that -M pc didn't always work with Xen. Now it does and we
>>> are already relying on it in libxl since
>>> 2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
>>> releases from 1.6 onward work well with Xen and -M pc. Older QEMU
>>> releases are considered ancient and unmaintained. This is what I was
>>> referring to in my last reply. I really meant "we should move away from
>>> xenfv and use a pc.* machine that does not create xen-platform by
>>> default".
>>>
>>> As you say the other issue is the version of the pc machine, especially
>>> in relation to save/restore. However since:
>>>
>>> commit 2bc047635b51abd41c917aa2b813211ee0de2c38
>>> Author: Anthony PERARD <address@hidden>
>>> Date:   Wed Nov 27 18:21:34 2013 +0000
>>>
>>>       libxl: Handle xen_platform_pci=0 case with qemu-xen.
>>>
>>> we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
>>> should change that too and backport the patch to 4.4. pc-i440fx-1.6
>>> seems like a good choice to me.
>> Use "-M pc" as default seems a good idea.
>> Use specific version maybe too.
>> This way the base hardware should stay the same even if updating qemu, is
>> right?
> Yep
>
>
>> If yes, this should also solve possible problem like windows
>> reactivation request for different hardware, right?
> Right
>
>
>> How about create also xl parameter to select the "pc" model?
> Having a VM config parameter for it is OK. In the past I argued against
> relying on such a parameter to solve all compatibility issues with QEMU
> because I would like libxl to be able to select the right QEMU machine
> automatically, without user intervention.  But the option could still be
> useful for debugging.
>

Excellent