On 8/2/23 02:13, Henry Wang wrote:
Hi Daniel,
Hey Henry!
-----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 <dpsm...@apertussolutions.com> --- 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!
Nope you are correct, I now need to go back and look if I sent patches out of the wrong branch or if I really did miss cleaning up the original declarations in the Arm tree. I fairly confident I made sure gitlab-ci passed before I cut the patches for sending, but since I had three different series in flight, I may have gotten jumbled. Apologies for the unnecessary churn here.
V/r, Daniel P. Smith
