On 06/24/15 00:12, Jordan Justen wrote:
> On 2015-06-23 12:53:49, Laszlo Ersek wrote:
>> At the moment we work with a UC default MTRR type, and set three memory
>> ranges to WB:
>> - [0, 640 KB),
>> - [1 MB, LowerMemorySize),
>> - [4 GB, 4 GB + UpperMemorySize).
>>
>> Unfortunately, coverage for the third range can fail with a high
>> likelihood. If the alignment of the base (ie. 4 GB) and the alignment of
>> the size (UpperMemorySize) differ, then MtrrLib creates a series of
>> variable MTRR entries, with power-of-two sized MTRR masks. And, it's
>> really easy to run out of variable MTRR entries, dependent on the
>> alignment difference.
>>
>> This is a problem because a Linux guest will loudly reject any high memory
>> that is not covered my MTRR.
>>
>> So, let's follow the inverse pattern (loosely inspired by SeaBIOS):
>> - flip the MTRR default type to WB,
>> - set [0, 640 KB) to WB -- fixed MTRRs have precedence over the default
>> type and variable MTRRs, so we can't avoid this,
>> - set [640 KB, 1 MB) to UC -- implemented with fixed MTRRs,
>> - set [LowerMemorySize, 4 GB) to UC -- should succeed with variable MTRRs
>> more likely than the other scheme (due to less chaotic alignment
>> differences).
>>
>> Effects of this patch can be observed by setting DEBUG_CACHE (0x00200000)
>> in PcdDebugPrintErrorLevel.
>>
>> Cc: Maoming <maoming.maom...@huawei.com>
>> Cc: Huangpeng (Peter) <peter.huangp...@huawei.com>
>> Cc: Wei Liu <wei.l...@citrix.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> Tested-by: Maoming <maoming.maom...@huawei.com>
>> Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
>> ---
>>
>> Notes:
>> v2:
>> - update comments [Jordan]
>> - calculate the size of the low 384 KB uncacheable hole as Jordan
>> suggested
>>
>> OvmfPkg/PlatformPei/MemDetect.c | 46 ++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c
>> b/OvmfPkg/PlatformPei/MemDetect.c
>> index b74308f..4778a55 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -252,6 +252,8 @@ QemuInitializeRam (
>> {
>> UINT64 LowerMemorySize;
>> UINT64 UpperMemorySize;
>> + MTRR_SETTINGS MtrrSettings;
>> + EFI_STATUS Status;
>>
>> DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
>>
>> @@ -272,12 +274,48 @@ QemuInitializeRam (
>> }
>> }
>>
>> - MtrrSetMemoryAttribute (BASE_1MB, LowerMemorySize - BASE_1MB,
>> CacheWriteBack);
>> + //
>> + // We'd like to keep the following ranges uncached:
>> + // - [640 KB, 1 MB)
>> + // - [LowerMemorySize, 4 GB)
>> + //
>> + // Everything else should be WB. Unfortunately, programming the inverse
>> (ie.
>> + // keeping the default UC, and configuring the complement set of the
>> above as
>> + // WB) is not reliable in general, because the end of the upper RAM can
>> have
>> + // practically any alignment, and we may not have enough variable MTRRs to
>> + // cover it exactly.
>> + //
>> + if (IsMtrrSupported ()) {
>> + MtrrGetAllMtrrs (&MtrrSettings);
>>
>> - MtrrSetMemoryAttribute (0, BASE_512KB + BASE_128KB, CacheWriteBack);
>> + //
>> + // MTRRs disabled, fixed MTRRs disabled, default type is uncached
>> + //
>> + ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
>> + ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
>> + ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
>>
>> - if (UpperMemorySize != 0) {
>> - MtrrSetMemoryAttribute (BASE_4GB, UpperMemorySize, CacheWriteBack);
>> + //
>> + // flip default type to writeback
>> + //
>> + SetMem (&MtrrSettings.Fixed, sizeof MtrrSettings.Fixed, 0x06);
>> + ZeroMem (&MtrrSettings.Variables, sizeof MtrrSettings.Variables);
>> + MtrrSettings.MtrrDefType |= BIT11 | BIT10 | 6;
>> + MtrrSetAllMtrrs (&MtrrSettings);
>> +
>> + //
>> + // Set memory from 640KB to 1MB to uncacheable
>> + //
>
> Thanks Laszlo. All the patches look good to me now.
>
> If I have a nit pick remaining, it would be the comments I suggested
> here. :)
>
> I think maybe 'memory' => 'memory range' seems better. What do you
> think?
OK.
>> + Status = MtrrSetMemoryAttribute (BASE_512KB + BASE_128KB,
>> + BASE_1MB - (BASE_512KB + BASE_128KB), CacheUncacheable);
>> + ASSERT_EFI_ERROR (Status);
>> +
>> + //
>> + // Set MMIO just below 4GB to uncacheable
>> + //
>
> You expressed some doubts about this, and I agree. To throw out
> another option, maybe:
>
> //
> // Set memory range from the "top of lower RAM" (RAM below 4GB) to
> // 4GB as uncacheable
> //
Works for me.
> Or, maybe:
>
> //
> // Set memory range for memory mapped IO below 4GB as uncacheable
> //
>
> Or, perhaps you have a better suggestion?
>
> Anyway, I don't think it is critical, so with or without changes,
> series Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
Thanks! I'll update these comments when I commit the patches to SVN. But
first I'd like to give Wei Liu and Maoming one or two days to re-test; I
wouldn't like to lose their Tested-by tags for patch #1.
Cheers!
Laszlo
>> + Status = MtrrSetMemoryAttribute (LowerMemorySize,
>> + SIZE_4GB - LowerMemorySize, CacheUncacheable);
>> + ASSERT_EFI_ERROR (Status);
>> }
>> }
>>
>> --
>> 1.8.3.1
>>
>>
>> ------------------------------------------------------------------------------
>> Monitor 25 network devices or servers for free with OpManager!
>> OpManager is web-based network management software that monitors
>> network devices and physical & virtual servers, alerts via email & sms
>> for fault. Monitor 25 devices for free with no restriction. Download now
>> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
