On Tue Jun 17, 2025 at 1:07 PM CEST, 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]);
Bah, missing break. And with a printk rather than a panic CI stayed green when it shouldn't have been. > + default: > + // Nonsensical size. default to 1. > + printk(XENLOG_WARNING "dt_read_number(%d) bad size", size); I'll add an ASSERT_UNREACHABLE() here for good measure. > + }; > > - while ( size-- ) > - r = (r << 32) | be32_to_cpu(*(cell++)); > return r; > } > > > base-commit: 14c57887f36937c1deb9eeca852c3a7595d2d0b8
