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 <da...@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 <afaer...@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(), > }; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg