Re: [Qemu-devel] [PATCH for 2.13 2/2] spapr: Add ibm, max-associativity-domains property

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]

On Thu, Apr 05, 2018 at 10:35:23AM -0400, Serhii Popovych wrote:
> Now recent kernels (i.e. since linux-stable commit a346137e9142
> ("powerpc/numa: Use ibm,max-associativity-domains to discover possible nodes")
> support this property to mark initially memory-less NUMA nodes as "possible"
> to allow further memory hot-add to them.
> 
> Advertise this property for pSeries machines to let guest kernels detect
> maximum supported node configuration and benefit from kernel side change
> when hot-add memory to specific, possibly empty before, NUMA node.
> 
> Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
> ---
>  hw/ppc/spapr.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3ad4545..e02fc94 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -909,6 +909,14 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
>          0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
>          cpu_to_be32(max_cpus / smp_threads),
>      };
> +    uint32_t maxdomains[] = {
> +        cpu_to_be32(5),
> +        cpu_to_be32(0),
> +        cpu_to_be32(0),
> +        cpu_to_be32(0),
> +        cpu_to_be32(nb_numa_nodes - 1),
> +        cpu_to_be32(max_cpus - 1),
> +    };

There's a minor problem here, which I didn't think of when I was
discussing this with you earlier.

(max_cpus - 1) in the last slot isn't quite right, because we need the
maximum vcpu id here.  Because of (complicated, historical) reasons we
don't allocate the vcpu ids contiguously in all cases, so it could be
larger than max_cpus - 1.

But, we don't actually need to handle that case.  We give 5 levels of
associativity on cpus, but only 4 on memory.  Having a quick look at
the NUMA code on the guest side, I'm pretty sure it's ok if we only
give maximum values to depth 4 here, so we can just drop the last cell
here.

>  
>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
>  
> @@ -945,6 +953,9 @@ static void spapr_dt_rtas(sPAPRMachineState *spapr, void *fdt)
>      _FDT(fdt_setprop(fdt, rtas, "ibm,associativity-reference-points",
>                       refpoints, sizeof(refpoints)));
>  
> +    _FDT(fdt_setprop(fdt, rtas, "ibm,max-associativity-domains",
> +                     maxdomains, sizeof(maxdomains)));
> +
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-error-log-max",
>                            RTAS_ERROR_LOG_MAX));
>      _FDT(fdt_setprop_cell(fdt, rtas, "rtas-event-scan-rate",

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]