On 2015/5/15 14:25, Jan Beulich wrote:
On 15.05.15 at 08:11, <tiejun.c...@intel.com> wrote:
On 2015/4/20 22:29, Jan Beulich wrote:
On 10.04.15 at 11:22, <tiejun.c...@intel.com> wrote:
@@ -119,10 +120,6 @@ int build_e820_table(struct e820entry *e820,
/* 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++;
I think the above comment needs adjustment with all this code
removed. I also wonder how meaningful the BUG_ON() is with
->low_mem_pgend no longer used for E820 table construction.
Perhaps this needs another BUG_ON() validating that the field
matches some value from memory_map.map[]?
But I think hvm_info->low_mem_pgend is still correct, right?
I think so, but as said it's becoming less used and hence less
relevant here.
Understood.
And
additionally, there's no any obvious flag to indicate which
memory_map.map[x] is that last low memory map.
I didn't imply it would be immediately obvious _how_ to do this.
I'm merely wanting to avoid leaving meaningless BUG_ON()s in
the code, while meaningful ones are amiss.
Maybe we should lookup all .map[] to get the lowest memory map and then
BUG_ON?
Even we may separate the
low memory to construct memory_map.map[]...
???
Sorry I just mean that the low memory is not represented with only one
memory_map.map[] in some cases. Is it impossible? Even in the future? Or
actually we always consider the lowest memory map?
@@ -159,16 +156,37 @@ int build_e820_table(struct e820entry *e820,
nr++;
}
-
- if ( hvm_info->high_mem_pgend )
+ /* Construct the remaining according memory_map[]. */
+ 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].addr = memory_map.map[i].addr;
+ e820[nr].size = memory_map.map[i].size;
+ e820[nr].type = memory_map.map[i].type;
Afaict you could use structure assignment here to make this
more readable.
Sorry, are you saying this?
memcpy(&e820[nr], &memory_map.map[i], sizeof(struct e820entry));
No, structure assignment (which, other than memcpy(), is type safe):
e820[nr] = memory_map.map[i];
Understood.
Thanks
Tiejun
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
