diff mbox

[v3,064/197] killall VIOsPAPRDeviceInfo

Message ID 1323721273-32404-65-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Dec. 12, 2011, 8:19 p.m. UTC 
This was doing something evil building a dt tree so we broke the device.
---
 hw/spapr_llan.c  |   39 ++++++++++++++++++++++++---------------
 hw/spapr_vio.c   |   52 +++++++++++++++++++++++++++++++++-------------------
 hw/spapr_vio.h   |   29 +++++++++++++++++++----------
 hw/spapr_vscsi.c |   35 ++++++++++++++++++++++-------------
 hw/spapr_vty.c   |   35 ++++++++++++++++++++++-------------
 5 files changed, 120 insertions(+), 70 deletions(-)

Comments

Michael Ellerman Dec. 13, 2011, 2:04 a.m. UTC | #1 
On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
> This was doing something evil building a dt tree so we broke the device.

> @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>      spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
>      spapr_rtas_register("quiesce", rtas_quiesce);
>  
> +#if 0
> +    /* Evil and broken */

By which you mean: works fine, broken by your patch?

> +
>      for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
>          VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
> +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
>  
>          if (qinfo->bus_info != &spapr_vio_bus_info) {
>              continue;
> @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>              info->hcalls(bus);
>          }
>      }
> +#endif

It's registering hcalls for each class of device we find on the spapr
vio bus. I don't understand why that is evil, but what do you suggest we
do instead?

cheers
Anthony Liguori Dec. 13, 2011, 2:10 a.m. UTC | #2 
On 12/12/2011 08:04 PM, Michael Ellerman wrote:
> On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
>> This was doing something evil building a dt tree so we broke the device.
>
>> @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>       spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
>>       spapr_rtas_register("quiesce", rtas_quiesce);
>>
>> +#if 0
>> +    /* Evil and broken */
>
> By which you mean: works fine, broken by your patch?

These patches were never supposed to go out.  Ignore this series entirely.

>
>> +
>>       for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
>>           VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
>> +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
>>
>>           if (qinfo->bus_info !=&spapr_vio_bus_info) {
>>               continue;
>> @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>               info->hcalls(bus);
>>           }
>>       }
>> +#endif
>
> It's registering hcalls for each class of device we find on the spapr
> vio bus. I don't understand why that is evil, but what do you suggest we
> do instead?

I talked to David about this, the hcalls can just be registered as part the 
device_init entry points.

If you must initialize them via a per-device callback, then you should walk it 
from the bus's children, not by walking the entire device model.  That was the 
bit that I was referring to as evil.   It's a layering violation.

Regards,

Anthony Liguori


> cheers
>
>
Michael Ellerman Dec. 13, 2011, 2:22 a.m. UTC | #3 
On Mon, 2011-12-12 at 20:10 -0600, Anthony Liguori wrote:
> On 12/12/2011 08:04 PM, Michael Ellerman wrote:
> > On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
> >> This was doing something evil building a dt tree so we broke the device.
> >
> >> @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >>       spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
> >>       spapr_rtas_register("quiesce", rtas_quiesce);
> >>
> >> +#if 0
> >> +    /* Evil and broken */
> >
> > By which you mean: works fine, broken by your patch?
> 
> These patches were never supposed to go out.  Ignore this series entirely.

But I just read all 197 of them ! ;)

> >>       for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
> >>           VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
> >> +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> >>
> >>           if (qinfo->bus_info !=&spapr_vio_bus_info) {
> >>               continue;
> >> @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >>               info->hcalls(bus);
> >>           }
> >>       }
> >> +#endif
> >
> > It's registering hcalls for each class of device we find on the spapr
> > vio bus. I don't understand why that is evil, but what do you suggest we
> > do instead?
> 
> I talked to David about this, the hcalls can just be registered as part the 
> device_init entry points.

OK I'll talk to him about it. I don't think device_init() works, because
we only want to register the hcalls if an instance of the device is
instantiated. But I guess we'll come up with something.

cheers
Anthony Liguori Dec. 13, 2011, 2:25 a.m. UTC | #4 
On 12/12/2011 08:22 PM, Michael Ellerman wrote:
> On Mon, 2011-12-12 at 20:10 -0600, Anthony Liguori wrote:
>> On 12/12/2011 08:04 PM, Michael Ellerman wrote:
>>> On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
>>>> This was doing something evil building a dt tree so we broke the device.
>>>
>>>> @@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>>>        spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
>>>>        spapr_rtas_register("quiesce", rtas_quiesce);
>>>>
>>>> +#if 0
>>>> +    /* Evil and broken */
>>>
>>> By which you mean: works fine, broken by your patch?
>>
>> These patches were never supposed to go out.  Ignore this series entirely.
>
> But I just read all 197 of them ! ;)
>
>>>>        for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
>>>>            VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
>>>> +        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
>>>>
>>>>            if (qinfo->bus_info !=&spapr_vio_bus_info) {
>>>>                continue;
>>>> @@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>>>>                info->hcalls(bus);
>>>>            }
>>>>        }
>>>> +#endif
>>>
>>> It's registering hcalls for each class of device we find on the spapr
>>> vio bus. I don't understand why that is evil, but what do you suggest we
>>> do instead?
>>
>> I talked to David about this, the hcalls can just be registered as part the
>> device_init entry points.
>
> OK I'll talk to him about it. I don't think device_init() works, because
> we only want to register the hcalls if an instance of the device is
> instantiated. But I guess we'll come up with something.

Since the hcalls are well known and CPUState doesn't get touched at all, why not 
register all of the possible hcalls in the VIO bus and then use a higher level 
interface to dispatch to devices?  I think that conceptionally makes more sense 
than the devices directly registering hcalls themselves.

I know you're dealing with an existing virtual I/O model, but it's strange to 
have an hcall dispatched directly to a device without going through any type of 
controller/bus hierarchy.  It doesn't fit how hardware works very well.

Regards,

Anthony Liguori

>
> cheers
>
David Gibson Dec. 13, 2011, 3:26 a.m. UTC | #5 
On Mon, Dec 12, 2011 at 08:25:51PM -0600, Anthony Liguori wrote:
> On 12/12/2011 08:22 PM, Michael Ellerman wrote:
> >On Mon, 2011-12-12 at 20:10 -0600, Anthony Liguori wrote:
> >>On 12/12/2011 08:04 PM, Michael Ellerman wrote:
> >>>On Mon, 2011-12-12 at 14:19 -0600, Anthony Liguori wrote:
> >>>>This was doing something evil building a dt tree so we broke the device.
> >>>
> >>>>@@ -711,8 +711,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >>>>       spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
> >>>>       spapr_rtas_register("quiesce", rtas_quiesce);
> >>>>
> >>>>+#if 0
> >>>>+    /* Evil and broken */
> >>>
> >>>By which you mean: works fine, broken by your patch?

Um, yeah.  It may have been evil, but it wasn't broken, whereas it
certainly is broken by this patch.

> >>These patches were never supposed to go out.  Ignore this series entirely.

Phew.

> >But I just read all 197 of them ! ;)
> >
> >>>>       for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
> >>>>           VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
> >>>>+        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> >>>>
> >>>>           if (qinfo->bus_info !=&spapr_vio_bus_info) {
> >>>>               continue;
> >>>>@@ -722,6 +726,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
> >>>>               info->hcalls(bus);
> >>>>           }
> >>>>       }
> >>>>+#endif
> >>>
> >>>It's registering hcalls for each class of device we find on the spapr
> >>>vio bus. I don't understand why that is evil, but what do you suggest we
> >>>do instead?
> >>
> >>I talked to David about this, the hcalls can just be registered as part the
> >>device_init entry points.
> >
> >OK I'll talk to him about it. I don't think device_init() works, because
> >we only want to register the hcalls if an instance of the device is
> >instantiated. But I guess we'll come up with something.

Hm, no, actually.  The reason we're stepping through the device infos
rather than vio devices is so that hcalls are registered per device
type rather than per device instance.

> Since the hcalls are well known and CPUState doesn't get touched at
> all, why not register all of the possible hcalls in the VIO bus and
> then use a higher level interface to dispatch to devices?  I think
> that conceptionally makes more sense than the devices directly
> registering hcalls themselves.

No, not really.  It means spapr_vio.c has to contain knowledge of
every possible VIO device.

> I know you're dealing with an existing virtual I/O model, but it's
> strange to have an hcall dispatched directly to a device without
> going through any type of controller/bus hierarchy.  It doesn't fit
> how hardware works very well.

But VIO devices don't work much like real hardware.  Other than the
CRQ and a few other hcalls, which are already handled at the bus
level, there is *no* common processing we can do for the device
specific hcalls.  Routing them through spapr_vio.c would just be
pointless redirection.


Anyway, I'll make a [atch to move the hypercall registrations to
device_init functions.
diff mbox

Patch

diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index 958702c..b06b274 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -484,21 +484,30 @@  static void vlan_hcalls(VIOsPAPRBus *bus)
     spapr_register_hypercall(H_MULTICAST_CTRL, h_multicast_ctrl);
 }
 
-static VIOsPAPRDeviceInfo spapr_vlan_info = {
-    .init = spapr_vlan_init,
-    .devnode = spapr_vlan_devnode,
-    .dt_name = "l-lan",
-    .dt_type = "network",
-    .dt_compatible = "IBM,l-lan",
-    .signal_mask = 0x1,
-    .hcalls = vlan_hcalls,
-    .qdev.name = "spapr-vlan",
-    .qdev.size = sizeof(VIOsPAPRVLANDevice),
-    .qdev.props = (Property[]) {
-        DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x10000000),
-        DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
-        DEFINE_PROP_END_OF_LIST(),
-    },
+static Property spapr_vlan_properties[] = {
+    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x10000000),
+    DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_vlan_class_init(ObjectClass *klass, void *data)
+{
+    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
+
+    k->init = spapr_vlan_init;
+    k->devnode = spapr_vlan_devnode;
+    k->dt_name = "l-lan";
+    k->dt_type = "network";
+    k->dt_compatible = "IBM,l-lan";
+    k->signal_mask = 0x1;
+    k->hcalls = vlan_hcalls;
+}
+
+static DeviceInfo spapr_vlan_info = {
+    .name = "spapr-vlan",
+    .size = sizeof(VIOsPAPRVLANDevice),
+    .props = spapr_vlan_properties,
+    .class_init = spapr_vlan_class_init,
 };
 
 static void spapr_vlan_register(void)
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 2dcc036..4761bf9 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -75,11 +75,11 @@  VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
 
 static char *vio_format_dev_name(VIOsPAPRDevice *dev)
 {
-    VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
+    VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
     char *name;
 
     /* Device tree style name device@reg */
-    if (asprintf(&name, "%s@%x", info->dt_name, dev->reg) < 0) {
+    if (asprintf(&name, "%s@%x", pc->dt_name, dev->reg) < 0) {
         return NULL;
     }
 
@@ -90,7 +90,7 @@  static char *vio_format_dev_name(VIOsPAPRDevice *dev)
 static int vio_make_devnode(VIOsPAPRDevice *dev,
                             void *fdt)
 {
-    VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
+    VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
     int vdevice_off, node_off, ret;
     char *dt_name;
 
@@ -115,17 +115,17 @@  static int vio_make_devnode(VIOsPAPRDevice *dev,
         return ret;
     }
 
-    if (info->dt_type) {
+    if (pc->dt_type) {
         ret = fdt_setprop_string(fdt, node_off, "device_type",
-                                 info->dt_type);
+                                 pc->dt_type);
         if (ret < 0) {
             return ret;
         }
     }
 
-    if (info->dt_compatible) {
+    if (pc->dt_compatible) {
         ret = fdt_setprop_string(fdt, node_off, "compatible",
-                                 info->dt_compatible);
+                                 pc->dt_compatible);
         if (ret < 0) {
             return ret;
         }
@@ -163,8 +163,8 @@  static int vio_make_devnode(VIOsPAPRDevice *dev,
         }
     }
 
-    if (info->devnode) {
-        ret = (info->devnode)(dev, fdt, node_off);
+    if (pc->devnode) {
+        ret = (pc->devnode)(dev, fdt, node_off);
         if (ret < 0) {
             return ret;
         }
@@ -623,8 +623,8 @@  static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
 
 static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
 {
-    VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
     VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
+    VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
     char *id;
 
     /* Don't overwrite ids assigned on the command line */
@@ -643,16 +643,16 @@  static int spapr_vio_busdev_init(DeviceState *qdev, DeviceInfo *qinfo)
 
     rtce_init(dev);
 
-    return info->init(dev);
+    return pc->init(dev);
 }
 
-void spapr_vio_bus_register_withprop(VIOsPAPRDeviceInfo *info)
+void spapr_vio_bus_register_withprop(DeviceInfo *info)
 {
-    info->qdev.init = spapr_vio_busdev_init;
-    info->qdev.bus_info = &spapr_vio_bus_info;
+    info->init = spapr_vio_busdev_init;
+    info->bus_info = &spapr_vio_bus_info;
 
-    assert(info->qdev.size >= sizeof(VIOsPAPRDevice));
-    qdev_register(&info->qdev);
+    assert(info->size >= sizeof(VIOsPAPRDevice));
+    qdev_register_subclass(info, TYPE_VIO_SPAPR_DEVICE);
 }
 
 static target_ulong h_vio_signal(CPUState *env, sPAPREnvironment *spapr,
@@ -662,15 +662,15 @@  static target_ulong h_vio_signal(CPUState *env, sPAPREnvironment *spapr,
     target_ulong reg = args[0];
     target_ulong mode = args[1];
     VIOsPAPRDevice *dev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
-    VIOsPAPRDeviceInfo *info;
+    VIOsPAPRDeviceClass *pc;
 
     if (!dev) {
         return H_PARAMETER;
     }
 
-    info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
+    pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
 
-    if (mode & ~info->signal_mask) {
+    if (mode & ~pc->signal_mask) {
         return H_PARAMETER;
     }
 
@@ -711,8 +711,12 @@  VIOsPAPRBus *spapr_vio_bus_init(void)
     spapr_rtas_register("ibm,set-tce-bypass", rtas_set_tce_bypass);
     spapr_rtas_register("quiesce", rtas_quiesce);
 
+#if 0
+    /* Evil and broken */
+
     for (qinfo = device_info_list; qinfo; qinfo = qinfo->next) {
         VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)qinfo;
+        VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
 
         if (qinfo->bus_info != &spapr_vio_bus_info) {
             continue;
@@ -722,6 +726,7 @@  VIOsPAPRBus *spapr_vio_bus_init(void)
             info->hcalls(bus);
         }
     }
+#endif
 
     return bus;
 }
@@ -741,9 +746,18 @@  static SysBusDeviceInfo spapr_vio_bridge_info = {
     .qdev.no_user = 1,
 };
 
+static TypeInfo spapr_vio_type_info = {
+    .name = TYPE_VIO_SPAPR_DEVICE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(VIOsPAPRDevice),
+    .abstract = true,
+    .class_size = sizeof(VIOsPAPRDeviceClass),
+};
+
 static void spapr_vio_register_devices(void)
 {
     sysbus_register_withprop(&spapr_vio_bridge_info);
+    type_register_static(&spapr_vio_type_info);
 }
 
 device_init(spapr_vio_register_devices)
diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
index a325a5f..89438a2 100644
--- a/hw/spapr_vio.h
+++ b/hw/spapr_vio.h
@@ -34,6 +34,14 @@  enum VIOsPAPR_TCEAccess {
 
 #define SPAPR_VTY_BASE_ADDRESS     0x30000000
 
+#define TYPE_VIO_SPAPR_DEVICE "vio-spapr-device"
+#define VIO_SPAPR_DEVICE(obj) \
+     OBJECT_CHECK(VIOsPAPRDevice, (obj), TYPE_VIO_SPAPR_DEVICE)
+#define VIO_SPAPR_DEVICE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(VIOsPAPRDeviceClass, (klass), TYPE_VIO_SPAPR_DEVICE)
+#define VIO_SPAPR_DEVICE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(VIOsPAPRDeviceClass, (obj), TYPE_VIO_SPAPR_DEVICE)
+
 struct VIOsPAPRDevice;
 
 typedef struct VIOsPAPR_RTCE {
@@ -47,6 +55,16 @@  typedef struct VIOsPAPR_CRQ {
     int(*SendFunc)(struct VIOsPAPRDevice *vdev, uint8_t *crq);
 } VIOsPAPR_CRQ;
 
+typedef struct VIOsPAPRDeviceClass {
+    DeviceClass parent_class;
+
+    const char *dt_name, *dt_type, *dt_compatible;
+    target_ulong signal_mask;
+    int (*init)(VIOsPAPRDevice *dev);
+    void (*hcalls)(VIOsPAPRBus *bus);
+    int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off);
+} VIOsPAPRDeviceClass;
+
 typedef struct VIOsPAPRDevice {
     DeviceState qdev;
     uint32_t reg;
@@ -70,18 +88,9 @@  typedef struct VIOsPAPRBus {
     BusState bus;
 } VIOsPAPRBus;
 
-typedef struct {
-    DeviceInfo qdev;
-    const char *dt_name, *dt_type, *dt_compatible;
-    target_ulong signal_mask;
-    int (*init)(VIOsPAPRDevice *dev);
-    void (*hcalls)(VIOsPAPRBus *bus);
-    int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off);
-} VIOsPAPRDeviceInfo;
-
 extern VIOsPAPRBus *spapr_vio_bus_init(void);
 extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
-extern void spapr_vio_bus_register_withprop(VIOsPAPRDeviceInfo *info);
+extern void spapr_vio_bus_register_withprop(DeviceInfo *info);
 extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
 
 extern int spapr_vio_signal(VIOsPAPRDevice *dev, target_ulong mode);
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 21d946e..b83bb7f 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -947,19 +947,28 @@  static int spapr_vscsi_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
     return 0;
 }
 
-static VIOsPAPRDeviceInfo spapr_vscsi_info = {
-    .init = spapr_vscsi_init,
-    .devnode = spapr_vscsi_devnode,
-    .dt_name = "v-scsi",
-    .dt_type = "vscsi",
-    .dt_compatible = "IBM,v-scsi",
-    .signal_mask = 0x00000001,
-    .qdev.name = "spapr-vscsi",
-    .qdev.size = sizeof(VSCSIState),
-    .qdev.props = (Property[]) {
-        DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x10000000),
-        DEFINE_PROP_END_OF_LIST(),
-    },
+static Property spapr_vscsi_properties[] = {
+    DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x10000000),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_vscsi_class_init(ObjectClass *klass, void *data)
+{
+    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
+
+    k->init = spapr_vscsi_init;
+    k->devnode = spapr_vscsi_devnode;
+    k->dt_name = "v-scsi";
+    k->dt_type = "vscsi";
+    k->dt_compatible = "IBM,v-scsi";
+    k->signal_mask = 0x00000001;
+}
+
+static DeviceInfo spapr_vscsi_info = {
+    .name = "spapr-vscsi",
+    .size = sizeof(VSCSIState),
+    .props = spapr_vscsi_properties,
+    .class_init = spapr_vscsi_class_init,
 };
 
 static void spapr_vscsi_register(void)
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index fddecd8..466c4f0 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -141,19 +141,28 @@  static void vty_hcalls(VIOsPAPRBus *bus)
     spapr_register_hypercall(H_GET_TERM_CHAR, h_get_term_char);
 }
 
-static VIOsPAPRDeviceInfo spapr_vty_info = {
-    .init = spapr_vty_init,
-    .dt_name = "vty",
-    .dt_type = "serial",
-    .dt_compatible = "hvterm1",
-    .hcalls = vty_hcalls,
-    .qdev.name = "spapr-vty",
-    .qdev.size = sizeof(VIOsPAPRVTYDevice),
-    .qdev.props = (Property[]) {
-        DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, SPAPR_VTY_BASE_ADDRESS, 0),
-        DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
-        DEFINE_PROP_END_OF_LIST(),
-    },
+static Property spapr_vty_properties[] = {
+    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, SPAPR_VTY_BASE_ADDRESS, 0),
+    DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_vty_class_init(ObjectClass *klass, void *data)
+{
+    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
+
+    k->init = spapr_vty_init;
+    k->dt_name = "vty";
+    k->dt_type = "serial";
+    k->dt_compatible = "hvterm1";
+    k->hcalls = vty_hcalls;
+}
+
+static DeviceInfo spapr_vty_info = {
+    .name = "spapr-vty",
+    .size = sizeof(VIOsPAPRVTYDevice),
+    .props = spapr_vty_properties,
+    .class_init = spapr_vty_class_init,
 };
 
 static VIOsPAPRDevice *vty_lookup(sPAPREnvironment *spapr, target_ulong reg)