Re: [Xen-devel] [RFC][PATCH 4/5] tools:firmware:hvmloader: reserve RMRR mappings in e820

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Chen, Tiejun
> Sent: Thursday, August 07, 2014 4:03 AM
> 
> We need to reserve all RMRR mappings in e820 to avoid any
> potential guest memory conflict.

strictly speaking besides reserving in e820, you should also poke later
MMIO BAR allocations to avoid confliction too. Currently it's relative
to low_mem_pgend, which is likely to be different from host layout
so it's still possible to see a virtual MMIO bar base conflicting to the
RMRR ranges which are supposed to be sparse.

Thanks
Kevin

> 
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> ---
>  tools/firmware/hvmloader/e820.c | 14 ++++++++++++++
>  tools/firmware/hvmloader/e820.h |  6 ++++++
>  tools/firmware/hvmloader/util.c | 13 +++++++++++++
>  tools/firmware/hvmloader/util.h |  1 +
>  4 files changed, 34 insertions(+)
> 
> diff --git a/tools/firmware/hvmloader/e820.c
> b/tools/firmware/hvmloader/e820.c
> index 2e05e93..8cf8c75 100644
> --- a/tools/firmware/hvmloader/e820.c
> +++ b/tools/firmware/hvmloader/e820.c
> @@ -74,6 +74,8 @@ int build_e820_table(struct e820entry *e820,
>                       unsigned int bios_image_base)
>  {
>      unsigned int nr = 0;
> +    struct e820map *e820_rmrr_map;
> +    unsigned int i = 0;
> 
>      if ( !lowmem_reserved_base )
>              lowmem_reserved_base = 0xA0000;
> @@ -124,6 +126,18 @@ int build_e820_table(struct e820entry *e820,
>      e820[nr].type = E820_RAM;
>      nr++;
> 
> +    /* We'd better reserve RMRR mapping for each VM to avoid potential
> +     * memory conflict.
> +     */
> +    e820_rmrr_map = get_rmrr_map_info();
> +    for ( i = 0; i <= e820_rmrr_map->nr_map; i++ )
> +    {
> +        e820[nr].addr = e820_rmrr_map->map[i].addr;
> +        e820[nr].size = e820_rmrr_map->map[i].size + 1;
> +        e820[nr].type = E820_RESERVED;
> +        nr++;
> +    }
> +
>      /*
>       * Explicitly reserve space for special pages.
>       * This space starts at RESERVED_MEMBASE an extends to cover
> various
> diff --git a/tools/firmware/hvmloader/e820.h
> b/tools/firmware/hvmloader/e820.h
> index b2ead7f..ae4cc55 100644
> --- a/tools/firmware/hvmloader/e820.h
> +++ b/tools/firmware/hvmloader/e820.h
> @@ -15,6 +15,12 @@ struct e820entry {
>      uint32_t type;
>  } __attribute__((packed));
> 
> +#define E820MAX 128
> +
> +struct e820map {
> +    int nr_map;
> +    struct e820entry map[E820MAX];
> +};
>  #endif /* __HVMLOADER_E820_H__ */
> 
>  /*
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index 80d822f..270700f 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -766,6 +766,19 @@ struct shared_info *get_shared_info(void)
>      return shared_info;
>  }
> 
> +struct e820map *get_rmrr_map_info(void)
> +{
> +    static struct e820map *e820_map = NULL;
> +
> +    if ( e820_map != NULL )
> +        return e820_map;
> +
> +    if ( hypercall_memory_op(XENMEM_RMRR_memory_map,
> e820_map) != 0 )
> +        BUG();
> +
> +    return e820_map;
> +}
> +
>  uint16_t get_cpu_mhz(void)
>  {
>      struct shared_info *shared_info = get_shared_info();
> diff --git a/tools/firmware/hvmloader/util.h
> b/tools/firmware/hvmloader/util.h
> index a70e4aa..26bfb8c 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -236,6 +236,7 @@ unsigned long create_pir_tables(void);
>  void smp_initialise(void);
> 
>  #include "e820.h"
> +struct e820map *get_rmrr_map_info(void);
>  int build_e820_table(struct e820entry *e820,
>                       unsigned int lowmem_reserved_base,
>                       unsigned int bios_image_base);
> --
> 1.9.1


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