On Mon, 27 Jan 2025 at 09:52, Michal Orzel <[email protected]> wrote:
> > > On 27/01/2025 12:19, Julien Grall wrote: > > > > > > > > > > > > On Mon, 27 Jan 2025 at 07:46, Michal Orzel <[email protected] > <mailto:[email protected]>> 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. Reading this, it sounds like you want to check against the alignment of « struct membank ». This is because the structure could gain a 64-bit field in the future and this would not be caught by the BUILD_BUG_ON. > Let me know how you would like to have this written in commit msg. I think it needs to be rephrased to make clear the alignment of struct membanks should be the same as struct membank. Cheers, > > ~Michal > >
