Re: [Qemu-devel] [PATCH v4 05/17] spapr_rtas: add get-sensor-state RTAS interface

[David Gibson]

On Tue, Dec 23, 2014 at 06:30:19AM -0600, Michael Roth wrote:
> From: Mike Day <address@hidden>

Even simple patches should have commit messages.

> Signed-off-by: Mike Day <address@hidden>
> Signed-off-by: Michael Roth <address@hidden>
> ---
>  hw/ppc/spapr_rtas.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 6aa325f..13e6e55 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -389,6 +389,39 @@ out_unimplemented:
>      rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>  }
>  
> +static void rtas_get_sensor_state(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> +                                  uint32_t token, uint32_t nargs,
> +                                  target_ulong args, uint32_t nret,
> +                                  target_ulong rets)
> +{
> +    uint32_t sensor_type = rtas_ld(args, 0);
> +    uint32_t sensor_index = rtas_ld(args, 1);

Need to validate nargs and nret first.

> +    sPAPRDRConnector *drc;
> +    sPAPRDRConnectorClass *drck;
> +    uint32_t entity_sense;
> +
> +    if (sensor_type != RTAS_SENSOR_TYPE_ENTITY_SENSE) {
> +        /* currently only DR-related sensors are implemented */
> +        DPRINTF("rtas_get_sensor_state: sensor/indicator not implemented: 
> %d\n",
> +                sensor_type);
> +        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);

I think your previous patch used RTAS_OUT_PARAM_ERROR instead of
RTAS_OUT_NOT_SUPORTED in the case of an unsupported indicator type.  I
imagine these should be consistent.

> +        return;
> +    }
> +
> +    drc = spapr_dr_connector_by_index(sensor_index);
> +    if (!drc) {
> +        DPRINTF("rtas_get_sensor_state: invalid sensor/DRC index: %xh\n",
> +                sensor_index);
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
> +    drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> +    entity_sense = drck->entity_sense(drc);
> +
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +    rtas_st(rets, 1, entity_sense);
> +}
> +
>  static struct rtas_call {
>      const char *name;
>      spapr_rtas_fn fn;
> @@ -524,6 +557,8 @@ static void core_rtas_register_types(void)
>                          rtas_get_power_level);
>      spapr_rtas_register(RTAS_SET_INDICATOR, "set-indicator",
>                          rtas_set_indicator);
> +    spapr_rtas_register(RTAS_GET_SENSOR_STATE, "get-sensor-state",
> +                        rtas_get_sensor_state);
>  }
>  
>  type_init(core_rtas_register_types)

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

pgpt5YKi3y6_b.pgp
Description: PGP signature