Patchwork [1/3] spapr: allow creating devices with -device

login
register
mail settings
Submitter Paolo Bonzini
Date May 24, 2011, 11:45 a.m.
Message ID <1306237507-19189-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/97149/
State New
Headers show

Comments

Paolo Bonzini - May 24, 2011, 11:45 a.m.
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_llan.c  |    7 +------
 hw/spapr_vio.c   |    5 +++++
 hw/spapr_vio.h   |   13 ++++---------
 hw/spapr_vscsi.c |    8 +-------
 hw/spapr_vty.c   |    8 +-------
 6 files changed, 17 insertions(+), 39 deletions(-)
Markus Armbruster - May 24, 2011, 1:03 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> 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.

Really?  Hasn't that always been the case?
Paolo Bonzini - May 24, 2011, 1:08 p.m.
On 05/24/2011 03:03 PM, Markus Armbruster wrote:
> >  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.
>
> Really?  Hasn't that always been the case?

What hasn't always been the case? :)

1) "the default VSCSI controller will not be created" -- no, this is new 
with scsi-cd: scsi-disk never was on the default_driver_table in vl.c, 
as you said in the commit message for af6bf13 (defaults: ide-cd, ide-hd 
and scsi-cd devices suppress default CD-ROM, 2011-05-18).  In fact, I 
believe you could add scsi-hd there too.

2) "without qdevification, you will not be able to add yours" -- that of 
course has always been the case.  But I never noticed because there was 
no way to avoid creating the default CD-ROM, and this in turn forced the 
non-qdev-clean creation of the VSCSI controller.

Paolo
Markus Armbruster - May 24, 2011, 1:34 p.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 05/24/2011 03:03 PM, Markus Armbruster wrote:
>> >  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.
>>
>> Really?  Hasn't that always been the case?
>
> What hasn't always been the case? :)
>
> 1) "the default VSCSI controller will not be created" -- no, this is
> new with scsi-cd: scsi-disk never was on the default_driver_table in
> vl.c, as you said in the commit message for af6bf13 (defaults: ide-cd,
> ide-hd and scsi-cd devices suppress default CD-ROM, 2011-05-18).  In
> fact, I believe you could add scsi-hd there too.

Aha.  The default CD-ROM also creates a default controller if
machine->use_scsi.  But as soon as you try -device scsi-cd, it
vanishes.  Hmm.

> 2) "without qdevification, you will not be able to add yours" -- that
> of course has always been the case.  But I never noticed because there
> was no way to avoid creating the default CD-ROM, and this in turn
> forced the non-qdev-clean creation of the VSCSI controller.

There's -nodefaults.
David Gibson - May 24, 2011, 10:12 p.m.
On Tue, May 24, 2011 at 01:45:05PM +0200, 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_llan.c  |    7 +------
>  hw/spapr_vio.c   |    5 +++++
>  hw/spapr_vio.h   |   13 ++++---------
>  hw/spapr_vscsi.c |    8 +-------
>  hw/spapr_vty.c   |    8 +-------
>  6 files changed, 17 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]);

Yeah, I was passing these in in the hope of avoiding tying the VIO
code too strongly to the XICS.  That was probably a foolish goal,
since PAPR does specify the XICS.

>          }
>      }
>  
> -    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_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..be535d6 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 = xics_find_qirq(spapr->icp, dev->vio_irq_num);

I'd prefer to see an spapr_allocate_irq() function, rather than open
coding this multiple times.

Since we're not trying to be PIC independent any more, I'm not sure
there's much point carrying both the irq number and the qirq around in
the device structure.  We could just to the xics_find_irq at the point
we need to issue the interrupt.  (I'd prefer to do it the other way
around, but there's no simple way to retrieve the irq number from the
qirq).

>      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 fdad3d2..89f7ce2 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -894,20 +894,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)
Paolo Bonzini - May 25, 2011, 7:29 a.m.
On 05/25/2011 12:12 AM, David Gibson wrote:
>> @@ -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 = xics_find_qirq(spapr->icp, dev->vio_irq_num);
>
> I'd prefer to see an spapr_allocate_irq() function, rather than open
> coding this multiple times.

I don't understand.  This is the only call to xics_find_qirq, unlike 
before this patch.  It's not open coded multiple times.

I can surely add a spapr_allocate_irq() function that would keep the 
code independent of the particular interrupt controller.  Would you 
prefer something that gives back the virtual IRQ number as well:

     qemu_irq *spapr_allocate_irq(uint32_t *p_vio_irq_num)

or should I keep that in the VIOsPAPRBus, like this:

     qemu_irq *spapr_allocate_irq(uint32_t p_vio_irq_num)

?  Should I pass a sPAPREnvironment too, or should it just use the global?

Paolo
David Gibson - May 30, 2011, 3:16 a.m.
On Wed, May 25, 2011 at 09:29:26AM +0200, Paolo Bonzini wrote:
> On 05/25/2011 12:12 AM, David Gibson wrote:
> >>@@ -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 = xics_find_qirq(spapr->icp, dev->vio_irq_num);
> >
> >I'd prefer to see an spapr_allocate_irq() function, rather than open
> >coding this multiple times.
> 
> I don't understand.  This is the only call to xics_find_qirq, unlike
> before this patch.  It's not open coded multiple times.

Uh, sorry, I wasn't thinking it through when I assumed it was
duplicated.

Actually, in any case I wouldn't mind multiple calls to
xics_find_qirq(), it's the actual allocation - the irq++ - being
potentially duplicated which bothers me.  It's not now, but with this
approach it would need to be when we add non VIO devices to the system
which is coming (PCI, virtio, ..).

> I can surely add a spapr_allocate_irq() function that would keep the
> code independent of the particular interrupt controller.  Would you
> prefer something that gives back the virtual IRQ number as well:
> 
>     qemu_irq *spapr_allocate_irq(uint32_t *p_vio_irq_num)
> 
> or should I keep that in the VIOsPAPRBus, like this:
> 
>     qemu_irq *spapr_allocate_irq(uint32_t p_vio_irq_num)

In fact just returning the xics irq num and using xics_find_qirq() on
that would be ok by me.  The point is that I don't want the policy for
irq allocation on the global interrupt controller to be open coded
here at the bus level.

> ?  Should I pass a sPAPREnvironment too, or should it just use the global?

Passing it would be preferable.

Patch

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_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..be535d6 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 = xics_find_qirq(spapr->icp, 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 fdad3d2..89f7ce2 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -894,20 +894,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)