Thread (6 messages) 6 messages, 3 authors, 2023-07-31

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

From: Aneesh Kumar K V <hidden>
Date: 2023-07-29 15:30:05

On 7/29/23 3:25 AM, Reza Arbab wrote:
quoted hunk ↗ jump to hunk
On Fri, Jul 28, 2023 at 04:05:55PM +0530, Aneesh Kumar K.V wrote:
quoted
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
[snip]
quoted
+    /*
+     * "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".
quoted
+        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.
quoted
+    }
+
+    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;
Thanks for correcting the right device tree node and testing the changes. Can I add

Co-authored-by: Reza Arbab [off-list ref]

-aneesh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help