Re: [PATCH v1 06/14] xen/riscv: riscv_of_processor_hartid() implementation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2025 17:57, Oleksii Kurochko wrote:
> @@ -13,3 +16,68 @@ void __init smp_clear_cpu_maps(void)
>      cpumask_set_cpu(0, &cpu_online_map);
>      cpumask_copy(&cpu_present_map, &cpu_possible_map);
>  }
> +
> +/**
> + * of_get_cpu_hwid - Get the hardware ID from a CPU device node
> + *
> + * @cpun: CPU number(logical index) for which device node is required
> + * @thread: The local thread number to get the hardware ID for.
> + *
> + * Return: The hardware ID for the CPU node or ~0ULL if not found.
> + */
> +static uint64_t of_get_cpu_hwid(struct dt_device_node *cpun, unsigned int 
> thread)

What does the "of" prefix stand for here? Looking at the function body I'm
really at a loss. (I was first guessing something like OpenFirmware, but
there's nothing here that would support that.)

As you're only fetching data - can cpun be pointer-to-const?

> +{
> +    const __be32 *cell;
> +    int ac;
> +    uint32_t len;
> +
> +    ac = dt_n_addr_cells(cpun);
> +    cell = dt_get_property(cpun, "reg", &len);
> +    if ( !cell || !ac || ((sizeof(*cell) * ac * (thread + 1)) > len) )
> +        return ~0ULL;

I'm sorry for my lack of DT knowledge, but what's "thread" representing here?
You only pass in 0 below, so it's unclear whether it's what one might expect
(the thread number on a multi-threaded core).

> +    cell += ac * thread;
> +    return dt_read_number(cell, ac);

Nit (you know what)

> +/*
> + * Returns the hart ID of the given device tree node, or -ENODEV if the node
> + * isn't an enabled and valid RISC-V hart node.
> + */
> +int riscv_of_processor_hartid(struct dt_device_node *node, unsigned long 
> *hart)

Similar question as above: What's "of" and what significance does the "riscv"
prefix have in RISC-V code?

Const-ness question again for "node".

> +{
> +    const char *isa;
> +
> +    if ( !dt_device_is_compatible(node, "riscv") )
> +    {
> +        printk("Found incompatible CPU\n");
> +        return -ENODEV;
> +    }
> +
> +    *hart = (unsigned long) of_get_cpu_hwid(node, 0);
> +    if ( *hart == ~0UL )

While for RV64 this won't matter, the difference in types (uint64_t returned,
unsigned long used) is still puzzling me. What's the deal?

Jan