From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7C3EB81993 for ; Thu, 5 Jan 2017 02:30:35 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D95354D697; Thu, 5 Jan 2017 10:30:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-100.phx2.redhat.com [10.3.116.100]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v05AUYkJ005672; Thu, 5 Jan 2017 05:30:34 -0500 To: Anthony PERARD , edk2-devel@ml01.01.org, xen-devel@lists.xenproject.org References: <20161208153340.2285-1-anthony.perard@citrix.com> <20161208153340.2285-7-anthony.perard@citrix.com> From: Laszlo Ersek Message-ID: <8cc382af-45b2-c836-2907-98f066cca05b@redhat.com> Date: Thu, 5 Jan 2017 11:30:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20161208153340.2285-7-anthony.perard@citrix.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 05 Jan 2017 10:30:35 +0000 (UTC) Subject: Re: [PATCH RFC 06/14] OvmfPkg/XenPlatformPei: Add xen PVH specific code X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jan 2017 10:30:35 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 12/08/16 16:33, Anthony PERARD wrote: > - learn about memory size from the E820 > - ignore error if host bridge devid is 0xffff, PVH does not have PCI and > reading from unexisting device return -1. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Anthony PERARD > --- > OvmfPkg/XenPlatformPei/MemDetect.c | 71 ++++++++++++++++++++++++++++++++++++++ > OvmfPkg/XenPlatformPei/Platform.c | 5 +++ > OvmfPkg/XenPlatformPei/Platform.h | 10 ++++++ > 3 files changed, 86 insertions(+) > > diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c b/OvmfPkg/XenPlatformPei/MemDetect.c > index 4ecdf5e..0d80775 100644 > --- a/OvmfPkg/XenPlatformPei/MemDetect.c > +++ b/OvmfPkg/XenPlatformPei/MemDetect.c > @@ -42,6 +42,70 @@ UINT8 mPhysMemAddressWidth; > STATIC UINT32 mS3AcpiReservedMemoryBase; > STATIC UINT32 mS3AcpiReservedMemorySize; > > +STATIC UINT32 mXenLowerMemorySize = 0; > +STATIC UINT64 mXenHighMemorySize = 0; > + > +VOID > +XenReadMemorySizes ( > + VOID > + ) > +{ > + EFI_E820_ENTRY64 *E820Map; > + UINT32 E820EntriesCount; > + EFI_STATUS Status; > + > + Status = XenGetE820Map (&E820Map, &E820EntriesCount); > + ASSERT_EFI_ERROR (Status); > + > + mXenLowerMemorySize = 0; > + mXenHighMemorySize = 0; > + > + 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; > + } > + > + if ((Entry->BaseAddr + Entry->Length) <= SIZE_16MB) { > + continue; > + } > + > + if (Entry->BaseAddr < SIZE_16MB) { > + UINT64 bottom = Entry->BaseAddr; > + UINT64 size = Entry->Length; > + size -= SIZE_16MB - bottom; > + bottom = SIZE_16MB; > + mXenLowerMemorySize += size; > + continue; > + } > + if (Entry->BaseAddr <= SIZE_4GB) { > + UINT64 size = Entry->Length; > + mXenLowerMemorySize += size; > + continue; > + } > + > + if (Entry->BaseAddr == SIZE_4GB) { > + mXenHighMemorySize = Entry->Length; > + continue; > + } > + > + DEBUG ((EFI_D_INFO, "%a: ignored XenE820 entry (0x%llx, 0x%llx)\n", > + __FUNCTION__, > + Entry->BaseAddr, > + Entry->Length)); > + } > + mXenLowerMemorySize += SIZE_16MB; > + } > +} > + > UINT32 > GetSystemMemorySizeBelow4gb ( > VOID > @@ -50,6 +114,9 @@ GetSystemMemorySizeBelow4gb ( > UINT8 Cmos0x34; > UINT8 Cmos0x35; > > + if (mXen && mXenLowerMemorySize) > + return mXenLowerMemorySize; > + > // > // CMOS 0x34/0x35 specifies the system memory above 16 MB. > // * CMOS(0x35) is the high byte > @@ -74,6 +141,10 @@ GetSystemMemorySizeAbove4gb ( > UINT32 Size; > UINTN CmosIndex; > > + // if lower memory is specified that way, return also high memory > + if (mXen && mXenLowerMemorySize) > + return mXenHighMemorySize; > + > // > // CMOS 0x5b-0x5d specifies the system memory above 4GB MB. > // * CMOS(0x5d) is the most significant size byte > diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c > index bf78878..9fc713c 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.c > +++ b/OvmfPkg/XenPlatformPei/Platform.c > @@ -362,6 +362,10 @@ MiscInitialization ( > AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL); > AcpiEnBit = ICH9_ACPI_CNTL_ACPI_EN; > break; > + case 0xffff: > + // xen PVH, ignore > + PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId, mHostBridgeDevId); > + return; Can we create a new macro for 0xFFFF in "OvmfPkg/Include/OvmfPlatforms.h", and use that here? Normally I would suggest a separate header file under "OvmfPkg/Include", similar to "Q35MchIch9.h" and "I440FxPiix4.h", and to include that new file in "OvmfPlatforms.h", but here we only need a single "chipset identifier", so I guess that can go directly into "OvmfPlatforms.h". I mainly suggest this because in patch 08/14, the same 0xFFFF case label is being added to code shared with QEMU, and using a verbose macro there is much better than a magic number. In turn, we should use the same macro here, I think. Looks OK otherwise. Thanks Laszlo > default: > DEBUG ((EFI_D_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n", > __FUNCTION__, mHostBridgeDevId)); > @@ -503,6 +507,7 @@ InitializeXenPlatform ( > DebugDumpCmos (); > > XenDetect (); > + XenReadMemorySizes (); > > BootModeInitialization (); > AddressWidthInitialization (); > diff --git a/OvmfPkg/XenPlatformPei/Platform.h b/OvmfPkg/XenPlatformPei/Platform.h > index eda765b..2948853 100644 > --- a/OvmfPkg/XenPlatformPei/Platform.h > +++ b/OvmfPkg/XenPlatformPei/Platform.h > @@ -101,4 +101,14 @@ extern BOOLEAN mS3Supported; > > extern UINT8 mPhysMemAddressWidth; > > +EFI_STATUS > +XenGetE820Map ( > + EFI_E820_ENTRY64 **Entries, > + UINT32 *Count > + ); > +VOID > +XenReadMemorySizes ( > + VOID > + ); > + > #endif // _PLATFORM_PEI_H_INCLUDED_ >