All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3)
@ 2013-06-18  9:59 Paul Durrant
  2013-06-18  9:59 ` [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:59 UTC (permalink / raw)
  To: qemu-devel, xen-devel

Because of concerns over backwards compatibility and a suggestion that
xenfv should be retired in favour of using the pc machine type I have re-
worked my original patch into 2 patches:

[PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM
[PATCH 2/2] Move hardcoded initialization of xen-platform device.

Application of both these patches allows alternative pc machine types to be
used with the accel=xen option, but preserves the hardcoded creation of
the xen-platform device only for machine type xenfv.

v3:
- Add test for xen_enabled() that went missing in v2

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:59 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3) Paul Durrant
  2013-06-18  9:59 ` [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
@ 2013-06-18  9:59 ` Paul Durrant
  2013-06-18 10:28   ` Andreas Färber
  2013-06-18 10:28   ` Andreas Färber
  2013-06-18  9:59 ` [PATCH 2/2] Move hardcoded initialization of xen-platform device Paul Durrant
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
  3 siblings, 2 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:59 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
initialization code for this machine type can easily be pulled into the
generic pc initialization code and guarded with a test for whether the xen
accelerator options is specified, which is more consistent with the way
other accelerators are used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..7bbb59a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -1,3 +1,4 @@
+
 /*
  * QEMU PC System Emulator
  *
@@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion *system_memory,
     DeviceState *icc_bridge;
     FWCfgState *fw_cfg = NULL;
 
+    if (xen_enabled() && xen_hvm_init() != 0) {
+        hw_error("xen hardware virtual machine initialisation failed");
+    }
+
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
@@ -320,9 +325,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
-    if (xen_hvm_init() != 0) {
-        hw_error("xen hardware virtual machine initialisation failed");
-    }
     pc_init_pci(args);
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:59 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3) Paul Durrant
@ 2013-06-18  9:59 ` Paul Durrant
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:59 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
initialization code for this machine type can easily be pulled into the
generic pc initialization code and guarded with a test for whether the xen
accelerator options is specified, which is more consistent with the way
other accelerators are used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..7bbb59a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -1,3 +1,4 @@
+
 /*
  * QEMU PC System Emulator
  *
@@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion *system_memory,
     DeviceState *icc_bridge;
     FWCfgState *fw_cfg = NULL;
 
+    if (xen_enabled() && xen_hvm_init() != 0) {
+        hw_error("xen hardware virtual machine initialisation failed");
+    }
+
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
@@ -320,9 +325,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
-    if (xen_hvm_init() != 0) {
-        hw_error("xen hardware virtual machine initialisation failed");
-    }
     pc_init_pci(args);
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 2/2] Move hardcoded initialization of xen-platform device.
  2013-06-18  9:59 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3) Paul Durrant
                   ` (2 preceding siblings ...)
  2013-06-18  9:59 ` [PATCH 2/2] Move hardcoded initialization of xen-platform device Paul Durrant
@ 2013-06-18  9:59 ` Paul Durrant
  2013-06-18 10:31   ` Andreas Färber
  2013-06-18 10:31   ` Andreas Färber
  3 siblings, 2 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:59 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Creation of the xen-platform device is currently hardcoded into machine
type pc's initialization code, guarded by a test for the whether the xen
accelerator is enabled. This patch moves the creation of xen-platform into
the initialization code of the xenfv machine type. This maintains backwards
compatibility for that machine type but allows more flexibility if another
machine type is used with Xen HVM domains.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7bbb59a..8dd487e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -179,9 +179,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());
@@ -325,7 +322,13 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
+    PCIBus *bus;
+
     pc_init_pci(args);
+
+    bus = pci_find_root_bus(0);
+    if (bus != NULL)
+        pci_create_simple(bus, -1, "xen-platform");
 }
 #endif
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] Move hardcoded initialization of xen-platform device.
  2013-06-18  9:59 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3) Paul Durrant
  2013-06-18  9:59 ` [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
@ 2013-06-18  9:59 ` Paul Durrant
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
  3 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:59 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Creation of the xen-platform device is currently hardcoded into machine
type pc's initialization code, guarded by a test for the whether the xen
accelerator is enabled. This patch moves the creation of xen-platform into
the initialization code of the xenfv machine type. This maintains backwards
compatibility for that machine type but allows more flexibility if another
machine type is used with Xen HVM domains.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7bbb59a..8dd487e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -179,9 +179,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());
@@ -325,7 +322,13 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
+    PCIBus *bus;
+
     pc_init_pci(args);
+
+    bus = pci_find_root_bus(0);
+    if (bus != NULL)
+        pci_create_simple(bus, -1, "xen-platform");
 }
 #endif
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
  2013-06-18 10:28   ` Andreas Färber
@ 2013-06-18 10:28   ` Andreas Färber
  2013-06-18 10:34     ` Paul Durrant
  2013-06-18 10:34     ` Paul Durrant
  1 sibling, 2 replies; 17+ messages in thread
From: Andreas Färber @ 2013-06-18 10:28 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Paolo Bonzini, qemu-devel, xen-devel

Am 18.06.2013 11:59, schrieb Paul Durrant:
> Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
> initialization code for this machine type can easily be pulled into the
> generic pc initialization code and guarded with a test for whether the xen
> accelerator options is specified, which is more consistent with the way
> other accelerators are used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d618570..7bbb59a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -1,3 +1,4 @@
> +
>  /*
>   * QEMU PC System Emulator
>   *

Unrelated whitespace change, please drop this hunk.

> @@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion *system_memory,
>      DeviceState *icc_bridge;
>      FWCfgState *fw_cfg = NULL;
>  
> +    if (xen_enabled() && xen_hvm_init() != 0) {
> +        hw_error("xen hardware virtual machine initialisation failed");

I see this is just a code movement, but maybe consider replacing that
with an fprintf()+exit(1)? Xen does not have any CPUs AFAIU and they
wouldn't be initialized at that point anyway, so hw_error() is not much
more than an fprintf()+abort().

Regards,
Andreas

> +    }
> +
>      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
> @@ -320,9 +325,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> -    if (xen_hvm_init() != 0) {
> -        hw_error("xen hardware virtual machine initialisation failed");
> -    }
>      pc_init_pci(args);
>  }
>  #endif

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
@ 2013-06-18 10:28   ` Andreas Färber
  2013-06-18 10:28   ` Andreas Färber
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Färber @ 2013-06-18 10:28 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Paolo Bonzini, qemu-devel, xen-devel

Am 18.06.2013 11:59, schrieb Paul Durrant:
> Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
> initialization code for this machine type can easily be pulled into the
> generic pc initialization code and guarded with a test for whether the xen
> accelerator options is specified, which is more consistent with the way
> other accelerators are used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d618570..7bbb59a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -1,3 +1,4 @@
> +
>  /*
>   * QEMU PC System Emulator
>   *

Unrelated whitespace change, please drop this hunk.

> @@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion *system_memory,
>      DeviceState *icc_bridge;
>      FWCfgState *fw_cfg = NULL;
>  
> +    if (xen_enabled() && xen_hvm_init() != 0) {
> +        hw_error("xen hardware virtual machine initialisation failed");

I see this is just a code movement, but maybe consider replacing that
with an fprintf()+exit(1)? Xen does not have any CPUs AFAIU and they
wouldn't be initialized at that point anyway, so hw_error() is not much
more than an fprintf()+abort().

Regards,
Andreas

> +    }
> +
>      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
> @@ -320,9 +325,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> -    if (xen_hvm_init() != 0) {
> -        hw_error("xen hardware virtual machine initialisation failed");
> -    }
>      pc_init_pci(args);
>  }
>  #endif

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] Move hardcoded initialization of xen-platform device.
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
@ 2013-06-18 10:31   ` Andreas Färber
  2013-06-18 10:31   ` Andreas Färber
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Färber @ 2013-06-18 10:31 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Paolo Bonzini, qemu-devel, xen-devel

Am 18.06.2013 11:59, schrieb Paul Durrant:
> Creation of the xen-platform device is currently hardcoded into machine
> type pc's initialization code, guarded by a test for the whether the xen
> accelerator is enabled. This patch moves the creation of xen-platform into
> the initialization code of the xenfv machine type. This maintains backwards
> compatibility for that machine type but allows more flexibility if another
> machine type is used with Xen HVM domains.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7bbb59a..8dd487e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -179,9 +179,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());
> @@ -325,7 +322,13 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> +    PCIBus *bus;
> +
>      pc_init_pci(args);
> +
> +    bus = pci_find_root_bus(0);
> +    if (bus != NULL)
> +        pci_create_simple(bus, -1, "xen-platform");

scripts/checkpatch.pl will complain about missing braces for if, but
otherwise looks okay to me.

Andreas

>  }
>  #endif
>  

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] Move hardcoded initialization of xen-platform device.
  2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
  2013-06-18 10:31   ` Andreas Färber
@ 2013-06-18 10:31   ` Andreas Färber
  1 sibling, 0 replies; 17+ messages in thread
From: Andreas Färber @ 2013-06-18 10:31 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Paolo Bonzini, qemu-devel, xen-devel

Am 18.06.2013 11:59, schrieb Paul Durrant:
> Creation of the xen-platform device is currently hardcoded into machine
> type pc's initialization code, guarded by a test for the whether the xen
> accelerator is enabled. This patch moves the creation of xen-platform into
> the initialization code of the xenfv machine type. This maintains backwards
> compatibility for that machine type but allows more flexibility if another
> machine type is used with Xen HVM domains.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7bbb59a..8dd487e 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -179,9 +179,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());
> @@ -325,7 +322,13 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> +    PCIBus *bus;
> +
>      pc_init_pci(args);
> +
> +    bus = pci_find_root_bus(0);
> +    if (bus != NULL)
> +        pci_create_simple(bus, -1, "xen-platform");

scripts/checkpatch.pl will complain about missing braces for if, but
otherwise looks okay to me.

Andreas

>  }
>  #endif
>  

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18 10:28   ` Andreas Färber
  2013-06-18 10:34     ` Paul Durrant
@ 2013-06-18 10:34     ` Paul Durrant
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18 10:34 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, qemu-devel, xen-devel

> -----Original Message-----
> From: Andreas Färber [mailto:afaerber@suse.de]
> Sent: 18 June 2013 11:29
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Paolo Bonzini
> Subject: Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type
> (accel=xen) for Xen HVM domains.
> 
> Am 18.06.2013 11:59, schrieb Paul Durrant:
> > Xen HVM domains normally spawn QEMU with a dedicated xenfv machine
> type. The
> > initialization code for this machine type can easily be pulled into the
> > generic pc initialization code and guarded with a test for whether the xen
> > accelerator options is specified, which is more consistent with the way
> > other accelerators are used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> >  hw/i386/pc_piix.c |    8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d618570..7bbb59a 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -1,3 +1,4 @@
> > +
> >  /*
> >   * QEMU PC System Emulator
> >   *
> 
> Unrelated whitespace change, please drop this hunk.
> 

Ok.

> > @@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion
> *system_memory,
> >      DeviceState *icc_bridge;
> >      FWCfgState *fw_cfg = NULL;
> >
> > +    if (xen_enabled() && xen_hvm_init() != 0) {
> > +        hw_error("xen hardware virtual machine initialisation failed");
> 
> I see this is just a code movement, but maybe consider replacing that
> with an fprintf()+exit(1)? Xen does not have any CPUs AFAIU and they
> wouldn't be initialized at that point anyway, so hw_error() is not much
> more than an fprintf()+abort().
> 

If that's preferred I can do that, but it makes it less apparent that this patch is just code movement.

  Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18 10:28   ` Andreas Färber
@ 2013-06-18 10:34     ` Paul Durrant
  2013-06-18 10:34     ` Paul Durrant
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18 10:34 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Paolo Bonzini, qemu-devel, xen-devel

> -----Original Message-----
> From: Andreas Färber [mailto:afaerber@suse.de]
> Sent: 18 June 2013 11:29
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org; Paolo Bonzini
> Subject: Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type
> (accel=xen) for Xen HVM domains.
> 
> Am 18.06.2013 11:59, schrieb Paul Durrant:
> > Xen HVM domains normally spawn QEMU with a dedicated xenfv machine
> type. The
> > initialization code for this machine type can easily be pulled into the
> > generic pc initialization code and guarded with a test for whether the xen
> > accelerator options is specified, which is more consistent with the way
> > other accelerators are used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> >  hw/i386/pc_piix.c |    8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d618570..7bbb59a 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -1,3 +1,4 @@
> > +
> >  /*
> >   * QEMU PC System Emulator
> >   *
> 
> Unrelated whitespace change, please drop this hunk.
> 

Ok.

> > @@ -91,6 +92,10 @@ static void pc_init1(MemoryRegion
> *system_memory,
> >      DeviceState *icc_bridge;
> >      FWCfgState *fw_cfg = NULL;
> >
> > +    if (xen_enabled() && xen_hvm_init() != 0) {
> > +        hw_error("xen hardware virtual machine initialisation failed");
> 
> I see this is just a code movement, but maybe consider replacing that
> with an fprintf()+exit(1)? Xen does not have any CPUs AFAIU and they
> wouldn't be initialized at that point anyway, so hw_error() is not much
> more than an fprintf()+abort().
> 

If that's preferred I can do that, but it makes it less apparent that this patch is just code movement.

  Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18 11:16 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v4) Paul Durrant
@ 2013-06-18 11:17 ` Paul Durrant
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18 11:17 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
initialization code for this machine type can easily be pulled into the
generic pc initialization code and guarded with a test for whether the xen
accelerator options is specified, which is more consistent with the way
other accelerators are used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..f96e0c2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -91,6 +91,11 @@ static void pc_init1(MemoryRegion *system_memory,
     DeviceState *icc_bridge;
     FWCfgState *fw_cfg = NULL;
 
+    if (xen_enabled() && xen_hvm_init() != 0) {
+        fprintf(stderr, "xen hardware virtual machine initialisation failed\n");
+        exit(1);
+    }
+
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
@@ -320,9 +325,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
-    if (xen_hvm_init() != 0) {
-        hw_error("xen hardware virtual machine initialisation failed");
-    }
     pc_init_pci(args);
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:43   ` Igor Mammedov
  2013-06-18  9:46     ` Paul Durrant
@ 2013-06-18  9:46     ` Paul Durrant
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:46 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, xen-devel

> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 18 June 2013 10:44
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Subject: Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type
> (accel=xen) for Xen HVM domains.
> 
> On Mon, 17 Jun 2013 13:36:36 +0100
> Paul Durrant <paul.durrant@citrix.com> wrote:
> 
> > Xen HVM domains normally spawn QEMU with a dedicated xenfv machine
> type. The
> > initialization code for this machine type can easily be pulled into the
> > generic pc initialization code and guarded with a test for whether the xen
> > accelerator options is specified, which is more consistent with the way
> > other accelerators are used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> >  hw/i386/pc_piix.c |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d618570..6d3f161 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -91,6 +91,10 @@ static void pc_init1(MemoryRegion
> *system_memory,
> >      DeviceState *icc_bridge;
> >      FWCfgState *fw_cfg = NULL;
> >
> > +    if (xen_hvm_init() != 0) {
> > +        hw_error("xen hardware virtual machine initialisation failed");
> > +    }
> > +
> would it work if starting QEMU without xen?
> 

Yikes, there's supposed to be a xen_enabled() test there too! Must have got lost when I re-based my patches. I'll fix and re-post.

  Paul

> >      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
> >      object_property_add_child(qdev_get_machine(), "icc-bridge",
> >                                OBJECT(icc_bridge), NULL);
> > @@ -320,9 +324,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
> >  #ifdef CONFIG_XEN
> >  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
> >  {
> > -    if (xen_hvm_init() != 0) {
> > -        hw_error("xen hardware virtual machine initialisation failed");
> > -    }
> >      pc_init_pci(args);
> >  }
> >  #endif
> > --
> > 1.7.10.4
> >
> >
> 
> 
> --
> Regards,
>   Igor

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-18  9:43   ` Igor Mammedov
@ 2013-06-18  9:46     ` Paul Durrant
  2013-06-18  9:46     ` Paul Durrant
  1 sibling, 0 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-18  9:46 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: qemu-devel, xen-devel

> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 18 June 2013 10:44
> To: Paul Durrant
> Cc: qemu-devel@nongnu.org; xen-devel@lists.xen.org
> Subject: Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type
> (accel=xen) for Xen HVM domains.
> 
> On Mon, 17 Jun 2013 13:36:36 +0100
> Paul Durrant <paul.durrant@citrix.com> wrote:
> 
> > Xen HVM domains normally spawn QEMU with a dedicated xenfv machine
> type. The
> > initialization code for this machine type can easily be pulled into the
> > generic pc initialization code and guarded with a test for whether the xen
> > accelerator options is specified, which is more consistent with the way
> > other accelerators are used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> >  hw/i386/pc_piix.c |    7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index d618570..6d3f161 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -91,6 +91,10 @@ static void pc_init1(MemoryRegion
> *system_memory,
> >      DeviceState *icc_bridge;
> >      FWCfgState *fw_cfg = NULL;
> >
> > +    if (xen_hvm_init() != 0) {
> > +        hw_error("xen hardware virtual machine initialisation failed");
> > +    }
> > +
> would it work if starting QEMU without xen?
> 

Yikes, there's supposed to be a xen_enabled() test there too! Must have got lost when I re-based my patches. I'll fix and re-post.

  Paul

> >      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
> >      object_property_add_child(qdev_get_machine(), "icc-bridge",
> >                                OBJECT(icc_bridge), NULL);
> > @@ -320,9 +324,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
> >  #ifdef CONFIG_XEN
> >  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
> >  {
> > -    if (xen_hvm_init() != 0) {
> > -        hw_error("xen hardware virtual machine initialisation failed");
> > -    }
> >      pc_init_pci(args);
> >  }
> >  #endif
> > --
> > 1.7.10.4
> >
> >
> 
> 
> --
> Regards,
>   Igor

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-17 12:36 ` [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
  2013-06-18  9:43   ` Igor Mammedov
@ 2013-06-18  9:43   ` Igor Mammedov
  2013-06-18  9:46     ` Paul Durrant
  2013-06-18  9:46     ` Paul Durrant
  1 sibling, 2 replies; 17+ messages in thread
From: Igor Mammedov @ 2013-06-18  9:43 UTC (permalink / raw)
  To: Paul Durrant; +Cc: qemu-devel, xen-devel

On Mon, 17 Jun 2013 13:36:36 +0100
Paul Durrant <paul.durrant@citrix.com> wrote:

> Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
> initialization code for this machine type can easily be pulled into the
> generic pc initialization code and guarded with a test for whether the xen
> accelerator options is specified, which is more consistent with the way
> other accelerators are used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d618570..6d3f161 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -91,6 +91,10 @@ static void pc_init1(MemoryRegion *system_memory,
>      DeviceState *icc_bridge;
>      FWCfgState *fw_cfg = NULL;
>  
> +    if (xen_hvm_init() != 0) {
> +        hw_error("xen hardware virtual machine initialisation failed");
> +    }
> +
would it work if starting QEMU without xen?

>      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
> @@ -320,9 +324,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> -    if (xen_hvm_init() != 0) {
> -        hw_error("xen hardware virtual machine initialisation failed");
> -    }
>      pc_init_pci(args);
>  }
>  #endif
> -- 
> 1.7.10.4
> 
> 


-- 
Regards,
  Igor

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-17 12:36 ` [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
@ 2013-06-18  9:43   ` Igor Mammedov
  2013-06-18  9:43   ` Igor Mammedov
  1 sibling, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2013-06-18  9:43 UTC (permalink / raw)
  To: Paul Durrant; +Cc: qemu-devel, xen-devel

On Mon, 17 Jun 2013 13:36:36 +0100
Paul Durrant <paul.durrant@citrix.com> wrote:

> Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
> initialization code for this machine type can easily be pulled into the
> generic pc initialization code and guarded with a test for whether the xen
> accelerator options is specified, which is more consistent with the way
> other accelerators are used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
>  hw/i386/pc_piix.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index d618570..6d3f161 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -91,6 +91,10 @@ static void pc_init1(MemoryRegion *system_memory,
>      DeviceState *icc_bridge;
>      FWCfgState *fw_cfg = NULL;
>  
> +    if (xen_hvm_init() != 0) {
> +        hw_error("xen hardware virtual machine initialisation failed");
> +    }
> +
would it work if starting QEMU without xen?

>      icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
>      object_property_add_child(qdev_get_machine(), "icc-bridge",
>                                OBJECT(icc_bridge), NULL);
> @@ -320,9 +324,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
>  #ifdef CONFIG_XEN
>  static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
>  {
> -    if (xen_hvm_init() != 0) {
> -        hw_error("xen hardware virtual machine initialisation failed");
> -    }
>      pc_init_pci(args);
>  }
>  #endif
> -- 
> 1.7.10.4
> 
> 


-- 
Regards,
  Igor

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains.
  2013-06-17 12:36 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v2) Paul Durrant
@ 2013-06-17 12:36 ` Paul Durrant
  2013-06-18  9:43   ` Igor Mammedov
  2013-06-18  9:43   ` Igor Mammedov
  0 siblings, 2 replies; 17+ messages in thread
From: Paul Durrant @ 2013-06-17 12:36 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: Paul Durrant

Xen HVM domains normally spawn QEMU with a dedicated xenfv machine type. The
initialization code for this machine type can easily be pulled into the
generic pc initialization code and guarded with a test for whether the xen
accelerator options is specified, which is more consistent with the way
other accelerators are used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 hw/i386/pc_piix.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index d618570..6d3f161 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -91,6 +91,10 @@ static void pc_init1(MemoryRegion *system_memory,
     DeviceState *icc_bridge;
     FWCfgState *fw_cfg = NULL;
 
+    if (xen_hvm_init() != 0) {
+        hw_error("xen hardware virtual machine initialisation failed");
+    }
+
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",
                               OBJECT(icc_bridge), NULL);
@@ -320,9 +324,6 @@ static void pc_init_isa(QEMUMachineInitArgs *args)
 #ifdef CONFIG_XEN
 static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
 {
-    if (xen_hvm_init() != 0) {
-        hw_error("xen hardware virtual machine initialisation failed");
-    }
     pc_init_pci(args);
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2013-06-18 11:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-18  9:59 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v3) Paul Durrant
2013-06-18  9:59 ` [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
2013-06-18 10:28   ` Andreas Färber
2013-06-18 10:28   ` Andreas Färber
2013-06-18 10:34     ` Paul Durrant
2013-06-18 10:34     ` Paul Durrant
2013-06-18  9:59 ` [PATCH 2/2] Move hardcoded initialization of xen-platform device Paul Durrant
2013-06-18  9:59 ` [Qemu-devel] " Paul Durrant
2013-06-18 10:31   ` Andreas Färber
2013-06-18 10:31   ` Andreas Färber
  -- strict thread matches above, loose matches on Subject: below --
2013-06-18 11:16 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v4) Paul Durrant
2013-06-18 11:17 ` [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
2013-06-17 12:36 [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v2) Paul Durrant
2013-06-17 12:36 ` [Qemu-devel] [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM domains Paul Durrant
2013-06-18  9:43   ` Igor Mammedov
2013-06-18  9:43   ` Igor Mammedov
2013-06-18  9:46     ` Paul Durrant
2013-06-18  9:46     ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.