From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v10][PATCH 07/16] hvmloader/e820: construct guest e820 table Date: Mon, 20 Jul 2015 22:35:04 +0800 Message-ID: <55AD0718.4080507@intel.com> References: <1437373023-14884-1-git-send-email-tiejun.chen@intel.com> <1437373023-14884-8-git-send-email-tiejun.chen@intel.com> <55ACFDF902000078000930C9@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55ACFDF902000078000930C9@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Wei Liu , Ian Campbell , Stefano Stabellini , Andrew Cooper , Ian Jackson , xen-devel@lists.xen.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org Looks just a little bit should be changed so I also paste this new online to try winning your Acked here, hvmloader/e820: construct guest e820 table Now use the hypervisor-supplied memory map to build our final e820 table: * Add regions for BIOS ranges and other special mappings not in the hypervisor map * Add in the hypervisor supplied regions * Adjust the lowmem and highmem regions if we've had to relocate memory (adding a highmem region if necessary) * Sort all the ranges so that they appear in memory order. CC: Keir Fraser CC: Jan Beulich CC: Andrew Cooper CC: Ian Jackson CC: Stefano Stabellini CC: Ian Campbell CC: Wei Liu Reviewed-by: George Dunlap Signed-off-by: Tiejun Chen --- tools/firmware/hvmloader/e820.c | 109 +++++++++++++++++++++++++++++++++++----- 1 file changed, 96 insertions(+), 13 deletions(-) diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c index 7a414ab..a6cacdf 100644 --- a/tools/firmware/hvmloader/e820.c +++ b/tools/firmware/hvmloader/e820.c @@ -105,7 +105,11 @@ 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; + uint32_t low_mem_end = hvm_info->low_mem_pgend << PAGE_SHIFT; + uint32_t add_high_mem = 0; + uint64_t high_mem_end = (uint64_t)hvm_info->high_mem_pgend << PAGE_SHIFT; + uint64_t map_start, map_size, map_end; if ( !lowmem_reserved_base ) lowmem_reserved_base = 0xA0000; @@ -149,13 +153,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 @@ -191,16 +188,102 @@ int build_e820_table(struct e820entry *e820, nr++; } + /* Low RAM goes here. Reserve space for special pages. */ + BUG_ON(low_mem_end < (2u << 20)); - if ( hvm_info->high_mem_pgend ) + /* + * Construct E820 table according to recorded memory map. + * + * 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. Reserved regions if they exist + * + * #3. High memory region if it exists + * + * Note we just have one low memory entry and one high mmeory entry if + * exists. + * + * But we may have relocated RAM to allocate sufficient MMIO previously + * so low_mem_pgend would be changed over there. And here memory_map[] + * records the original low/high memory, so if low_mem_end is less than + * the original we need to revise low/high memory range firstly. + */ + 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; + map_start = memory_map.map[i].addr; + map_size = memory_map.map[i].size; + map_end = map_start + map_size; + + /* If we need to adjust lowmem. */ + if ( memory_map.map[i].type == E820_RAM && + low_mem_end > map_start && low_mem_end < map_end ) + { + add_high_mem = map_end - low_mem_end; + memory_map.map[i].size = low_mem_end - map_start; + break; + } + } + + /* If we need to adjust highmem. */ + if ( add_high_mem ) + { + /* Modify the existing highmem region if it exists. */ + for ( i = 0; i < memory_map.nr_map; i++ ) + { + map_start = memory_map.map[i].addr; + map_size = memory_map.map[i].size; + map_end = map_start + map_size; + + if ( memory_map.map[i].type == E820_RAM && + map_start == ((uint64_t)1 << 32)) + { + memory_map.map[i].size += add_high_mem; + break; + } + } + + /* If there was no highmem region, just create one. */ + if ( i == memory_map.nr_map ) + { + memory_map.map[i].addr = ((uint64_t)1 << 32); + memory_map.map[i].size = add_high_mem; + memory_map.map[i].type = E820_RAM; + memory_map.nr_map++; + } + + /* A sanity check if high memory is broken. */ + BUG_ON( high_mem_end != + memory_map.map[i].addr + memory_map.map[i].size); + } + + /* Now fill e820. */ + for ( i = 0; i < memory_map.nr_map; i++ ) + { + e820[nr] = memory_map.map[i]; nr++; } + /* Finally we need to sort 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 = e820[j]; + + e820[j] = e820[i]; + e820[i] = tmp; + } + } + } + return nr; } -- 1.9.1 Thanks Tiejun On 2015/7/20 19:56, Jan Beulich wrote: >>>> On 20.07.15 at 08:16, wrote: >> + /* If there was no highmem region, just create one. */ >> + if ( i == memory_map.nr_map ) >> + { >> + memory_map.map[i].addr = ((uint64_t)1 << 32); >> + memory_map.map[i].size = add_high_mem; >> + memory_map.map[i].type = E820_RAM; > > Don't you need to increment memory_map.nr_map here? > >> + } >> + >> + /* A sanity check if high memory is broken. */ >> + BUG_ON( high_mem_end != >> + memory_map.map[i].addr + memory_map.map[i].size); >> + } >> + >> + /* Now fulfill e820. */ > > s/fulfill/fill/. > >> + /* Finally we need to sort all e820 entries. */ >> + for ( j = 0; j < nr-1; j++ ) >> + { >> + for ( i = j+1; i < nr; i++ ) > > Blanks around binary operators please. > > Jan > >