On 01/27/2015 06:42 PM, Michael Roth wrote:
> Quoting Tyrel Datwyler (2015-01-27 16:05:52)
>> On 01/27/2015 01:36 PM, Michael Roth wrote:
>>> Quoting David Gibson (2015-01-26 23:24:11)
>>>> On Sun, Jan 25, 2015 at 11:21:26PM -0600, Michael Roth wrote:
>>>>> Quoting David Gibson (2015-01-16 00:21:55)
>>>>>> On Tue, Dec 23, 2014 at 06:30:17AM -0600, Michael Roth wrote:
>>>>>>> From: Nathan Fontenot <nf...@linux.vnet.ibm.com>
>>>>>>>
>>>>>>> Signed-off-by: Nathan Fontenot <nf...@linux.vnet.ibm.com>
>>>>>>> Signed-off-by: Michael Roth <mdr...@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.
>>>>>
>>>>> In this particular case the calls act on a "power domain" which isn't
>>>>> actually modeled in QEMU (we just assume a single "live-insertion" domain
>>>>> which just magically does everything we want), so I think it makes
>>>>> sense to leave these here.
>>>>
>>>> Yeah, fair enough.
>>>
>>> Hmm, looking at it again, set-indicator and get-sensor-state aren't actually
>>> specific to DR, but might be extended to handle a number of other types of
>>> sensors in the future ("Reset Component", "Error Log", and "Global Interrupt
>>> Queue Control" may be interesting in this regard).
>>>
>>> So it looks like only configure-connector is specifically for DR. Still
>>> planning on moving it to spapr_drc_rtas.c, unless you'd prefer not to
>>> at this point (it'll be lonely for the foreseable future).
>>
>> I will point out that configure-connector would be used for
>> hibernation/migration if we ever implement it as it is defined by PAPR.
>> I know for migration there had been some talk in the past about someday
>> doing it the right way.
>
> Interesting. Do you know if calls are still tied to DRCs in this case, or
> is is there something more general like fetching the entire device-tree? I
> didn't see anything in PAPR+ 2.7 outside of the DR use-case where it acts
> on a particular DRC.
>
It does act on a specific DRC. When doing device tree update for
migration/hibernation
we first call rtas_update_nodes which passes back list containing among other
things
a drc index for each new node that needs to be added to the device tree.
-Nathan
>>
>> -Tyrel
>>
>>>
>>>>
>>>>> But for the others it does make sense to tie them with spapr_drc.c, or
>>>>> maybe spapr_drc_rtas.c to maintain the encapsulation of DRC state behind
>>>>> well-defined accessors.
>>>>
>>>> Ok.
>>>>
>>>> --
>>>> 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
