[Qemu-devel] [PATCH v2 0/3] spapr qdevification

[Qemu-devel] [PATCH v2 0/3] spapr qdevification

[Paolo Bonzini]

This series fixes some problems with spapr's qdev interface.  Patch
1 is the important one, which makes it possible to use -device
to create vio devices.  The other two are cosmetic.

v1->v2:
        abstracted the call to xics_find_qirq behind spapr_find_qirq

Paolo Bonzini (3):
  spapr: proper qdevification
  spapr: prepare for qdevification of irq
  spapr: make irq customizable via qdev

 hw/spapr.c       |   15 +++++----------
 hw/spapr_llan.c  |   11 ++---------
 hw/spapr_vio.c   |   11 +++++++++++
 hw/spapr_vio.h   |   18 +++++++++---------
 hw/spapr_vscsi.c |   12 ++----------
 hw/spapr_vty.c   |   10 ++--------
 6 files changed, 31 insertions(+), 46 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH v2 1/3] spapr: proper qdevification

[Paolo Bonzini]

Right now the spapr devices cannot be instantiated with -device,
because the IRQs need to be passed to the spapr_*_create functions.
Do this instead in the bus's init wrapper.

This is particularly important with the conversion from scsi-disk
to scsi-{cd,hd} that Markus made.  After his patches, if you
specify a scsi-cd device attached to an if=none drive, the default
VSCSI controller will not be created and, without qdevification,
you will not be able to add yours.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr.c       |   15 +++++----------
 hw/spapr.h       |    6 ++++++
 hw/spapr_llan.c  |    7 +------
 hw/spapr_vio.c   |    5 +++++
 hw/spapr_vio.h   |   13 ++++---------
 hw/spapr_vscsi.c |    8 +-------
 hw/spapr_vty.c   |    8 +-------
 7 files changed, 23 insertions(+), 39 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index 109b774..07b2165 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -298,7 +298,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
     long kernel_size, initrd_size, fw_size;
     long pteg_shift = 17;
     char *filename;
-    int irq = 16;
 
     spapr = qemu_malloc(sizeof(*spapr));
     cpu_ppc_hypercall = emulate_spapr_hypercall;
@@ -360,15 +359,14 @@ static void ppc_spapr_init(ram_addr_t ram_size,
     /* Set up VIO bus */
     spapr->vio_bus = spapr_vio_bus_init();
 
-    for (i = 0; i < MAX_SERIAL_PORTS; i++, irq++) {
+    for (i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
             spapr_vty_create(spapr->vio_bus, SPAPR_VTY_BASE_ADDRESS + i,
-                             serial_hds[i], xics_find_qirq(spapr->icp, irq),
-                             irq);
+                             serial_hds[i]);
         }
     }
 
-    for (i = 0; i < nb_nics; i++, irq++) {
+    for (i = 0; i < nb_nics; i++) {
         NICInfo *nd = &nd_table[i];
 
         if (!nd->model) {
@@ -376,8 +374,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
         }
 
         if (strcmp(nd->model, "ibmveth") == 0) {
-            spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd,
-                              xics_find_qirq(spapr->icp, irq), irq);
+            spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd);
         } else {
             fprintf(stderr, "pSeries (sPAPR) platform does not support "
                     "NIC model '%s' (only ibmveth is supported)\n",
@@ -387,9 +384,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
     }
 
     for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
-        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i,
-                           xics_find_qirq(spapr->icp, irq), irq);
-        irq++;
+        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
     }
 
     if (kernel_filename) {
diff --git a/hw/spapr.h b/hw/spapr.h
index b52133a..4130c13 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -278,6 +278,12 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
 target_ulong spapr_hypercall(CPUState *env, target_ulong opcode,
                              target_ulong *args);
 
+static inline qemu_irq *spapr_find_qirq(sPAPREnvironment *spapr,
+                                        int irq_num)
+{
+    return xics_find_qirq(spapr->icp, irq_num);
+}
+
 static inline uint32_t rtas_ld(target_ulong phys, int n)
 {
     return ldl_phys(phys + 4*n);
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index c18efc7..2597748 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -195,11 +195,9 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
     return 0;
 }
 
-void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd,
-                       qemu_irq qirq, uint32_t vio_irq_num)
+void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd)
 {
     DeviceState *dev;
-    VIOsPAPRDevice *sdev;
 
     dev = qdev_create(&bus->bus, "spapr-vlan");
     qdev_prop_set_uint32(dev, "reg", reg);
@@ -207,9 +205,6 @@ void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd,
     qdev_set_nic_properties(dev, nd);
 
     qdev_init_nofail(dev);
-    sdev = (VIOsPAPRDevice *)dev;
-    sdev->qirq = qirq;
-    sdev->vio_irq_num = vio_irq_num;
 }
 
 static int spapr_vlan_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 481a804..8b31bc6 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -32,6 +32,7 @@
 
 #include "hw/spapr.h"
 #include "hw/spapr_vio.h"
+#include "hw/xics.h"
 
 #ifdef CONFIG_FDT
 #include <libfdt.h>
@@ -595,6 +596,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
 {
     VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
     VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
+    VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
     char *id;
 
     if (asprintf(&id, "%s@%x", info->dt_name, dev->reg) < 0) {
@@ -602,6 +604,8 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
     }
 
     dev->qdev.id = id;
+    dev->vio_irq_num = bus->irq++;
+    dev->qirq = spapr_find_qirq(spapr, dev->vio_irq_num);
 
     rtce_init(dev);
 
@@ -656,6 +660,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
 
     qbus = qbus_create(&spapr_vio_bus_info, dev, "spapr-vio");
     bus = DO_UPCAST(VIOsPAPRBus, bus, qbus);
+    bus->irq = 16;
 
     /* hcall-vio */
     spapr_register_hypercall(H_VIO_SIGNAL, h_vio_signal);
diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
index 603a8c4..faa5d94 100644
--- a/hw/spapr_vio.h
+++ b/hw/spapr_vio.h
@@ -62,6 +62,7 @@ typedef struct VIOsPAPRDevice {
 
 typedef struct VIOsPAPRBus {
     BusState bus;
+    int irq;
 } VIOsPAPRBus;
 
 typedef struct {
@@ -98,15 +99,9 @@ uint64_t ldq_tce(VIOsPAPRDevice *dev, uint64_t taddr);
 int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq);
 
 void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
-void spapr_vty_create(VIOsPAPRBus *bus,
-                      uint32_t reg, CharDriverState *chardev,
-                      qemu_irq qirq, uint32_t vio_irq_num);
-
-void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd,
-                       qemu_irq qirq, uint32_t vio_irq_num);
-
-void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg,
-                        qemu_irq qirq, uint32_t vio_irq_num);
+void spapr_vty_create(VIOsPAPRBus *bus, uint32_t reg, CharDriverState *chardev);
+void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd);
+void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg);
 
 int spapr_tce_set_bypass(uint32_t unit, uint32_t enable);
 void spapr_vio_quiesce(void);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 9928334..dc258a1 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -931,20 +931,14 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
     return 0;
 }
 
-void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg,
-                        qemu_irq qirq, uint32_t vio_irq_num)
+void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg)
 {
     DeviceState *dev;
-    VIOsPAPRDevice *sdev;
 
     dev = qdev_create(&bus->bus, "spapr-vscsi");
     qdev_prop_set_uint32(dev, "reg", reg);
 
     qdev_init_nofail(dev);
-
-    sdev = (VIOsPAPRDevice *)dev;
-    sdev->qirq = qirq;
-    sdev->vio_irq_num = vio_irq_num;
 }
 
 static int spapr_vscsi_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index 6fc0105..fa97cf7 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -115,20 +115,14 @@ static target_ulong h_get_term_char(CPUState *env, sPAPREnvironment *spapr,
     return H_SUCCESS;
 }
 
-void spapr_vty_create(VIOsPAPRBus *bus,
-                      uint32_t reg, CharDriverState *chardev,
-                      qemu_irq qirq, uint32_t vio_irq_num)
+void spapr_vty_create(VIOsPAPRBus *bus, uint32_t reg, CharDriverState *chardev)
 {
     DeviceState *dev;
-    VIOsPAPRDevice *sdev;
 
     dev = qdev_create(&bus->bus, "spapr-vty");
     qdev_prop_set_uint32(dev, "reg", reg);
     qdev_prop_set_chr(dev, "chardev", chardev);
     qdev_init_nofail(dev);
-    sdev = (VIOsPAPRDevice *)dev;
-    sdev->qirq = qirq;
-    sdev->vio_irq_num = vio_irq_num;
 }
 
 static void vty_hcalls(VIOsPAPRBus *bus)
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH v2 1/3] spapr: proper qdevification

[Alexander Graf]

On 05/26/2011 11:52 AM, Paolo Bonzini wrote:
> Right now the spapr devices cannot be instantiated with -device,
> because the IRQs need to be passed to the spapr_*_create functions.
> Do this instead in the bus's init wrapper.
>
> This is particularly important with the conversion from scsi-disk
> to scsi-{cd,hd} that Markus made.  After his patches, if you
> specify a scsi-cd device attached to an if=none drive, the default
> VSCSI controller will not be created and, without qdevification,
> you will not be able to add yours.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> Cc: Alexander Graf<agraf@suse.de>
> Cc: David Gibson<david@gibson.dropbear.id.au>
> ---
>   hw/spapr.c       |   15 +++++----------
>   hw/spapr.h       |    6 ++++++
>   hw/spapr_llan.c  |    7 +------
>   hw/spapr_vio.c   |    5 +++++
>   hw/spapr_vio.h   |   13 ++++---------
>   hw/spapr_vscsi.c |    8 +-------
>   hw/spapr_vty.c   |    8 +-------
>   7 files changed, 23 insertions(+), 39 deletions(-)
>
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 109b774..07b2165 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -298,7 +298,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>       long kernel_size, initrd_size, fw_size;
>       long pteg_shift = 17;
>       char *filename;
> -    int irq = 16;
>
>       spapr = qemu_malloc(sizeof(*spapr));
>       cpu_ppc_hypercall = emulate_spapr_hypercall;
> @@ -360,15 +359,14 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>       /* Set up VIO bus */
>       spapr->vio_bus = spapr_vio_bus_init();
>
> -    for (i = 0; i<  MAX_SERIAL_PORTS; i++, irq++) {
> +    for (i = 0; i<  MAX_SERIAL_PORTS; i++) {
>           if (serial_hds[i]) {
>               spapr_vty_create(spapr->vio_bus, SPAPR_VTY_BASE_ADDRESS + i,
> -                             serial_hds[i], xics_find_qirq(spapr->icp, irq),
> -                             irq);
> +                             serial_hds[i]);
>           }
>       }
>
> -    for (i = 0; i<  nb_nics; i++, irq++) {
> +    for (i = 0; i<  nb_nics; i++) {
>           NICInfo *nd =&nd_table[i];
>
>           if (!nd->model) {
> @@ -376,8 +374,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>           }
>
>           if (strcmp(nd->model, "ibmveth") == 0) {
> -            spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd,
> -                              xics_find_qirq(spapr->icp, irq), irq);
> +            spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd);
>           } else {
>               fprintf(stderr, "pSeries (sPAPR) platform does not support "
>                       "NIC model '%s' (only ibmveth is supported)\n",
> @@ -387,9 +384,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>       }
>
>       for (i = 0; i<= drive_get_max_bus(IF_SCSI); i++) {
> -        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i,
> -                           xics_find_qirq(spapr->icp, irq), irq);
> -        irq++;
> +        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
>       }
>
>       if (kernel_filename) {
> diff --git a/hw/spapr.h b/hw/spapr.h
> index b52133a..4130c13 100644
> --- a/hw/spapr.h
> +++ b/hw/spapr.h
> @@ -278,6 +278,12 @@ void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn);
>   target_ulong spapr_hypercall(CPUState *env, target_ulong opcode,
>                                target_ulong *args);
>
> +static inline qemu_irq *spapr_find_qirq(sPAPREnvironment *spapr,
> +                                        int irq_num)
> +{
> +    return xics_find_qirq(spapr->icp, irq_num);
> +}
> +

This breaks with current HEAD. I've added a fix to my tree:

diff --git a/hw/spapr.h b/hw/spapr.h
index 4130c13..a725d4a 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -1,6 +1,8 @@
  #if !defined(__HW_SPAPR_H__)
  #define __HW_SPAPR_H__

+#include "hw/xics.h"
+
  struct VIOsPAPRBus;
  struct icp_state;

@@ -278,7 +280,7 @@ void spapr_register_hypercall(target_ulong opcode, 
spapr_hcall_fn fn);
  target_ulong spapr_hypercall(CPUState *env, target_ulong opcode,
                               target_ulong *args);

-static inline qemu_irq *spapr_find_qirq(sPAPREnvironment *spapr,
+static inline qemu_irq spapr_find_qirq(sPAPREnvironment *spapr,
                                          int irq_num)
  {
      return xics_find_qirq(spapr->icp, irq_num);


Alex

[Qemu-devel] [PATCH v2 2/3] spapr: prepare for qdevification of irq

[Paolo Bonzini]

Restructure common properties for sPAPR devices so that IRQ definitions
can be added in one place.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr_llan.c  |    4 +---
 hw/spapr_vio.h   |    5 +++++
 hw/spapr_vscsi.c |    4 +---
 hw/spapr_vty.c   |    2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index 2597748..abe1297 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -495,9 +495,7 @@ static VIOsPAPRDeviceInfo spapr_vlan = {
     .qdev.name = "spapr-vlan",
     .qdev.size = sizeof(VIOsPAPRVLANDevice),
     .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0x1000),
-        DEFINE_PROP_UINT32("dma-window", VIOsPAPRDevice, rtce_window_size,
-                           0x10000000),
+        DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x10000000),
         DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
         DEFINE_PROP_END_OF_LIST(),
     },
diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
index faa5d94..7eb5367 100644
--- a/hw/spapr_vio.h
+++ b/hw/spapr_vio.h
@@ -60,6 +60,11 @@ typedef struct VIOsPAPRDevice {
     VIOsPAPR_CRQ crq;
 } VIOsPAPRDevice;
 
+#define DEFINE_SPAPR_PROPERTIES(type, field, default_reg, default_dma_window) \
+        DEFINE_PROP_UINT32("reg", type, field.reg, default_reg), \
+        DEFINE_PROP_UINT32("dma-window", type, field.rtce_window_size, \
+                           default_dma_window)
+
 typedef struct VIOsPAPRBus {
     BusState bus;
     int irq;
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index dc258a1..7d26ec0 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -968,9 +968,7 @@ static VIOsPAPRDeviceInfo spapr_vscsi = {
     .qdev.name = "spapr-vscsi",
     .qdev.size = sizeof(VSCSIState),
     .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0x2000),
-        DEFINE_PROP_UINT32("dma-window", VIOsPAPRDevice,
-                           rtce_window_size, 0x10000000),
+        DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x10000000),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index fa97cf7..abc41f8 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -140,7 +140,7 @@ static VIOsPAPRDeviceInfo spapr_vty = {
     .qdev.name = "spapr-vty",
     .qdev.size = sizeof(VIOsPAPRVTYDevice),
     .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("reg", VIOsPAPRDevice, reg, 0),
+        DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, 0, 0),
         DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
         DEFINE_PROP_END_OF_LIST(),
     },
-- 
1.7.4.4

[Qemu-devel] [PATCH v2 3/3] spapr: make irq customizable via qdev

[Paolo Bonzini]

This also lets the user see the irq in "info qtree".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr_vio.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 8b31bc6..877c4d6 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -52,6 +52,10 @@
 static struct BusInfo spapr_vio_bus_info = {
     .name       = "spapr-vio",
     .size       = sizeof(VIOsPAPRBus),
+    .props = (Property[]) {
+        DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, vio_irq_num, 0), \
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
@@ -604,7 +608,9 @@ static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
     }
 
     dev->qdev.id = id;
-    dev->vio_irq_num = bus->irq++;
+    if (!dev->vio_irq_num) {
+        dev->vio_irq_num = bus->irq++;
+    }
     dev->qirq = spapr_find_qirq(spapr, dev->vio_irq_num);
 
     rtce_init(dev);
-- 
1.7.4.4