Re: [Xen-devel] [PATCH 07/10] xen: arm: store per-boot module type instead of relying on index

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 16 Jun 2014, Ian Campbell wrote:
>  void __init discard_initial_modules(void)
>  {
>      struct bootmodules *mi = &bootinfo.modules;
>      int i;
>  
> -    for ( i = MOD_DISCARD_FIRST; i <= mi->nr_mods; i++ )
> +    for ( i = 0; i <= mi->nr_mods; i++ )
>      {
>          paddr_t s = mi->module[i].start;
>          paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
>  
> -        dt_unreserved_regions(s, e, init_domheap_pages, 0);
> +        if ( mi->module[i].kind > BOOTMOD_LAST_PRESERVE )
> +            dt_unreserved_regions(s, e, init_domheap_pages, 0);
>      }

Given that we have a nice enum now, it seems like a step back to me
relying on kind > BOOTMOD_LAST_PRESERVE.
I would get rid of BOOTMOD_LAST_PRESERVE and instead check specifically
for BOOTMOD_XEN and BOOTMOD_FDT, that are the only ones we want to
preserve.

I admit that it is more a matter of taste than anything else.

The rest is fine.


>      mi->nr_mods = 0;
> @@ -360,9 +399,7 @@ static paddr_t __init get_xen_paddr(void)
>      printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
>             paddr, paddr + min_size);
>  
> -    bootinfo.modules.module[MOD_XEN].start = paddr;
> -    bootinfo.modules.module[MOD_XEN].size = min_size;
> -
> +    add_boot_module(BOOTMOD_XEN, paddr, min_size, NULL);
>      return paddr;
>  }
>  
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 85aa866..57c98cb 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -5,14 +5,19 @@
>  
>  #define NR_MEM_BANKS 8
>  
> -#define MOD_XEN    0
> -#define MOD_FDT    1
> -#define MOD_KERNEL 2
> -#define MOD_INITRD 3
> -#define MOD_XSM    4
> -#define NR_MODULES 5
> +#define MAX_MODULES 5 /* Current maximum useful modules */
> +
> +typedef enum {
> +    BOOTMOD_XEN,
> +    BOOTMOD_FDT,
> +    /* Everything up to here is not freed after start of day */
> +    BOOTMOD_LAST_PRESERVE = BOOTMOD_FDT,
> +    BOOTMOD_KERNEL,
> +    BOOTMOD_RAMDISK,
> +    BOOTMOD_XSM,
> +    BOOTMOD_UNKNOWN
> +}  bootmodulekind;
>  
> -#define MOD_DISCARD_FIRST MOD_FDT
>  
>  struct membank {
>      paddr_t start;
> @@ -24,16 +29,18 @@ struct meminfo {
>      struct membank bank[NR_MEM_BANKS];
>  };
>  
> +#define BOOTMOD_MAX_CMDLINE 1024
>  struct bootmodule {
> +    bootmodulekind kind;
>      paddr_t start;
>      paddr_t size;
> -    char cmdline[1024];
> +    char cmdline[BOOTMOD_MAX_CMDLINE];
>  };
>  
>  struct bootmodules {
>      int nr_mods;
>      /* Module 0 is Xen itself, followed by the provided modules-proper */
> -    struct bootmodule module[NR_MODULES];
> +    struct bootmodule module[MAX_MODULES];
>  };
>  
>  struct bootinfo {
> @@ -56,6 +63,10 @@ void discard_initial_modules(void);
>  size_t __init boot_fdt_info(const void *fdt, paddr_t paddr);
>  const char __init *boot_fdt_cmdline(const void *fdt);
>  
> +void add_boot_module(bootmodulekind kind, paddr_t start, paddr_t size,
> +                     const char *cmdline);
> +struct bootmodule *boot_module_find_by_kind(bootmodulekind kind);
> +
>  #endif
>  /*
>   * Local variables:
> -- 
> 1.7.10.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel