On 06/22/15 21:10, Laszlo Ersek wrote:
> On 06/22/15 20:55, Jordan Justen wrote:
>> On 2015-06-16 10:05:27, 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>
>>> ---
>>>  OvmfPkg/PlatformPei/MemDetect.c | 43 
>>> +++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 39 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c 
>>> b/OvmfPkg/PlatformPei/MemDetect.c
>>> index 1228063..e872126 100644
>>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>>> @@ -194,6 +194,8 @@ QemuInitializeRam (
>>>  {
>>>    UINT64                      LowerMemorySize;
>>>    UINT64                      UpperMemorySize;
>>> +  MTRR_SETTINGS               MtrrSettings;
>>> +  EFI_STATUS                  Status;
>>>  
>>>    DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
>>>  
>>> @@ -214,12 +216,45 @@ 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);
>>> +
>>> +    //
>>> +    // punch holes
>>
>> What about: "Set memory from 640KB - 1MB to uncacheable"
> 
> Okay.
> 
>>
>>> +    //
>>> +    Status = MtrrSetMemoryAttribute (BASE_512KB + BASE_128KB,
>>> +               SIZE_256KB + SIZE_128KB, CacheUncacheable);
>>
>> What about instead specifying length as:
>>
>> BASE_1MB - (BASE_512KB + BASE_128KB)
> 
> Will do.
> 
>>> +    ASSERT_EFI_ERROR (Status);
>>> +
>>
>> How about a comment: "Set MMIO just below 4GB to uncacheable"
> 
> Hmm, "just below" doesn't quite reflect (to me) that it is supposed to
> cover the entire 32-bit PCI hole, which can be gigabytes big. Do you
> have a proposal that reflects this more accurately?

Nevermind, I went with your initial suggestion.

> 
>>> +    Status = MtrrSetMemoryAttribute (LowerMemorySize,
>>> +               SIZE_4GB - LowerMemorySize, CacheUncacheable);
>>> +    ASSERT_EFI_ERROR (Status);
>>
>> Patches 2-4:
>> Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
> 
> Thank you. (I'll post a v2 later.)
> Laszlo
> 
>>
>>>    }
>>>  }
>>>  
>>> -- 
>>> 1.8.3.1
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> _______________________________________________
>>> 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
> 


------------------------------------------------------------------------------
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

Reply via email to