[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 06/14] xen/riscv: dt_processor_hartid() implementation
On 5/22/25 9:50 AM, Jan Beulich wrote:
On 21.05.2025 18:03, Oleksii Kurochko wrote:--- a/xen/arch/riscv/smpboot.c +++ b/xen/arch/riscv/smpboot.c @@ -1,5 +1,8 @@ #include <xen/cpumask.h> +#include <xen/device_tree.h> +#include <xen/errno.h> #include <xen/init.h> +#include <xen/types.h> #include <xen/sections.h>Nit: The latter insertion wants to move one line down. Yet then - isn't xen/stdint.h sufficient here? __be32 used in dt_get_hartid() is defined in xen/types.h. @@ -14,3 +17,69 @@ 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 ( !cell || !ac || ((sizeof(*cell) * ac) > len) )Does DT make any guarantees for this multiplication to not overflow? I haven't tried of DTC checks such things during compilation but considering that ac value is uin32_t value (according to DT spec) then overflow could really happen. I will add the following to check an overflow: if ( ac > ((sizeof(size_t) * BIT_PER_BYTE) / sizeof(*cell)) ) { printk("%s: overflow detected\n", __func__); return ~0UL; } + 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=%lu is not available\n", *hartid);Considering that hart ID assignment is outside of our control, would we perhaps better (uniformly) log such using %#lx? It makes sense, DTC when generates dts from dtb also uses hex number, so it could help to find a failure node faster. + return -ENODEV; + } + + if ( (ret = dt_property_read_string(node, "riscv,isa", &isa)) ) + { + printk("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hartid); + return ret; + } + + if ( isa[0] != 'r' || isa[1] != 'v' ) + { + printk("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hartid, + isa); + return -EINVAL;As before -EINVAL is appropriate when input arguments have wrong values. Here, however, you found an unexpected value in something the platform has presented to you. While not entirely appropriate either, maybe -ENODEV again (if nothing better can be found)? I don't see better candidate. Interesting if some reserved region exists for user defined errors. ~ Oleksii
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |