[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] xen/arm: Add memory overlap check for bootinfo.reserved_mem



On Wed, 14 Dec 2022, Henry Wang wrote:
> As we are having more and more types of static region, and all of
> these static regions are defined in bootinfo.reserved_mem, it is
> necessary to add the overlap check of reserved memory regions in Xen,
> because such check will help user to identify the misconfiguration in
> the device tree at the early stage of boot time.
> 
> Currently we have 3 types of static region, namely
> (1) static memory
> (2) static heap
> (3) static shared memory
> 
> (1) and (2) are parsed by the function `device_tree_get_meminfo()` and
> (3) is parsed using its own logic. All of parsed information of these
> types will be stored in `struct meminfo`.
> 
> Therefore, to unify the overlap checking logic for all of these types,
> this commit firstly introduces a helper `meminfo_overlap_check()` and
> a function `check_reserved_regions_overlap()` to check if an input
> physical address range is overlapping with the existing memory regions
> defined in bootinfo. After that, use `check_reserved_regions_overlap()`
> in `device_tree_get_meminfo()` to do the overlap check of (1) and (2)
> and replace the original overlap check of (3) with
> `check_reserved_regions_overlap()`.
> 
> Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> v1 -> v2:
> 1. Split original `overlap_check()` to `meminfo_overlap_check()`.
> 2. Rework commit message.
> ---
>  xen/arch/arm/bootfdt.c           | 13 +++++-----
>  xen/arch/arm/include/asm/setup.h |  2 ++
>  xen/arch/arm/setup.c             | 42 ++++++++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 0085c28d74..e2f6c7324b 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -88,6 +88,9 @@ static int __init device_tree_get_meminfo(const void *fdt, 
> int node,
>      for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
>      {
>          device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> +        if ( mem == &bootinfo.reserved_mem &&
> +             check_reserved_regions_overlap(start, size) )
> +            return -EINVAL;
>          /* Some DT may describe empty bank, ignore them */
>          if ( !size )
>              continue;
> @@ -482,7 +485,9 @@ static int __init process_shm_node(const void *fdt, int 
> node,
>                  return -EINVAL;
>              }
>  
> -            if ( (end <= mem->bank[i].start) || (paddr >= bank_end) )
> +            if ( check_reserved_regions_overlap(paddr, size) )
> +                return -EINVAL;
> +            else
>              {
>                  if ( strcmp(shm_id, mem->bank[i].shm_id) != 0 )
>                      continue;
> @@ -493,12 +498,6 @@ static int __init process_shm_node(const void *fdt, int 
> node,
>                      return -EINVAL;
>                  }
>              }
> -            else
> -            {
> -                printk("fdt: shared memory region overlap with an existing 
> entry %#"PRIpaddr" - %#"PRIpaddr"\n",
> -                        mem->bank[i].start, bank_end);
> -                return -EINVAL;
> -            }
>          }
>      }
>  
> diff --git a/xen/arch/arm/include/asm/setup.h 
> b/xen/arch/arm/include/asm/setup.h
> index fdbf68aadc..6a9f88ecbb 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -143,6 +143,8 @@ void fw_unreserved_regions(paddr_t s, paddr_t e,
>  size_t boot_fdt_info(const void *fdt, paddr_t paddr);
>  const char *boot_fdt_cmdline(const void *fdt);
>  
> +int check_reserved_regions_overlap(paddr_t region_start, paddr_t 
> region_size);
> +
>  struct bootmodule *add_boot_module(bootmodule_kind kind,
>                                     paddr_t start, paddr_t size, bool domU);
>  struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 1f26f67b90..e6eeb3a306 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -261,6 +261,31 @@ static void __init dt_unreserved_regions(paddr_t s, 
> paddr_t e,
>      cb(s, e);
>  }
>  
> +static int __init meminfo_overlap_check(struct meminfo *meminfo,
> +                                        paddr_t region_start,
> +                                        paddr_t region_end)
> +{
> +    paddr_t bank_start = INVALID_PADDR, bank_end = 0;
> +    unsigned int i, bank_num = meminfo->nr_banks;
> +
> +    for ( i = 0; i < bank_num; i++ )
> +    {
> +        bank_start = meminfo->bank[i].start;
> +        bank_end = bank_start + meminfo->bank[i].size;
> +
> +        if ( region_end <= bank_start || region_start >= bank_end )
> +            continue;
> +        else
> +        {
> +            printk("Region %#"PRIpaddr" - %#"PRIpaddr" overlapping with 
> bank[%u] %#"PRIpaddr" - %#"PRIpaddr"\n",
> +                   region_start, region_end, i, bank_start, bank_end);
> +            return -EINVAL;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  void __init fw_unreserved_regions(paddr_t s, paddr_t e,
>                                    void (*cb)(paddr_t, paddr_t),
>                                    unsigned int first)
> @@ -271,7 +296,24 @@ void __init fw_unreserved_regions(paddr_t s, paddr_t e,
>          cb(s, e);
>  }
>  
> +/*
> + * Given an input physical address range, check if this range is overlapping
> + * with the existing reserved memory regions defined in bootinfo.
> + * Return 0 if the input physical address range is not overlapping with any
> + * existing reserved memory regions, otherwise -EINVAL.
> + */
> +int __init check_reserved_regions_overlap(paddr_t region_start,
> +                                          paddr_t region_size)
> +{
> +    paddr_t region_end = region_start + region_size;
> +
> +    /* Check if input region is overlapping with bootinfo.reserved_mem banks 
> */
> +    if ( meminfo_overlap_check(&bootinfo.reserved_mem,
> +                               region_start, region_end) )
> +        return -EINVAL;
>  
> +    return 0;
> +}
>  
>  struct bootmodule __init *add_boot_module(bootmodule_kind kind,
>                                            paddr_t start, paddr_t size,
> -- 
> 2.25.1
> 



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.