spapr: Correct max associativity domains value for non-NUMA configs

[Qemu-devel] [PATCH for 2.13] spapr: Correct max associativity domains value for non-NUMA configs

Posted by Serhii Popovych 5 years, 12 months ago

In non-NUMA configurations nb_numa_nodes is zero and we set 5th cell
in ibm,max-associativity-domains to -1. That causes to stall Linux
guests during boot after following line:

[    0.000000] NUMA associativity depth for CPU/Memory: 4

Make last possible NUMA in property zero to correct support for
non-NUMA guests.

Fixes: c1df49a670ef ("spapr: Add ibm,max-associativity-domains property")
Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7b2bc4e..bff2125 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -914,7 +914,7 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
         cpu_to_be32(0),
         cpu_to_be32(0),
         cpu_to_be32(0),
-        cpu_to_be32(nb_numa_nodes - 1),
+        cpu_to_be32(nb_numa_nodes ? nb_numa_nodes - 1 : 0),
     };
 
     _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH for 2.13] spapr: Correct max associativity domains value for non-NUMA configs

Posted by Greg Kurz 5 years, 12 months ago

Cc'ing David who should always be in the recipient list when posting ppc related
patches :)

On Tue, 17 Apr 2018 12:21:35 -0400
Serhii Popovych <spopovyc@redhat.com> wrote:

> In non-NUMA configurations nb_numa_nodes is zero and we set 5th cell
> in ibm,max-associativity-domains to -1. That causes to stall Linux
> guests during boot after following line:
> 
> [    0.000000] NUMA associativity depth for CPU/Memory: 4
> 
> Make last possible NUMA in property zero to correct support for
> non-NUMA guests.
> 

Alternatively, as suggested by David in some other mail, you could drop the
property in this case. I've checked it fixes the hang too, and it probably
makes more sense than exposing only zeroes.

> Fixes: c1df49a670ef ("spapr: Add ibm,max-associativity-domains property")

Since c1df49a670ef hasn't hit master yet, I guess we should squash this
patch (or any alternative) there to preserve bisect, ie, either David
does it for you or you post a v4 of your previous series.

> Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7b2bc4e..bff2125 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -914,7 +914,7 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
>          cpu_to_be32(0),
>          cpu_to_be32(0),
>          cpu_to_be32(0),
> -        cpu_to_be32(nb_numa_nodes - 1),
> +        cpu_to_be32(nb_numa_nodes ? nb_numa_nodes - 1 : 0),
>      };
>  
>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));

Re: [Qemu-devel] [Qemu-ppc] [PATCH for 2.13] spapr: Correct max associativity domains value for non-NUMA configs

Posted by David Gibson 5 years, 12 months ago

On Tue, Apr 17, 2018 at 07:17:51PM +0200, Greg Kurz wrote:
> Cc'ing David who should always be in the recipient list when posting ppc related
> patches :)
> 
> On Tue, 17 Apr 2018 12:21:35 -0400
> Serhii Popovych <spopovyc@redhat.com> wrote:
> 
> > In non-NUMA configurations nb_numa_nodes is zero and we set 5th cell
> > in ibm,max-associativity-domains to -1. That causes to stall Linux
> > guests during boot after following line:
> > 
> > [    0.000000] NUMA associativity depth for CPU/Memory: 4
> > 
> > Make last possible NUMA in property zero to correct support for
> > non-NUMA guests.
> > 
> 
> Alternatively, as suggested by David in some other mail, you could drop the
> property in this case. I've checked it fixes the hang too, and it probably
> makes more sense than exposing only zeroes.

Actually, I think this is the better solution. qemu treats "not NUMA"
and "exactly one NUMA node" differently for historical reasons, but I
don't think that actually makes a whole lot of sense.  I think
advertising them identically so we don't generate more special cases
on the guest side is a better idea.

> > Fixes: c1df49a670ef ("spapr: Add ibm,max-associativity-domains property")
> 
> Since c1df49a670ef hasn't hit master yet, I guess we should squash this
> patch (or any alternative) there to preserve bisect, ie, either David
> does it for you or you post a v4 of your previous series.

Right, I've folded these together in my tree so we don't get an
interval of broken commits.

> 
> > Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
> > ---
> >  hw/ppc/spapr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7b2bc4e..bff2125 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -914,7 +914,7 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
> >          cpu_to_be32(0),
> >          cpu_to_be32(0),
> >          cpu_to_be32(0),
> > -        cpu_to_be32(nb_numa_nodes - 1),
> > +        cpu_to_be32(nb_numa_nodes ? nb_numa_nodes - 1 : 0),
> >      };
> >  
> >      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
> 
> 

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH for 2.13] spapr: Correct max associativity domains value for non-NUMA configs

Posted by Greg Kurz 5 years, 12 months ago

On Wed, 18 Apr 2018 11:49:08 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Apr 17, 2018 at 07:17:51PM +0200, Greg Kurz wrote:
> > Cc'ing David who should always be in the recipient list when posting ppc related
> > patches :)
> > 
> > On Tue, 17 Apr 2018 12:21:35 -0400
> > Serhii Popovych <spopovyc@redhat.com> wrote:
> >   
> > > In non-NUMA configurations nb_numa_nodes is zero and we set 5th cell
> > > in ibm,max-associativity-domains to -1. That causes to stall Linux
> > > guests during boot after following line:
> > > 
> > > [    0.000000] NUMA associativity depth for CPU/Memory: 4
> > > 
> > > Make last possible NUMA in property zero to correct support for
> > > non-NUMA guests.
> > >   
> > 
> > Alternatively, as suggested by David in some other mail, you could drop the
> > property in this case. I've checked it fixes the hang too, and it probably
> > makes more sense than exposing only zeroes.  
> 
> Actually, I think this is the better solution. qemu treats "not NUMA"
> and "exactly one NUMA node" differently for historical reasons, but I
> don't think that actually makes a whole lot of sense.  I think
> advertising them identically so we don't generate more special cases
> on the guest side is a better idea.
> 

Fair enough. Thanks for the explanation.

> > > Fixes: c1df49a670ef ("spapr: Add ibm,max-associativity-domains property")  
> > 
> > Since c1df49a670ef hasn't hit master yet, I guess we should squash this
> > patch (or any alternative) there to preserve bisect, ie, either David
> > does it for you or you post a v4 of your previous series.  
> 
> Right, I've folded these together in my tree so we don't get an
> interval of broken commits.
> 
> >   
> > > Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
> > > ---
> > >  hw/ppc/spapr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 7b2bc4e..bff2125 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -914,7 +914,7 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
> > >          cpu_to_be32(0),
> > >          cpu_to_be32(0),
> > >          cpu_to_be32(0),
> > > -        cpu_to_be32(nb_numa_nodes - 1),
> > > +        cpu_to_be32(nb_numa_nodes ? nb_numa_nodes - 1 : 0),
> > >      };
> > >  
> > >      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));  
> > 
> >   
>