It looks like the code to parse the /cpus node in DTB tree
assumed that all subnodes would represent the cpus. Unfortunately,
it is not the case any more as recent QEMU on both RPI 4 and Odroid
show a structure looking like this:

cpus {
        #address-cells = <0x02>;
        #size-cells = <0x00>;

        cpu-map {
                cluster0 {
                        core0 {
                                cpu = <0x09>;
                        };

                        core1 {
                                cpu = <0x0a>;
                        };
                };

                cluster1 {

                        core0 {
                                cpu = <0x0b>;
                        };

                        core1 {
                                cpu = <0x0c>;
                        };

                        core2 {
                                cpu = <0x0d>;
                        };

                        core3 {
                                cpu = <0x0e>;
                        };
                };
        };

        cpu@0 {
                device_type = "cpu";
                compatible = "arm,cortex-a53";
                reg = <0x00 0x00>;
                enable-method = "psci";
                capacity-dmips-mhz = <0x250>;
                next-level-cache = <0x4c>;
                #cooling-cells = <0x02>;
                cpu-supply = <0x4d>;
                operating-points-v2 = <0x4e>;
                clocks = <0x02 0xbb>;
                clock-latency = <0xc350>;
                phandle = <0x09>;
        };

        cpu@1 {
                device_type = "cpu";
                compatible = "arm,cortex-a53";
                reg = <0x00 0x01>;
                enable-method = "psci";
                capacity-dmips-mhz = <0x250>;
                next-level-cache = <0x4c>;
                #cooling-cells = <0x02>;
                cpu-supply = <0x4d>;
                operating-points-v2 = <0x4e>;
                clocks = <0x02 0xbb>;
                clock-latency = <0xc350>;
                phandle = <0x0a>;
        };
};

As one can notice, besides the cpu subnodes there is also "cpu-map" one
which at this moment we do not care about. And more to the point the
dtb_parse_cpus_count() and dtb_parse_cpus_mpid() functions in
arch-dtb.cc would try to parse those non-cpu subnodes and lead to
detecting wrong number of CPUs and OSv would end up hanging while
waiting for all secondary CPUs to come up.

So this patch fixes the code to parse the /cpus node by iterating
over all subnodes but filtering only those that have the
'device_type' property set to 'cpu'. This patch also combines
dtb_parse_cpus_count() and dtb_parse_cpus_mpid() into a single 
dtb_parse_cpus() function.

For more information about cpus information in DTB tree please read
https://www.kernel.org/doc/Documentation/devicetree/bindings/cpu/cpu-topology.txt.

Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
---
 arch/aarch64/arch-dtb.cc | 57 +++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/arch/aarch64/arch-dtb.cc b/arch/aarch64/arch-dtb.cc
index d7241963..9aeeaa28 100644
--- a/arch/aarch64/arch-dtb.cc
+++ b/arch/aarch64/arch-dtb.cc
@@ -381,9 +381,12 @@ bool dtb_get_gic_v2(u64 *dist, size_t *dist_len, u64 *cpu, 
size_t *cpu_len)
     return true;
 }
 
-/* this gets the cpus node and returns the number of cpu elements in it. */
+/* this parses the cpus node and mpidr values and returns the number of cpu in 
it. */
+#define DTB_MAX_CPU_COUNT 32
 static int dtb_cpu_count = -1;
-static int dtb_parse_cpus_count()
+static u64 dtb_cpus_mpids[DTB_MAX_CPU_COUNT];
+
+static int dtb_parse_cpus()
 {
     int node, subnode, count;
     if (!dtb)
@@ -393,9 +396,24 @@ static int dtb_parse_cpus_count()
     if (node < 0)
         return -1;
 
+    u64 *mpids = dtb_cpus_mpids;
     for (count = 0, subnode = fdt_first_subnode(dtb, node);
          subnode >= 0;
-         count++,   subnode = fdt_next_subnode(dtb, subnode)) {
+         subnode = fdt_next_subnode(dtb, subnode)) {
+
+        if (count > DTB_MAX_CPU_COUNT) {
+            abort("dtb_parse_cpus_mpid: number of cpus greater than maximum. 
Increase the DTB_MAX_CPU_COUNT!\n");
+        }
+
+        // Only count subnode that have a property "device_type" with value 
"cpu"
+        auto property = fdt_get_property(dtb, subnode, "device_type", NULL);
+        if (property) {
+            if (!strncmp("cpu", property->data, 3)) {
+                (void)dtb_get_reg(subnode, mpids);
+                mpids++;
+                count++;
+            }
+        }
     }
     return count;
 }
@@ -405,33 +423,6 @@ int dtb_get_cpus_count()
     return dtb_cpu_count;
 }
 
-/* this gets the cpu mpidr values for all cpus */
-#define DTB_MAX_CPU_COUNT 32
-static u64 dtb_cpus_mpids[DTB_MAX_CPU_COUNT];
-bool dtb_parse_cpus_mpid(u64 *mpids, int n)
-{
-    int node, subnode;
-
-    if (n > DTB_MAX_CPU_COUNT) {
-        abort("dtb_parse_cpus_mpid: number of cpus greater than maximum. 
Increase the DTB_MAX_CPU_COUNT!\n");
-    }
-
-    if (!dtb)
-        return false;
-
-    node = fdt_path_offset(dtb, "/cpus");
-    if (node < 0)
-        return false;
-
-    for (subnode = fdt_first_subnode(dtb, node);
-         n > 0 && subnode >= 0;
-         subnode = fdt_next_subnode(dtb, subnode), n--, mpids++) {
-
-        (void)dtb_get_reg(subnode, mpids);
-    }
-    return true;
-}
-
 bool dtb_get_cpus_mpid(u64 *mpids, int n) {
     for (auto i = 0; i < n; i++) {
         mpids[i] = dtb_cpus_mpids[i];
@@ -789,9 +780,9 @@ void  __attribute__((constructor(init_prio::dtb))) 
dtb_setup()
         abort("dtb_setup: cannot find cmdline after dtb move.\n");
     }
     // Parse some dtb configuration ahead of time
-    dtb_cpu_count = dtb_parse_cpus_count();
-    if (!dtb_parse_cpus_mpid(dtb_cpus_mpids, dtb_cpu_count)) {
-        abort("dtb_setup: failed to parse cpu mpid.\n");
+    dtb_cpu_count = dtb_parse_cpus();
+    if (dtb_cpu_count == -1) {
+        abort("dtb_setup: failed to parse the cpus node.\n");
     }
 
     dtb_timer_irq = dtb_parse_timer_irq();
-- 
2.35.1

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/20220510155249.105849-1-jwkozaczuk%40gmail.com.

Reply via email to