Re: [PATCH] xen/arm: use defines for boot module indexes instead of open coded numbers

[Julien Grall <julien.grall@linaro.org>]

On 08/22/2013 04:24 PM, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

> ---
>  xen/arch/arm/domain_build.c   |    6 +++---
>  xen/arch/arm/kernel.c         |   10 +++++-----
>  xen/arch/arm/setup.c          |    7 +++----
>  xen/common/device_tree.c      |    4 ++--
>  xen/include/xen/device_tree.h |   10 ++++++++--
>  5 files changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 01492bb..9ca663a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -142,9 +142,9 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>      const char *bootargs = NULL;
>      int prop;
>  
> -    if ( early_info.modules.nr_mods >= 1 &&
> -         early_info.modules.module[1].cmdline[0] )
> -        bootargs = &early_info.modules.module[1].cmdline[0];
> +    if ( early_info.modules.nr_mods >= MOD_KERNEL &&
> +         early_info.modules.module[MOD_KERNEL].cmdline[0] )
> +        bootargs = &early_info.modules.module[MOD_KERNEL].cmdline[0];
>  
>      for ( prop = fdt_first_property_offset(fdt, node);
>            prop >= 0;
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 1417429..f12f895 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -307,10 +307,10 @@ int kernel_prepare(struct kernel_info *info)
>  
>      paddr_t start, size;
>  
> -    if ( early_info.modules.nr_mods > 1 )
> +    if ( early_info.modules.nr_mods > MOD_INITRD )
>          panic("Cannot handle dom0 initrd yet\n");
>  
> -    if ( early_info.modules.nr_mods < 1 )
> +    if ( early_info.modules.nr_mods < MOD_KERNEL )
>      {

Not related to this patch. Do we have a reason to keep the flash
workaround in Xen?

>          printk("No boot modules found, trying flash\n");
>          start = KERNEL_FLASH_ADDRESS;
> @@ -319,9 +319,9 @@ int kernel_prepare(struct kernel_info *info)
>      }
>      else
>      {
> -        printk("Loading kernel from boot module 1\n");
> -        start = early_info.modules.module[1].start;
> -        size = early_info.modules.module[1].size;
> +        printk("Loading kernel from boot module %d\n", MOD_KERNEL);
> +        start = early_info.modules.module[MOD_KERNEL].start;
> +        size = early_info.modules.module[MOD_KERNEL].size;
>          info->load_attr = BUFFERABLE;
>      }
>  
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e82df88..afdb290 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -152,7 +152,7 @@ void __init discard_initial_modules(void)
>      struct dt_module_info *mi = &early_info.modules;
>      int i;
>  
> -    for ( i = 1; i <= mi->nr_mods; i++ )
> +    for ( i = MOD_DISCARD_FIRST; i <= mi->nr_mods; i++ )
>      {
>          paddr_t s = mi->module[i].start;
>          paddr_t e = s + PAGE_ALIGN(mi->module[i].size);
> @@ -277,9 +277,8 @@ static paddr_t __init get_xen_paddr(void)
>      early_printk("Placing Xen at 0x%"PRIpaddr"-0x%"PRIpaddr"\n",
>                   paddr, paddr + min_size);
>  
> -    /* Xen is module 0 */
> -    early_info.modules.module[0].start = paddr;
> -    early_info.modules.module[0].size = min_size;
> +    early_info.modules.module[MOD_XEN].start = paddr;
> +    early_info.modules.module[MOD_XEN].size = min_size;
>  
>      return paddr;
>  }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 1249985..3a3c99c 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -445,9 +445,9 @@ static void __init process_multiboot_node(const void *fdt, int node,
>      int len;
>  
>      if ( fdt_node_check_compatible(fdt, node, "xen,linux-zimage") == 0 )
> -        nr = 1;
> +        nr = MOD_KERNEL;
>      else if ( fdt_node_check_compatible(fdt, node, "xen,linux-initrd") == 0)
> -        nr = 2;
> +        nr = MOD_INITRD;
>      else
>          early_panic("%s not a known xen multiboot type\n", name);
>  
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index faf727f..402cef2 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -19,7 +19,13 @@
>  #define DEVICE_TREE_MAX_DEPTH 16
>  
>  #define NR_MEM_BANKS 8
> -#define NR_MODULES 2
> +
> +#define MOD_XEN 0
> +#define MOD_KERNEL 1
> +#define MOD_INITRD 2
> +#define NR_MODULES 3
> +
> +#define MOD_DISCARD_FIRST MOD_KERNEL
>  
>  struct membank {
>      paddr_t start;
> @@ -40,7 +46,7 @@ struct dt_mb_module {
>  struct dt_module_info {
>      int nr_mods;
>      /* Module 0 is Xen itself, followed by the provided modules-proper */
> -    struct dt_mb_module module[NR_MODULES + 1];
> +    struct dt_mb_module module[NR_MODULES];
>  };
>  
>  struct dt_early_info {
> 


-- 
Julien Grall