diff mbox

[4/4] pseries: Implement automatic PAPR VIO address allocation

Message ID 1333515728-9769-5-git-send-email-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson April 4, 2012, 5:02 a.m. UTC
PAPR virtual IO (VIO) devices require a unique, but otherwise arbitrary,
"address" used as a token to the hypercalls which manipulate them.

Currently the pseries machine code does an ok job of allocating these
addresses when the legacy -net nic / -serial and so forth options are used
but will fail to allocate them properly when using -device.

Specifically, you can use -device if all addresses are explicitly assigned.
Without explicit assignment, only one VIO device of each type (network,
console, SCSI) will be assigned properly, any further ones will attempt
to take the same address leading to a fatal error.

This patch fixes the situation by adding a proper address allocator to the
VIO "bus" code.  This is used both by -device and the legacy options and
default devices.  Addresses can still be explicitly assigned with -device
options if desired.

This patch changes the (guest visible) numbering of VIO devices, but since
their addresses are discovered using the device tree and already differ
from the numbering found on existing PowerVM systems, this does not break
compatibility.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/spapr.c       |    7 ++---
 hw/spapr_llan.c  |    5 +--
 hw/spapr_vio.c   |   74 ++++++++++++++++++++++++++++++++----------------------
 hw/spapr_vio.h   |   13 ++++-----
 hw/spapr_vscsi.c |    5 +--
 hw/spapr_vty.c   |    5 +--
 6 files changed, 59 insertions(+), 50 deletions(-)

Comments

Andreas Färber April 15, 2012, 5:42 p.m. UTC | #1
Am 04.04.2012 07:02, schrieb David Gibson:
> PAPR virtual IO (VIO) devices require a unique, but otherwise arbitrary,
> "address" used as a token to the hypercalls which manipulate them.
> 
> Currently the pseries machine code does an ok job of allocating these
> addresses when the legacy -net nic / -serial and so forth options are used
> but will fail to allocate them properly when using -device.
> 
> Specifically, you can use -device if all addresses are explicitly assigned.
> Without explicit assignment, only one VIO device of each type (network,
> console, SCSI) will be assigned properly, any further ones will attempt
> to take the same address leading to a fatal error.
> 
> This patch fixes the situation by adding a proper address allocator to the
> VIO "bus" code.  This is used both by -device and the legacy options and
> default devices.  Addresses can still be explicitly assigned with -device
> options if desired.
> 
> This patch changes the (guest visible) numbering of VIO devices, but since
> their addresses are discovered using the device tree and already differ
> from the numbering found on existing PowerVM systems, this does not break
> compatibility.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/spapr.c       |    7 ++---
>  hw/spapr_llan.c  |    5 +--
>  hw/spapr_vio.c   |   74 ++++++++++++++++++++++++++++++++----------------------
>  hw/spapr_vio.h   |   13 ++++-----
>  hw/spapr_vscsi.c |    5 +--
>  hw/spapr_vty.c   |    5 +--
>  6 files changed, 59 insertions(+), 50 deletions(-)

Reviewed-by: Andreas Färber <afaerber@suse.de>

Technically this change looks okay but I'd appreciate a second reviewer
as to what side-effects this change in numbering might have, so I'm
leaving this one to Alex.

Andreas

> 
> diff --git a/hw/spapr.c b/hw/spapr.c
> index bfaf260..cca20f9 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -631,8 +631,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>  
>      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]);
> +            spapr_vty_create(spapr->vio_bus, serial_hds[i]);
>          }
>      }
>  
> @@ -650,14 +649,14 @@ 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);
> +            spapr_vlan_create(spapr->vio_bus, nd);
>          } else {
>              pci_nic_init_nofail(&nd_table[i], nd->model, NULL);
>          }
>      }
>  
>      for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
> -        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
> +        spapr_vscsi_create(spapr->vio_bus);
>      }
>  
>      if (rma_size < (MIN_RMA_SLOF << 20)) {
> diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
> index 32dce17..a0020e9 100644
> --- a/hw/spapr_llan.c
> +++ b/hw/spapr_llan.c
> @@ -195,12 +195,11 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
>      return 0;
>  }
>  
> -void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd)
> +void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
>  {
>      DeviceState *dev;
>  
>      dev = qdev_create(&bus->bus, "spapr-vlan");
> -    qdev_prop_set_uint32(dev, "reg", reg);
>  
>      qdev_set_nic_properties(dev, nd);
>  
> @@ -473,7 +472,7 @@ static target_ulong h_multicast_ctrl(CPUPPCState *env, sPAPREnvironment *spapr,
>  }
>  
>  static Property spapr_vlan_properties[] = {
> -    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x10000000),
> +    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x10000000),
>      DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
> index 97d029a..1411f84 100644
> --- a/hw/spapr_vio.c
> +++ b/hw/spapr_vio.c
> @@ -620,41 +620,35 @@ static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
>      rtas_st(rets, 0, 0);
>  }
>  
> -static int spapr_vio_check_reg(VIOsPAPRDevice *sdev)
> +static void spapr_vio_busdev_reset(void *opaque)
>  {
> -    VIOsPAPRDevice *other_sdev;
> -    DeviceState *qdev;
> -    VIOsPAPRBus *sbus;
> +    VIOsPAPRDevice *dev = (VIOsPAPRDevice *)opaque;
> +
> +    if (dev->crq.qsize) {
> +        free_crq(dev);
> +    }
> +}
>  
> -    sbus = DO_UPCAST(VIOsPAPRBus, bus, sdev->qdev.parent_bus);
> +static VIOsPAPRDevice *reg_conflict(VIOsPAPRDevice *dev)
> +{
> +    VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
> +    DeviceState *qdev;
> +    VIOsPAPRDevice *other;
>  
>      /*
> -     * Check two device aren't given clashing addresses by the user (or some
> -     * other mechanism). We have to open code this because we have to check
> -     * for matches with devices other than us.
> +     * Check for a device other than the given one which is already
> +     * using the requested address. We have to open code this because
> +     * the given dev might already be in the list.
>       */
> -    QTAILQ_FOREACH(qdev, &sbus->bus.children, sibling) {
> -        other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
> +    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
> +        other = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
>  
> -        if (other_sdev != sdev && other_sdev->reg == sdev->reg) {
> -            fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
> -                    object_get_typename(OBJECT(sdev)),
> -                    object_get_typename(OBJECT(qdev)),
> -                    sdev->reg);
> -            return -EEXIST;
> +        if (other != dev && other->reg == dev->reg) {
> +            return other;
>          }
>      }
>  
> -    return 0;
> -}
> -
> -static void spapr_vio_busdev_reset(void *opaque)
> -{
> -    VIOsPAPRDevice *dev = (VIOsPAPRDevice *)opaque;
> -
> -    if (dev->crq.qsize) {
> -        free_crq(dev);
> -    }
> +    return NULL;    
>  }
>  
>  static int spapr_vio_busdev_init(DeviceState *qdev)
> @@ -662,11 +656,30 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
>      VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
>      VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
>      char *id;
> -    int ret;
>  
> -    ret = spapr_vio_check_reg(dev);
> -    if (ret) {
> -        return ret;
> +    if (dev->reg != -1) {
> +        /*
> +         * Explicitly assigned address, just verify that no-one else
> +         * is using it.  other mechanism). We have to open code this
> +         * rather than using spapr_vio_find_by_reg() because sdev
> +         * itself is already in the list.
> +         */
> +        VIOsPAPRDevice *other = reg_conflict(dev);
> +
> +        if (other) {
> +            fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
> +                    object_get_typename(OBJECT(qdev)),
> +                    object_get_typename(OBJECT(&other->qdev)),
> +                    dev->reg);
> +            return -1;
> +        }
> +    } else {
> +        /* Need to assign an address */
> +        VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
> +
> +        do {
> +            dev->reg = bus->next_reg++;
> +        } while (reg_conflict(dev));
>      }
>  
>      /* Don't overwrite ids assigned on the command line */
> @@ -728,6 +741,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
>  
>      qbus = qbus_create(&spapr_vio_bus_info, dev, "spapr-vio");
>      bus = DO_UPCAST(VIOsPAPRBus, bus, qbus);
> +    bus->next_reg = 0x1000;
>  
>      /* hcall-vio */
>      spapr_register_hypercall(H_VIO_SIGNAL, h_vio_signal);
> diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
> index fd29c5e..420398c 100644
> --- a/hw/spapr_vio.h
> +++ b/hw/spapr_vio.h
> @@ -32,8 +32,6 @@ enum VIOsPAPR_TCEAccess {
>      SPAPR_TCE_RW = 3,
>  };
>  
> -#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)
> @@ -82,13 +80,14 @@ struct VIOsPAPRDevice {
>      VIOsPAPR_CRQ crq;
>  };
>  
> -#define DEFINE_SPAPR_PROPERTIES(type, field, default_reg, default_dma_window) \
> -        DEFINE_PROP_UINT32("reg", type, field.reg, default_reg), \
> +#define DEFINE_SPAPR_PROPERTIES(type, field, default_dma_window)       \
> +        DEFINE_PROP_UINT32("reg", type, field.reg, -1),                \
>          DEFINE_PROP_UINT32("dma-window", type, field.rtce_window_size, \
>                             default_dma_window)
>  
>  struct VIOsPAPRBus {
>      BusState bus;
> +    uint32_t next_reg;
>      int (*init)(VIOsPAPRDevice *dev);
>      int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off);
>  };
> @@ -119,9 +118,9 @@ int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq);
>  
>  VIOsPAPRDevice *vty_lookup(sPAPREnvironment *spapr, target_ulong reg);
>  void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
> -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);
> +void spapr_vty_create(VIOsPAPRBus *bus, CharDriverState *chardev);
> +void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd);
> +void spapr_vscsi_create(VIOsPAPRBus *bus);
>  
>  VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus);
>  
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index 2167017..de2ba68 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -920,12 +920,11 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>      return 0;
>  }
>  
> -void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg)
> +void spapr_vscsi_create(VIOsPAPRBus *bus)
>  {
>      DeviceState *dev;
>  
>      dev = qdev_create(&bus->bus, "spapr-vscsi");
> -    qdev_prop_set_uint32(dev, "reg", reg);
>  
>      qdev_init_nofail(dev);
>  }
> @@ -948,7 +947,7 @@ static int spapr_vscsi_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
>  }
>  
>  static Property spapr_vscsi_properties[] = {
> -    DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x10000000),
> +    DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x10000000),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
> index a30c040..c9674f3 100644
> --- a/hw/spapr_vty.c
> +++ b/hw/spapr_vty.c
> @@ -123,18 +123,17 @@ static target_ulong h_get_term_char(CPUPPCState *env, sPAPREnvironment *spapr,
>      return H_SUCCESS;
>  }
>  
> -void spapr_vty_create(VIOsPAPRBus *bus, uint32_t reg, CharDriverState *chardev)
> +void spapr_vty_create(VIOsPAPRBus *bus, CharDriverState *chardev)
>  {
>      DeviceState *dev;
>  
>      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);
>  }
>  
>  static Property spapr_vty_properties[] = {
> -    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, SPAPR_VTY_BASE_ADDRESS, 0),
> +    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, 0),
>      DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
>      DEFINE_PROP_END_OF_LIST(),
>  };
David Gibson April 16, 2012, 4:02 a.m. UTC | #2
On Sun, Apr 15, 2012 at 07:42:58PM +0200, Andreas Färber wrote:
> Am 04.04.2012 07:02, schrieb David Gibson:
> > PAPR virtual IO (VIO) devices require a unique, but otherwise arbitrary,
> > "address" used as a token to the hypercalls which manipulate them.
> > 
> > Currently the pseries machine code does an ok job of allocating these
> > addresses when the legacy -net nic / -serial and so forth options are used
> > but will fail to allocate them properly when using -device.
> > 
> > Specifically, you can use -device if all addresses are explicitly assigned.
> > Without explicit assignment, only one VIO device of each type (network,
> > console, SCSI) will be assigned properly, any further ones will attempt
> > to take the same address leading to a fatal error.
> > 
> > This patch fixes the situation by adding a proper address allocator to the
> > VIO "bus" code.  This is used both by -device and the legacy options and
> > default devices.  Addresses can still be explicitly assigned with -device
> > options if desired.
> > 
> > This patch changes the (guest visible) numbering of VIO devices, but since
> > their addresses are discovered using the device tree and already differ
> > from the numbering found on existing PowerVM systems, this does not break
> > compatibility.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/spapr.c       |    7 ++---
> >  hw/spapr_llan.c  |    5 +--
> >  hw/spapr_vio.c   |   74 ++++++++++++++++++++++++++++++++----------------------
> >  hw/spapr_vio.h   |   13 ++++-----
> >  hw/spapr_vscsi.c |    5 +--
> >  hw/spapr_vty.c   |    5 +--
> >  6 files changed, 59 insertions(+), 50 deletions(-)
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> 
> Technically this change looks okay but I'd appreciate a second reviewer
> as to what side-effects this change in numbering might have, so I'm
> leaving this one to Alex.

Hm, ok.  As with the interrupt mapping patch, because we communicate
all the addressing information to the guest through the device tree,
the exact numbering really doesn't matter.
diff mbox

Patch

diff --git a/hw/spapr.c b/hw/spapr.c
index bfaf260..cca20f9 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -631,8 +631,7 @@  static void ppc_spapr_init(ram_addr_t ram_size,
 
     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]);
+            spapr_vty_create(spapr->vio_bus, serial_hds[i]);
         }
     }
 
@@ -650,14 +649,14 @@  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);
+            spapr_vlan_create(spapr->vio_bus, nd);
         } else {
             pci_nic_init_nofail(&nd_table[i], nd->model, NULL);
         }
     }
 
     for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
-        spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
+        spapr_vscsi_create(spapr->vio_bus);
     }
 
     if (rma_size < (MIN_RMA_SLOF << 20)) {
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index 32dce17..a0020e9 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -195,12 +195,11 @@  static int spapr_vlan_init(VIOsPAPRDevice *sdev)
     return 0;
 }
 
-void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd)
+void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
 {
     DeviceState *dev;
 
     dev = qdev_create(&bus->bus, "spapr-vlan");
-    qdev_prop_set_uint32(dev, "reg", reg);
 
     qdev_set_nic_properties(dev, nd);
 
@@ -473,7 +472,7 @@  static target_ulong h_multicast_ctrl(CPUPPCState *env, sPAPREnvironment *spapr,
 }
 
 static Property spapr_vlan_properties[] = {
-    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x10000000),
+    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x10000000),
     DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index 97d029a..1411f84 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -620,41 +620,35 @@  static void rtas_quiesce(sPAPREnvironment *spapr, uint32_t token,
     rtas_st(rets, 0, 0);
 }
 
-static int spapr_vio_check_reg(VIOsPAPRDevice *sdev)
+static void spapr_vio_busdev_reset(void *opaque)
 {
-    VIOsPAPRDevice *other_sdev;
-    DeviceState *qdev;
-    VIOsPAPRBus *sbus;
+    VIOsPAPRDevice *dev = (VIOsPAPRDevice *)opaque;
+
+    if (dev->crq.qsize) {
+        free_crq(dev);
+    }
+}
 
-    sbus = DO_UPCAST(VIOsPAPRBus, bus, sdev->qdev.parent_bus);
+static VIOsPAPRDevice *reg_conflict(VIOsPAPRDevice *dev)
+{
+    VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
+    DeviceState *qdev;
+    VIOsPAPRDevice *other;
 
     /*
-     * Check two device aren't given clashing addresses by the user (or some
-     * other mechanism). We have to open code this because we have to check
-     * for matches with devices other than us.
+     * Check for a device other than the given one which is already
+     * using the requested address. We have to open code this because
+     * the given dev might already be in the list.
      */
-    QTAILQ_FOREACH(qdev, &sbus->bus.children, sibling) {
-        other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
+    QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
+        other = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
 
-        if (other_sdev != sdev && other_sdev->reg == sdev->reg) {
-            fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
-                    object_get_typename(OBJECT(sdev)),
-                    object_get_typename(OBJECT(qdev)),
-                    sdev->reg);
-            return -EEXIST;
+        if (other != dev && other->reg == dev->reg) {
+            return other;
         }
     }
 
-    return 0;
-}
-
-static void spapr_vio_busdev_reset(void *opaque)
-{
-    VIOsPAPRDevice *dev = (VIOsPAPRDevice *)opaque;
-
-    if (dev->crq.qsize) {
-        free_crq(dev);
-    }
+    return NULL;    
 }
 
 static int spapr_vio_busdev_init(DeviceState *qdev)
@@ -662,11 +656,30 @@  static int spapr_vio_busdev_init(DeviceState *qdev)
     VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
     VIOsPAPRDeviceClass *pc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
     char *id;
-    int ret;
 
-    ret = spapr_vio_check_reg(dev);
-    if (ret) {
-        return ret;
+    if (dev->reg != -1) {
+        /*
+         * Explicitly assigned address, just verify that no-one else
+         * is using it.  other mechanism). We have to open code this
+         * rather than using spapr_vio_find_by_reg() because sdev
+         * itself is already in the list.
+         */
+        VIOsPAPRDevice *other = reg_conflict(dev);
+
+        if (other) {
+            fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
+                    object_get_typename(OBJECT(qdev)),
+                    object_get_typename(OBJECT(&other->qdev)),
+                    dev->reg);
+            return -1;
+        }
+    } else {
+        /* Need to assign an address */
+        VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
+
+        do {
+            dev->reg = bus->next_reg++;
+        } while (reg_conflict(dev));
     }
 
     /* Don't overwrite ids assigned on the command line */
@@ -728,6 +741,7 @@  VIOsPAPRBus *spapr_vio_bus_init(void)
 
     qbus = qbus_create(&spapr_vio_bus_info, dev, "spapr-vio");
     bus = DO_UPCAST(VIOsPAPRBus, bus, qbus);
+    bus->next_reg = 0x1000;
 
     /* hcall-vio */
     spapr_register_hypercall(H_VIO_SIGNAL, h_vio_signal);
diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
index fd29c5e..420398c 100644
--- a/hw/spapr_vio.h
+++ b/hw/spapr_vio.h
@@ -32,8 +32,6 @@  enum VIOsPAPR_TCEAccess {
     SPAPR_TCE_RW = 3,
 };
 
-#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)
@@ -82,13 +80,14 @@  struct VIOsPAPRDevice {
     VIOsPAPR_CRQ crq;
 };
 
-#define DEFINE_SPAPR_PROPERTIES(type, field, default_reg, default_dma_window) \
-        DEFINE_PROP_UINT32("reg", type, field.reg, default_reg), \
+#define DEFINE_SPAPR_PROPERTIES(type, field, default_dma_window)       \
+        DEFINE_PROP_UINT32("reg", type, field.reg, -1),                \
         DEFINE_PROP_UINT32("dma-window", type, field.rtce_window_size, \
                            default_dma_window)
 
 struct VIOsPAPRBus {
     BusState bus;
+    uint32_t next_reg;
     int (*init)(VIOsPAPRDevice *dev);
     int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off);
 };
@@ -119,9 +118,9 @@  int spapr_vio_send_crq(VIOsPAPRDevice *dev, uint8_t *crq);
 
 VIOsPAPRDevice *vty_lookup(sPAPREnvironment *spapr, target_ulong reg);
 void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
-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);
+void spapr_vty_create(VIOsPAPRBus *bus, CharDriverState *chardev);
+void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd);
+void spapr_vscsi_create(VIOsPAPRBus *bus);
 
 VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus);
 
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 2167017..de2ba68 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -920,12 +920,11 @@  static int spapr_vscsi_init(VIOsPAPRDevice *dev)
     return 0;
 }
 
-void spapr_vscsi_create(VIOsPAPRBus *bus, uint32_t reg)
+void spapr_vscsi_create(VIOsPAPRBus *bus)
 {
     DeviceState *dev;
 
     dev = qdev_create(&bus->bus, "spapr-vscsi");
-    qdev_prop_set_uint32(dev, "reg", reg);
 
     qdev_init_nofail(dev);
 }
@@ -948,7 +947,7 @@  static int spapr_vscsi_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
 }
 
 static Property spapr_vscsi_properties[] = {
-    DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x2000, 0x10000000),
+    DEFINE_SPAPR_PROPERTIES(VSCSIState, vdev, 0x10000000),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
index a30c040..c9674f3 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -123,18 +123,17 @@  static target_ulong h_get_term_char(CPUPPCState *env, sPAPREnvironment *spapr,
     return H_SUCCESS;
 }
 
-void spapr_vty_create(VIOsPAPRBus *bus, uint32_t reg, CharDriverState *chardev)
+void spapr_vty_create(VIOsPAPRBus *bus, CharDriverState *chardev)
 {
     DeviceState *dev;
 
     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);
 }
 
 static Property spapr_vty_properties[] = {
-    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, SPAPR_VTY_BASE_ADDRESS, 0),
+    DEFINE_SPAPR_PROPERTIES(VIOsPAPRVTYDevice, sdev, 0),
     DEFINE_PROP_CHR("chardev", VIOsPAPRVTYDevice, chardev),
     DEFINE_PROP_END_OF_LIST(),
 };