Quoting David Gibson (2017-06-06 03:32:19)
> In theory the RTAS set-indicator call can be used for a number of
> "indicators" defined by PAPR. In practice the only ones we're ever likely
> to implement are those used for Dynamic Reconfiguration (i.e. hotplug).
> Because of this, the current implementation determines the associated DRC
> object, before dispatching based on the type of indicator.
>
> However, this means we also need a check that we're dealing with a DR
> related indicator at all, which duplicates some of the logic from the
> switch further down.
>
> Even though it means a bit of code duplication, things work out cleaner if
> we delegate the DRC lookup to the individual indicator type functions -
> and it also allows some further cleanups.
>
> While we're there, remove references to "sensor", a copy/paste artefact
> from the related, but distinct "get-sensor" call.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr_drc.c | 84
> ++++++++++++++++++++++++++++-------------------------
> hw/ppc/trace-events | 2 --
> 2 files changed, 44 insertions(+), 42 deletions(-)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 1fc51c9..6c2fa93 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -869,74 +869,78 @@ out:
> * RTAS calls
> */
>
> -static bool sensor_type_is_dr(uint32_t sensor_type)
> +static uint32_t rtas_set_isolation_state(uint32_t idx, uint32_t state)
> {
> - switch (sensor_type) {
> - case RTAS_SENSOR_TYPE_ISOLATION_STATE:
> - case RTAS_SENSOR_TYPE_DR:
> - case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
> - return true;
> + sPAPRDRConnector *drc = spapr_drc_by_index(idx);
> + sPAPRDRConnectorClass *drck;
> +
> + if (!drc) {
> + return RTAS_OUT_PARAM_ERROR;
> }
>
> - return false;
> + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> + return drck->set_isolation_state(drc, state);
> }
>
> -static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> - uint32_t token, uint32_t nargs,
> - target_ulong args, uint32_t nret,
> - target_ulong rets)
> +static uint32_t rtas_set_allocation_state(uint32_t idx, uint32_t state)
> {
> - uint32_t sensor_type;
> - uint32_t sensor_index;
> - uint32_t sensor_state;
> - uint32_t ret = RTAS_OUT_SUCCESS;
> - sPAPRDRConnector *drc;
> + sPAPRDRConnector *drc = spapr_drc_by_index(idx);
> sPAPRDRConnectorClass *drck;
>
> - if (nargs != 3 || nret != 1) {
> - ret = RTAS_OUT_PARAM_ERROR;
> - goto out;
> + if (!drc) {
> + return RTAS_OUT_PARAM_ERROR;
> }
>
> - sensor_type = rtas_ld(args, 0);
> - sensor_index = rtas_ld(args, 1);
> - sensor_state = rtas_ld(args, 2);
> + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> + return drck->set_allocation_state(drc, state);
> +}
>
> - if (!sensor_type_is_dr(sensor_type)) {
> - goto out_unimplemented;
> - }
> +static uint32_t rtas_set_indicator_state(uint32_t idx, uint32_t state)
> +{
> + sPAPRDRConnector *drc = spapr_drc_by_index(idx);
> + sPAPRDRConnectorClass *drck;
>
> - /* if this is a DR sensor we can assume sensor_index == drc_index */
> - drc = spapr_drc_by_index(sensor_index);
> if (!drc) {
> - trace_spapr_rtas_set_indicator_invalid(sensor_index);
> + return RTAS_OUT_PARAM_ERROR;
> + }
> +
> + drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> + return drck->set_indicator_state(drc, state);
> +}
> +
> +static void rtas_set_indicator(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> + uint32_t token,
> + uint32_t nargs, target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> + uint32_t type, idx, state;
> + uint32_t ret = RTAS_OUT_SUCCESS;
> +
> + if (nargs != 3 || nret != 1) {
> ret = RTAS_OUT_PARAM_ERROR;
> goto out;
> }
> - drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>
> - switch (sensor_type) {
> + type = rtas_ld(args, 0);
> + idx = rtas_ld(args, 1);
> + state = rtas_ld(args, 2);
> +
> + switch (type) {
> case RTAS_SENSOR_TYPE_ISOLATION_STATE:
> - ret = drck->set_isolation_state(drc, sensor_state);
> + ret = rtas_set_isolation_state(idx, state);
> break;
> case RTAS_SENSOR_TYPE_DR:
> - ret = drck->set_indicator_state(drc, sensor_state);
> + ret = rtas_set_indicator_state(idx, state);
> break;
> case RTAS_SENSOR_TYPE_ALLOCATION_STATE:
> - ret = drck->set_allocation_state(drc, sensor_state);
> + ret = rtas_set_allocation_state(idx, state);
> break;
> default:
> - goto out_unimplemented;
> + ret = RTAS_OUT_NOT_SUPPORTED;
> }
>
> out:
> rtas_st(rets, 0, ret);
> - return;
> -
> -out_unimplemented:
> - /* currently only DR-related sensors are implemented */
> - trace_spapr_rtas_set_indicator_not_supported(sensor_index, sensor_type);
> - rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> }
>
> static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 4979397..581fa85 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -60,8 +60,6 @@ spapr_ovec_parse_vector(int vector, int byte, uint16_t
> vec_len, uint8_t entry) "
> spapr_ovec_populate_dt(int byte, uint16_t vec_len, uint8_t entry) "encoding
> guest vector byte %3d / %3d: 0x%.2x"
>
> # hw/ppc/spapr_rtas.c
> -spapr_rtas_set_indicator_invalid(uint32_t index) "sensor index: 0x%"PRIx32
> -spapr_rtas_set_indicator_not_supported(uint32_t index, uint32_t type)
> "sensor index: 0x%"PRIx32", type: %"PRIu32
> spapr_rtas_get_sensor_state_not_supported(uint32_t index, uint32_t type)
> "sensor index: 0x%"PRIx32", type: %"PRIu32
> spapr_rtas_get_sensor_state_invalid(uint32_t index) "sensor index: 0x%"PRIx32
> spapr_rtas_ibm_configure_connector_invalid(uint32_t index) "DRC index:
> 0x%"PRIx32
> --
> 2.9.4
>
