Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Liming,

On 4/11/19 2:32 AM, Liming Gao wrote:
> Push on commit ddb8cedce7e07b87c0ac6b84cd750a6d3dac47c8

I see in the git history the authorship passed
from: rebecca@bluestop.org <rebecca@bluestop.org>
to: Rebecca Cran via Groups.Io <rebecca=bluestop.org@groups.io>

I wonder if we shouldn't block pushes until the
BaseTools/Scripts/PatchCheck.py tool get fixed to avoid such mistakes
which are likely to get reproduced now that we switched to Groups.Io.

Thoughts?

Regards,

Phil.

>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Liming Gao
>> Sent: Monday, April 08, 2019 9:16 PM
>> To: Philippe Mathieu-Daudé <philmd@redhat.com>; rebecca@bluestop.org
>> Cc: devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments
>> and improve E820 debug output
>>
>> Reviewed-by: Liming Gao <liming.gao@intel.com>
>>
>>> -----Original Message-----
>>> From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
>>> Sent: Friday, April 5, 2019 5:05 PM
>>> To: rebecca@bluestop.org; Gao, Liming <liming.gao@intel.com>
>>> Cc: devel@edk2.groups.io
>>> Subject: Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix
>> comments and improve E820 debug output
>>>
>>> On 4/4/19 7:56 PM, Rebecca Cran via Groups.Io wrote:
>>>> Fix a few typos in LegacyBiosBuildE820, and improve the debug output of
>>>> the E820 table to pad with zeros instead of spaces, remove extra hyphens
>>>> and display the memory type in decimal.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Rebecca Cran <rebecca@bluestop.org>
>>>
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>>
>>>> ---
>>>>  .../Csm/LegacyBiosDxe/LegacyBootSupport.c              | 10 +++++-----
>>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git
>> a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
>>> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
>>>> index a7b8e6a9a0..8c415cdfc6 100644
>>>> ---
>> a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
>>>> +++
>> b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
>>>> @@ -1711,9 +1711,9 @@ LegacyBiosBuildE820 (
>>>>
>>>>    do {
>>>>      //
>>>> -    // Use size returned back plus 1 descriptor for the AllocatePool.
>>>> +    // Use size returned for the AllocatePool.
>>>>      // We don't just multiply by 2 since the "for" loop below terminates on
>>>> -    // EfiMemoryMapEnd which is dependent upon EfiMemoryMapSize.
>> Otherwize
>>>> +    // EfiMemoryMapEnd which is dependent upon EfiMemoryMapSize.
>> Otherwise
>>>>      // we process bogus entries and create bogus E820 entries.
>>>>      //
>>>>      EfiMemoryMap = (EFI_MEMORY_DESCRIPTOR *) AllocatePool
>> (EfiMemoryMapSize);
>>>> @@ -1801,7 +1801,7 @@ LegacyBiosBuildE820 (
>>>>      MemoryBlockLength = (UINT64) (LShiftU64 (EfiEntry->NumberOfPages,
>> 12));
>>>>      if ((EfiEntry->PhysicalStart + MemoryBlockLength) < 0x100000) {
>>>>        //
>>>> -      // Skip the memory block is under 1MB
>>>> +      // Skip the memory block if under 1MB
>>>>        //
>>>>      } else {
>>>>        if (EfiEntry->PhysicalStart < 0x100000) {
>>>> @@ -1926,7 +1926,7 @@ LegacyBiosBuildE820 (
>>>>    *Size = (UINTN) (Index * sizeof (EFI_E820_ENTRY64));
>>>>
>>>>    //
>>>> -  // Determine OS usable memory above 1Mb
>>>> +  // Determine OS usable memory above 1MB
>>>>    //
>>>>    Private->IntThunk->EfiToLegacy16BootTable.OsMemoryAbove1Mb =
>> 0x0000;
>>>>    for (TempIndex = Above1MIndex; TempIndex < Index; TempIndex++) {
>>>> @@ -1948,7 +1948,7 @@ LegacyBiosBuildE820 (
>>>>    // Print DEBUG information
>>>>    //
>>>>    for (TempIndex = 0; TempIndex < Index; TempIndex++) {
>>>> -    DEBUG((EFI_D_INFO, "E820[%2d]: 0x%16lx ---- 0x%16lx, Type = 0x%x
>> \n",
>>>> +    DEBUG((EFI_D_INFO, "E820[%2d]: 0x%016lx - 0x%016lx, Type = %d\n",
>>>>        TempIndex,
>>>>        E820Table[TempIndex].BaseAddr,
>>>>        (E820Table[TempIndex].BaseAddr + E820Table[TempIndex].Length),
>>>>
>>
>>
> 
> 
> 
>