Re: [Qemu-devel] [PATCH v4 1/8] spapr: Consider max_cpus during xics initialization

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 06/05/2015 05:07 PM, Bharata B Rao wrote:
> On Fri, Jun 05, 2015 at 03:30:24PM +1000, Alexey Kardashevskiy wrote:
>> On 06/05/2015 02:25 PM, Bharata B Rao wrote:
>>> Use max_cpus instead of smp_cpus when intializating xics system. Also
>>> report max_cpus in ibm,interrupt-server-ranges device tree property of
>>> interrupt controller node.
>>>
>>> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>> ---
>>>   hw/ppc/spapr.c | 7 +++----
>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index acc7233..9270234 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -308,7 +308,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>       GString *hypertas = g_string_sized_new(256);
>>>       GString *qemu_hypertas = g_string_sized_new(256);
>>>       uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
>>> -    uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
>>> +    uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(max_cpus)};
>>>       int smt = kvmppc_smt_threads();
>>>       unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
>>>       QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>>> @@ -1454,9 +1454,8 @@ static void ppc_spapr_init(MachineState *machine)
>>>
>>>       /* Set up Interrupt Controller before we create the VCPUs */
>>>       spapr->icp = xics_system_init(machine,
>>> -                                  DIV_ROUND_UP(smp_cpus * kvmppc_smt_threads(),
>>> -                                               smp_threads),
>>> -                                  XICS_IRQS);
>>> +                                  DIV_ROUND_UP(max_cpus * kvmppc_smt_threads(),
>>> +                                               smp_threads), XICS_IRQS);
>>
>>
>> Please do not change the formatting of "XICS_IRQS);".
>
> Hmmm why ? I thought I saved a line!


Looks weird. There were 3 parameters, aligned. Now there are two and third 
one hides behind DIV_ROUND_UP. And we can afford an extra line ;)

And this change is not related to what the patch does, the patch does
s/smp_cpus/max_cpus/ and when I see another unrelated change - this 
confuses me.


> Again checkpatch.pl doesn't complain.

Well, you can ignore me - after all I am not the one to takes these patches 
further :)



-- 
Alexey