Re: [PATCH v5 01/10] x86: Replace arch-specific boot_module with common one

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.07.2025 12:56, Alejandro Vallejo wrote:
> These types resemble each other very closely in layout and intent,
> and with "struct boot_module" already in common code it makes perfect
> sense to merge them. In order to do so, add an arch-specific area for
> x86-specific tidbits, and rename identical fields with conflicting
> names.
> 
> No functional change intended.
> 
> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

I'm largely okay with this change, just one question:

> --- a/xen/include/xen/bootfdt.h
> +++ b/xen/include/xen/bootfdt.h
> @@ -7,6 +7,10 @@
>  #include <xen/macros.h>
>  #include <xen/xmalloc.h>
>  
> +#if __has_include(<asm/bootfdt.h>)
> +#include <asm/bootfdt.h>
> +#endif
> +
>  #define MIN_FDT_ALIGN 8
>  
>  #define NR_MEM_BANKS 256
> @@ -108,6 +112,10 @@ struct boot_module {
>      bool domU;
>      paddr_t start;
>      paddr_t size;
> +
> +#if __has_include(<asm/bootfdt.h>)
> +    struct arch_boot_module arch;
> +#endif
>  };

The pre-existing domU field isn't used by x86. Shouldn't that move into Arm's
(to be created) struct arch_boot_module? Or is that intended to become
dependent upon CONFIG_DOM0LESS_BOOT? (While we apparently didn't adopt Misra
rule 2.2, this is imo precisely the situation where we would better follow it:
An unused struct field raises questions.)

Jan