[PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output

[PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output

[Rebecca Cran]

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>
---
 .../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),
-- 
2.21.0

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

[Philippe Mathieu-Daudé]

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

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

[Liming Gao]

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

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

[Liming Gao]

Push on commit ddb8cedce7e07b87c0ac6b84cd750a6d3dac47c8

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

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

[Philippe Mathieu-Daudé]

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

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

[Liming Gao]

Phil:
  I use Outlook to receive the patch mail, and save as mail file. This file shows From: Rebecca Cran via Groups.Io <rebecca=bluestop.org@groups.io>. So, I use it as patch author. I also check my patch with the same way. It shows From: Liming Gao <liming.gao@intel.com>. This is correct. So, I don't know this is user setting issue or group.io issue. 

Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Philippe Mathieu-Daudé
> Sent: Thursday, April 11, 2019 6:59 PM
> To: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Cc: devel@edk2.groups.io; rebecca@bluestop.org; Stephano Cetola <stephano.cetola@linux.intel.com>
> Subject: Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output
> 
> 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),
> >>>>
> >>
> >>
> >
> >
> >
> >
> 
> 

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

[Philippe Mathieu-Daudé]

Hi Liming, Rebecca.

On 4/11/19 3:52 PM, Liming Gao wrote:
> Phil:
>   I use Outlook to receive the patch mail, and save as mail file. This file shows From: Rebecca Cran via Groups.Io <rebecca=bluestop.org@groups.io>. So, I use it as patch author. I also check my patch with the same way. It shows From: Liming Gao <liming.gao@intel.com>. This is correct. So, I don't know this is user setting issue or group.io issue. 

On https://edk2.groups.io/g/devel/editprofile

So on Groups.Io I went to
- Your groups
  -> EDK2
     -> devel
        -> Suscription
           -> Group Profile
              -> Edit Group Profile

I see 2 fields:

- User Name
- Display Name

The latter field is described as:

  The name listed with your posts. Note: When you post,
  your email address is also shown to group members who
  receive messages via email.

My guess is Groups.Io rewrites the sender From if an user change one of
these fields.

Rebecca: do you remember if you modified something in your profile?
How are you sending your patches to the list?

Sorry you became our 'patches via groups.io' guinea pig ;)

Thanks,

Phil.

> Thanks
> Liming
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Philippe Mathieu-Daudé
>> Sent: Thursday, April 11, 2019 6:59 PM
>> To: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>
>> Cc: devel@edk2.groups.io; rebecca@bluestop.org; Stephano Cetola <stephano.cetola@linux.intel.com>
>> Subject: Re: [edk2-devel] [PATCH] IntelFrameworkModulePkg: Fix comments and improve E820 debug output
>>
>> 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>
>>>>>
>>>>>> ---
[...]

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

[Laszlo Ersek]

On 04/11/19 16:32, Philippe Mathieu-Daudé wrote:
> Hi Liming, Rebecca.
> 
> On 4/11/19 3:52 PM, Liming Gao wrote:
>> Phil:
>>   I use Outlook to receive the patch mail, and save as mail file. This file shows From: Rebecca Cran via Groups.Io <rebecca=bluestop.org@groups.io>. So, I use it as patch author. I also check my patch with the same way. It shows From: Liming Gao <liming.gao@intel.com>. This is correct. So, I don't know this is user setting issue or group.io issue. 
> 
> On https://edk2.groups.io/g/devel/editprofile
> 
> So on Groups.Io I went to
> - Your groups
>   -> EDK2
>      -> devel
>         -> Suscription
>            -> Group Profile
>               -> Edit Group Profile
> 
> I see 2 fields:
> 
> - User Name
> - Display Name
> 
> The latter field is described as:
> 
>   The name listed with your posts. Note: When you post,
>   your email address is also shown to group members who
>   receive messages via email.
> 
> My guess is Groups.Io rewrites the sender From if an user change one of
> these fields.
> 
> Rebecca: do you remember if you modified something in your profile?
> How are you sending your patches to the list?
> 
> Sorry you became our 'patches via groups.io' guinea pig ;)

My understanding is that this is unrelated to groups.io. The same kind of email mangling used to happen a *lot* on the old <01.org> list. (From a number of senders.) Here's an example:

(1) Locate the message at

  https://lists.01.org/pipermail/edk2-devel/2019-March/038118.html

It looks correct, right?

(2) Now locate the same message in a different archive, e.g. at:

  http://mid.mail-archive.com/20190324020033.50367-1-rebecca@bluestop.org

Notice it says "Rebecca Cran via edk2-devel".

(3) In my local copy of that message, I see the following headers:

- From: Rebecca Cran via edk2-devel <edk2-devel@lists.01.org>
- Reply-To: Rebecca Cran <rebecca@bluestop.org>
- Sender: "edk2-devel" <edk2-devel-bounces@lists.01.org>

The damage is done before the patch receives the list(s), and/or is fed to git-am on the maintainer side.

If I remember correctly, this is somehow related to Outlook (i.e., the sender's MUA / first MTA), not the mailing list(s), or git.

Thanks
Laszlo

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

[Rebecca Cran]

On 2019-04-11 11:35, Laszlo Ersek wrote:
> On 04/11/19 16:32, Philippe Mathieu-Daudé wrote:
>>
>> Rebecca: do you remember if you modified something in your profile?
>> How are you sending your patches to the list?
>>
>> Sorry you became our 'patches via groups.io' guinea pig ;)

Yes, I've changed those fields.


> If I remember correctly, this is somehow related to Outlook (i.e., the sender's MUA / first MTA), not the mailing list(s), or git.


I'm sending emails on Thunderbird via Postfix, so there's no Outlook 
involved.


-- 
Rebecca Cran

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

[rebecca]

On 2019-04-11 13:30, Rebecca Cran via Groups.Io wrote:
>
> I'm sending emails on Thunderbird via Postfix, so there's no Outlook 
> involved.
>
>

Let's try again: I've unset the groups.io fields, so I wonder if that'll 
change things.

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

[Philippe Mathieu-Daudé]

On 4/11/19 9:53 PM, Rebecca Cran wrote:
> On 2019-04-11 13:30, Rebecca Cran via Groups.Io wrote:
>>
>> I'm sending emails on Thunderbird via Postfix, so there's no Outlook
>> involved.
>>
> 
> Let's try again: I've unset the groups.io fields, so I wonder if that'll
> change things.

Yes, I see your From address correctly now.

Now, how do we proceed to avoid the same problem with other users?

IMHO a git pre-push hook on the maintainer side would be easier to maintain.

Regards,

Phil.

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

[Laszlo Ersek]

On 04/11/19 21:59, Philippe Mathieu-Daudé wrote:
> On 4/11/19 9:53 PM, Rebecca Cran wrote:
>> On 2019-04-11 13:30, Rebecca Cran via Groups.Io wrote:
>>>
>>> I'm sending emails on Thunderbird via Postfix, so there's no Outlook
>>> involved.
>>>
>>
>> Let's try again: I've unset the groups.io fields, so I wonder if that'll
>> change things.
> 
> Yes, I see your From address correctly now.

Me too. I stand corrected! :)

(It remains a mistery though why the same issue occurred on the old list
/ with mailman2, multiple times.)

> Now, how do we proceed to avoid the same problem with other users?
> 
> IMHO a git pre-push hook on the maintainer side would be easier to maintain.

I'd be happy to adopt one. Can you recommend specifics?

Also -- can you please describe the problematic groups.io settings
again? (I've re-read your previous description now, but it's unclear.)

This would be important because Stephano could either take it to the
groups.io guys to fix, or add another item to our suggested settings,
for when people join the group.

Thanks,
Laszlo

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

[Laszlo Ersek]

On 04/11/19 21:59, Philippe Mathieu-Daudé wrote:
> On 4/11/19 9:53 PM, Rebecca Cran wrote:
>> On 2019-04-11 13:30, Rebecca Cran via Groups.Io wrote:
>>>
>>> I'm sending emails on Thunderbird via Postfix, so there's no Outlook
>>> involved.
>>>
>>
>> Let's try again: I've unset the groups.io fields, so I wonder if that'll
>> change things.
> 
> Yes, I see your From address correctly now.

Actually, I have to withdraw my previous confirmation.

When I wrote

  "Me too. I stand corrected"

previously, I was replying to the copy of the thread that was delivered
to me directly. However, now that I'm looking at my list folder (i.e. at
messages reflected by groups.io), the only change that I'm seeing
between Rebecca's last two emails is:

- before: "Rebecca Cran via Groups.Io <rebecca=bluestop.org@groups.io>"
- after: "rebecca via Groups.Io <rebecca=bluestop.org@groups.io>"

So, if a maintainer applies a patch directly from the list, with git-am,
it will *still* not have right authorship information.

Thanks
Laszlo

> 
> Now, how do we proceed to avoid the same problem with other users?
> 
> IMHO a git pre-push hook on the maintainer side would be easier to maintain.
> 
> Regards,
> 
> Phil.
> 
> 
>