Hi Alejandro, On 17/06/2025 12:07, Alejandro Vallejo wrote:
The DT spec declares only two number types for a property: u32 and u64, as per Table 2.3 in Section 2.2.4. Remove unbounded loop and replace with a switch statement. Default to a size of 1 cell in the nonsensical size case, with a warning printed on the Xen console.Suggested-by: Daniel P. Smith" <dpsm...@apertussolutions.com> Signed-off-by: Alejandro Vallejo <agarc...@amd.com> --- Based on this suggestion by Daniel: https://lore.kernel.org/xen-devel/a66c11c4-cfac-4934-b1f5-e07c728db...@apertussolutions.com/ I'd be happier panicking there, seeing how DTs are by their very nature trusted blobs. But I suspect defaulting to something will find less resistance in review. I don't care much either way. --- xen/include/xen/device_tree.h | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index 75017e4266..2daef8659e 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -261,10 +261,19 @@ void intc_dt_preinit(void); /* Helper to read a big number; size is in cells (not bytes) */ static inline u64 dt_read_number(const __be32 *cell, int size) { - u64 r = 0; + u64 r = be32_to_cpu(*cell); + + switch ( size ) + { + case 1: + break; + case 2: + r = (r << 32) | be32_to_cpu(cell[1]); + default: + // Nonsensical size. default to 1. + printk(XENLOG_WARNING "dt_read_number(%d) bad size", size);
Aside what Andrew wrote. I would consider to use at least ASSERT_UNREACHABLE() for debug build. I am not sure what's the best approach for release build. But this likely want to a XENLOG_ERR.
Cheers, -- Julien Grall
