qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes
@ 2018-04-11 18:41 Serhii Popovych
  2018-04-11 18:41 ` [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property Serhii Popovych
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Serhii Popovych @ 2018-04-11 18:41 UTC (permalink / raw)
  To: qemu-ppc; +Cc: bharata, qemu-devel, david

Now PowerPC Linux kernel supports hot-add to NUMA nodes not populated
initially with memory we can enable such support in qemu. This requires
two changes:

  o Add device tree property "ibm,max-associativity-domains" to let
    guest kernel chance to find max possible NUMA node

  o Revert  commit b556854bd852 ("spapr: Don't allow memory hotplug to
    memory less nodes") to remove check for hot-add to memory-less node.

See description messges for individual changes for more details.

v3:
  - Make layer for max_cpus unspecified instead of setting it to zero.
    Not adding cpu_to_be32(spapr_vcpu_id(spapr, max_cpus - 1)) because
    at the moment we only want max number for numa nodes to enable feat.
  - Rebase to current state of master branch.

v2:
  - Reorder patches in series according to description above.
  - Add extra coment to revert noticing return to previous behaviour for
    guests without support for hot-add to empty node.
  - Drop max_cpus from topology in property due to vcpu id discontiguous
    allocations. Thanks to David Gibson for extra explanation.
  - Rebase to current state of master branch.

Serhii Popovych (2):
  spapr: Add ibm,max-associativity-domains property
  Revert "spapr: Don't allow memory hotplug to memory less nodes"

 hw/ppc/spapr.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-11 18:41 [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes Serhii Popovych
@ 2018-04-11 18:41 ` Serhii Popovych
  2018-04-13  7:57   ` Bharata B Rao
  2018-04-11 18:42 ` [Qemu-devel] [PATCH for 2.13 v3 2/2] Revert "spapr: Don't allow memory hotplug to memory less nodes" Serhii Popovych
  2018-04-12  2:21 ` [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes David Gibson
  2 siblings, 1 reply; 9+ messages in thread
From: Serhii Popovych @ 2018-04-11 18:41 UTC (permalink / raw)
  To: qemu-ppc; +Cc: bharata, qemu-devel, david

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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a81570e..c05bbad 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -910,6 +910,13 @@ 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(4),
+        cpu_to_be32(0),
+        cpu_to_be32(0),
+        cpu_to_be32(0),
+        cpu_to_be32(nb_numa_nodes - 1),
+    };
 
     _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
 
@@ -946,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",
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH for 2.13 v3 2/2] Revert "spapr: Don't allow memory hotplug to memory less nodes"
  2018-04-11 18:41 [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes Serhii Popovych
  2018-04-11 18:41 ` [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property Serhii Popovych
@ 2018-04-11 18:42 ` Serhii Popovych
  2018-04-12  2:21 ` [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes David Gibson
  2 siblings, 0 replies; 9+ messages in thread
From: Serhii Popovych @ 2018-04-11 18:42 UTC (permalink / raw)
  To: qemu-ppc; +Cc: bharata, qemu-devel, david

This reverts commit b556854bd8524c26b8be98ab1bfdf0826831e793.

Leave change @node type from uint32_t to to int from reverted commit
because node < 0 is always false.

Note that implementing capability or some trick to detect if guest
kernel does not support hot-add to memory: this returns previous
behavour where memory added to first non-empty node.

Signed-off-by: Serhii Popovych <spopovyc@redhat.com>
---
 hw/ppc/spapr.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c05bbad..1e7983c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3488,28 +3488,6 @@ static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
             return;
         }
 
-        /*
-         * Currently PowerPC kernel doesn't allow hot-adding memory to
-         * memory-less node, but instead will silently add the memory
-         * to the first node that has some memory. This causes two
-         * unexpected behaviours for the user.
-         *
-         * - Memory gets hotplugged to a different node than what the user
-         *   specified.
-         * - Since pc-dimm subsystem in QEMU still thinks that memory belongs
-         *   to memory-less node, a reboot will set things accordingly
-         *   and the previously hotplugged memory now ends in the right node.
-         *   This appears as if some memory moved from one node to another.
-         *
-         * So until kernel starts supporting memory hotplug to memory-less
-         * nodes, just prevent such attempts upfront in QEMU.
-         */
-        if (nb_numa_nodes && !numa_info[node].node_mem) {
-            error_setg(errp, "Can't hotplug memory to memory-less node %d",
-                       node);
-            return;
-        }
-
         spapr_memory_plug(hotplug_dev, dev, node, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE)) {
         spapr_core_plug(hotplug_dev, dev, errp);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes
  2018-04-11 18:41 [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes Serhii Popovych
  2018-04-11 18:41 ` [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property Serhii Popovych
  2018-04-11 18:42 ` [Qemu-devel] [PATCH for 2.13 v3 2/2] Revert "spapr: Don't allow memory hotplug to memory less nodes" Serhii Popovych
@ 2018-04-12  2:21 ` David Gibson
  2 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2018-04-12  2:21 UTC (permalink / raw)
  To: Serhii Popovych; +Cc: qemu-ppc, bharata, qemu-devel

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

On Wed, Apr 11, 2018 at 02:41:58PM -0400, Serhii Popovych wrote:
> Now PowerPC Linux kernel supports hot-add to NUMA nodes not populated
> initially with memory we can enable such support in qemu. This requires
> two changes:
> 
>   o Add device tree property "ibm,max-associativity-domains" to let
>     guest kernel chance to find max possible NUMA node
> 
>   o Revert  commit b556854bd852 ("spapr: Don't allow memory hotplug to
>     memory less nodes") to remove check for hot-add to memory-less node.
> 
> See description messges for individual changes for more details.
> 
> v3:
>   - Make layer for max_cpus unspecified instead of setting it to zero.
>     Not adding cpu_to_be32(spapr_vcpu_id(spapr, max_cpus - 1)) because
>     at the moment we only want max number for numa nodes to enable feat.
>   - Rebase to current state of master branch.
> 
> v2:
>   - Reorder patches in series according to description above.
>   - Add extra coment to revert noticing return to previous behaviour for
>     guests without support for hot-add to empty node.
>   - Drop max_cpus from topology in property due to vcpu id discontiguous
>     allocations. Thanks to David Gibson for extra explanation.
>   - Rebase to current state of master branch.
> 
> Serhii Popovych (2):
>   spapr: Add ibm,max-associativity-domains property
>   Revert "spapr: Don't allow memory hotplug to memory less nodes"

Applied to ppc-for-2.13, thanks.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-11 18:41 ` [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property Serhii Popovych
@ 2018-04-13  7:57   ` Bharata B Rao
  2018-04-16 16:47     ` Serhii Popovych
  0 siblings, 1 reply; 9+ messages in thread
From: Bharata B Rao @ 2018-04-13  7:57 UTC (permalink / raw)
  To: Serhii Popovych; +Cc: qemu-ppc, qemu-devel, david

On Wed, Apr 11, 2018 at 02:41:59PM -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 | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a81570e..c05bbad 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -910,6 +910,13 @@ 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(4),
> +        cpu_to_be32(0),
> +        cpu_to_be32(0),
> +        cpu_to_be32(0),
> +        cpu_to_be32(nb_numa_nodes - 1),
> +    };
> 
>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
> 
> @@ -946,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",
> -- 
> 1.8.3.1

This commit causes hash guest with latest guest kernel to hang at early boot.

Quiescing Open Firmware ...
Booting Linux via __start() @ 0x0000000002000000 ...
[    0.000000] hash-mmu: Page sizes from device-tree:
[    0.000000] hash-mmu: base_shift=12: shift=12, sllp=0x0000, avpnm=0x00000000, tlbiel=1, penc=0
[    0.000000] hash-mmu: base_shift=16: shift=16, sllp=0x0110, avpnm=0x00000000, tlbiel=1, penc=1
[    0.000000] Using 1TB segments
[    0.000000] hash-mmu: Initializing hash mmu with SLB
[    0.000000] Linux version 4.16.0-rc7+ (root@localhost.localdomain) (gcc version 7.1.1 20170622 (Red Hat 7.1.1-3) (GCC)) #60 SMP Wed Apr 11 10:36:22 IST 2018
[    0.000000] Found initrd at 0xc000000003c00000:0xc000000004f9a34c
[    0.000000] Using pSeries machine description
[    0.000000] bootconsole [udbg0] enabled
[    0.000000] Partition configured for 32 cpus.
[    0.000000] CPU maps initialized for 1 thread per core
[    0.000000] -----------------------------------------------------
[    0.000000] ppc64_pft_size    = 0x1a
[    0.000000] phys_mem_size     = 0x200000000
[    0.000000] dcache_bsize      = 0x80
[    0.000000] icache_bsize      = 0x80
[    0.000000] cpu_features      = 0x077c7a6c18500249
[    0.000000]   possible        = 0xffffffff18500649
[    0.000000]   always          = 0x0000000018100040
[    0.000000] cpu_user_features = 0xdc0065c2 0xae000000
[    0.000000] mmu_features      = 0x78006001
[    0.000000] firmware_features = 0x00000001415a445f
[    0.000000] htab_hash_mask    = 0x7ffff
[    0.000000] -----------------------------------------------------

No progess after this.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-13  7:57   ` Bharata B Rao
@ 2018-04-16 16:47     ` Serhii Popovych
  2018-04-17  3:58       ` Bharata B Rao
  0 siblings, 1 reply; 9+ messages in thread
From: Serhii Popovych @ 2018-04-16 16:47 UTC (permalink / raw)
  To: bharata; +Cc: qemu-ppc, qemu-devel, david

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

Bharata B Rao wrote:
> On Wed, Apr 11, 2018 at 02:41:59PM -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 | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a81570e..c05bbad 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -910,6 +910,13 @@ 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(4),
>> +        cpu_to_be32(0),
>> +        cpu_to_be32(0),
>> +        cpu_to_be32(0),
>> +        cpu_to_be32(nb_numa_nodes - 1),
>> +    };
>>
>>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
>>
>> @@ -946,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",
>> -- 
>> 1.8.3.1
> 
> This commit causes hash guest with latest guest kernel to hang at early boot.

I use v4.16 tag from stable and can't reproduce on P8 machine reported
issue.

Could you please share more details about your setup, kernel commit id
you spot problem?

> 
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x0000000002000000 ...
> [    0.000000] hash-mmu: Page sizes from device-tree:
> [    0.000000] hash-mmu: base_shift=12: shift=12, sllp=0x0000, avpnm=0x00000000, tlbiel=1, penc=0
> [    0.000000] hash-mmu: base_shift=16: shift=16, sllp=0x0110, avpnm=0x00000000, tlbiel=1, penc=1
> [    0.000000] Using 1TB segments
> [    0.000000] hash-mmu: Initializing hash mmu with SLB
> [    0.000000] Linux version 4.16.0-rc7+ (root@localhost.localdomain) (gcc version 7.1.1 20170622 (Red Hat 7.1.1-3) (GCC)) #60 SMP Wed Apr 11 10:36:22 IST 2018
> [    0.000000] Found initrd at 0xc000000003c00000:0xc000000004f9a34c
> [    0.000000] Using pSeries machine description
> [    0.000000] bootconsole [udbg0] enabled
> [    0.000000] Partition configured for 32 cpus.
> [    0.000000] CPU maps initialized for 1 thread per core
> [    0.000000] -----------------------------------------------------
> [    0.000000] ppc64_pft_size    = 0x1a
> [    0.000000] phys_mem_size     = 0x200000000
> [    0.000000] dcache_bsize      = 0x80
> [    0.000000] icache_bsize      = 0x80
> [    0.000000] cpu_features      = 0x077c7a6c18500249
> [    0.000000]   possible        = 0xffffffff18500649
> [    0.000000]   always          = 0x0000000018100040
> [    0.000000] cpu_user_features = 0xdc0065c2 0xae000000
> [    0.000000] mmu_features      = 0x78006001
> [    0.000000] firmware_features = 0x00000001415a445f
> [    0.000000] htab_hash_mask    = 0x7ffff
> [    0.000000] -----------------------------------------------------
> 
> No progess after this.
> 
> 


-- 
Thanks,
Serhii


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-16 16:47     ` Serhii Popovych
@ 2018-04-17  3:58       ` Bharata B Rao
  2018-04-17  6:19         ` David Gibson
  0 siblings, 1 reply; 9+ messages in thread
From: Bharata B Rao @ 2018-04-17  3:58 UTC (permalink / raw)
  To: Serhii Popovych; +Cc: qemu-ppc, qemu-devel, david

On Mon, Apr 16, 2018 at 07:47:29PM +0300, Serhii Popovych wrote:
> Bharata B Rao wrote:
> > On Wed, Apr 11, 2018 at 02:41:59PM -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 | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index a81570e..c05bbad 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -910,6 +910,13 @@ 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(4),
> >> +        cpu_to_be32(0),
> >> +        cpu_to_be32(0),
> >> +        cpu_to_be32(0),
> >> +        cpu_to_be32(nb_numa_nodes - 1),
> >> +    };
> >>
> >>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
> >>
> >> @@ -946,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",
> >> -- 
> >> 1.8.3.1
> > 
> > This commit causes hash guest with latest guest kernel to hang at early boot.
> 
> I use v4.16 tag from stable and can't reproduce on P8 machine reported
> issue.
> 
> Could you please share more details about your setup, kernel commit id
> you spot problem?

I am on 4.16.0-rc7 (commit id: 0b412605ef5f)

BTW this happens only for non-NUMA guest.

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-17  3:58       ` Bharata B Rao
@ 2018-04-17  6:19         ` David Gibson
  2018-04-17 16:26           ` Serhii Popovych
  0 siblings, 1 reply; 9+ messages in thread
From: David Gibson @ 2018-04-17  6:19 UTC (permalink / raw)
  To: Bharata B Rao; +Cc: Serhii Popovych, qemu-ppc, qemu-devel

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

On Tue, Apr 17, 2018 at 09:28:42AM +0530, Bharata B Rao wrote:
> On Mon, Apr 16, 2018 at 07:47:29PM +0300, Serhii Popovych wrote:
> > Bharata B Rao wrote:
> > > On Wed, Apr 11, 2018 at 02:41:59PM -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 | 10 ++++++++++
> > >>  1 file changed, 10 insertions(+)
> > >>
> > >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > >> index a81570e..c05bbad 100644
> > >> --- a/hw/ppc/spapr.c
> > >> +++ b/hw/ppc/spapr.c
> > >> @@ -910,6 +910,13 @@ 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(4),
> > >> +        cpu_to_be32(0),
> > >> +        cpu_to_be32(0),
> > >> +        cpu_to_be32(0),
> > >> +        cpu_to_be32(nb_numa_nodes - 1),
> > >> +    };
> > >>
> > >>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
> > >>
> > >> @@ -946,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",
> > > 
> > > This commit causes hash guest with latest guest kernel to hang at early boot.
> > 
> > I use v4.16 tag from stable and can't reproduce on P8 machine reported
> > issue.
> > 
> > Could you please share more details about your setup, kernel commit id
> > you spot problem?
> 
> I am on 4.16.0-rc7 (commit id: 0b412605ef5f)
> 
> BTW this happens only for non-NUMA guest.

Ah, that might explain it.  With no NUMA nodes specified, I think this
code will put a -1 into the max-associativity-domains property, which
is probably causing the mess.  If we don't have NUMA (nb_numa_nodes ==
0) we probably want to either omit the property entirely, or clamp
that 5th cell to 0.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property
  2018-04-17  6:19         ` David Gibson
@ 2018-04-17 16:26           ` Serhii Popovych
  0 siblings, 0 replies; 9+ messages in thread
From: Serhii Popovych @ 2018-04-17 16:26 UTC (permalink / raw)
  To: David Gibson, Bharata B Rao; +Cc: qemu-ppc, qemu-devel

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

David Gibson wrote:
> On Tue, Apr 17, 2018 at 09:28:42AM +0530, Bharata B Rao wrote:
>> On Mon, Apr 16, 2018 at 07:47:29PM +0300, Serhii Popovych wrote:
>>> Bharata B Rao wrote:
>>>> On Wed, Apr 11, 2018 at 02:41:59PM -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 | 10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index a81570e..c05bbad 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -910,6 +910,13 @@ 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(4),
>>>>> +        cpu_to_be32(0),
>>>>> +        cpu_to_be32(0),
>>>>> +        cpu_to_be32(0),
>>>>> +        cpu_to_be32(nb_numa_nodes - 1),
>>>>> +    };
>>>>>
>>>>>      _FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
>>>>>
>>>>> @@ -946,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",
>>>>
>>>> This commit causes hash guest with latest guest kernel to hang at early boot.
>>>
>>> I use v4.16 tag from stable and can't reproduce on P8 machine reported
>>> issue.
>>>
>>> Could you please share more details about your setup, kernel commit id
>>> you spot problem?
>>
>> I am on 4.16.0-rc7 (commit id: 0b412605ef5f)
>>
>> BTW this happens only for non-NUMA guest.
> 
> Ah, that might explain it.  With no NUMA nodes specified, I think this
> code will put a -1 into the max-associativity-domains property, which
> is probably causing the mess.  If we don't have NUMA (nb_numa_nodes ==
> 0) we probably want to either omit the property entirely, or clamp
> that 5th cell to 0.
> 
Ok, proposed fix already posted to qemu-devel. Sorry forgot to CC.

Mail subject contains (w/o quotas):
"spapr: Correct max associativity domains value for non-NUMA configs"

Tested with v4.16 tag:

  o Before : non-NUMA configs: able to reproduce, stall during boot
  o After  : non-NUMA configs: not reproducible, boot is ok

-- 
Thanks,
Serhii


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-17 16:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-11 18:41 [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes Serhii Popovych
2018-04-11 18:41 ` [Qemu-devel] [PATCH for 2.13 v3 1/2] spapr: Add ibm, max-associativity-domains property Serhii Popovych
2018-04-13  7:57   ` Bharata B Rao
2018-04-16 16:47     ` Serhii Popovych
2018-04-17  3:58       ` Bharata B Rao
2018-04-17  6:19         ` David Gibson
2018-04-17 16:26           ` Serhii Popovych
2018-04-11 18:42 ` [Qemu-devel] [PATCH for 2.13 v3 2/2] Revert "spapr: Don't allow memory hotplug to memory less nodes" Serhii Popovych
2018-04-12  2:21 ` [Qemu-devel] [PATCH for 2.13 v3 0/2] target/ppc: Support adding memory to initially memory-less NUMA nodes David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).