Re: [Xen-devel] [v8][PATCH 06/17] tools/libxc: check if modules space is overlapping with reserved device memory

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, Dec 01, 2014 at 05:24:24PM +0800, Tiejun Chen wrote:
> In case of reserved device memory overlapping with ram, it also probably

s/also//
> overlap with modules space so we need to check these reserved device
s/overlap/overlaps/

What is 'modules space'?

> memory as well.

s/reserved device memory/E820_RSV/ ?

> 
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> ---
>  tools/libxc/xc_hvm_build_x86.c | 94 
> +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 79 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
> index c81a25b..ddcf06d 100644
> --- a/tools/libxc/xc_hvm_build_x86.c
> +++ b/tools/libxc/xc_hvm_build_x86.c
> @@ -54,9 +54,82 @@
>  
>  #define VGA_HOLE_SIZE (0x20)
>  
> +/*
> + * Check whether there exists mmio hole in the specified memory range.
> + * Returns 1 if exists, else returns 0.
> + */
> +static int check_mmio_hole(uint64_t start, uint64_t memsize,
> +                           uint64_t mmio_start, uint64_t mmio_size)
> +{
> +    if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size )
> +        return 0;
> +    else
> +        return 1;
> +}
> +
> +/* Getting all reserved device memory map info. */
> +static struct xen_reserved_device_memory
> +*xc_get_reserved_device_memory_map(xc_interface *xch, unsigned int 
> nr_entries,
> +                                   uint32_t dom)
> +{
> +    struct xen_reserved_device_memory *xrdm = NULL;
> +    int rc = xc_reserved_device_memory_map(xch, dom, xrdm, &nr_entries);
> +
> +    if ( rc < 0 )
> +    {
> +        if ( errno == ENOBUFS )
> +        {
> +            if ( (xrdm = malloc(nr_entries *
> +                                sizeof(xen_reserved_device_memory_t))) == 
> NULL )
> +            {
> +                PERROR("Could not allocate memory.");
> +                return 0;
> +            }
> +            rc = xc_reserved_device_memory_map(xch, dom, xrdm, &nr_entries);
> +            if ( rc )
> +            {
> +                PERROR("Could not get reserved device memory maps.");
> +                free(xrdm);
> +                return 0;

Uhhh, is that the right error to return?

Don't you mean ERR_PTR logic? Or 'return NULL' ?


> +            }
> +        }
> +        else
> +            PERROR("Could not get reserved device memory maps.");
> +    }
> +
> +    return xrdm;
> +}
> +
> +static int xc_check_modules_space(xc_interface *xch, uint64_t *mstart_out,
> +                                  uint64_t *mend_out, uint32_t dom)
> +{
> +    unsigned int i = 0, nr_entries = 0;
> +    uint64_t rdm_start = 0, rdm_end = 0;
> +    struct xen_reserved_device_memory *rdm_map =
> +                        xc_get_reserved_device_memory_map(xch, nr_entries, 
> dom);
> +

You need to check whether 'rdm_map' is NULL.

> +    for ( i = 0; i < nr_entries; i++ )
> +    {
> +        rdm_start = (uint64_t)rdm_map[i].start_pfn << XC_PAGE_SHIFT;
> +        rdm_end = rdm_start + ((uint64_t)rdm_map[i].nr_pages << 
> XC_PAGE_SHIFT);
> +
> +        /* Just use check_mmio_hole() to check modules ranges. */
> +        if ( check_mmio_hole(rdm_start,
> +                             rdm_end - rdm_start,
> +                             *mstart_out, *mend_out) )
> +            return -1;
> +    }
> +    
> +    free(rdm_map);
> +
> +    return 0;
> +}
> +
>  static int modules_init(struct xc_hvm_build_args *args,
>                          uint64_t vend, struct elf_binary *elf,
> -                        uint64_t *mstart_out, uint64_t *mend_out)
> +                        uint64_t *mstart_out, uint64_t *mend_out,
> +                        xc_interface *xch,
> +                        uint32_t dom)
>  {
>  #define MODULE_ALIGN 1UL << 7
>  #define MB_ALIGN     1UL << 20
> @@ -80,6 +153,10 @@ static int modules_init(struct xc_hvm_build_args *args,
>      if ( *mend_out > vend )    
>          return -1;
>  
> +    /* Is it overlapping with reserved device memory? */
> +    if ( xc_check_modules_space(xch, mstart_out, mend_out, dom) )
> +        return -1;
> +
>      if ( args->acpi_module.length != 0 )
>          args->acpi_module.guest_addr_out = *mstart_out;
>      if ( args->smbios_module.length != 0 )
> @@ -226,19 +303,6 @@ static int loadmodules(xc_interface *xch,
>      return rc;
>  }
>  
> -/*
> - * Check whether there exists mmio hole in the specified memory range.
> - * Returns 1 if exists, else returns 0.
> - */
> -static int check_mmio_hole(uint64_t start, uint64_t memsize,
> -                           uint64_t mmio_start, uint64_t mmio_size)
> -{
> -    if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size )
> -        return 0;
> -    else
> -        return 1;
> -}
> -

This movement of 'check_mmio_hole' needs to be a seperate patch.

>  static int setup_guest(xc_interface *xch,
>                         uint32_t dom, struct xc_hvm_build_args *args,
>                         char *image, unsigned long image_size)
> @@ -282,7 +346,7 @@ static int setup_guest(xc_interface *xch,
>          goto error_out;
>      }
>  
> -    if ( modules_init(args, v_end, &elf, &m_start, &m_end) != 0 )
> +    if ( modules_init(args, v_end, &elf, &m_start, &m_end, xch, dom) != 0 )
>      {
>          ERROR("Insufficient space to load modules.");
>          goto error_out;
> -- 
> 1.9.1
> 

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