Re: HVM support for e820_host (Was: Bug: Limitation of <=2GB RAM in domU persists with 4.3.0)

[Gordan Bobic <gordan@bobich.net>]

 On Thu, 05 Sep 2013 19:01:03 -0400, Konrad Rzeszutek Wilk 
 <konrad.wilk@oracle.com> wrote:
> Gordan Bobic <gordan@bobich.net> wrote:
>>On 09/05/2013 11:23 PM, Konrad Rzeszutek Wilk wrote:
>>> Gordan Bobic <gordan@bobich.net> wrote:
>>>> Right, finally got around to trying this with the latest patch.
>>>>
>>>> With e820_host=0 things work as before:
>>>>
>>>> (XEN) HVM3: BIOS map:
>>>> (XEN) HVM3:  f0000-fffff: Main BIOS
>>>> (XEN) HVM3: E820 table:
>>>> (XEN) HVM3:  [00]: 00000000:00000000 - 00000000:0009e000: RAM
>>>> (XEN) HVM3:  [01]: 00000000:0009e000 - 00000000:000a0000: RESERVED
>>>> (XEN) HVM3:  HOLE: 00000000:000a0000 - 00000000:000e0000
>>>> (XEN) HVM3:  [02]: 00000000:000e0000 - 00000000:00100000: RESERVED
>>>> (XEN) HVM3:  [03]: 00000000:00100000 - 00000000:e0000000: RAM
>>>> (XEN) HVM3:  HOLE: 00000000:e0000000 - 00000000:fc000000
>>>> (XEN) HVM3:  [04]: 00000000:fc000000 - 00000001:00000000: RESERVED
>>>> (XEN) HVM3:  [05]: 00000001:00000000 - 00000002:1f800000: RAM
>>>>
>>>>
>>>> I seem to be getting two different E820 table dumps with
>>e820_host=1:
>>>>
>>>> (XEN) HVM1: BIOS map:
>>>> (XEN) HVM1:  f0000-fffff: Main BIOS
>>>> (XEN) HVM1: build_e820_table:91 got 8 op.nr_entries
>>>> (XEN) HVM1: E820 table:
>>>> (XEN) HVM1:  [00]: 00000000:00000000 - 00000000:3f790000: RAM
>>>> (XEN) HVM1:  [01]: 00000000:3f790000 - 00000000:3f79e000: ACPI
>>>> (XEN) HVM1:  [02]: 00000000:3f79e000 - 00000000:3f7d0000: NVS
>>>> (XEN) HVM1:  [03]: 00000000:3f7d0000 - 00000000:3f7e0000: RESERVED
>>>> (XEN) HVM1:  HOLE: 00000000:3f7e0000 - 00000000:3f7e7000
>>>> (XEN) HVM1:  [04]: 00000000:3f7e7000 - 00000000:40000000: RESERVED
>>>> (XEN) HVM1:  HOLE: 00000000:40000000 - 00000000:fee00000
>>>> (XEN) HVM1:  [05]: 00000000:fee00000 - 00000000:fee01000: RESERVED
>>>> (XEN) HVM1:  HOLE: 00000000:fee01000 - 00000000:ffc00000
>>>> (XEN) HVM1:  [06]: 00000000:ffc00000 - 00000001:00000000: RESERVED
>>>> (XEN) HVM1:  [07]: 00000001:00000000 - 00000001:68870000: RAM
>>>> (XEN) HVM1: E820 table:
>>>> (XEN) HVM1:  [00]: 00000000:00000000 - 00000000:0009e000: RAM
>>>> (XEN) HVM1:  [01]: 00000000:0009e000 - 00000000:000a0000: RESERVED
>>>> (XEN) HVM1:  HOLE: 00000000:000a0000 - 00000000:000e0000
>>>> (XEN) HVM1:  [02]: 00000000:000e0000 - 00000000:00100000: RESERVED
>>>> (XEN) HVM1:  [03]: 00000000:00100000 - 00000000:a7800000: RAM
>>>> (XEN) HVM1:  HOLE: 00000000:a7800000 - 00000000:fc000000
>>>> (XEN) HVM1:  [04]: 00000000:fc000000 - 00000001:00000000: RESERVED
>>>> (XEN) HVM1: Invoking ROMBIOS ...
>>>>
>>>> I cannot quite figure out what is going on here - these tables 
>>>> can't
>>>> both be true.
>>>>
>>>
>>> Right.  The code just prints the E820 that was constructed b/c of 
>>> the
>>e820_host =1 parameter as the first output.  Then the second one is
>>what was constructed originally.
>>>
>>> The code that would tie in the E820 from the hyper call and the 
>>> alter
>>how the hvmloader sets it up is not yet done.
>>>
>>>
>>>> Looking at the IOMEM on the host, the IOMEM begins at 0xa8000000 
>>>> and
>>>> goes more or less contiguously up to 0xfec8b000.
>>>>
>>>> Looking at dmesg on domU, the e820 map more or less matches the
>>second
>>>> dump above.
>>>
>>> Right.  That is correct since the patch I sent just outputs stuff.
>>No real changes to the E820 yet.
>>
>>I thought this did that in hvmloader/e820c:
>>hypercall_memory_op ( XENMEM_memory_map, &op);
>>
>>Gordan
>
> No.  They just gets the E820 that is stashed in the hypervisor for
> the guest.  The PV guest would use it but hvmloader is not. This is
> what would needed to be implemented to allow hvmloader construct  the
> E820 on its own.

 Right. So so in hvmloader/e820.c we now have the host based map in
 struct e820entry map[E820MAX];

 The rest of the function then goes and constructs the standard HVM
 e820 map in the passed in
 struct e820entry *e820

 So all that needs to happen here is if e820_host is set, fill e820[]
 by copying map[] up to the hvm_info->low_mem_pgend
 (or hvm_info->high_mem_pgend if it is set). I am guessing that
 SeaBIOS and other existing stuff might break if the host map is
 just copied in verbatim, so presumably I need to add/dedupe the
 non-RAM parts of the maps.

 Is that right? Nothing else needs to happen?

 The following questions arise:

 1) What to do in case of overlaps? On my specific hardware,
 the key difference in the end map will be that the hole at:
 (XEN) HVM1:  HOLE: 00000000:40000000 - 00000000:fee00000
 will end up being created in domU.

 2) Do only the holes need to be pulled from the host or
 the entire map? Would hvmloader/seabios/whatever know
 what to do if passed a map that is different from what
 they might expect (i.e. different from what the current
 hvmloader provides)? Or would this be likely to cause
 extensive further breakages?

 3) At the moment I am leaning toward just pulling in the
 holes from the host e820, mirroring them in domU.
 3.1) Marking them as "reserved" would likely fix the
 problem that was my primary motivation for doing this
 in the first place. Having said that - with all of
 the 1GB-3GB space marked as reserved, I'm not sure where
 the IOMEM would end up mapped in domU - things might just
 break. If marking the dom0 hole as a hole in domU without
 ensuring pBAR=vBAR, the PCI device in domU might get
 mapped with where another device is in dom0, which might
 cause the same problem.

 At the moment, I think the expedient thing to do is make
 domU map holes as per dom0 and ignore other non-RAM
 areas. This may (by luck) or may not fix my immediate problem
 (RAM in domU clobbering host's mapped IOMEM), but at
 least it would cover the pre-requisite hole mapping for
 the next step which is vBAR=pBAR.

 I light of this, however, depending on the answer to 2)
 above, it may not be practical for e820_host option do do
 what it actually means for HVMs, at least not to the same
 extent as happens for PV. It would only do a part of it
 (initial vHOLE=pHOLE, to later be extended to the more
 specific case of vBAR=pBAR).

 Does this sound reasonable?

 Gordan