Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 04/10] hw/ppc: Replace global smp variables with machine smp properties

[Like Xu <like.xu@linux.intel.com>]

On 2019/6/6 16:20, Greg Kurz wrote:
> On Thu, 6 Jun 2019 13:07:32 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> On Wed, Jun 05, 2019 at 11:54:56PM -0300, Eduardo Habkost wrote:
>>> On Wed, Jun 05, 2019 at 11:52:41PM -0300, Eduardo Habkost wrote:
>>>> On Sun, May 19, 2019 at 04:54:22AM +0800, Like Xu wrote:
>>>>> The global smp variables in ppc are replaced with smp machine properties.
>>>>>
>>>>> A local variable of the same name would be introduced in the declaration
>>>>> phase if it's used widely in the context OR replace it on the spot if it's
>>>>> only used once. No semantic changes.
>>>>>
>>>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>>>
>>>> Any objections from the ppc maintainers to queueing this through
>>>> the Machine Core tree?
>>>
>>> Oops, CCing the ppc maintainers.
>>
>> No objection here.
>>
>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>>
> 
> Just one nit...
> 
> [...]
> 
>>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>>> index ee24212765..c9ffe9786c 100644
>>>>> --- a/hw/ppc/spapr_rtas.c
>>>>> +++ b/hw/ppc/spapr_rtas.c
>>>>> @@ -231,6 +231,8 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>>>>>                                             target_ulong args,
>>>>>                                             uint32_t nret, target_ulong rets)
>>>>>   {
>>>>> +    MachineState *ms = MACHINE(qdev_get_machine());
> 
> rtas_ibm_get_system_parameter() has a SpaprMachineState *spapr argument, no
> need to rely on qdev_get_machine().

I will fix it in the next (rebased) version. Thank you, Greg.

> 
> But this can be fixed in a followup patch I guess. Not worth holding the
> patchset because of that.
> 
>>>>> +    unsigned int max_cpus = ms->smp.max_cpus;
>>>>>       target_ulong parameter = rtas_ld(args, 0);
>>>>>       target_ulong buffer = rtas_ld(args, 1);
>>>>>       target_ulong length = rtas_ld(args, 2);
>>>>> @@ -244,7 +246,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>>>>>                                             "MaxPlatProcs=%d",
>>>>>                                             max_cpus,
>>>>>                                             current_machine->ram_size / MiB,
>>>>> -                                          smp_cpus,
>>>>> +                                          ms->smp.cpus,
>>>>>                                             max_cpus);
>>>>>           ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
>>>>>           g_free(param_val);
>>>>    
>>>    
>>
>