diff mbox

[1/5] spapr: Introduce DRC subclasses

Message ID 20170602072952.25454-2-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson June 2, 2017, 7:29 a.m. UTC
Currently we only have a single QOM type for all DRCs, but lots of
places where we switch behaviour based on the DRC's PAPR defined type.
This is a poor use of our existing type system.

So, instead create QOM subclasses for each PAPR defined DRC type.  We
also introduce intermediate subclasses for physical and logical DRCs,
a division which will be useful later on.

Instead of being stored in the DRC object itself, the PAPR type is now
stored in the class structure.  There are still many places where we
switch directly on the PAPR type value, but this at least provides the
basis to start to remove those.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c             |   5 +-
 hw/ppc/spapr_drc.c         | 120 +++++++++++++++++++++++++++++++++------------
 hw/ppc/spapr_pci.c         |   3 +-
 include/hw/ppc/spapr_drc.h |  47 ++++++++++++++++--
 4 files changed, 136 insertions(+), 39 deletions(-)

Comments

Michael Roth June 3, 2017, 10:05 p.m. UTC | #1
Quoting David Gibson (2017-06-02 02:29:48)
> Currently we only have a single QOM type for all DRCs, but lots of
> places where we switch behaviour based on the DRC's PAPR defined type.
> This is a poor use of our existing type system.
> 
> So, instead create QOM subclasses for each PAPR defined DRC type.  We
> also introduce intermediate subclasses for physical and logical DRCs,
> a division which will be useful later on.
> 
> Instead of being stored in the DRC object itself, the PAPR type is now
> stored in the class structure.  There are still many places where we
> switch directly on the PAPR type value, but this at least provides the
> basis to start to remove those.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c             |   5 +-
>  hw/ppc/spapr_drc.c         | 120 +++++++++++++++++++++++++++++++++------------
>  hw/ppc/spapr_pci.c         |   3 +-
>  include/hw/ppc/spapr_drc.h |  47 ++++++++++++++++--
>  4 files changed, 136 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5d10366..456f9e7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1911,7 +1911,7 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
>          uint64_t addr;
> 
>          addr = i * lmb_size + spapr->hotplug_memory.base;
> -        drc = spapr_dr_connector_new(OBJECT(spapr), SPAPR_DR_CONNECTOR_TYPE_LMB,
> +        drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
>                                       addr/lmb_size);
>          qemu_register_reset(spapr_drc_reset, drc);
>      }
> @@ -2008,8 +2008,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> 
>          if (mc->has_hotpluggable_cpus) {
>              sPAPRDRConnector *drc =
> -                spapr_dr_connector_new(OBJECT(spapr),
> -                                       SPAPR_DR_CONNECTOR_TYPE_CPU,
> +                spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
>                                         (core_id / smp_threads) * smt);
> 
>              qemu_register_reset(spapr_drc_reset, drc);
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index a35314e..690b41f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -70,14 +70,23 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
>      return shift;
>  }
> 
> +sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
> +{
> +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +
> +    return 1 << drck->typeshift;
> +}
> +
>  uint32_t spapr_drc_index(sPAPRDRConnector *drc)
>  {
> +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +
>      /* no set format for a drc index: it only needs to be globally
>       * unique. this is how we encode the DRC type on bare-metal
>       * however, so might as well do that here
>       */
> -    return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) |
> -            (drc->id & DRC_INDEX_ID_MASK);
> +    return (drck->typeshift << DRC_INDEX_TYPE_SHIFT)
> +        | (drc->id & DRC_INDEX_ID_MASK);
>  }
> 
>  static uint32_t set_isolation_state(sPAPRDRConnector *drc,
> @@ -107,7 +116,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
>       * If the LMB being removed doesn't belong to a DIMM device that is
>       * actually being unplugged, fail the isolation request here.
>       */
> -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_LMB) {
> +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
>          if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
>               !drc->awaiting_release) {
>              return RTAS_OUT_HW_ERROR;
> @@ -177,7 +186,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
>          }
>      }
> 
> -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
>          drc->allocation_state = state;
>          if (drc->awaiting_release &&
>              drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> @@ -191,11 +200,6 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
>      return RTAS_OUT_SUCCESS;
>  }
> 
> -sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
> -{
> -    return drc->type;
> -}
> -
>  static const char *get_name(sPAPRDRConnector *drc)
>  {
>      return drc->name;
> @@ -217,7 +221,7 @@ static void set_signalled(sPAPRDRConnector *drc)
>  static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
>  {
>      if (drc->dev) {
> -        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> +        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
>              drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
>              /* for logical DR, we return a state of UNUSABLE
>               * iff the allocation state UNUSABLE.
> @@ -235,7 +239,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
>              *state = SPAPR_DR_ENTITY_SENSE_PRESENT;
>          }
>      } else {
> -        if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +        if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
>              /* PCI devices, and only PCI devices, use EMPTY
>               * in cases where we'd otherwise use UNUSABLE
>               */
> @@ -368,7 +372,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>          error_setg(errp, "an attached device is still awaiting release");
>          return;
>      }
> -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
>          g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE);
>      }
>      g_assert(fdt || coldplug);
> @@ -380,7 +384,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>       * may be accessing the device, we can easily crash the guest, so we
>       * we defer completion of removal in such cases to the reset() hook.
>       */
> -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
>          drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
>      }
>      drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
> @@ -398,10 +402,10 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
>       * 'physical' DR resources such as PCI where each device/resource is
>       * signalled individually.
>       */
> -    drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI)
> +    drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI)
>                       ? true : coldplug;
> 
> -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
>          drc->awaiting_allocation = true;
>      }
> 
> @@ -441,7 +445,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
>          return;
>      }
> 
> -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
>          drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
>          trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
>          drc->awaiting_release = true;
> @@ -458,8 +462,8 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
> 
>      drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
> 
> -    /* Calling release callbacks based on drc->type. */
> -    switch (drc->type) {
> +    /* Calling release callbacks based on spapr_drc_type(drc). */
> +    switch (spapr_drc_type(drc)) {
>      case SPAPR_DR_CONNECTOR_TYPE_CPU:
>          spapr_core_release(drc->dev);
>          break;
> @@ -515,7 +519,7 @@ static void reset(DeviceState *d)
>          }
> 
>          /* non-PCI devices may be awaiting a transition to UNUSABLE */
> -        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> +        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
>              drc->awaiting_release) {
>              drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
>          }
> @@ -544,7 +548,7 @@ static bool spapr_drc_needed(void *opaque)
>       * If there is dev plugged in, we need to migrate the DRC state when
>       * it is different from cold-plugged state
>       */
> -    switch (drc->type) {
> +    switch (spapr_drc_type(drc)) {
>      case SPAPR_DR_CONNECTOR_TYPE_PCI:
>          rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) &&
>                 (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) &&
> @@ -630,17 +634,12 @@ static void unrealize(DeviceState *d, Error **errp)
>      }
>  }
> 
> -sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> -                                         sPAPRDRConnectorType type,
> +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
>                                           uint32_t id)
>  {
> -    sPAPRDRConnector *drc =
> -        SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
> +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type));
>      char *prop_name;
> 
> -    g_assert(type);
> -
> -    drc->type = type;
>      drc->id = id;
>      drc->owner = owner;
>      prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
> @@ -670,7 +669,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
>       * 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 (drc->type) {
> +    switch (spapr_drc_type(drc)) {
>      case SPAPR_DR_CONNECTOR_TYPE_CPU:
>          drc->name = g_strdup_printf("CPU %d", id);
>          break;
> @@ -689,7 +688,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
>      }
> 
>      /* PCI slot always start in a USABLE state, and stay there */
> -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
>          drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
>      }
> 
> @@ -741,6 +740,27 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
>      dk->user_creatable = false;
>  }
> 
> +static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
> +{
> +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> +
> +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
> +}
> +
> +static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
> +{
> +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> +
> +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
> +}
> +
> +static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
> +{
> +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> +
> +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
> +}
> +
>  static const TypeInfo spapr_dr_connector_info = {
>      .name          = TYPE_SPAPR_DR_CONNECTOR,
>      .parent        = TYPE_DEVICE,
> @@ -750,6 +770,39 @@ static const TypeInfo spapr_dr_connector_info = {
>      .class_init    = spapr_dr_connector_class_init,
>  };
> 
> +static const TypeInfo spapr_drc_physical_info = {
> +    .name          = TYPE_SPAPR_DRC_PHYSICAL,
> +    .parent        = TYPE_SPAPR_DR_CONNECTOR,
> +    .instance_size = sizeof(sPAPRDRConnector),
> +};
> +
> +static const TypeInfo spapr_drc_logical_info = {
> +    .name          = TYPE_SPAPR_DRC_LOGICAL,
> +    .parent        = TYPE_SPAPR_DR_CONNECTOR,
> +    .instance_size = sizeof(sPAPRDRConnector),
> +};

It doesn't look like there's any intent of being able to instantiate
a "generic" logical/physical DRC for future types, so I think these
should be marked .abstract = true.

> +
> +static const TypeInfo spapr_drc_cpu_info = {
> +    .name          = TYPE_SPAPR_DRC_CPU,
> +    .parent        = TYPE_SPAPR_DRC_LOGICAL,
> +    .instance_size = sizeof(sPAPRDRConnector),
> +    .class_init    = spapr_drc_cpu_class_init,
> +};
> +
> +static const TypeInfo spapr_drc_pci_info = {
> +    .name          = TYPE_SPAPR_DRC_PCI,
> +    .parent        = TYPE_SPAPR_DRC_PHYSICAL,
> +    .instance_size = sizeof(sPAPRDRConnector),
> +    .class_init    = spapr_drc_pci_class_init,
> +};
> +
> +static const TypeInfo spapr_drc_lmb_info = {
> +    .name          = TYPE_SPAPR_DRC_LMB,
> +    .parent        = TYPE_SPAPR_DRC_LOGICAL,
> +    .instance_size = sizeof(sPAPRDRConnector),
> +    .class_init    = spapr_drc_lmb_class_init,
> +};
> +
>  /* helper functions for external users */
> 
>  sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
> @@ -858,7 +911,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>              continue;
>          }
> 
> -        if ((drc->type & drc_type_mask) == 0) {
> +        if ((spapr_drc_type(drc) & drc_type_mask) == 0) {
>              continue;
>          }
> 
> @@ -878,7 +931,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
> 
>          /* ibm,drc-types */
>          drc_types = g_string_append(drc_types,
> -                                    spapr_drc_get_type_str(drc->type));
> +                                    spapr_drc_get_type_str(spapr_drc_type(drc)));
>          drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
>      }
> 
> @@ -1210,6 +1263,11 @@ out:
>  static void spapr_drc_register_types(void)
>  {
>      type_register_static(&spapr_dr_connector_info);
> +    type_register_static(&spapr_drc_physical_info);
> +    type_register_static(&spapr_drc_logical_info);
> +    type_register_static(&spapr_drc_cpu_info);
> +    type_register_static(&spapr_drc_pci_info);
> +    type_register_static(&spapr_drc_lmb_info);
> 
>      spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
>                          rtas_set_indicator);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 7a208a7..50d673b 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1761,8 +1761,7 @@ 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++) {
> -            spapr_dr_connector_new(OBJECT(phb),
> -                                   SPAPR_DR_CONNECTOR_TYPE_PCI,
> +            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
>                                     (sphb->index << 16) | i);
>          }
>      }
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 10e7c24..f77be38 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -26,6 +26,48 @@
>  #define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
>                                               TYPE_SPAPR_DR_CONNECTOR)
> 
> +#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
> +#define SPAPR_DRC_PHSYICALGET_CLASS(obj) \

typo                 ^

> +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHYSICAL)
> +#define SPAPR_DRC_PHYSICAL_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
> +                           TYPE_SPAPR_DRC_PHSYICAL)

typo                                         ^

> +#define SPAPR_DRC_PHYSICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +                                             TYPE_SPAPR_DRC_PHYSICAL)
> +
> +#define TYPE_SPAPR_DRC_LOGICAL "spapr-drc-logical"
> +#define SPAPR_DRC_LOGICAL_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LOGICAL)
> +#define SPAPR_DRC_LOGICAL_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
> +                           TYPE_SPAPR_DRC_PHSYICAL)

*LOGICAL

> +#define SPAPR_DRC_LOGICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +                                             TYPE_SPAPR_DRC_LOGICAL)
> +
> +#define TYPE_SPAPR_DRC_CPU "spapr-drc-cpu"
> +#define SPAPR_DRC_CPU_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_CPU)
> +#define SPAPR_DRC_CPU_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_CPU)
> +#define SPAPR_DRC_CPU(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +                                        TYPE_SPAPR_DRC_CPU)
> +
> +#define TYPE_SPAPR_DRC_PCI "spapr-drc-pci"
> +#define SPAPR_DRC_PCI_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PCI)
> +#define SPAPR_DRC_PCI_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PCI)
> +#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +                                        TYPE_SPAPR_DRC_PCI)
> +
> +#define TYPE_SPAPR_DRC_LMB "spapr-drc-lmb"
> +#define SPAPR_DRC_LMB_GET_CLASS(obj) \
> +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LMB)
> +#define SPAPR_DRC_LMB_CLASS(klass) \
> +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_LMB)
> +#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> +                                        TYPE_SPAPR_DRC_LMB)
> +
>  /*
>   * Various hotplug types managed by sPAPRDRConnector
>   *
> @@ -134,7 +176,6 @@ typedef struct sPAPRDRConnector {
>      /*< private >*/
>      DeviceState parent;
> 
> -    sPAPRDRConnectorType type;
>      uint32_t id;
>      Object *owner;
>      const char *name;
> @@ -163,6 +204,7 @@ typedef struct sPAPRDRConnectorClass {
>      DeviceClass parent;
> 
>      /*< public >*/
> +    sPAPRDRConnectorTypeShift typeshift;
> 
>      /* accessors for guest-visible (generally via RTAS) DR state */
>      uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
> @@ -186,8 +228,7 @@ typedef struct sPAPRDRConnectorClass {
>  uint32_t spapr_drc_index(sPAPRDRConnector *drc);
>  sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
> 
> -sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> -                                         sPAPRDRConnectorType type,
> +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
>                                           uint32_t id);
>  sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
>  sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
> -- 
> 2.9.4
>
David Gibson June 4, 2017, 10:25 a.m. UTC | #2
On Sat, Jun 03, 2017 at 05:05:26PM -0500, Michael Roth wrote:
> Quoting David Gibson (2017-06-02 02:29:48)
> > Currently we only have a single QOM type for all DRCs, but lots of
> > places where we switch behaviour based on the DRC's PAPR defined type.
> > This is a poor use of our existing type system.
> > 
> > So, instead create QOM subclasses for each PAPR defined DRC type.  We
> > also introduce intermediate subclasses for physical and logical DRCs,
> > a division which will be useful later on.
> > 
> > Instead of being stored in the DRC object itself, the PAPR type is now
> > stored in the class structure.  There are still many places where we
> > switch directly on the PAPR type value, but this at least provides the
> > basis to start to remove those.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr.c             |   5 +-
> >  hw/ppc/spapr_drc.c         | 120 +++++++++++++++++++++++++++++++++------------
> >  hw/ppc/spapr_pci.c         |   3 +-
> >  include/hw/ppc/spapr_drc.h |  47 ++++++++++++++++--
> >  4 files changed, 136 insertions(+), 39 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5d10366..456f9e7 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1911,7 +1911,7 @@ static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
> >          uint64_t addr;
> > 
> >          addr = i * lmb_size + spapr->hotplug_memory.base;
> > -        drc = spapr_dr_connector_new(OBJECT(spapr), SPAPR_DR_CONNECTOR_TYPE_LMB,
> > +        drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
> >                                       addr/lmb_size);
> >          qemu_register_reset(spapr_drc_reset, drc);
> >      }
> > @@ -2008,8 +2008,7 @@ static void spapr_init_cpus(sPAPRMachineState *spapr)
> > 
> >          if (mc->has_hotpluggable_cpus) {
> >              sPAPRDRConnector *drc =
> > -                spapr_dr_connector_new(OBJECT(spapr),
> > -                                       SPAPR_DR_CONNECTOR_TYPE_CPU,
> > +                spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
> >                                         (core_id / smp_threads) * smt);
> > 
> >              qemu_register_reset(spapr_drc_reset, drc);
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index a35314e..690b41f 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -70,14 +70,23 @@ static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
> >      return shift;
> >  }
> > 
> > +sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +
> > +    return 1 << drck->typeshift;
> > +}
> > +
> >  uint32_t spapr_drc_index(sPAPRDRConnector *drc)
> >  {
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> > +
> >      /* no set format for a drc index: it only needs to be globally
> >       * unique. this is how we encode the DRC type on bare-metal
> >       * however, so might as well do that here
> >       */
> > -    return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) |
> > -            (drc->id & DRC_INDEX_ID_MASK);
> > +    return (drck->typeshift << DRC_INDEX_TYPE_SHIFT)
> > +        | (drc->id & DRC_INDEX_ID_MASK);
> >  }
> > 
> >  static uint32_t set_isolation_state(sPAPRDRConnector *drc,
> > @@ -107,7 +116,7 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
> >       * If the LMB being removed doesn't belong to a DIMM device that is
> >       * actually being unplugged, fail the isolation request here.
> >       */
> > -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_LMB) {
> > +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
> >          if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
> >               !drc->awaiting_release) {
> >              return RTAS_OUT_HW_ERROR;
> > @@ -177,7 +186,7 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
> >          }
> >      }
> > 
> > -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >          drc->allocation_state = state;
> >          if (drc->awaiting_release &&
> >              drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> > @@ -191,11 +200,6 @@ static uint32_t set_allocation_state(sPAPRDRConnector *drc,
> >      return RTAS_OUT_SUCCESS;
> >  }
> > 
> > -sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
> > -{
> > -    return drc->type;
> > -}
> > -
> >  static const char *get_name(sPAPRDRConnector *drc)
> >  {
> >      return drc->name;
> > @@ -217,7 +221,7 @@ static void set_signalled(sPAPRDRConnector *drc)
> >  static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
> >  {
> >      if (drc->dev) {
> > -        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> > +        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> >              drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> >              /* for logical DR, we return a state of UNUSABLE
> >               * iff the allocation state UNUSABLE.
> > @@ -235,7 +239,7 @@ static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
> >              *state = SPAPR_DR_ENTITY_SENSE_PRESENT;
> >          }
> >      } else {
> > -        if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +        if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >              /* PCI devices, and only PCI devices, use EMPTY
> >               * in cases where we'd otherwise use UNUSABLE
> >               */
> > @@ -368,7 +372,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> >          error_setg(errp, "an attached device is still awaiting release");
> >          return;
> >      }
> > -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >          g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE);
> >      }
> >      g_assert(fdt || coldplug);
> > @@ -380,7 +384,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> >       * may be accessing the device, we can easily crash the guest, so we
> >       * we defer completion of removal in such cases to the reset() hook.
> >       */
> > -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >          drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
> >      }
> >      drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
> > @@ -398,10 +402,10 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
> >       * 'physical' DR resources such as PCI where each device/resource is
> >       * signalled individually.
> >       */
> > -    drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI)
> > +    drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI)
> >                       ? true : coldplug;
> > 
> > -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >          drc->awaiting_allocation = true;
> >      }
> > 
> > @@ -441,7 +445,7 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
> >          return;
> >      }
> > 
> > -    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> > +    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> >          drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
> >          trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
> >          drc->awaiting_release = true;
> > @@ -458,8 +462,8 @@ static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
> > 
> >      drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
> > 
> > -    /* Calling release callbacks based on drc->type. */
> > -    switch (drc->type) {
> > +    /* Calling release callbacks based on spapr_drc_type(drc). */
> > +    switch (spapr_drc_type(drc)) {
> >      case SPAPR_DR_CONNECTOR_TYPE_CPU:
> >          spapr_core_release(drc->dev);
> >          break;
> > @@ -515,7 +519,7 @@ static void reset(DeviceState *d)
> >          }
> > 
> >          /* non-PCI devices may be awaiting a transition to UNUSABLE */
> > -        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> > +        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
> >              drc->awaiting_release) {
> >              drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
> >          }
> > @@ -544,7 +548,7 @@ static bool spapr_drc_needed(void *opaque)
> >       * If there is dev plugged in, we need to migrate the DRC state when
> >       * it is different from cold-plugged state
> >       */
> > -    switch (drc->type) {
> > +    switch (spapr_drc_type(drc)) {
> >      case SPAPR_DR_CONNECTOR_TYPE_PCI:
> >          rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) &&
> >                 (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) &&
> > @@ -630,17 +634,12 @@ static void unrealize(DeviceState *d, Error **errp)
> >      }
> >  }
> > 
> > -sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> > -                                         sPAPRDRConnectorType type,
> > +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
> >                                           uint32_t id)
> >  {
> > -    sPAPRDRConnector *drc =
> > -        SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
> > +    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type));
> >      char *prop_name;
> > 
> > -    g_assert(type);
> > -
> > -    drc->type = type;
> >      drc->id = id;
> >      drc->owner = owner;
> >      prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
> > @@ -670,7 +669,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> >       * 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 (drc->type) {
> > +    switch (spapr_drc_type(drc)) {
> >      case SPAPR_DR_CONNECTOR_TYPE_CPU:
> >          drc->name = g_strdup_printf("CPU %d", id);
> >          break;
> > @@ -689,7 +688,7 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> >      }
> > 
> >      /* PCI slot always start in a USABLE state, and stay there */
> > -    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> > +    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
> >          drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
> >      }
> > 
> > @@ -741,6 +740,27 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
> >      dk->user_creatable = false;
> >  }
> > 
> > +static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> > +
> > +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
> > +}
> > +
> > +static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> > +
> > +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
> > +}
> > +
> > +static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
> > +{
> > +    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
> > +
> > +    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
> > +}
> > +
> >  static const TypeInfo spapr_dr_connector_info = {
> >      .name          = TYPE_SPAPR_DR_CONNECTOR,
> >      .parent        = TYPE_DEVICE,
> > @@ -750,6 +770,39 @@ static const TypeInfo spapr_dr_connector_info = {
> >      .class_init    = spapr_dr_connector_class_init,
> >  };
> > 
> > +static const TypeInfo spapr_drc_physical_info = {
> > +    .name          = TYPE_SPAPR_DRC_PHYSICAL,
> > +    .parent        = TYPE_SPAPR_DR_CONNECTOR,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +};
> > +
> > +static const TypeInfo spapr_drc_logical_info = {
> > +    .name          = TYPE_SPAPR_DRC_LOGICAL,
> > +    .parent        = TYPE_SPAPR_DR_CONNECTOR,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +};
> 
> It doesn't look like there's any intent of being able to instantiate
> a "generic" logical/physical DRC for future types, so I think these
> should be marked .abstract = true.

Ah, good idea.

> > +
> > +static const TypeInfo spapr_drc_cpu_info = {
> > +    .name          = TYPE_SPAPR_DRC_CPU,
> > +    .parent        = TYPE_SPAPR_DRC_LOGICAL,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +    .class_init    = spapr_drc_cpu_class_init,
> > +};
> > +
> > +static const TypeInfo spapr_drc_pci_info = {
> > +    .name          = TYPE_SPAPR_DRC_PCI,
> > +    .parent        = TYPE_SPAPR_DRC_PHYSICAL,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +    .class_init    = spapr_drc_pci_class_init,
> > +};
> > +
> > +static const TypeInfo spapr_drc_lmb_info = {
> > +    .name          = TYPE_SPAPR_DRC_LMB,
> > +    .parent        = TYPE_SPAPR_DRC_LOGICAL,
> > +    .instance_size = sizeof(sPAPRDRConnector),
> > +    .class_init    = spapr_drc_lmb_class_init,
> > +};
> > +
> >  /* helper functions for external users */
> > 
> >  sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
> > @@ -858,7 +911,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
> >              continue;
> >          }
> > 
> > -        if ((drc->type & drc_type_mask) == 0) {
> > +        if ((spapr_drc_type(drc) & drc_type_mask) == 0) {
> >              continue;
> >          }
> > 
> > @@ -878,7 +931,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
> > 
> >          /* ibm,drc-types */
> >          drc_types = g_string_append(drc_types,
> > -                                    spapr_drc_get_type_str(drc->type));
> > +                                    spapr_drc_get_type_str(spapr_drc_type(drc)));
> >          drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
> >      }
> > 
> > @@ -1210,6 +1263,11 @@ out:
> >  static void spapr_drc_register_types(void)
> >  {
> >      type_register_static(&spapr_dr_connector_info);
> > +    type_register_static(&spapr_drc_physical_info);
> > +    type_register_static(&spapr_drc_logical_info);
> > +    type_register_static(&spapr_drc_cpu_info);
> > +    type_register_static(&spapr_drc_pci_info);
> > +    type_register_static(&spapr_drc_lmb_info);
> > 
> >      spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
> >                          rtas_set_indicator);
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 7a208a7..50d673b 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1761,8 +1761,7 @@ 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++) {
> > -            spapr_dr_connector_new(OBJECT(phb),
> > -                                   SPAPR_DR_CONNECTOR_TYPE_PCI,
> > +            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
> >                                     (sphb->index << 16) | i);
> >          }
> >      }
> > diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> > index 10e7c24..f77be38 100644
> > --- a/include/hw/ppc/spapr_drc.h
> > +++ b/include/hw/ppc/spapr_drc.h
> > @@ -26,6 +26,48 @@
> >  #define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> >                                               TYPE_SPAPR_DR_CONNECTOR)
> > 
> > +#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
> > +#define SPAPR_DRC_PHSYICALGET_CLASS(obj) \
> 
> typo                 ^
> 
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHYSICAL)
> > +#define SPAPR_DRC_PHYSICAL_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
> > +                           TYPE_SPAPR_DRC_PHSYICAL)
> 
> typo                                         ^
> 
> > +#define SPAPR_DRC_PHYSICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                             TYPE_SPAPR_DRC_PHYSICAL)
> > +
> > +#define TYPE_SPAPR_DRC_LOGICAL "spapr-drc-logical"
> > +#define SPAPR_DRC_LOGICAL_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LOGICAL)
> > +#define SPAPR_DRC_LOGICAL_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
> > +                           TYPE_SPAPR_DRC_PHSYICAL)
> 
> *LOGICAL

Thank you, corrected.

> > +#define SPAPR_DRC_LOGICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                             TYPE_SPAPR_DRC_LOGICAL)
> > +
> > +#define TYPE_SPAPR_DRC_CPU "spapr-drc-cpu"
> > +#define SPAPR_DRC_CPU_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_CPU)
> > +#define SPAPR_DRC_CPU_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_CPU)
> > +#define SPAPR_DRC_CPU(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                        TYPE_SPAPR_DRC_CPU)
> > +
> > +#define TYPE_SPAPR_DRC_PCI "spapr-drc-pci"
> > +#define SPAPR_DRC_PCI_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PCI)
> > +#define SPAPR_DRC_PCI_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PCI)
> > +#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                        TYPE_SPAPR_DRC_PCI)
> > +
> > +#define TYPE_SPAPR_DRC_LMB "spapr-drc-lmb"
> > +#define SPAPR_DRC_LMB_GET_CLASS(obj) \
> > +        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LMB)
> > +#define SPAPR_DRC_LMB_CLASS(klass) \
> > +        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_LMB)
> > +#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
> > +                                        TYPE_SPAPR_DRC_LMB)
> > +
> >  /*
> >   * Various hotplug types managed by sPAPRDRConnector
> >   *
> > @@ -134,7 +176,6 @@ typedef struct sPAPRDRConnector {
> >      /*< private >*/
> >      DeviceState parent;
> > 
> > -    sPAPRDRConnectorType type;
> >      uint32_t id;
> >      Object *owner;
> >      const char *name;
> > @@ -163,6 +204,7 @@ typedef struct sPAPRDRConnectorClass {
> >      DeviceClass parent;
> > 
> >      /*< public >*/
> > +    sPAPRDRConnectorTypeShift typeshift;
> > 
> >      /* accessors for guest-visible (generally via RTAS) DR state */
> >      uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
> > @@ -186,8 +228,7 @@ typedef struct sPAPRDRConnectorClass {
> >  uint32_t spapr_drc_index(sPAPRDRConnector *drc);
> >  sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
> > 
> > -sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
> > -                                         sPAPRDRConnectorType type,
> > +sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
> >                                           uint32_t id);
> >  sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
> >  sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,
>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5d10366..456f9e7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1911,7 +1911,7 @@  static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
         uint64_t addr;
 
         addr = i * lmb_size + spapr->hotplug_memory.base;
-        drc = spapr_dr_connector_new(OBJECT(spapr), SPAPR_DR_CONNECTOR_TYPE_LMB,
+        drc = spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_LMB,
                                      addr/lmb_size);
         qemu_register_reset(spapr_drc_reset, drc);
     }
@@ -2008,8 +2008,7 @@  static void spapr_init_cpus(sPAPRMachineState *spapr)
 
         if (mc->has_hotpluggable_cpus) {
             sPAPRDRConnector *drc =
-                spapr_dr_connector_new(OBJECT(spapr),
-                                       SPAPR_DR_CONNECTOR_TYPE_CPU,
+                spapr_dr_connector_new(OBJECT(spapr), TYPE_SPAPR_DRC_CPU,
                                        (core_id / smp_threads) * smt);
 
             qemu_register_reset(spapr_drc_reset, drc);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index a35314e..690b41f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -70,14 +70,23 @@  static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
     return shift;
 }
 
+sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
+    return 1 << drck->typeshift;
+}
+
 uint32_t spapr_drc_index(sPAPRDRConnector *drc)
 {
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+
     /* no set format for a drc index: it only needs to be globally
      * unique. this is how we encode the DRC type on bare-metal
      * however, so might as well do that here
      */
-    return (get_type_shift(drc->type) << DRC_INDEX_TYPE_SHIFT) |
-            (drc->id & DRC_INDEX_ID_MASK);
+    return (drck->typeshift << DRC_INDEX_TYPE_SHIFT)
+        | (drc->id & DRC_INDEX_ID_MASK);
 }
 
 static uint32_t set_isolation_state(sPAPRDRConnector *drc,
@@ -107,7 +116,7 @@  static uint32_t set_isolation_state(sPAPRDRConnector *drc,
      * If the LMB being removed doesn't belong to a DIMM device that is
      * actually being unplugged, fail the isolation request here.
      */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_LMB) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
         if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
              !drc->awaiting_release) {
             return RTAS_OUT_HW_ERROR;
@@ -177,7 +186,7 @@  static uint32_t set_allocation_state(sPAPRDRConnector *drc,
         }
     }
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->allocation_state = state;
         if (drc->awaiting_release &&
             drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
@@ -191,11 +200,6 @@  static uint32_t set_allocation_state(sPAPRDRConnector *drc,
     return RTAS_OUT_SUCCESS;
 }
 
-sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc)
-{
-    return drc->type;
-}
-
 static const char *get_name(sPAPRDRConnector *drc)
 {
     return drc->name;
@@ -217,7 +221,7 @@  static void set_signalled(sPAPRDRConnector *drc)
 static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
 {
     if (drc->dev) {
-        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
             drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
             /* for logical DR, we return a state of UNUSABLE
              * iff the allocation state UNUSABLE.
@@ -235,7 +239,7 @@  static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
             *state = SPAPR_DR_ENTITY_SENSE_PRESENT;
         }
     } else {
-        if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+        if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
             /* PCI devices, and only PCI devices, use EMPTY
              * in cases where we'd otherwise use UNUSABLE
              */
@@ -368,7 +372,7 @@  static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
         error_setg(errp, "an attached device is still awaiting release");
         return;
     }
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         g_assert(drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE);
     }
     g_assert(fdt || coldplug);
@@ -380,7 +384,7 @@  static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
      * may be accessing the device, we can easily crash the guest, so we
      * we defer completion of removal in such cases to the reset() hook.
      */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
     }
     drc->indicator_state = SPAPR_DR_INDICATOR_STATE_ACTIVE;
@@ -398,10 +402,10 @@  static void attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt,
      * 'physical' DR resources such as PCI where each device/resource is
      * signalled individually.
      */
-    drc->signalled = (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI)
+    drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI)
                      ? true : coldplug;
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->awaiting_allocation = true;
     }
 
@@ -441,7 +445,7 @@  static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
         return;
     }
 
-    if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+    if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
         drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
         trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
         drc->awaiting_release = true;
@@ -458,8 +462,8 @@  static void detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
 
     drc->indicator_state = SPAPR_DR_INDICATOR_STATE_INACTIVE;
 
-    /* Calling release callbacks based on drc->type. */
-    switch (drc->type) {
+    /* Calling release callbacks based on spapr_drc_type(drc). */
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
         spapr_core_release(drc->dev);
         break;
@@ -515,7 +519,7 @@  static void reset(DeviceState *d)
         }
 
         /* non-PCI devices may be awaiting a transition to UNUSABLE */
-        if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+        if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
             drc->awaiting_release) {
             drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
         }
@@ -544,7 +548,7 @@  static bool spapr_drc_needed(void *opaque)
      * If there is dev plugged in, we need to migrate the DRC state when
      * it is different from cold-plugged state
      */
-    switch (drc->type) {
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_PCI:
         rc = !((drc->isolation_state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) &&
                (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) &&
@@ -630,17 +634,12 @@  static void unrealize(DeviceState *d, Error **errp)
     }
 }
 
-sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
-                                         sPAPRDRConnectorType type,
+sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
                                          uint32_t id)
 {
-    sPAPRDRConnector *drc =
-        SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
+    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(object_new(type));
     char *prop_name;
 
-    g_assert(type);
-
-    drc->type = type;
     drc->id = id;
     drc->owner = owner;
     prop_name = g_strdup_printf("dr-connector[%"PRIu32"]",
@@ -670,7 +669,7 @@  sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
      * 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 (drc->type) {
+    switch (spapr_drc_type(drc)) {
     case SPAPR_DR_CONNECTOR_TYPE_CPU:
         drc->name = g_strdup_printf("CPU %d", id);
         break;
@@ -689,7 +688,7 @@  sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
     }
 
     /* PCI slot always start in a USABLE state, and stay there */
-    if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
+    if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
         drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
     }
 
@@ -741,6 +740,27 @@  static void spapr_dr_connector_class_init(ObjectClass *k, void *data)
     dk->user_creatable = false;
 }
 
+static void spapr_drc_cpu_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_CPU;
+}
+
+static void spapr_drc_pci_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_PCI;
+}
+
+static void spapr_drc_lmb_class_init(ObjectClass *k, void *data)
+{
+    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_CLASS(k);
+
+    drck->typeshift = SPAPR_DR_CONNECTOR_TYPE_SHIFT_LMB;
+}
+
 static const TypeInfo spapr_dr_connector_info = {
     .name          = TYPE_SPAPR_DR_CONNECTOR,
     .parent        = TYPE_DEVICE,
@@ -750,6 +770,39 @@  static const TypeInfo spapr_dr_connector_info = {
     .class_init    = spapr_dr_connector_class_init,
 };
 
+static const TypeInfo spapr_drc_physical_info = {
+    .name          = TYPE_SPAPR_DRC_PHYSICAL,
+    .parent        = TYPE_SPAPR_DR_CONNECTOR,
+    .instance_size = sizeof(sPAPRDRConnector),
+};
+
+static const TypeInfo spapr_drc_logical_info = {
+    .name          = TYPE_SPAPR_DRC_LOGICAL,
+    .parent        = TYPE_SPAPR_DR_CONNECTOR,
+    .instance_size = sizeof(sPAPRDRConnector),
+};
+
+static const TypeInfo spapr_drc_cpu_info = {
+    .name          = TYPE_SPAPR_DRC_CPU,
+    .parent        = TYPE_SPAPR_DRC_LOGICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_cpu_class_init,
+};
+
+static const TypeInfo spapr_drc_pci_info = {
+    .name          = TYPE_SPAPR_DRC_PCI,
+    .parent        = TYPE_SPAPR_DRC_PHYSICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_pci_class_init,
+};
+
+static const TypeInfo spapr_drc_lmb_info = {
+    .name          = TYPE_SPAPR_DRC_LMB,
+    .parent        = TYPE_SPAPR_DRC_LOGICAL,
+    .instance_size = sizeof(sPAPRDRConnector),
+    .class_init    = spapr_drc_lmb_class_init,
+};
+
 /* helper functions for external users */
 
 sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index)
@@ -858,7 +911,7 @@  int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
             continue;
         }
 
-        if ((drc->type & drc_type_mask) == 0) {
+        if ((spapr_drc_type(drc) & drc_type_mask) == 0) {
             continue;
         }
 
@@ -878,7 +931,7 @@  int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
 
         /* ibm,drc-types */
         drc_types = g_string_append(drc_types,
-                                    spapr_drc_get_type_str(drc->type));
+                                    spapr_drc_get_type_str(spapr_drc_type(drc)));
         drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
     }
 
@@ -1210,6 +1263,11 @@  out:
 static void spapr_drc_register_types(void)
 {
     type_register_static(&spapr_dr_connector_info);
+    type_register_static(&spapr_drc_physical_info);
+    type_register_static(&spapr_drc_logical_info);
+    type_register_static(&spapr_drc_cpu_info);
+    type_register_static(&spapr_drc_pci_info);
+    type_register_static(&spapr_drc_lmb_info);
 
     spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
                         rtas_set_indicator);
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 7a208a7..50d673b 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1761,8 +1761,7 @@  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++) {
-            spapr_dr_connector_new(OBJECT(phb),
-                                   SPAPR_DR_CONNECTOR_TYPE_PCI,
+            spapr_dr_connector_new(OBJECT(phb), TYPE_SPAPR_DRC_PCI,
                                    (sphb->index << 16) | i);
         }
     }
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 10e7c24..f77be38 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -26,6 +26,48 @@ 
 #define SPAPR_DR_CONNECTOR(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
                                              TYPE_SPAPR_DR_CONNECTOR)
 
+#define TYPE_SPAPR_DRC_PHYSICAL "spapr-drc-physical"
+#define SPAPR_DRC_PHSYICALGET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PHYSICAL)
+#define SPAPR_DRC_PHYSICAL_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
+                           TYPE_SPAPR_DRC_PHSYICAL)
+#define SPAPR_DRC_PHYSICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                             TYPE_SPAPR_DRC_PHYSICAL)
+
+#define TYPE_SPAPR_DRC_LOGICAL "spapr-drc-logical"
+#define SPAPR_DRC_LOGICAL_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LOGICAL)
+#define SPAPR_DRC_LOGICAL_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, \
+                           TYPE_SPAPR_DRC_PHSYICAL)
+#define SPAPR_DRC_LOGICAL(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                             TYPE_SPAPR_DRC_LOGICAL)
+
+#define TYPE_SPAPR_DRC_CPU "spapr-drc-cpu"
+#define SPAPR_DRC_CPU_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_CPU)
+#define SPAPR_DRC_CPU_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_CPU)
+#define SPAPR_DRC_CPU(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_CPU)
+
+#define TYPE_SPAPR_DRC_PCI "spapr-drc-pci"
+#define SPAPR_DRC_PCI_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_PCI)
+#define SPAPR_DRC_PCI_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_PCI)
+#define SPAPR_DRC_PCI(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_PCI)
+
+#define TYPE_SPAPR_DRC_LMB "spapr-drc-lmb"
+#define SPAPR_DRC_LMB_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(sPAPRDRConnectorClass, obj, TYPE_SPAPR_DRC_LMB)
+#define SPAPR_DRC_LMB_CLASS(klass) \
+        OBJECT_CLASS_CHECK(sPAPRDRConnectorClass, klass, TYPE_SPAPR_DRC_LMB)
+#define SPAPR_DRC_LMB(obj) OBJECT_CHECK(sPAPRDRConnector, (obj), \
+                                        TYPE_SPAPR_DRC_LMB)
+
 /*
  * Various hotplug types managed by sPAPRDRConnector
  *
@@ -134,7 +176,6 @@  typedef struct sPAPRDRConnector {
     /*< private >*/
     DeviceState parent;
 
-    sPAPRDRConnectorType type;
     uint32_t id;
     Object *owner;
     const char *name;
@@ -163,6 +204,7 @@  typedef struct sPAPRDRConnectorClass {
     DeviceClass parent;
 
     /*< public >*/
+    sPAPRDRConnectorTypeShift typeshift;
 
     /* accessors for guest-visible (generally via RTAS) DR state */
     uint32_t (*set_isolation_state)(sPAPRDRConnector *drc,
@@ -186,8 +228,7 @@  typedef struct sPAPRDRConnectorClass {
 uint32_t spapr_drc_index(sPAPRDRConnector *drc);
 sPAPRDRConnectorType spapr_drc_type(sPAPRDRConnector *drc);
 
-sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
-                                         sPAPRDRConnectorType type,
+sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
                                          uint32_t id);
 sPAPRDRConnector *spapr_dr_connector_by_index(uint32_t index);
 sPAPRDRConnector *spapr_dr_connector_by_id(sPAPRDRConnectorType type,