xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/hvmloader: sync memory map[]
@ 2015-07-28  7:27 Tiejun Chen
  2015-07-28 10:49 ` Andrew Cooper
  0 siblings, 1 reply; 3+ messages in thread
From: Tiejun Chen @ 2015-07-28  7:27 UTC (permalink / raw)
  To: xen-devel
  Cc: Keir Fraser, Ian Campbell, Stefano Stabellini, George Dunlap,
	Andrew Cooper, Ian Jackson, Jan Beulich, Wei Liu

Currently we always use memory map[] to help hvmloader construct e820 table
but hvmloader may have relocated RAM to support mmio allocation or just
populated ram to ensure we can have enough room to load ovmf. Anyway we
need to sync these changes into memory map[].

CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 tools/firmware/hvmloader/e820.c | 105 ++++++++++++++++++----------------------
 tools/firmware/hvmloader/pci.c  |   3 ++
 tools/firmware/hvmloader/util.c |   3 ++
 tools/firmware/hvmloader/util.h |   3 ++
 4 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/tools/firmware/hvmloader/e820.c b/tools/firmware/hvmloader/e820.c
index a6cacdf..f4ccacb 100644
--- a/tools/firmware/hvmloader/e820.c
+++ b/tools/firmware/hvmloader/e820.c
@@ -55,6 +55,54 @@ void memory_map_setup(void)
     }
 }
 
+/*
+ * Sometimes hvmloader may have relocated RAM so low_mem_pgend/high_mem_end
+ * would be changed over there. But memory_map[] just records the
+ * original low/high memory, so we need to sync these entries once
+ * hvmloader modifies low/high memory.
+ */
+void adjust_memory_map(void)
+{
+    uint32_t low_mem_end = hvm_info->low_mem_pgend << PAGE_SHIFT;
+    uint64_t high_mem_end = (uint64_t)hvm_info->high_mem_pgend << PAGE_SHIFT;
+    unsigned int i;
+
+    for ( i = 0; i < memory_map.nr_map; i++ )
+    {
+        uint64_t map_start = memory_map.map[i].addr;
+        uint64_t map_size = memory_map.map[i].size;
+        uint64_t 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 )
+        {
+            memory_map.map[i].size = low_mem_end - map_start;
+            continue;
+        }
+
+        /* Modify the existing highmem region if it exists. */
+        if ( memory_map.map[i].type == E820_RAM &&
+             high_mem_end && map_start == ((uint64_t)1 << 32) )
+        {
+            if ( high_mem_end != map_end )
+                memory_map.map[i].size = high_mem_end - map_start;
+            high_mem_end = 0;
+            continue;
+        }
+    }
+
+    /* If there was no highmem region, just create one. */
+    if ( high_mem_end )
+    {
+        memory_map.map[i].addr = ((uint64_t)1 << 32);
+        memory_map.map[i].size =
+                ((uint64_t)hvm_info->high_mem_pgend << PAGE_SHIFT) -
+                    memory_map.map[i].addr;
+        memory_map.map[i].type = E820_RAM;
+    }
+}
+
 void dump_e820_table(struct e820entry *e820, unsigned int nr)
 {
     uint64_t last_end = 0, start, end;
@@ -107,9 +155,6 @@ int build_e820_table(struct e820entry *e820,
 {
     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;
@@ -208,63 +253,9 @@ int build_e820_table(struct e820entry *e820,
      *
      * 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++ )
     {
-        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++;
     }
diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index ff81fba..69b98d6 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -383,6 +383,9 @@ void pci_setup(void)
         hvm_info->high_mem_pgend += nr_pages;
     }
 
+    /* Sync memory map[] if necessary. */
+    adjust_memory_map();
+
     high_mem_resource.base = ((uint64_t)hvm_info->high_mem_pgend) << PAGE_SHIFT;
     if ( high_mem_resource.base < 1ull << 32 )
     {
diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 122e3fa..32ddcd0 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -432,6 +432,9 @@ void mem_hole_populate_ram(xen_pfn_t mfn, uint32_t nr_mfns)
         if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
             BUG();
     }
+
+    /* Sync memory map[]. */
+    adjust_memory_map();
 }
 
 static uint32_t alloc_up = RESERVED_MEMORY_DYNAMIC_START - 1;
diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 1100a3b..132d915 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -227,6 +227,9 @@ void pci_setup(void);
 /* Setup memory map  */
 void memory_map_setup(void);
 
+/* Sync memory map */
+void adjust_memory_map(void);
+
 /* Prepare the 32bit BIOS */
 uint32_t rombios_highbios_setup(void);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/hvmloader: sync memory map[]
  2015-07-28  7:27 [PATCH] tools/hvmloader: sync memory map[] Tiejun Chen
@ 2015-07-28 10:49 ` Andrew Cooper
  2015-07-28 13:27   ` Ian Campbell
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2015-07-28 10:49 UTC (permalink / raw)
  To: Tiejun Chen, xen-devel
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, George Dunlap,
	Ian Jackson, Jan Beulich, Keir Fraser

On 28/07/15 08:27, Tiejun Chen wrote:
> Currently we always use memory map[] to help hvmloader construct e820 table
> but hvmloader may have relocated RAM to support mmio allocation or just
> populated ram to ensure we can have enough room to load ovmf. Anyway we
> need to sync these changes into memory map[].
>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools/hvmloader: sync memory map[]
  2015-07-28 10:49 ` Andrew Cooper
@ 2015-07-28 13:27   ` Ian Campbell
  0 siblings, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2015-07-28 13:27 UTC (permalink / raw)
  To: Andrew Cooper, Tiejun Chen, xen-devel
  Cc: Keir Fraser, Stefano Stabellini, George Dunlap, Ian Jackson,
	Jan Beulich, Wei Liu

On Tue, 2015-07-28 at 11:49 +0100, Andrew Cooper wrote:
> On 28/07/15 08:27, Tiejun Chen wrote:
> > Currently we always use memory map[] to help hvmloader construct e820 
> > table
> > but hvmloader may have relocated RAM to support mmio allocation or just
> > populated ram to ensure we can have enough room to load ovmf. Anyway we
> > need to sync these changes into memory map[].
> > 
> > CC: Keir Fraser <keir@xen.org>
> > CC: Jan Beulich <jbeulich@suse.com>
> > CC: Andrew Cooper <andrew.cooper3@citrix.com>
> > CC: Ian Jackson <ian.jackson@eu.citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: George Dunlap <george.dunlap@eu.citrix.com>
> > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Applied, thanks.

Ian.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-07-28 13:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-28  7:27 [PATCH] tools/hvmloader: sync memory map[] Tiejun Chen
2015-07-28 10:49 ` Andrew Cooper
2015-07-28 13:27   ` Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).