Re: [Qemu-devel] [PATCH v2 05/14] spapr_pci: add get/set-power-level RTAS interfaces

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Alexey Kardashevskiy (2013-12-15 21:09:09)
> On 12/06/2013 09:32 AM, 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_pci.c |   22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 1046ec8..8df44a3 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -488,6 +488,26 @@ static void rtas_set_indicator(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> >      rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >  }
> >  
> > +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)
> > +{
> > +    uint32_t power_lvl = rtas_ld(args, 1);
> > +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > +    rtas_st(rets, 1, power_lvl);
> > +}
> > +
> > +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)
> > +{
> > +    /* return SUCCESS with a power level of 100 */
> > +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > +    rtas_st(rets, 1, 100);
> > +}
> > +
> 
> The PAPR spec says that rtas_set_power_level() returns "Actual_level The
> power level actually set" but rtas_get_power_level() always returns 100
> (full power).
> 
> Is this inconsistency here for a reason?

We advertise all PHB power-domains as being a "live insert connector", -1,
which is considered invalid to call rtas_set_power_level with. So I think
it should work either way. Will remove the inconsistency and re-test to
confirm.

> 
> 
> >  static int pci_spapr_swizzle(int slot, int pin)
> >  {
> >      return (slot + pin) % PCI_NUM_PINS;
> > @@ -1051,6 +1071,8 @@ void spapr_pci_rtas_init(void)
> >          spapr_rtas_register("ibm,change-msi", rtas_ibm_change_msi);
> >      }
> >      spapr_rtas_register("set-indicator", rtas_set_indicator);
> > +    spapr_rtas_register("set-power-level", rtas_set_power_level);
> > +    spapr_rtas_register("get-power-level", rtas_get_power_level);
> >  }
> >  
> >  static void spapr_pci_register_types(void)
> > 
> 
> 
> -- 
> Alexey