Re: [Xen-devel] [v3][PATCH 08/16] hvmloader/e820: construct guest e820 table

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

> From: Chen, Tiejun
> Sent: Thursday, June 11, 2015 9:15 AM
> 
> Now we can use that memory map to build our final
> e820 table but it may need to reorder all e820
> entries.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> ---
>  tools/firmware/hvmloader/e820.c | 62
> +++++++++++++++++++++++++++++++----------
>  1 file changed, 48 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c
> index 2e05e93..c39b0aa 100644
> --- a/tools/firmware/hvmloader/e820.c
> +++ b/tools/firmware/hvmloader/e820.c
> @@ -73,7 +73,8 @@ int build_e820_table(struct e820entry *e820,
>                       unsigned int lowmem_reserved_base,
>                       unsigned int bios_image_base)
>  {
> -    unsigned int nr = 0;
> +    unsigned int nr = 0, i, j;
> +    uint64_t low_mem_pgend = hvm_info->low_mem_pgend << PAGE_SHIFT;

You may call it low_mem_end to differentiate from original
low_mem_pgend since one means actual address while the
other means pfn.

> 
>      if ( !lowmem_reserved_base )
>              lowmem_reserved_base = 0xA0000;
> @@ -117,13 +118,6 @@ int build_e820_table(struct e820entry *e820,
>      e820[nr].type = E820_RESERVED;
>      nr++;
> 
> -    /* Low RAM goes here. Reserve space for special pages. */
> -    BUG_ON((hvm_info->low_mem_pgend << PAGE_SHIFT) < (2u << 20));
> -    e820[nr].addr = 0x100000;
> -    e820[nr].size = (hvm_info->low_mem_pgend << PAGE_SHIFT) - e820[nr].addr;
> -    e820[nr].type = E820_RAM;
> -    nr++;
> -
>      /*
>       * Explicitly reserve space for special pages.
>       * This space starts at RESERVED_MEMBASE an extends to cover various
> @@ -159,16 +153,56 @@ int build_e820_table(struct e820entry *e820,
>          nr++;
>      }
> 
> -
> -    if ( hvm_info->high_mem_pgend )
> +    /*
> +     * Construct the remaining according memory_map.

"Construct E820 table according to recorded memory map"

> +     *
> +     * Note memory_map includes,

The memory map created by toolstack may include:

> +     *
> +     * #1. Low memory region
> +     *
> +     * Low RAM starts at least from 1M to make sure all standard regions
> +     * of the PC memory map, like BIOS, VGA memory-mapped I/O and vgabios,
> +     * have enough space.
> +     *
> +     * #2. RDM region if it exists

"Reserved regions if they exist"

> +     *
> +     * #3. High memory region if it exists
> +     */
> +    for ( i = 0; i < memory_map.nr_map; i++ )
>      {
> -        e820[nr].addr = ((uint64_t)1 << 32);
> -        e820[nr].size =
> -            ((uint64_t)hvm_info->high_mem_pgend << PAGE_SHIFT) - 
> e820[nr].addr;
> -        e820[nr].type = E820_RAM;
> +        e820[nr] = memory_map.map[i];
>          nr++;
>      }
> 
> +    /* Low RAM goes here. Reserve space for special pages. */
> +    BUG_ON(low_mem_pgend < (2u << 20));
> +    /*
> +     * We may need to adjust real lowmem end since we may
> +     * populate RAM to get enough MMIO previously.
> +     */
> +    for ( i = 0; i < memory_map.nr_map; i++ )

since you already translate memory map into e820 earlier, here
you should use 'nr' instead of memory_map.nr_map.

> +    {
> +        uint64_t end = e820[i].addr + e820[i].size;
> +        if ( e820[i].type == E820_RAM &&
> +             low_mem_pgend > e820[i].addr && low_mem_pgend < end )
> +            e820[i].size = low_mem_pgend - e820[i].addr;
> +    }

Sorry I may miss the code but could you elaborate where the
low_mem_pgend is changed after memory map is created? If
it happens within hvmloader, suppose the amount of reduced
memory from original E820_RAM entry should be added to
another E820_RAM entry for highmem, right?

> +
> +    /* Finally we need to reorder all e820 entries. */
> +    for ( j = 0; j < nr-1; j++ )
> +    {
> +        for ( i = j+1; i < nr; i++ )
> +        {
> +            if ( e820[j].addr > e820[i].addr )
> +            {
> +                struct e820entry tmp;
> +                tmp = e820[j];
> +                e820[j] = e820[i];
> +                e820[i] = tmp;
> +            }
> +        }
> +    }
> +
>      return nr;
>  }
> 
> --
> 1.9.1


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