Re: [PATCH 00/10] Use a better algorithm to calculate MTRR

[Laszlo Ersek <lersek@redhat.com>]

On 03/31/17 16:25, Ni, Ruiyu wrote:
> Just fixed the build failure. sorry about that.

No problem -- I wasn't annoyed or anything (such breakages are
unavoidable without a centralized build farm, as no developer can
test-build the tree with all the supported toolchains), I just wanted to
report it.

Thank you for the prompt action!
Laszlo

>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, March 31, 2017 5:11 PM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH 00/10] Use a better algorithm to calculate MTRR
>>
>> On 03/29/17 05:03, Ruiyu Ni wrote:
>>> The new algorithm finds out the more optimal MTRR solution for
>>> current memory type settings.
>>> Compare against the original algorithm, the new one guarantees
>>> to find the correct MTRR solution, but doesn't guarantee to
>>> find the most optimal MTRR solution.
>>>
>>> Ruiyu Ni (10):
>>>   UefiCpuPkg/MtrrLib: Correct typo in comments and remove TABs
>>>   UefiCpuPkg/MtrrLib: Add CacheInvalid enum type to MtrrLib.h
>>>   UefiCpuPkg/MtrrLib: IsMtrrSupported uses definitions in Msr.h
>>>   UefiCpuPkg/MtrrLib: GetVariableMtrrCountWorker uses definitions in
>>>     Msr.h
>>>   UefiCpuPkg/MtrrLib: Add MtrrLib prefix to ProgramFixedMtrr
>>>   UefiCpuPkg/MtrrLib: Add MtrrLib prefix to several internal functions
>>>   UefiCpuPkg/MtrrLib: MtrrLibInitializeMtrrMask() uses definitions in
>>>     CpuId.h
>>>   UefiCpuPkg/MtrrLib: Use a better algorithm to calculate MTRR
>>>   UefiCpuPkg/MtrrLib: Refine MtrrGetMemoryAttributeByAddressWorker
>>>   UefiCpuPkg/MtrrLib: All functions use definitions in Msr.h
>>>
>>>  UefiCpuPkg/Include/Library/MtrrLib.h |   17 +-
>>>  UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 1838 ++++++++++++++++++++--------------
>>>  2 files changed, 1068 insertions(+), 787 deletions(-)
>>>
>>
>> This series seems to trigger a number of build failures with GCC:
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c: In function 'MtrrSetMemoryAttributeWorker':
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:2051:90: error: suggest parentheses around arithmetic in operand of '|'
>> [-Werror=parentheses]
>>         VariableSettings->Mtrr[Index].Mask = (~(OriginalVariableMtrr[Index].Length - 1)) & MtrrValidAddressMask |
>> BIT11;
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c: In function 'MtrrLibGetMtrrNumber':
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:945:24: error: 'Length' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   LeastRightMtrrNumber = MtrrLibGetPositiveMtrrNumber (BaseAddress, Length, Alignment0);
>>   ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:927:17: error: 'BaseAddress' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>     BaseAddress += BaseAlignment;
>>     ~~~~~~~~~~~~^~~~~~~~~~~~~~~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c: In function 'MtrrLibSetMemoryType':
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:1430:6: error: 'LengthRight' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   if (LengthRight == 0) {
>>      ^
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:1444:31: error: 'LengthLeft' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>     Ranges[StartIndex].Length = LengthLeft;
>>     ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c: In function 'MtrrSetMemoryAttributeWorker':
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:2085:3: error: 'OriginalVariableMtrrCount' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   for (Index = 0; Index < OriginalVariableMtrrCount; Index++) {
>>   ^~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:1833:30: error: 'VariableSettings' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   MTRR_VARIABLE_SETTINGS    *VariableSettings;
>>                              ^~~~~~~~~~~~~~~~
>>
>> UefiCpuPkg/Library/MtrrLib/MtrrLib.c:2105:10: error: 'Status' may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   return Status;
>>          ^~~~~~
>> cc1: all warnings being treated as errors
>> GNUmakefile:328: recipe for target
>> 'Build/OvmfIa32/DEBUG_GCC49/IA32/UefiCpuPkg/Library/MtrrLib/MtrrLib/OUTPUT/MtrrLib.obj' failed
>> make: *** [Build/OvmfIa32/DEBUG_GCC49/IA32/UefiCpuPkg/Library/MtrrLib/MtrrLib/OUTPUT/MtrrLib.obj] Error 1
>>
>> Thanks
>> Laszlo