On Mon, Jul 29, 2019 at 04:39:32PM +0100, Anthony PERARD wrote:

When running as a Xen PVH guest, there is no CMOS to read the memory
size from. Rework GetSystemMemorySize(Below|Above)4gb() so they can
work without CMOS by reading the e820 table.

Rework XenPublishRamRegions to also care for the reserved and ACPI
entry in the e820 table. The region that was added by InitializeXen()
isn't needed as that same entry is in the e820 table provided by
hvmloader.

MTRR settings aren't modified anymore, on HVM it's already done by
hvmloader, on PVH it is supposed to have sane default. MTRR will need
to be done properly but keeping what's already been done by programmes

^ firmware

that has runned before OVMF will do for now.

^ ran

diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
index a21d657357..182e96cc5b 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -276,9 +276,12 @@ XenPublishRamRegions (
VOID
)
{
- EFI_E820_ENTRY64 *E820Map;
- UINT32 E820EntriesCount;
- EFI_STATUS Status;
+ EFI_E820_ENTRY64 *E820Map;
+ UINT32 E820EntriesCount;
+ EFI_STATUS Status;
+ EFI_E820_ENTRY64 *Entry;
+ UINTN Index;
+ EFI_PHYSICAL_ADDRESS LocalApic;

DEBUG ((DEBUG_INFO, "Using memory map provided by Xen\n"));

@@ -287,26 +290,47 @@ XenPublishRamRegions (
//
E820EntriesCount = 0;
Status = XenGetE820Map (&E820Map, &E820EntriesCount);
-
ASSERT_EFI_ERROR (Status);

- if (E820EntriesCount > 0) {
- EFI_E820_ENTRY64 *Entry;
- UINT32 Loop;
-
- for (Loop = 0; Loop < E820EntriesCount; Loop++) {
- Entry = E820Map + Loop;
-
- //
- // Only care about RAM
- //
- if (Entry->Type != EfiAcpiAddressRangeMemory) {
- continue;
+ LocalApic = PcdGet32(PcdCpuLocalApicBaseAddress);
+ AddIoMemoryBaseSizeHob (LocalApic, SIZE_1MB);
+
+ for (Index = 0; Index < E820EntriesCount; Index++) {
+ UINT64 Base;
+ UINT64 End;
+
+ Entry = &E820Map[Index];
+
+ //
+ // Round up the start address, and round down the end address.
+ //
+ Base = ALIGN_VALUE (Entry->BaseAddr, (UINT64)EFI_PAGE_SIZE);
+ End = (Entry->BaseAddr + Entry->Length) & ~(UINT64)EFI_PAGE_MASK;
+
+ switch (Entry->Type) {
+ case EfiAcpiAddressRangeMemory:
+ AddMemoryRangeHob (Base, End);
+ break;
+ case EfiAcpiAddressRangeACPI:
+ AddReservedMemoryRangeHob (Base, End, FALSE);
+ break;
+ case EfiAcpiAddressRangeReserved:
+ if (Base < LocalApic && LocalApic < End) {

Don't you also need to check for equality? In case such region starts
at Base == LocalApic?

I guess it doesn't matter that much since this is to workaround a
specific issue with hvmloader, but I would like to see this sorted out
in hvmloader so that there's no clash anymore.

Thanks, Roger.