On Thu, 8 Jun 2017 15:09:29 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
> 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>
> ---
Reviewed-by: Greg Kurz <gr...@kaod.org>
> 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);
pgpl1UuAqtiWE.pgp
Description: OpenPGP digital signature
