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

[David Gibson]

On Fri, Jun 05, 2015 at 06:01:53PM +1000, Alexey Kardashevskiy wrote:
> 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 :)

I don't think it's that important, but there are a couple of other
small things to fix in the series, so you might as well revert the
formatting as Alexey suggests at the same time.

-- 
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

pgpeFQxv0HX25.pgp
Description: PGP signature