[PATCH v5 1/9] xen/riscv: dt_processor_hartid() implementation

Oleksii Kurochko posted 9 patches 6 months ago
Only 7 patches received!
There is a newer version of this series
[PATCH v5 1/9] xen/riscv: dt_processor_hartid() implementation
Posted by Oleksii Kurochko 6 months ago
Implements dt_processor_hartid() to get the hart ID of the given
device tree node and do some checks if CPU is available and given device
tree node has proper riscv,isa property.

As a helper function dt_get_hartid() is introduced to deal specifically
with reg propery of a CPU device node.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V5:
 - Rework ac overflow check in dt_get_hartid().
 - Update if check with (ac > 2) before dt_read_number() as it return uin64_t, so
   with ac > 2 it will return incorrect results. In case (ac > 2) return ~0UL,
   so this hartid will be skipped.
---
Changes in V4:
 - Sort properly inculsion of xen/types.h in riscv/smpboot.c.
 - Add overflow check of got address cell value in dt_get_hartid().
 - Use "%#lx" for loging recieved hartid from DTB.
 - s/-EINVAL/-ENODEV for recieved isa string from DTB.
---
Changes in V3:
 - s/dt_processor_cpuid/dt_processor_hartid.
 - s/dt_get_cpuid/dt_get_hartid.
 - use ~0UL in dt_get_cpuid() and in the comment above it.
 - change type for local variable ac in dt_get_cpuid() to unsigned int.
 - Update the return errors for dt_processor_cpuid().
 - Update the commit message + subject: s/cpuid/hartid.
---
Changes in V2:
 - s/of_get_cpu_hwid()/dt_get_cpu_id().
 - Update prototype of dt_get_cpu_hwid(), use pointer-to-const for cpun arg.
 - Add empty line before last return in dt_get_cpu_hwid().
 - s/riscv_of_processor_hartid/dt_processor_cpuid().
 - Use pointer-to_const for node argument of dt_processor_cpuid().
 - Use for hart_id unsigned long type as according to the spec for RV128
   mhartid register will be 128 bit long.
 - Update commit message and subject.
 - use 'CPU' instead of 'HART'.
 - Drop thread argument of dt_get_cpu_id() (of_get_cpu_hwid) as it is
   expected to be always 0 according to RISC-V's DTS binding.
---
 xen/arch/riscv/include/asm/smp.h |  4 ++
 xen/arch/riscv/smpboot.c         | 77 ++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/xen/arch/riscv/include/asm/smp.h b/xen/arch/riscv/include/asm/smp.h
index 5e170b57b3..eb58b6576b 100644
--- a/xen/arch/riscv/include/asm/smp.h
+++ b/xen/arch/riscv/include/asm/smp.h
@@ -26,6 +26,10 @@ static inline void set_cpuid_to_hartid(unsigned long cpuid,
 
 void setup_tp(unsigned int cpuid);
 
+struct dt_device_node;
+int dt_processor_hartid(const struct dt_device_node *node,
+                        unsigned long *hartid);
+
 #endif
 
 /*
diff --git a/xen/arch/riscv/smpboot.c b/xen/arch/riscv/smpboot.c
index 0f9c2cc54a..470f6d1311 100644
--- a/xen/arch/riscv/smpboot.c
+++ b/xen/arch/riscv/smpboot.c
@@ -1,6 +1,9 @@
 #include <xen/cpumask.h>
+#include <xen/device_tree.h>
+#include <xen/errno.h>
 #include <xen/init.h>
 #include <xen/sections.h>
+#include <xen/types.h>
 
 #include <asm/current.h>
 
@@ -14,3 +17,77 @@ void __init smp_prepare_boot_cpu(void)
     cpumask_set_cpu(0, &cpu_possible_map);
     cpumask_set_cpu(0, &cpu_online_map);
 }
+
+/**
+ * dt_get_hartid - Get the hartid from a CPU device node
+ *
+ * @cpun: CPU number(logical index) for which device node is required
+ *
+ * Return: The hartid for the CPU node or ~0UL if not found.
+ */
+static unsigned long dt_get_hartid(const struct dt_device_node *cpun)
+{
+    const __be32 *cell;
+    unsigned int ac;
+    uint32_t len;
+
+    ac = dt_n_addr_cells(cpun);
+    cell = dt_get_property(cpun, "reg", &len);
+
+    /*
+     * If ac > 2, the result may be truncated or meaningless unless
+     * dt_read_number() supports wider integers.
+     *
+     * TODO: drop (ac > 2) when dt_read_number() will support wider
+     *       integers.
+     */
+    if ( !cell || !ac || (ac > 2) || (ac > len / sizeof(*cell)) )
+        return ~0UL;
+
+    return dt_read_number(cell, ac);
+}
+
+/*
+ * Returns the hartid of the given device tree node, or -ENODEV if the node
+ * isn't an enabled and valid RISC-V hart node.
+ */
+int dt_processor_hartid(const struct dt_device_node *node,
+                        unsigned long *hartid)
+{
+    const char *isa;
+    int ret;
+
+    if ( !dt_device_is_compatible(node, "riscv") )
+    {
+        printk("Found incompatible CPU\n");
+        return -ENODEV;
+    }
+
+    *hartid = dt_get_hartid(node);
+    if ( *hartid == ~0UL )
+    {
+        printk("Found CPU without CPU ID\n");
+        return -ENODATA;
+    }
+
+    if ( !dt_device_is_available(node))
+    {
+        printk("CPU with hartid=%#lx is not available\n", *hartid);
+        return -ENODEV;
+    }
+
+    if ( (ret = dt_property_read_string(node, "riscv,isa", &isa)) )
+    {
+        printk("CPU with hartid=%#lx has no \"riscv,isa\" property\n", *hartid);
+        return ret;
+    }
+
+    if ( isa[0] != 'r' || isa[1] != 'v' )
+    {
+        printk("CPU with hartid=%#lx has an invalid ISA of \"%s\"\n", *hartid,
+               isa);
+        return -ENODEV;
+    }
+
+    return 0;
+}
-- 
2.49.0
Re: [PATCH v5 1/9] xen/riscv: dt_processor_hartid() implementation
Posted by Jan Beulich 6 months ago
On 13.06.2025 17:48, Oleksii Kurochko wrote:
> Implements dt_processor_hartid() to get the hart ID of the given
> device tree node and do some checks if CPU is available and given device
> tree node has proper riscv,isa property.
> 
> As a helper function dt_get_hartid() is introduced to deal specifically
> with reg propery of a CPU device node.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>


Acked-by: Jan Beulich <jbeulich@suse.com>

> @@ -14,3 +17,77 @@ void __init smp_prepare_boot_cpu(void)
>      cpumask_set_cpu(0, &cpu_possible_map);
>      cpumask_set_cpu(0, &cpu_online_map);
>  }
> +
> +/**
> + * dt_get_hartid - Get the hartid from a CPU device node
> + *
> + * @cpun: CPU number(logical index) for which device node is required
> + *
> + * Return: The hartid for the CPU node or ~0UL if not found.
> + */
> +static unsigned long dt_get_hartid(const struct dt_device_node *cpun)
> +{
> +    const __be32 *cell;
> +    unsigned int ac;
> +    uint32_t len;
> +
> +    ac = dt_n_addr_cells(cpun);
> +    cell = dt_get_property(cpun, "reg", &len);

I'll commit as is, but I'd like to note that such could be more compact:

    unsigned int ac = dt_n_addr_cells(cpun);
    uint32_t len;
    const __be32 *cell = dt_get_property(cpun, "reg", &len);

> +    /*
> +     * If ac > 2, the result may be truncated or meaningless unless
> +     * dt_read_number() supports wider integers.
> +     *
> +     * TODO: drop (ac > 2) when dt_read_number() will support wider
> +     *       integers.
> +     */
> +    if ( !cell || !ac || (ac > 2) || (ac > len / sizeof(*cell)) )
> +        return ~0UL;
> +
> +    return dt_read_number(cell, ac);

I'd further like to note that this will silently truncate from 64 to 32
bits (when ac == 2) for RV32.

Jan