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