Re: [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

["Dong, Eric" <eric.dong@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4256 bytes --]

Hi Laszlo,

Thanks for your advice. I truly received the r-b from Dandan, but I just found she just reply to me.  But I truly forgot to include you in the r-b list. I will follow your suggestion next time.

Thanks,
Eric
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Thursday, September 6, 2018 8:04 PM
To: Dong, Eric <eric.dong@intel.com>
Cc: edk2-devel@lists.01.org; Bi, Dandan <dandan.bi@intel.com>
Subject: Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

Hi Eric,

On 09/05/18 09:57, Laszlo Ersek wrote:
> On 09/05/18 08:22, Eric Dong wrote:
>> Fix trailing white spaces and invalid line ending issue.
>>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Eric Dong <eric.dong@intel.com>
>> ---
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c                               | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/MpLib.h                               | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf                      | 2 +-
>>  UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 
>> 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index 377876643f..5c562d4759 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -1014,7 +1014,7 @@ WakeUpAP (
>>          CpuData = &CpuMpData->CpuData[Index];
>>          //
>>          // All AP(include disabled AP) will be woke up by INIT-SIPI-SIPI, but
>> -        // the AP procedure will be skipped for disabled AP because AP state 
>> +        // the AP procedure will be skipped for disabled AP because 
>> + AP state
>>          // is not CpuStateReady.
>>          //
>>          if (GetApState (CpuData) == CpuStateDisabled && 
>> !WakeUpDisabledAps) { diff --git 
>> a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> index 75f3fdda1d..773db76b61 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> @@ -257,7 +257,7 @@ struct _CPU_MP_DATA {
>>    // Whether need to use Init-Sipi-Sipi to wake up the APs.
>>    // Two cases need to set this value to TRUE. One is in HLT
>>    // loop mode, the other is resume from S3 which loop mode
>> -  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
>> +  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
>>    // driver.
>>    //
>>    BOOLEAN                        WakeUpByInitSipiSipi;
>> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
>> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> index 43a3b3b036..81036f0b12 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> @@ -67,4 +67,4 @@
>>    gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                   ## SOMETIMES_CONSUMES
>>  
>>  [Guids]
>> -  gEdkiiS3SmmInitDoneGuid
>> \ No newline at end of file
>> +  gEdkiiS3SmmInitDoneGuid
>> diff --git 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
>> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> index 4143ee4bb1..fa7e107e39 100644
>> --- 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLi
>> +++ b.c
>> @@ -516,7 +516,7 @@ AllocateAcpiCpuData (
>>  
>>    //
>>    // Allocate buffer for empty RegisterTable and 
>> PreSmmInitRegisterTable for all CPUs
>> -  //
>> +  //
>>    TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
>>    RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
>>    ASSERT (RegisterTable != NULL);
>>
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

I think you may have made a mistake when committing this patch (commit e23d9c3ed86c); you added Dandan's R-b, but on the list, the one R-b you got was from me, not Dandan.

Not a big issue, but as a best practice for the future, I recommend copying the feedback tags verbatim from the reply emails, using the clipboard.

Thanks
Laszlo

[-- Attachment #2: Type: message/rfc822, Size: 4261 bytes --]

From: "Bi, Dandan" <dandan.bi@intel.com>
To: "Dong, Eric" <eric.dong@intel.com>
Subject: RE: [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.
Date: Wed, 5 Sep 2018 07:29:09 +0000
Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB3BB71DCA@shsmsx102.ccr.corp.intel.com>

Reviewed-by: Dandan Bi <dandan.bi@intel.com>

Thanks,
Dandan

-----Original Message-----
From: Dong, Eric
Sent: Wednesday, September 5, 2018 2:22 PM
To: edk2-devel@lists.01.org
Cc: Bi, Dandan <dandan.bi@intel.com>
Subject: [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

Fix trailing white spaces and invalid line ending issue.

Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c                               | 2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h                               | 2 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf                      | 2 +-
 UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 377876643f..5c562d4759 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1014,7 +1014,7 @@ WakeUpAP (
         CpuData = &CpuMpData->CpuData[Index];
         //
         // All AP(include disabled AP) will be woke up by INIT-SIPI-SIPI, but
-        // the AP procedure will be skipped for disabled AP because AP state
+        // the AP procedure will be skipped for disabled AP because AP
+ state
         // is not CpuStateReady.
         //
         if (GetApState (CpuData) == CpuStateDisabled && !WakeUpDisabledAps) { diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 75f3fdda1d..773db76b61 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -257,7 +257,7 @@ struct _CPU_MP_DATA {
   // Whether need to use Init-Sipi-Sipi to wake up the APs.
   // Two cases need to set this value to TRUE. One is in HLT
   // loop mode, the other is resume from S3 which loop mode
-  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
+  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
   // driver.
   //
   BOOLEAN                        WakeUpByInitSipiSipi;
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 43a3b3b036..81036f0b12 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -67,4 +67,4 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                   ## SOMETIMES_CONSUMES

 [Guids]
-  gEdkiiS3SmmInitDoneGuid
\ No newline at end of file
+  gEdkiiS3SmmInitDoneGuid
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index 4143ee4bb1..fa7e107e39 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -516,7 +516,7 @@ AllocateAcpiCpuData (

   //
   // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs
-  //
+  //
   TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
   RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
   ASSERT (RegisterTable != NULL);
--
2.16.2.windows.1