RE: [PATCH 2/2] fdt: make fdt handling reusable across arch

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Daniel,

> -----Original Message-----
> Subject: [PATCH 2/2] fdt: make fdt handling reusable across arch
> 
> This refactors reusable code from Arm's bootfdt.c and device-tree.h that is
> general fdt handling code.  The Kconfig parameter CORE_DEVICE_TREE is
> introduced for when the ability of parsing DTB files is needed by a capability
> such as hyperlaunch.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> ---
>  MAINTAINERS                   |   2 +
>  xen/arch/arm/bootfdt.c        | 141 +------------------------------
>  xen/common/Kconfig            |   4 +
>  xen/common/Makefile           |   3 +-
>  xen/common/fdt.c              | 153 ++++++++++++++++++++++++++++++++++
>  xen/include/xen/device_tree.h |  50 +----------
>  xen/include/xen/fdt.h         |  79 ++++++++++++++++++
>  7 files changed, 242 insertions(+), 190 deletions(-)
>  create mode 100644 xen/common/fdt.c
>  create mode 100644 xen/include/xen/fdt.h
> 
> +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;
> +}
> +
> +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);
> +}

When I tested this patch by our internal CI (I applied this patch on top
of today's staging:
c2026b88b5 xen/arm/IRQ: uniform irq_set_affinity() with x86 version),
there are some build errors as below:

aarch64-linux-gnu-gcc -MMD -MP -MF ./.asm-offsets.s.d -DBUILD_ID 
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes 
-Wdeclaration-after-statement -Wno-unused-but-set-variable 
-Wno-unused-local-typedefs -O2 -fomit-frame-pointer -nostdinc -fno-builtin 
-fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ 
-include /jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/config.h 
-mcpu=generic -mgeneral-regs-only -mno-outline-atomics -I./include 
-I./arch/arm/include 
-I/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include 
-I/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include -fno-pie 
-fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables 
-Wnested-externs -S -g0 -o asm-offsets.s.new -MQ asm-offsets.s 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c
In file included from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:13:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/setup.h:162:6:
 error: redundant redeclaration of 'device_tree_get_reg' 
[-Werror=redundant-decls]
  162 | void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
      |      ^~~~~~~~~~~~~~~~~~~
In file included from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/device_tree.h:17,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/smp.h:6,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/smp.h:4,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/rwlock.h:6,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/sched.h:7,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:9:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/fdt.h:64:13: 
note: previous declaration of 'device_tree_get_reg' with type 'void(const 
__be32 **, u32,  u32,  u64 *, u64 *)' {aka 'void(const unsigned int **, 
unsigned int,  unsigned int,  long unsigned int *, long unsigned int *)'}
   64 | void __init device_tree_get_reg(
      |             ^~~~~~~~~~~~~~~~~~~
In file included from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:13:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/setup.h:165:5:
 error: redundant redeclaration of 'device_tree_get_u32' 
[-Werror=redundant-decls]
  165 | u32 device_tree_get_u32(const void *fdt, int node,
      |     ^~~~~~~~~~~~~~~~~~~
In file included from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/device_tree.h:17,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/smp.h:6,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/smp.h:4,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/rwlock.h:6,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/sched.h:7,
                 from 
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:9:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/fdt.h:68:12: 
note: previous declaration of 'device_tree_get_u32' with type 'u32(const void 
*, int,  const char *, u32)' {aka 'unsigned int(const void *, int,  const char 
*, unsigned int)'}
   68 | u32 __init device_tree_get_u32(
      |            ^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[2]: *** [/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/./build.mk:45: 
asm-offsets.s] Error 1
make[1]: *** [/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/Makefile:597: 
xen] Error 2
make[1]: Leaving directory 
'/jenkins/workspace/ais-xenbits-xen/build/xen-fvp-base'
make: *** [Makefile:183: __sub-make] Error 2
make: Leaving directory '/jenkins/workspace/ais-xenbits-xen/layers/xen/xen'

The staging itself passed the CI check. Did I miss some context to make
this patch works? Could you please provide some hints? Thanks in advance!

Kind regards,
Henry