Re: [PATCH 09/10] arm setup: use common integer-typed bootmod definition

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 1 Jul 2023, Christopher Clark wrote:
> This change enables inclusion of <xen/bootinfo.h> in Arm builds,
> required for subsequent patches in this series.
> 
> It replaces the enum definition of bootmodule_kind with bootmod_type_t
> as enums are not fixed size as needed for packed structs which are
> needed for boot structures in subsequent changes.
> 
> Signed-off-by: Christopher Clark <christopher.w.clark@xxxxxxxxx>
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> 
> ---
> New for v2 series.
> 
>  xen/arch/arm/include/asm/setup.h | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/setup.h 
> b/xen/arch/arm/include/asm/setup.h
> index 19dc637d55..7e0598217a 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -3,6 +3,7 @@
>  
>  #include <public/version.h>
>  #include <asm/p2m.h>
> +#include <xen/bootinfo.h>

I see this as very positive, but then I do think we should use a single
definition of struct bootmodule, struct bootinfo and struct bootcmdline.

I don't think it is a good idea to have struct bootinfo here and struct
boot_info in xen/include/xen/bootinfo.h with a similar definition and
purpose.

I think we should have a single definition of struct bootinfo and struct
bootmodule that works everywhere.


>  #include <xen/device_tree.h>
>  
>  #define MIN_FDT_ALIGN 8
> @@ -12,15 +13,7 @@
>  
>  #define MAX_MODULES 32 /* Current maximum useful modules */
>  
> -typedef enum {
> -    BOOTMOD_XEN,
> -    BOOTMOD_FDT,
> -    BOOTMOD_KERNEL,
> -    BOOTMOD_RAMDISK,
> -    BOOTMOD_XSM,
> -    BOOTMOD_GUEST_DTB,
> -    BOOTMOD_UNKNOWN
> -}  bootmodule_kind;
> +typedef bootmod_type_t bootmodule_kind;

Why can't we use an enum for it?