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

[Alexey Kardashevskiy]

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 <address@hidden>
> > > ---
> > >   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