The allocation-state indicator should only actually be implemented for "logical" DRCs, not physical ones. Factor a check for this, and also for valid indicator state values into rtas_set_allocation_state(). Because they don't exist for physical DRCs, there's no reason that we'd ever want more than one method implementation, so it can just be a plain function.
In addition, the setting to USABLE and setting to UNUSABLE paths in set_allocation_state() don't actually have much in common. So, split the method separate functions for each parameter value (drc_set_usable() and drc_set_unusable()). Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> --- hw/ppc/spapr_drc.c | 85 +++++++++++++++++++++++++--------------------- include/hw/ppc/spapr_drc.h | 2 -- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 7e17f34..9e01d7b 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -114,44 +114,43 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc, return RTAS_OUT_SUCCESS; } -static uint32_t set_allocation_state(sPAPRDRConnector *drc, - sPAPRDRAllocationState state) +static uint32_t drc_set_usable(sPAPRDRConnector *drc) { - trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state); - - if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) { - /* if there's no resource/device associated with the DRC, there's - * no way for us to put it in an allocation state consistent with - * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should - * result in an RTAS return code of -3 / "no such indicator" + /* if there's no resource/device associated with the DRC, there's + * no way for us to put it in an allocation state consistent with + * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should + * result in an RTAS return code of -3 / "no such indicator" + */ + if (!drc->dev) { + return RTAS_OUT_NO_SUCH_INDICATOR; + } + if (drc->awaiting_release && drc->awaiting_allocation) { + /* kernel is acknowledging a previous hotplug event + * while we are already removing it. + * it's safe to ignore awaiting_allocation here since we know the + * situation is predicated on the guest either already having done + * so (boot-time hotplug), or never being able to acquire in the + * first place (hotplug followed by immediate unplug). */ - if (!drc->dev) { - return RTAS_OUT_NO_SUCH_INDICATOR; - } - if (drc->awaiting_release && drc->awaiting_allocation) { - /* kernel is acknowledging a previous hotplug event - * while we are already removing it. - * it's safe to ignore awaiting_allocation here since we know the - * situation is predicated on the guest either already having done - * so (boot-time hotplug), or never being able to acquire in the - * first place (hotplug followed by immediate unplug). - */ - drc->awaiting_allocation_skippable = true; - return RTAS_OUT_NO_SUCH_INDICATOR; - } + drc->awaiting_allocation_skippable = true; + return RTAS_OUT_NO_SUCH_INDICATOR; } - 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) { - uint32_t drc_index = spapr_drc_index(drc); - trace_spapr_drc_set_allocation_state_finalizing(drc_index); - spapr_drc_detach(drc, DEVICE(drc->dev), NULL); - } else if (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) { - drc->awaiting_allocation = false; - } + drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE; + drc->awaiting_allocation = false; + + return RTAS_OUT_SUCCESS; +} + +static uint32_t drc_set_unusable(sPAPRDRConnector *drc) +{ + drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_UNUSABLE; + if (drc->awaiting_release) { + uint32_t drc_index = spapr_drc_index(drc); + trace_spapr_drc_set_allocation_state_finalizing(drc_index); + spapr_drc_detach(drc, DEVICE(drc->dev), NULL); } + return RTAS_OUT_SUCCESS; } @@ -553,7 +552,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) dk->realize = realize; dk->unrealize = unrealize; drck->set_isolation_state = set_isolation_state; - drck->set_allocation_state = set_allocation_state; drck->release_pending = release_pending; /* * Reason: it crashes FIXME find and document the real reason @@ -823,14 +821,23 @@ static uint32_t rtas_set_isolation_state(uint32_t idx, uint32_t state) static uint32_t rtas_set_allocation_state(uint32_t idx, uint32_t state) { sPAPRDRConnector *drc = spapr_drc_by_index(idx); - sPAPRDRConnectorClass *drck; - if (!drc) { - return RTAS_OUT_PARAM_ERROR; + if (!drc || !object_dynamic_cast(OBJECT(drc), TYPE_SPAPR_DRC_LOGICAL)) { + return RTAS_OUT_NO_SUCH_INDICATOR; } - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - return drck->set_allocation_state(drc, state); + trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state); + + switch (state) { + case SPAPR_DR_ALLOCATION_STATE_USABLE: + return drc_set_usable(drc); + + case SPAPR_DR_ALLOCATION_STATE_UNUSABLE: + return drc_set_unusable(drc); + + default: + return RTAS_OUT_PARAM_ERROR; + } } static uint32_t rtas_set_dr_indicator(uint32_t idx, uint32_t state) diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index f24188d..0e09afb 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -220,8 +220,6 @@ typedef struct sPAPRDRConnectorClass { /* accessors for guest-visible (generally via RTAS) DR state */ uint32_t (*set_isolation_state)(sPAPRDRConnector *drc, sPAPRDRIsolationState state); - uint32_t (*set_allocation_state)(sPAPRDRConnector *drc, - sPAPRDRAllocationState state); /* QEMU interfaces for managing hotplug operations */ bool (*release_pending)(sPAPRDRConnector *drc); -- 2.9.4