* [PATCH v2 0/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM
@ 2017-08-04 23:00 Laszlo Ersek
2017-08-04 23:00 ` [PATCH v2 1/1] " Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2017-08-04 23:00 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen
Version 2 of the patch posted at
https://lists.01.org/pipermail/edk2-devel/2017-July/012304.html
addressing Jordan's feedback (see the changes noted on patch v2 1/1).
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1468526
Repo: https://github.com/lersek/edk2.git
Branch: highram1tb_v2
Cc: Jordan Justen <jordan.l.justen@intel.com>
Thanks
Laszlo
Laszlo Ersek (1):
OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high
RAM
OvmfPkg/PlatformPei/MemDetect.c | 129 +++++++++++++++++++-
1 file changed, 127 insertions(+), 2 deletions(-)
--
2.13.1.3.g8be5a757fa67
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM
2017-08-04 23:00 [PATCH v2 0/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM Laszlo Ersek
@ 2017-08-04 23:00 ` Laszlo Ersek
2017-08-05 0:17 ` Jordan Justen
0 siblings, 1 reply; 4+ messages in thread
From: Laszlo Ersek @ 2017-08-04 23:00 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen
In OVMF we currently get the upper (>=4GB) memory size with the
GetSystemMemorySizeAbove4gb() function.
The GetSystemMemorySizeAbove4gb() function is used in two places:
(1) It is the starting point of the calculations in GetFirstNonAddress().
GetFirstNonAddress() in turn
- determines the placement of the 64-bit PCI MMIO aperture,
- provides input for the GCD memory space map's sizing (see
AddressWidthInitialization(), and the CPU HOB in
MiscInitialization()),
- influences the permanent PEI RAM cap (the DXE core's page tables,
built in permanent PEI RAM, grow as the RAM to map grows).
(2) In QemuInitializeRam(), GetSystemMemorySizeAbove4gb() determines the
single memory descriptor HOB that we produce for the upper memory.
Respectively, there are two problems with GetSystemMemorySizeAbove4gb():
(1) It reads a 24-bit count of 64KB RAM chunks from the CMOS, and
therefore cannot return a larger value than one terabyte.
(2) It cannot express discontiguous high RAM.
Starting with version 1.7.0, QEMU has provided the fw_cfg file called
"etc/e820". Refer to the following QEMU commits:
- 0624c7f916b4 ("e820: pass high memory too.", 2013-10-10),
- 7d67110f2d9a ("pc: add etc/e820 fw_cfg file", 2013-10-18)
- 7db16f2480db ("pc: register e820 entries for ram", 2013-10-10)
Ever since these commits in v1.7.0 -- with the last QEMU release being
v2.9.0, and v2.10.0 under development --, the only two RAM entries added
to this E820 map correspond to the below-4GB RAM range, and the above-4GB
RAM range. And, the above-4GB range exactly matches the CMOS registers in
question; see the use of "pcms->above_4g_mem_size":
pc_q35_init() | pc_init1()
pc_memory_init()
e820_add_entry(0x100000000ULL, pcms->above_4g_mem_size, E820_RAM);
pc_cmos_init()
val = pcms->above_4g_mem_size / 65536;
rtc_set_memory(s, 0x5b, val);
rtc_set_memory(s, 0x5c, val >> 8);
rtc_set_memory(s, 0x5d, val >> 16);
Therefore, remedy the above OVMF limitations as follows:
(1) Start off GetFirstNonAddress() by scanning the E820 map for the
highest exclusive >=4GB RAM address. Fall back to the CMOS if the E820
map is unavailable. Base all further calculations (such as 64-bit PCI
MMIO aperture placement, GCD sizing etc) on this value.
At the moment, the only difference this change makes is that we can
have more than 1TB above 4GB -- given that the sole "high RAM" entry
in the E820 map matches the CMOS exactly, modulo the most significant
bits (see above).
However, Igor plans to add discontiguous (cold-plugged) high RAM to
the fw_cfg E820 RAM map later on, and then this scanning will adapt
automatically.
(2) In QemuInitializeRam(), describe the high RAM regions from the E820
map one by one with memory HOBs. Fall back to the CMOS only if the
E820 map is missing.
Again, right now this change only makes a difference if there is at
least 1TB high RAM. Later on it will adapt to discontiguous high RAM
(regardless of its size) automatically.
-*-
Implementation details: introduce the ScanOrAdd64BitE820Ram() function,
which reads the E820 entries from fw_cfg, and finds the highest exclusive
>=4GB RAM address, or produces memory resource descriptor HOBs for RAM
entries that start at or above 4GB. The RAM map is not read in a single
go, because its size can vary, and in PlatformPei we should stay away from
dynamic memory allocation, for the following reasons:
- "Pool" allocations are limited to ~64KB, are served from HOBs, and
cannot be released ever.
- "Page" allocations are seriously limited before PlatformPei installs the
permanent PEI RAM. Furthermore, page allocations can only be released in
DXE, with dedicated code (so the address would have to be passed on with
a HOB or PCD).
- Raw memory allocation HOBs would require the same freeing in DXE.
Therefore we process each E820 entry as soon as it is read from fw_cfg.
-*-
Considering the impact of high RAM on the DXE core:
A few years ago, installing high RAM as *tested* would cause the DXE core
to inhabit such ranges rather than carving out its home from the permanent
PEI RAM. Fortunately, this was fixed in the following edk2 commit:
3a05b13106d1, "MdeModulePkg DxeCore: Take the range in resource HOB for
PHIT as higher priority", 2015-09-18
which I regression-tested at the time:
http://mid.mail-archive.com/55FC27B0.4070807@redhat.com
Later on, OVMF was changed to install its high RAM as tested (effectively
"arming" the earlier DXE core change for OVMF), in the following edk2
commit:
035ce3b37c90, "OvmfPkg/PlatformPei: Add memory above 4GB as tested",
2016-04-21
which I also regression-tested at the time:
http://mid.mail-archive.com/571E8B90.1020102@redhat.com
Therefore adding more "tested memory" HOBs is safe.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1468526
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- rename E820HighRamIterate() to ScanOrAdd64BitE820Ram() [Jordan]
- fold E820HighRamFindHighestExclusiveAddress() and
E820HighRamAddMemoryHob() into ScanOrAdd64BitE820Ram(), dependent on
the optional "MaxAddress" output parameter [Jordan]
OvmfPkg/PlatformPei/MemDetect.c | 129 +++++++++++++++++++-
1 file changed, 127 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 97f3fa5afcf5..2b2f3e4bec55 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -19,6 +19,7 @@ Module Name:
//
// The package level header files this module uses
//
+#include <IndustryStandard/E820.h>
#include <IndustryStandard/Q35MchIch9.h>
#include <PiPei.h>
@@ -103,6 +104,109 @@ Q35TsegMbytesInitialization (
}
+/**
+ Iterate over the RAM entries in QEMU's fw_cfg E820 RAM map that start outside
+ of the 32-bit address range.
+
+ Find the highest exclusive >=4GB RAM address, or produce memory resource
+ descriptor HOBs for RAM entries that start at or above 4GB.
+
+ @param[out] MaxAddress If MaxAddress is NULL, then ScanOrAdd64BitE820Ram()
+ produces memory resource descriptor HOBs for RAM
+ entries that start at or above 4GB.
+
+ Otherwise, MaxAddress holds the highest exclusive
+ >=4GB RAM address on output. If QEMU's fw_cfg E820
+ RAM map contains no RAM entry that starts outside of
+ the 32-bit address range, then MaxAddress is exactly
+ 4GB on output.
+
+ @retval EFI_SUCCESS The fw_cfg E820 RAM map was found and processed.
+
+ @retval EFI_PROTOCOL_ERROR The RAM map was found, but its size wasn't a
+ whole multiple of sizeof(EFI_E820_ENTRY64). No
+ RAM entry was processed.
+
+ @return Error codes from QemuFwCfgFindFile(). No RAM
+ entry was processed.
+**/
+STATIC
+EFI_STATUS
+ScanOrAdd64BitE820Ram (
+ OUT UINT64 *MaxAddress OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ FIRMWARE_CONFIG_ITEM FwCfgItem;
+ UINTN FwCfgSize;
+ EFI_E820_ENTRY64 E820Entry;
+ UINTN Processed;
+
+ Status = QemuFwCfgFindFile ("etc/e820", &FwCfgItem, &FwCfgSize);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ if (FwCfgSize % sizeof E820Entry != 0) {
+ return EFI_PROTOCOL_ERROR;
+ }
+
+ if (MaxAddress != NULL) {
+ *MaxAddress = BASE_4GB;
+ }
+
+ QemuFwCfgSelectItem (FwCfgItem);
+ for (Processed = 0; Processed < FwCfgSize; Processed += sizeof E820Entry) {
+ QemuFwCfgReadBytes (sizeof E820Entry, &E820Entry);
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: Base=0x%Lx Length=0x%Lx Type=%u\n",
+ __FUNCTION__,
+ E820Entry.BaseAddr,
+ E820Entry.Length,
+ E820Entry.Type
+ ));
+ if (E820Entry.Type == EfiAcpiAddressRangeMemory &&
+ E820Entry.BaseAddr >= BASE_4GB) {
+ if (MaxAddress == NULL) {
+ UINT64 Base;
+ UINT64 End;
+
+ //
+ // Round up the start address, and round down the end address.
+ //
+ Base = ALIGN_VALUE (E820Entry.BaseAddr, (UINT64)EFI_PAGE_SIZE);
+ End = (E820Entry.BaseAddr + E820Entry.Length) &
+ ~(UINT64)EFI_PAGE_MASK;
+ if (Base < End) {
+ AddMemoryRangeHob (Base, End);
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: AddMemoryRangeHob [0x%Lx, 0x%Lx)\n",
+ __FUNCTION__,
+ Base,
+ End
+ ));
+ }
+ } else {
+ UINT64 Candidate;
+
+ Candidate = E820Entry.BaseAddr + E820Entry.Length;
+ if (Candidate > *MaxAddress) {
+ *MaxAddress = Candidate;
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: MaxAddress=0x%Lx\n",
+ __FUNCTION__,
+ *MaxAddress
+ ));
+ }
+ }
+ }
+ }
+ return EFI_SUCCESS;
+}
+
+
UINT32
GetSystemMemorySizeBelow4gb (
VOID
@@ -170,7 +274,22 @@ GetFirstNonAddress (
UINT64 HotPlugMemoryEnd;
RETURN_STATUS PcdStatus;
- FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
+ //
+ // set FirstNonAddress to suppress incorrect compiler/analyzer warnings
+ //
+ FirstNonAddress = 0;
+
+ //
+ // If QEMU presents an E820 map, then get the highest exclusive >=4GB RAM
+ // address from it. This can express an address >= 4GB+1TB.
+ //
+ // Otherwise, get the flat size of the memory above 4GB from the CMOS (which
+ // can only express a size smaller than 1TB), and add it to 4GB.
+ //
+ Status = ScanOrAdd64BitE820Ram (&FirstNonAddress);
+ if (EFI_ERROR (Status)) {
+ FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
+ }
//
// If DXE is 32-bit, then we're done; PciBusDxe will degrade 64-bit MMIO
@@ -525,7 +644,13 @@ QemuInitializeRam (
AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
}
- if (UpperMemorySize != 0) {
+ //
+ // If QEMU presents an E820 map, then create memory HOBs for the >=4GB RAM
+ // entries. Otherwise, create a single memory HOB with the flat >=4GB
+ // memory size read from the CMOS.
+ //
+ Status = ScanOrAdd64BitE820Ram (NULL);
+ if (EFI_ERROR (Status) && UpperMemorySize != 0) {
AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
}
}
--
2.13.1.3.g8be5a757fa67
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM
2017-08-04 23:00 ` [PATCH v2 1/1] " Laszlo Ersek
@ 2017-08-05 0:17 ` Jordan Justen
2017-08-05 1:54 ` Laszlo Ersek
0 siblings, 1 reply; 4+ messages in thread
From: Jordan Justen @ 2017-08-05 0:17 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01
On 2017-08-04 16:00:43, Laszlo Ersek wrote:
> In OVMF we currently get the upper (>=4GB) memory size with the
> GetSystemMemorySizeAbove4gb() function.
>
> The GetSystemMemorySizeAbove4gb() function is used in two places:
>
> (1) It is the starting point of the calculations in GetFirstNonAddress().
> GetFirstNonAddress() in turn
> - determines the placement of the 64-bit PCI MMIO aperture,
> - provides input for the GCD memory space map's sizing (see
> AddressWidthInitialization(), and the CPU HOB in
> MiscInitialization()),
> - influences the permanent PEI RAM cap (the DXE core's page tables,
> built in permanent PEI RAM, grow as the RAM to map grows).
>
> (2) In QemuInitializeRam(), GetSystemMemorySizeAbove4gb() determines the
> single memory descriptor HOB that we produce for the upper memory.
>
> Respectively, there are two problems with GetSystemMemorySizeAbove4gb():
>
> (1) It reads a 24-bit count of 64KB RAM chunks from the CMOS, and
> therefore cannot return a larger value than one terabyte.
>
> (2) It cannot express discontiguous high RAM.
>
> Starting with version 1.7.0, QEMU has provided the fw_cfg file called
> "etc/e820". Refer to the following QEMU commits:
>
> - 0624c7f916b4 ("e820: pass high memory too.", 2013-10-10),
> - 7d67110f2d9a ("pc: add etc/e820 fw_cfg file", 2013-10-18)
> - 7db16f2480db ("pc: register e820 entries for ram", 2013-10-10)
>
> Ever since these commits in v1.7.0 -- with the last QEMU release being
> v2.9.0, and v2.10.0 under development --, the only two RAM entries added
> to this E820 map correspond to the below-4GB RAM range, and the above-4GB
> RAM range. And, the above-4GB range exactly matches the CMOS registers in
> question; see the use of "pcms->above_4g_mem_size":
>
> pc_q35_init() | pc_init1()
> pc_memory_init()
> e820_add_entry(0x100000000ULL, pcms->above_4g_mem_size, E820_RAM);
> pc_cmos_init()
> val = pcms->above_4g_mem_size / 65536;
> rtc_set_memory(s, 0x5b, val);
> rtc_set_memory(s, 0x5c, val >> 8);
> rtc_set_memory(s, 0x5d, val >> 16);
>
> Therefore, remedy the above OVMF limitations as follows:
>
> (1) Start off GetFirstNonAddress() by scanning the E820 map for the
> highest exclusive >=4GB RAM address. Fall back to the CMOS if the E820
> map is unavailable. Base all further calculations (such as 64-bit PCI
> MMIO aperture placement, GCD sizing etc) on this value.
>
> At the moment, the only difference this change makes is that we can
> have more than 1TB above 4GB -- given that the sole "high RAM" entry
> in the E820 map matches the CMOS exactly, modulo the most significant
> bits (see above).
>
> However, Igor plans to add discontiguous (cold-plugged) high RAM to
> the fw_cfg E820 RAM map later on, and then this scanning will adapt
> automatically.
>
> (2) In QemuInitializeRam(), describe the high RAM regions from the E820
> map one by one with memory HOBs. Fall back to the CMOS only if the
> E820 map is missing.
>
> Again, right now this change only makes a difference if there is at
> least 1TB high RAM. Later on it will adapt to discontiguous high RAM
> (regardless of its size) automatically.
>
> -*-
>
> Implementation details: introduce the ScanOrAdd64BitE820Ram() function,
> which reads the E820 entries from fw_cfg, and finds the highest exclusive
> >=4GB RAM address, or produces memory resource descriptor HOBs for RAM
> entries that start at or above 4GB. The RAM map is not read in a single
> go, because its size can vary, and in PlatformPei we should stay away from
> dynamic memory allocation, for the following reasons:
>
> - "Pool" allocations are limited to ~64KB, are served from HOBs, and
> cannot be released ever.
>
> - "Page" allocations are seriously limited before PlatformPei installs the
> permanent PEI RAM. Furthermore, page allocations can only be released in
> DXE, with dedicated code (so the address would have to be passed on with
> a HOB or PCD).
>
> - Raw memory allocation HOBs would require the same freeing in DXE.
>
> Therefore we process each E820 entry as soon as it is read from fw_cfg.
>
> -*-
>
> Considering the impact of high RAM on the DXE core:
>
> A few years ago, installing high RAM as *tested* would cause the DXE core
> to inhabit such ranges rather than carving out its home from the permanent
> PEI RAM. Fortunately, this was fixed in the following edk2 commit:
>
> 3a05b13106d1, "MdeModulePkg DxeCore: Take the range in resource HOB for
> PHIT as higher priority", 2015-09-18
>
> which I regression-tested at the time:
>
> http://mid.mail-archive.com/55FC27B0.4070807@redhat.com
>
> Later on, OVMF was changed to install its high RAM as tested (effectively
> "arming" the earlier DXE core change for OVMF), in the following edk2
> commit:
>
> 035ce3b37c90, "OvmfPkg/PlatformPei: Add memory above 4GB as tested",
> 2016-04-21
>
> which I also regression-tested at the time:
>
> http://mid.mail-archive.com/571E8B90.1020102@redhat.com
>
> Therefore adding more "tested memory" HOBs is safe.
!
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1468526
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
>
> - rename E820HighRamIterate() to ScanOrAdd64BitE820Ram() [Jordan]
>
> - fold E820HighRamFindHighestExclusiveAddress() and
> E820HighRamAddMemoryHob() into ScanOrAdd64BitE820Ram(), dependent on
> the optional "MaxAddress" output parameter [Jordan]
>
> OvmfPkg/PlatformPei/MemDetect.c | 129 +++++++++++++++++++-
> 1 file changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
> index 97f3fa5afcf5..2b2f3e4bec55 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -19,6 +19,7 @@ Module Name:
> //
> // The package level header files this module uses
> //
> +#include <IndustryStandard/E820.h>
> #include <IndustryStandard/Q35MchIch9.h>
> #include <PiPei.h>
>
> @@ -103,6 +104,109 @@ Q35TsegMbytesInitialization (
> }
>
>
> +/**
> + Iterate over the RAM entries in QEMU's fw_cfg E820 RAM map that start outside
> + of the 32-bit address range.
> +
> + Find the highest exclusive >=4GB RAM address, or produce memory resource
> + descriptor HOBs for RAM entries that start at or above 4GB.
> +
> + @param[out] MaxAddress If MaxAddress is NULL, then ScanOrAdd64BitE820Ram()
> + produces memory resource descriptor HOBs for RAM
> + entries that start at or above 4GB.
> +
> + Otherwise, MaxAddress holds the highest exclusive
> + >=4GB RAM address on output. If QEMU's fw_cfg E820
> + RAM map contains no RAM entry that starts outside of
> + the 32-bit address range, then MaxAddress is exactly
> + 4GB on output.
> +
> + @retval EFI_SUCCESS The fw_cfg E820 RAM map was found and processed.
> +
> + @retval EFI_PROTOCOL_ERROR The RAM map was found, but its size wasn't a
> + whole multiple of sizeof(EFI_E820_ENTRY64). No
> + RAM entry was processed.
> +
> + @return Error codes from QemuFwCfgFindFile(). No RAM
> + entry was processed.
> +**/
> +STATIC
> +EFI_STATUS
> +ScanOrAdd64BitE820Ram (
> + OUT UINT64 *MaxAddress OPTIONAL
> + )
> +{
> + EFI_STATUS Status;
> + FIRMWARE_CONFIG_ITEM FwCfgItem;
> + UINTN FwCfgSize;
> + EFI_E820_ENTRY64 E820Entry;
> + UINTN Processed;
> +
> + Status = QemuFwCfgFindFile ("etc/e820", &FwCfgItem, &FwCfgSize);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + if (FwCfgSize % sizeof E820Entry != 0) {
> + return EFI_PROTOCOL_ERROR;
> + }
> +
> + if (MaxAddress != NULL) {
> + *MaxAddress = BASE_4GB;
> + }
> +
> + QemuFwCfgSelectItem (FwCfgItem);
> + for (Processed = 0; Processed < FwCfgSize; Processed += sizeof E820Entry) {
> + QemuFwCfgReadBytes (sizeof E820Entry, &E820Entry);
> + DEBUG ((
> + DEBUG_VERBOSE,
> + "%a: Base=0x%Lx Length=0x%Lx Type=%u\n",
> + __FUNCTION__,
> + E820Entry.BaseAddr,
> + E820Entry.Length,
> + E820Entry.Type
> + ));
> + if (E820Entry.Type == EfiAcpiAddressRangeMemory &&
> + E820Entry.BaseAddr >= BASE_4GB) {
> + if (MaxAddress == NULL) {
> + UINT64 Base;
> + UINT64 End;
> +
> + //
> + // Round up the start address, and round down the end address.
> + //
> + Base = ALIGN_VALUE (E820Entry.BaseAddr, (UINT64)EFI_PAGE_SIZE);
> + End = (E820Entry.BaseAddr + E820Entry.Length) &
> + ~(UINT64)EFI_PAGE_MASK;
I had set non-aligned Base/End one level out, and then rounded them here.
> + if (Base < End) {
> + AddMemoryRangeHob (Base, End);
> + DEBUG ((
> + DEBUG_VERBOSE,
> + "%a: AddMemoryRangeHob [0x%Lx, 0x%Lx)\n",
> + __FUNCTION__,
> + Base,
> + End
> + ));
> + }
> + } else {
> + UINT64 Candidate;
> +
> + Candidate = E820Entry.BaseAddr + E820Entry.Length;
Therefore I already had End here.
> + if (Candidate > *MaxAddress) {
> + *MaxAddress = Candidate;
> + DEBUG ((
> + DEBUG_VERBOSE,
> + "%a: MaxAddress=0x%Lx\n",
> + __FUNCTION__,
> + *MaxAddress
> + ));
> + }
I just had:
*MaxAddress = MAX(*MaxAddress, End);
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> + }
> + }
> + }
> + return EFI_SUCCESS;
> +}
> +
> +
> UINT32
> GetSystemMemorySizeBelow4gb (
> VOID
> @@ -170,7 +274,22 @@ GetFirstNonAddress (
> UINT64 HotPlugMemoryEnd;
> RETURN_STATUS PcdStatus;
>
> - FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
> + //
> + // set FirstNonAddress to suppress incorrect compiler/analyzer warnings
> + //
> + FirstNonAddress = 0;
> +
> + //
> + // If QEMU presents an E820 map, then get the highest exclusive >=4GB RAM
> + // address from it. This can express an address >= 4GB+1TB.
> + //
> + // Otherwise, get the flat size of the memory above 4GB from the CMOS (which
> + // can only express a size smaller than 1TB), and add it to 4GB.
> + //
> + Status = ScanOrAdd64BitE820Ram (&FirstNonAddress);
> + if (EFI_ERROR (Status)) {
> + FirstNonAddress = BASE_4GB + GetSystemMemorySizeAbove4gb ();
> + }
>
> //
> // If DXE is 32-bit, then we're done; PciBusDxe will degrade 64-bit MMIO
> @@ -525,7 +644,13 @@ QemuInitializeRam (
> AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
> }
>
> - if (UpperMemorySize != 0) {
> + //
> + // If QEMU presents an E820 map, then create memory HOBs for the >=4GB RAM
> + // entries. Otherwise, create a single memory HOB with the flat >=4GB
> + // memory size read from the CMOS.
> + //
> + Status = ScanOrAdd64BitE820Ram (NULL);
> + if (EFI_ERROR (Status) && UpperMemorySize != 0) {
> AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
> }
> }
> --
> 2.13.1.3.g8be5a757fa67
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM
2017-08-05 0:17 ` Jordan Justen
@ 2017-08-05 1:54 ` Laszlo Ersek
0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2017-08-05 1:54 UTC (permalink / raw)
To: Jordan Justen, edk2-devel-01
On 08/05/17 02:17, Jordan Justen wrote:
> On 2017-08-04 16:00:43, Laszlo Ersek wrote:
>> + if (E820Entry.Type == EfiAcpiAddressRangeMemory &&
>> + E820Entry.BaseAddr >= BASE_4GB) {
>> + if (MaxAddress == NULL) {
>> + UINT64 Base;
>> + UINT64 End;
>> +
>> + //
>> + // Round up the start address, and round down the end address.
>> + //
>> + Base = ALIGN_VALUE (E820Entry.BaseAddr, (UINT64)EFI_PAGE_SIZE);
>> + End = (E820Entry.BaseAddr + E820Entry.Length) &
>> + ~(UINT64)EFI_PAGE_MASK;
>
> I had set non-aligned Base/End one level out, and then rounded them here.
>
>> + if (Base < End) {
>> + AddMemoryRangeHob (Base, End);
>> + DEBUG ((
>> + DEBUG_VERBOSE,
>> + "%a: AddMemoryRangeHob [0x%Lx, 0x%Lx)\n",
>> + __FUNCTION__,
>> + Base,
>> + End
>> + ));
>> + }
>> + } else {
>> + UINT64 Candidate;
>> +
>> + Candidate = E820Entry.BaseAddr + E820Entry.Length;
>
> Therefore I already had End here.
>
>> + if (Candidate > *MaxAddress) {
>> + *MaxAddress = Candidate;
>> + DEBUG ((
>> + DEBUG_VERBOSE,
>> + "%a: MaxAddress=0x%Lx\n",
>> + __FUNCTION__,
>> + *MaxAddress
>> + ));
>> + }
>
> I just had:
>
> *MaxAddress = MAX(*MaxAddress, End);
Whenever you go to the trouble of coding up your suggestions, please
post them too! I'd welcome and appreciate that. I might even answer with
an R-b, and then it's faster for everyone. (And it's nice to have some
patches with dual S-o-b's :) )
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Thank you, pushed as commit 1fceaddb12b5.
Laszlo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-05 1:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-04 23:00 [PATCH v2 0/1] OvmfPkg/PlatformPei: support >=1TB high RAM, and discontiguous high RAM Laszlo Ersek
2017-08-04 23:00 ` [PATCH v2 1/1] " Laszlo Ersek
2017-08-05 0:17 ` Jordan Justen
2017-08-05 1:54 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox