Re: [PATCH v3 08/14] xen/dt: Move bootfdt functions to xen/bootfdt.h

[Alejandro Vallejo <agarciav@amd.com>]

On Sat Jun 14, 2025 at 3:16 AM CEST, Stefano Stabellini wrote:
> On Fri, 13 Jun 2025, Alejandro Vallejo wrote:
>> Part of an unpicking process to extract bootfdt contents independent of bootinfo
>> to a separate file for x86 to take.
>> 
>> Move functions required for early FDT parsing from device_tree.h and arm's
>> setup.h onto bootfdt.h
>> 
>> Declaration motion only. Not a functional change.
>> 
>> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
>> ---
>> v3:
>>   * Avoid mutations during code motion
>> ---
>>  xen/include/xen/bootfdt.h     | 62 +++++++++++++++++++++++++++++++++++
>>  xen/include/xen/device_tree.h | 40 +---------------------
>>  2 files changed, 63 insertions(+), 39 deletions(-)
>> 
>> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
>> index 8ea52290b7..b6ae7d6aa6 100644
>> --- a/xen/include/xen/bootfdt.h
>> +++ b/xen/include/xen/bootfdt.h
>> @@ -2,6 +2,7 @@
>>  #ifndef XEN_BOOTFDT_H
>>  #define XEN_BOOTFDT_H
>>  
>> +#include <xen/byteorder.h>
>>  #include <xen/types.h>
>>  #include <xen/kernel.h>
>>  #include <xen/macros.h>
>> @@ -16,8 +17,53 @@
>>  #define NR_MEM_BANKS 256
>>  #define NR_SHMEM_BANKS 32
>>  
>> +/* Default #address and #size cells */
>> +#define DT_ROOT_NODE_ADDR_CELLS_DEFAULT 2
>> +#define DT_ROOT_NODE_SIZE_CELLS_DEFAULT 1
>> +
>>  #define MAX_MODULES 32 /* Current maximum useful modules */
>>  
>> +#define DEVICE_TREE_MAX_DEPTH 16
>> +
>> +/* 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;
>> +
>> +    while ( size-- )
>> +        r = (r << 32) | be32_to_cpu(*(cell++));
>> +    return r;
>> +}
>> +
>> +static inline u64 dt_next_cell(int s, const __be32 **cellp)
>> +{
>> +    const __be32 *p = *cellp;
>> +
>> +    *cellp = p + s;
>> +    return dt_read_number(p, s);
>> +}
>> +
>> +typedef int (*device_tree_node_func)(const void *fdt,
>> +                                     int node, const char *name, int depth,
>> +                                     u32 address_cells, u32 size_cells,
>> +                                     void *data);
>> +
>> +/**
>> + * device_tree_for_each_node - iterate over all device tree sub-nodes
>> + * @fdt: flat device tree.
>> + * @node: parent node to start the search from
>> + * @func: function to call for each sub-node.
>> + * @data: data to pass to @func.
>> + *
>> + * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
>> + *
>> + * Returns 0 if all nodes were iterated over successfully.  If @func
>> + * returns a value different from 0, that value is returned immediately.
>> + */
>> +int device_tree_for_each_node(const void *fdt, int node,
>> +                              device_tree_node_func func,
>> +                              void *data);
>> +
>>  typedef enum {
>>      BOOTMOD_XEN,
>>      BOOTMOD_FDT,
>> @@ -260,4 +306,20 @@ static inline struct membanks *membanks_xzalloc(unsigned int nr,
>>      return banks;
>>  }
>>  
>> +/*
>> + * Interpret the property `prop_name` of `node` as a u32.
>> + *
>> + * Returns the property value on success; otherwise returns `dflt`.
>> + */
>> +u32 device_tree_get_u32(const void *fdt, int node,
>> +                        const char *prop_name, u32 dflt);
>> +
>> +/*
>> + * Interpret the property `prop_name` of `node` as a "reg".
>> + *
>> + * Returns outputs in `start` and `size`.
>> + */
>> +void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
>> +                         uint32_t size_cells, paddr_t *start, paddr_t *size);
>> +
>>  #endif /* XEN_BOOTFDT_H */
>> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
>> index 75017e4266..0a22b1ba1d 100644
>> --- a/xen/include/xen/device_tree.h
>> +++ b/xen/include/xen/device_tree.h
>> @@ -10,6 +10,7 @@
>>  #ifndef __XEN_DEVICE_TREE_H__
>>  #define __XEN_DEVICE_TREE_H__
>>  
>> +#include <xen/bootfdt.h>
>>  #include <xen/byteorder.h>
>
> This should not be needed?

I wanted to avoid having to touch include sites. Let me check how many affected
places there are and fix up accordingly if any needs adjusting.

Cheers,
Alejandro