Re: [Qemu-devel] [PATCH v4 03/17] spapr_rtas: add get/set-power-level RTAS interfaces

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2722 bytes --]

On Tue, Dec 23, 2014 at 06:30:17AM -0600, Michael Roth wrote:
> From: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> 
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  hw/ppc/spapr_rtas.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 2ec2a8e..a2fb533 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -290,6 +290,27 @@ static void rtas_ibm_os_term(PowerPCCPU *cpu,
>      rtas_st(rets, 0, ret);
>  }
>  
> +static void rtas_set_power_level(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> +                                 uint32_t token, uint32_t nargs,
> +                                 target_ulong args, uint32_t nret,
> +                                 target_ulong rets)
> +{
> +    /* we currently only use a single, "live insert" powerdomain for
> +     * hotplugged/dlpar'd resources, so the power is always live/full (100)
> +     */

Even so, you should at least validate the number of args and rets, and
preferably check that the user isn't attempt to set something for some
other, non-existent power domain.

> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +    rtas_st(rets, 1, 100);
> +}
> +
> +static void rtas_get_power_level(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> +                                  uint32_t token, uint32_t nargs,
> +                                  target_ulong args, uint32_t nret,
> +                                  target_ulong rets)
> +{
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +    rtas_st(rets, 1, 100);
> +}
> +
>  static struct rtas_call {
>      const char *name;
>      spapr_rtas_fn fn;
> @@ -419,6 +440,10 @@ static void core_rtas_register_types(void)
>                          rtas_ibm_set_system_parameter);
>      spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term",
>                          rtas_ibm_os_term);
> +    spapr_rtas_register(RTAS_SET_POWER_LEVEL, "set-power-level",
> +                        rtas_set_power_level);
> +    spapr_rtas_register(RTAS_GET_POWER_LEVEL, "get-power-level",
> +                        rtas_get_power_level);
>  }
>  
>  type_init(core_rtas_register_types)

This code should probably go in spapr_drc.c.  The idea that spapr_rtas
was just the RTAS dispatch code, and RTAS functions that had no other
home.  Generally RTAS functions should live with the devices they're
connected to.

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]