At present, spapr_dr_connector_new assigns a name to the drc based on its type and ID. The few places the DRC name are used, however, are supposed to give some sort of human-useful information to correlate guest-side problems with host-side resources (for PCI devices it's supposed to be a physical location code).
The owners who set up the DRCs are in a much better position to supply this information than the DRC core. We already have a "name" R/O property, so make it R/W and set from the callers. We change the "name" property from a new-style QOM one to the property list form, simply because it's a bit less verbose that way. Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 16 +++++++++--- hw/ppc/spapr_drc.c | 61 ++++++++++++---------------------------------- hw/ppc/spapr_pci.c | 29 ++++++++++++++++++++-- include/hw/ppc/spapr_drc.h | 5 ++-- 4 files changed, 57 insertions(+), 54 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 86e6228..671cdbb 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1909,11 +1909,14 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr) for (i = 0; i < nr_lmbs; i++) { sPAPRDRConnector *drc; + char *drc_name; uint64_t addr; addr = i * lmb_size + spapr->hotplug_memory.base; + drc_name = g_strdup_printf("LMB %"PRId64, addr / lmb_size); drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB, - addr/lmb_size); + addr / lmb_size, drc_name, &error_abort); + g_free(drc_name); qemu_register_reset(spapr_drc_reset, drc); } } @@ -2008,9 +2011,14 @@ static void spapr_init_cpus(sPAPRMachineState *spapr) int core_id = i * smp_threads; if (mc->has_hotpluggable_cpus) { - sPAPRDRConnector *drc = - spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU, - (core_id / smp_threads) * smt); + int id = (core_id / smp_threads) * smt; + char *drc_name; + sPAPRDRConnector *drc; + + drc_name = g_strdup_printf("CPU %d", id); + drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU, + id, drc_name, &error_abort); + g_free(drc_name); qemu_register_reset(spapr_drc_reset, drc); } diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 9140b48..1fc51c9 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -213,12 +213,6 @@ static void prop_get_index(Object *obj, Visitor *v, const char *name, visit_type_uint32(v, name, &value, errp); } -static char *prop_get_name(Object *obj, Error **errp) -{ - sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj); - return g_strdup(drc->name); -} - static void prop_get_fdt(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { @@ -564,9 +558,11 @@ static void unrealize(DeviceState *d, Error **errp) } sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, - uint32_t id) + uint32_t id, const char *name, + Error **errp) { sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type)); + Error *local_err = NULL; char *prop_name; drc->id = id; @@ -574,48 +570,16 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", spapr_drc_index(drc)); object_property_add_child(owner, prop_name, OBJECT(drc), NULL); - object_property_set_bool(OBJECT(drc), true, "realized", NULL); g_free(prop_name); - /* human-readable name for a DRC to encode into the DT - * description. this is mainly only used within a guest in place - * of the unique DRC index. - * - * in the case of VIO/PCI devices, it corresponds to a - * "location code" that maps a logical device/function (DRC index) - * to a physical (or virtual in the case of VIO) location in the - * system by chaining together the "location label" for each - * encapsulating component. - * - * since this is more to do with diagnosing physical hardware - * issues than guest compatibility, we choose location codes/DRC - * names that adhere to the documented format, but avoid encoding - * the entire topology information into the label/code, instead - * just using the location codes based on the labels for the - * endpoints (VIO/PCI adaptor connectors), which is basically - * just "C" followed by an integer ID. - * - * DRC names as documented by PAPR+ v2.7, 13.5.2.4 - * location codes as documented by PAPR+ v2.7, 12.3.1.5 - */ - switch (spapr_drc_type(drc)) { - case SPAPR_DR_CONNECTOR_TYPE_CPU: - drc->name = g_strdup_printf("CPU %d", id); - break; - case SPAPR_DR_CONNECTOR_TYPE_PHB: - drc->name = g_strdup_printf("PHB %d", id); - break; - case SPAPR_DR_CONNECTOR_TYPE_VIO: - case SPAPR_DR_CONNECTOR_TYPE_PCI: - drc->name = g_strdup_printf("C%d", id); - break; - case SPAPR_DR_CONNECTOR_TYPE_LMB: - drc->name = g_strdup_printf("LMB %d", id); - break; - default: - g_assert(false); + object_property_set_str(OBJECT(drc), name, "name", &local_err); + if (local_err) { + error_propagate(errp, local_err); + return NULL; } + object_property_set_bool(OBJECT(drc), true, "realized", NULL); + /* PCI slot always start in a USABLE state, and stay there */ if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) { drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE; @@ -631,16 +595,21 @@ static void spapr_dr_connector_instance_init(Object *obj) object_property_add_uint32_ptr(obj, "id", &drc->id, NULL); object_property_add(obj, "index", "uint32", prop_get_index, NULL, NULL, NULL, NULL); - object_property_add_str(obj, "name", prop_get_name, NULL, NULL); object_property_add(obj, "fdt", "struct", prop_get_fdt, NULL, NULL, NULL, NULL); } +static Property spapr_drc_properties[] = { + DEFINE_PROP_STRING("name", sPAPRDRConnector, name), + DEFINE_PROP_END_OF_LIST(), +}; + static void spapr_dr_connector_class_init(ObjectClass *k, void *data) { DeviceClass *dk = DEVICE_CLASS(k); sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k); + dk->props = spapr_drc_properties; dk->reset = reset; dk->realize = realize; dk->unrealize = unrealize; diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 5cba8f9..967d7c3 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1570,10 +1570,10 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) sPAPRTCETable *tcet; const unsigned windows_supported = sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1; + Error *local_err = NULL; if (sphb->index != (uint32_t)-1) { sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); - Error *local_err = NULL; if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn[0] != (uint32_t)-1) || (sphb->dma_liobn[1] != (uint32_t)-1 && windows_supported == 2) @@ -1759,8 +1759,33 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) /* allocate connectors for child PCI devices */ if (sphb->dr_enabled) { for (i = 0; i < PCI_SLOT_MAX * 8; i++) { + int id = sphb->index << 16 | i; + char *drc_name; + + /* Name for a DRC for the DT. For PCI devices, it is a + * "location code" mapping a logical device/function (DRC + * index) to a physical location in the system. + * + * This is more to do with diagnosing physical hardware + * issues than guest compatibility, so choose names that + * adhere to the documented format, but avoid encoding the + * entire topology information into the label/code, + * instead just using the location codes based on the + * labels for the endpoints (VIO/PCI adaptor connectors), + * which is basically just "C" followed by an integer ID. + * + * DRC names as documented by PAPR+ v2.7, 13.5.2.4 + * location codes as documented by PAPR+ v2.7, 12.3.1.5 + */ + + drc_name = g_strdup_printf("C%d", id); spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI, - (sphb->index << 16) | i); + id, drc_name, &local_err); + g_free(drc_name); + if (local_err) { + error_propagate(errp, local_err); + return; + } } } diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 795b7dd..d437e0a 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -184,7 +184,7 @@ typedef struct sPAPRDRConnector { uint32_t id; Object *owner; - const char *name; + char *name; /* sensor/indicator states */ uint32_t isolation_state; @@ -236,7 +236,8 @@ uint32_t spapr_drc_index(sPAPRDRConnector *drc); sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc); sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type, - uint32_t id); + uint32_t id, const char *name, + Error **errp); sPAPRDRConnector *spapr_drc_by_index(uint32_t index); sPAPRDRConnector *spapr_drc_by_id(const char *type, uint32_t id); int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner, -- 2.9.4