Re: [for-4.20][PATCH 1/2] device-tree: bootfdt: Fix build issue when CONFIG_PHYS_ADDR_T_32=y

[Michal Orzel <michal.orzel@xxxxxxx>]

On 27/01/2025 12:19, Julien Grall wrote:
>       
> 
> 
> 
> 
> On Mon, 27 Jan 2025 at 07:46, Michal Orzel <michal.orzel@xxxxxxx 
> <mailto:michal.orzel@xxxxxxx>> wrote:
> 
>     On Arm32, when CONFIG_PHYS_ADDR_T_32 is set, a build failure is observed:
>     common/device-tree/bootfdt.c: In function 'build_assertions':
>     ./include/xen/macros.h:47:31: error: static assertion failed: 
> "!(alignof(struct membanks) != 8)"
>        47 | #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond 
> ")"); })
>           |                               ^~~~~~~~~~~~~~
>     common/device-tree/bootfdt.c:31:5: note: in expansion of macro 
> 'BUILD_BUG_ON'
>        31 |     BUILD_BUG_ON(alignof(struct membanks) != 8);
> 
>     When CONFIG_PHYS_ADDR_T_32 is set, paddr_t is defined as unsigned long,
>     therefore the struct membanks alignment is 4B. Fix it.
> 
> 
> Usually, we add a BUILD_BUG_ON when other parts of the code rely on a 
> specific property (in this case alignment). Can you explain in the commit 
> message why the new check is still ok?
Well, the change itself reflects the change in alignment requirement.
When paddr_t is 64b (Arm64, Arm32 with PA=40b) the alignment is 8B.
On Arm32 with PA=32b, the alignment is 4B because paddr_t is 4B.

AFAICT you requested this check back then, because struct membanks contains 
flexible array member 'bank' of type struct membank.
The alignment requirement of struct membanks becomes the requirement of struct 
membank whose largest type is paddr_t.
Let me know how you would like to have this written in commit msg.

~Michal