Re: [PATCH v3 1/2] powerpc/mm: Cleanup memory block size probing

[Reza Arbab]

On Fri, Jul 28, 2023 at 04:05:55PM +0530, Aneesh Kumar K.V wrote:
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
[snip]
+       /*
+        * "ibm,coherent-device-memory with linux,usable-memory = 0
+        * Force 256MiB block size. Work around for GPUs on P9 PowerNV
+        * linux,usable-memory == 0 implies driver managed memory and
+        * we can't use large memory block size due to hotplug/unplug
+        * limitations.
+        */
+       compatible = of_get_flat_dt_prop(node, "compatible", NULL);
+       if (compatible && !strcmp(compatible, "ibm,coherent-device-memory")) {
+               int len = 0;
+               const __be32 *usm;
+
+               usm = of_get_flat_dt_prop(node, "linux,drconf-usable-memory", 
&len);

I think this should be "linux,usable-memory".

+               if (usm && !len) {
+                       *block_size = SZ_256M;
+                       return 1;
+               }

This isn't quite right. The criteria is not that the property itself has 
no registers, it's that the base/size combo has size zero.

If you fold in the patch appended to the end of this mail, things worked 
for me.

+       }
+
+       reg = of_get_flat_dt_prop(node, "reg", &l);
+       endp = reg + (l / sizeof(__be32));
+
+       while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) {
+               u64 base, size;
+
+               base = dt_mem_next_cell(dt_root_addr_cells, &reg);
+               size = dt_mem_next_cell(dt_root_size_cells, &reg);
+
+               if (size == 0)
+                       continue;
+
+               update_memory_block_size(block_size, size);
+       }
+       /* continue looking for other memory device types */
+       return 0;
+}
+
+/*
+ * start with 1G memory block size. Early init will
+ * fix this with correct value.
+ */
+unsigned long memory_block_size __ro_after_init = 1UL << 30;

Could use SZ_1G here.

With the following fixup, I got 256MiB blocks on a system with
"ibm,coherent-device-memory" nodes.

diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index dbed37d6cffb..1ac58e72a885 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -487,7 +487,6 @@ static int __init probe_memory_block_size(unsigned long 
node, const char *uname,
                                          depth, void *data)
 {
        const char *type;
-       const char *compatible;
        unsigned long *block_size = (unsigned long *)data;
        const __be32 *reg, *endp;
        int l;
@@ -532,38 +531,38 @@ static int __init probe_memory_block_size(unsigned long 
node, const char *uname,
        if (type == NULL || strcmp(type, "memory") != 0)
                return 0;
 
-	/*
-        * "ibm,coherent-device-memory with linux,usable-memory = 0
-        * Force 256MiB block size. Work around for GPUs on P9 PowerNV
-        * linux,usable-memory == 0 implies driver managed memory and
-        * we can't use large memory block size due to hotplug/unplug
-        * limitations.
-        */
-       compatible = of_get_flat_dt_prop(node, "compatible", NULL);
-       if (compatible && !strcmp(compatible, "ibm,coherent-device-memory")) {
-               int len = 0;
-               const __be32 *usm;
-
-               usm = of_get_flat_dt_prop(node, "linux,drconf-usable-memory", 
&len);
-               if (usm && !len) {
-                       *block_size = SZ_256M;
-                       return 1;
-               }
-       }
+       reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
+       if (!reg)
+               reg = of_get_flat_dt_prop(node, "reg", &l);
+       if (!reg)
+               return 0;
 
-	reg = of_get_flat_dt_prop(node, "reg", &l);
        endp = reg + (l / sizeof(__be32));
 
 	while ((endp - reg) >= (dt_root_addr_cells + dt_root_size_cells)) {
+               const char *compatible;
                u64 base, size;
 
 		base = dt_mem_next_cell(dt_root_addr_cells, &reg);
                size = dt_mem_next_cell(dt_root_size_cells, &reg);
 
-		if (size == 0)
+               if (size) {
+                       update_memory_block_size(block_size, size);
                        continue;
+               }
 
-		update_memory_block_size(block_size, size);
+               /*
+                * ibm,coherent-device-memory with linux,usable-memory = 0
+                * Force 256MiB block size. Work around for GPUs on P9 PowerNV
+                * linux,usable-memory == 0 implies driver managed memory and
+                * we can't use large memory block size due to hotplug/unplug
+                * limitations.
+                */
+               compatible = of_get_flat_dt_prop(node, "compatible", NULL);
+               if (compatible && !strcmp(compatible, 
"ibm,coherent-device-memory")) {
+                       *block_size = SZ_256M;
+                       return 1;
+               }
        }
        /* continue looking for other memory device types */
        return 0;
--
Reza Arbab