On Sat May 31, 2025 at 2:39 AM CEST, dmkhn wrote:
> On Fri, May 30, 2025 at 02:02:23PM +0200, Alejandro Vallejo wrote:
>> ... back into bootfdt.c
>>
>> These will be required by x86 later on to detect modules in the early scan of
>> the FDT. They are independent of bootinfo, so it's cleaner for them to exist
>> in
>> a separate file.
>>
>> Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <agarc...@amd.com>
>> ---
>> xen/common/device-tree/Makefile | 1 +
>> xen/common/device-tree/bootfdt.c | 97 ++++++++++++++++++++++++
>> xen/common/device-tree/bootinfo-fdt.c | 104 --------------------------
>> 3 files changed, 98 insertions(+), 104 deletions(-)
>> create mode 100644 xen/common/device-tree/bootfdt.c
>>
>> diff --git a/xen/common/device-tree/Makefile
>> b/xen/common/device-tree/Makefile
>> index bb6d5ddec5..922c5bba9b 100644
>> --- a/xen/common/device-tree/Makefile
>> +++ b/xen/common/device-tree/Makefile
>> @@ -1,3 +1,4 @@
>> +obj-y += bootfdt.init.o
>> obj-y += bootinfo-fdt.init.o
>> obj-y += bootinfo.init.o
>> obj-y += device-tree.o
>> diff --git a/xen/common/device-tree/bootfdt.c
>> b/xen/common/device-tree/bootfdt.c
>> new file mode 100644
>> index 0000000000..5decf17faf
>> --- /dev/null
>> +++ b/xen/common/device-tree/bootfdt.c
>> @@ -0,0 +1,97 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#include <xen/bootfdt.h>
>> +#include <xen/bug.h>
>> +#include <xen/lib.h>
>> +#include <xen/libfdt/libfdt.h>
>> +
>> +uint32_t __init device_tree_get_u32(const void *fdt, int node,
>> + const char *prop_name, uint32_t dflt)
>> +{
>> + const struct fdt_property *prop;
>> +
>> + prop = fdt_get_property(fdt, node, prop_name, NULL);
>> + if ( !prop || prop->len < sizeof(u32) )
>> + return dflt;
>> +
>> + return fdt32_to_cpu(*(uint32_t*)prop->data);
>> +}
>> +
>> +int __init device_tree_for_each_node(const void *fdt, int node,
>> + device_tree_node_func func,
>> + void *data)
>> +{
>> + /*
>> + * We only care about relative depth increments, assume depth of
>> + * node is 0 for simplicity.
>> + */
>> + int depth = 0;
>> + const int first_node = node;
>> + u32 address_cells[DEVICE_TREE_MAX_DEPTH];
>> + u32 size_cells[DEVICE_TREE_MAX_DEPTH];
>> + int ret;
>> +
>> + do {
>> + const char *name = fdt_get_name(fdt, node, NULL);
>> + u32 as, ss;
>> +
>> + if ( depth >= DEVICE_TREE_MAX_DEPTH )
>> + {
>> + printk("Warning: device tree node `%s' is nested too deep\n",
>> + name);
>
> Use XENLOG_WARNING?
>
>> + continue;
>> + }
>> +
>> + as = depth > 0 ? address_cells[depth-1] :
>> DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
>> + ss = depth > 0 ? size_cells[depth-1] :
>> DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
>> +
>> + address_cells[depth] = device_tree_get_u32(fdt, node,
>> + "#address-cells", as);
>> + size_cells[depth] = device_tree_get_u32(fdt, node,
>> + "#size-cells", ss);
>> +
>> + /* skip the first node */
>> + if ( node != first_node )
>> + {
>> + ret = func(fdt, node, name, depth, as, ss, data);
>> + if ( ret != 0 )
>> + return ret;
>> + }
>> +
>> + node = fdt_next_node(fdt, node, &depth);
>> + } while ( node >= 0 && depth > 0 );
>> +
>> + return 0;
>> +}
>> +
>> +void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
>> + uint32_t size_cells, paddr_t *start,
>> + paddr_t *size)
>> +{
>> + uint64_t dt_start, dt_size;
>> +
>> + /*
>> + * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
>> + * Thus, there is an implicit cast from uint64_t to paddr_t.
>> + */
>> + dt_start = dt_next_cell(address_cells, cell);
>> + dt_size = dt_next_cell(size_cells, cell);
>> +
>> + if ( dt_start != (paddr_t)dt_start )
>> + {
>> + printk("Physical address greater than max width supported\n");
>
> I would add current and expected dt_start values to the printout.
>
>> + WARN();
>> + }
>> +
>> + if ( dt_size != (paddr_t)dt_size )
>> + {
>> + printk("Physical size greater than max width supported\n");
>> + WARN();
>> + }
>> +
>> + /*
>> + * Xen will truncate the address/size if it is greater than the maximum
>> + * supported width and it will give an appropriate warning.
>> + */
>> + *start = dt_start;
>> + *size = dt_size;
>> +}
>> diff --git a/xen/common/device-tree/bootinfo-fdt.c
>> b/xen/common/device-tree/bootinfo-fdt.c
>> index bb5f45771e..736f877616 100644
>> --- a/xen/common/device-tree/bootinfo-fdt.c
>> +++ b/xen/common/device-tree/bootinfo-fdt.c
>> @@ -112,39 +112,6 @@ static bool __init device_tree_is_memory_node(const
>> void *fdt, int node,
>> return true;
>> }
>>
>> -void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
>> - uint32_t size_cells, paddr_t *start,
>> - paddr_t *size)
>> -{
>> - uint64_t dt_start, dt_size;
>> -
>> - /*
>> - * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
>> - * Thus, there is an implicit cast from uint64_t to paddr_t.
>> - */
>> - dt_start = dt_next_cell(address_cells, cell);
>> - dt_size = dt_next_cell(size_cells, cell);
>> -
>> - if ( dt_start != (paddr_t)dt_start )
>> - {
>> - printk("Physical address greater than max width supported\n");
>> - WARN();
>> - }
>> -
>> - if ( dt_size != (paddr_t)dt_size )
>> - {
>> - printk("Physical size greater than max width supported\n");
>> - WARN();
>> - }
>> -
>> - /*
>> - * Xen will truncate the address/size if it is greater than the maximum
>> - * supported width and it will give an appropriate warning.
>> - */
>> - *start = dt_start;
>> - *size = dt_size;
>> -}
>> -
>> static int __init device_tree_get_meminfo(const void *fdt, int node,
>> const char *prop_name,
>> u32 address_cells, u32 size_cells,
>> @@ -205,77 +172,6 @@ static int __init device_tree_get_meminfo(const void
>> *fdt, int node,
>> return 0;
>> }
>>
>> -u32 __init device_tree_get_u32(const void *fdt, int node,
>> - const char *prop_name, u32 dflt)
>> -{
>> - const struct fdt_property *prop;
>> -
>> - prop = fdt_get_property(fdt, node, prop_name, NULL);
>> - if ( !prop || prop->len < sizeof(u32) )
>> - return dflt;
>> -
>> - return fdt32_to_cpu(*(uint32_t*)prop->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 __init device_tree_for_each_node(const void *fdt, int node,
>> - device_tree_node_func func,
>> - void *data)
>> -{
>> - /*
>> - * We only care about relative depth increments, assume depth of
>> - * node is 0 for simplicity.
>> - */
>> - int depth = 0;
>> - const int first_node = node;
>> - u32 address_cells[DEVICE_TREE_MAX_DEPTH];
>> - u32 size_cells[DEVICE_TREE_MAX_DEPTH];
>> - int ret;
>> -
>> - do {
>> - const char *name = fdt_get_name(fdt, node, NULL);
>> - u32 as, ss;
>> -
>> - if ( depth >= DEVICE_TREE_MAX_DEPTH )
>> - {
>> - printk("Warning: device tree node `%s' is nested too deep\n",
>> - name);
>> - continue;
>> - }
>> -
>> - as = depth > 0 ? address_cells[depth-1] :
>> DT_ROOT_NODE_ADDR_CELLS_DEFAULT;
>> - ss = depth > 0 ? size_cells[depth-1] :
>> DT_ROOT_NODE_SIZE_CELLS_DEFAULT;
>> -
>> - address_cells[depth] = device_tree_get_u32(fdt, node,
>> - "#address-cells", as);
>> - size_cells[depth] = device_tree_get_u32(fdt, node,
>> - "#size-cells", ss);
>> -
>> - /* skip the first node */
>> - if ( node != first_node )
>> - {
>> - ret = func(fdt, node, name, depth, as, ss, data);
>> - if ( ret != 0 )
>> - return ret;
>> - }
>> -
>> - node = fdt_next_node(fdt, node, &depth);
>> - } while ( node >= 0 && depth > 0 );
>> -
>> - return 0;
>> -}
>> -
>> static int __init process_memory_node(const void *fdt, int node,
>> const char *name, int depth,
>> u32 address_cells, u32 size_cells,
>> --
>> 2.43.0
>>
>>
As I answered to Daniel, as I'd rather not introduce functional changes in
code motion commits. I draw the line at trivial retyping (e.g: u32->uint32_t)
Otherwise it's just very confusing to future readers.
Cheers,
Alejandro
