Now we can use that memory map to build our final
e820 table but it may need to reorder all e820
entries.
"it" being what? I'm afraid I can't really make sense of the second
half of the sentence...
I hope the following can work for you,
...
but finally we should sort them into an increasing order since
we shouldn't assume the original order is always good.
--- a/tools/firmware/hvmloader/e820.c
+++ b/tools/firmware/hvmloader/e820.c
@@ -108,7 +108,9 @@ int build_e820_table(struct e820entry *e820,
unsigned int lowmem_reserved_base,
unsigned int bios_image_base)
[snip]
+ */
+ for ( i = 0; i < memory_map.nr_map; i++ )
+ {
+ uint64_t end = e820[i].addr + e820[i].size;
Either loop index/boundary or used array are wrong here: In the
earlier loop you copied memory_map[0...nr_map-1] to
e820[n...n+nr_map-1], but here you're looping looking at
e820[0...nr_map-1]
You're right. I should lookup all e820[] like this,
for ( i = 0; i < nr; i++ )
+ if ( e820[i].type == E820_RAM &&
+ low_mem_end > e820[i].addr && low_mem_end < end )
Assuming you mean to look at the RDM e820[] entries here, this
is not a correct check: You don't care about partly or fully
contained, all you care about is whether low_mem_end extends
beyond the start of the region.
Here I'm looking at the e820 entry indicating low memory. Because
low_mem_end = hvm_info->low_mem_pgend << PAGE_SHIFT;
and when we allocate MMIO in pci.c, its possible to populate RAM so
hvm_info->low_mem_pgend would be changed over there. So we need to
compensate this loss with high memory. Here memory_map[] also records
the original low/high memory, so if low_mem_end is less-than the
original we need this compensation.
So here we have two steps to address this issue,
#1. Calculate the loss
+ {
+ add_high_mem = end - low_mem_end;
+ e820[i].size = low_mem_end - e820[i].addr;
+ }
+ }
+
+ /*
+ * And then we also need to adjust highmem.
+ */
A single line comment should use the respective comment style.
#2. Compensate the loss
+ if ( add_high_mem )
+ {
+ for ( i = 0; i < memory_map.nr_map; i++ )
s/memory_map.nr_map/nr
+ {
+ if ( e820[i].type == E820_RAM &&
+ e820[i].addr > (1ull << 32))
+ e820[i].size += add_high_mem;
+ }
+ }
But looking at the code I think the comment should be extended to
state that we currently expect there to be exactly one such RAM
region.
I can add this at the beginning of #1 loop,
Its possible to relocate 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 in e820.
Thanks
Tiejun
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
