Hi Laszlo,

Got it. Go ahead.

Thanks,
Eric

-----Original Message-----
From: Laszlo Ersek <ler...@redhat.com> 
Sent: Wednesday, March 4, 2020 8:23 PM
To: devel@edk2.groups.io; Dong, Eric <eric.d...@intel.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Igor Mammedov 
<imamm...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; Justen, Jordan L 
<jordan.l.jus...@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com>; 
Philippe Mathieu-Daudé <phi...@redhat.com>; Ni, Ray <ray...@intel.com>; Boris 
Ostrovsky <boris.ostrov...@oracle.com>
Subject: Re: [edk2-devel] [PATCH v2 02/16] UefiCpuPkg/PiSmmCpuDxeSmm: fix S3 
Resume for CPU hotplug

Hi Eric,

On 02/28/20 04:05, Dong, Eric wrote:
> Hi Laszlo,
>
> Thanks for your patch. The change make sense base on the comments in 
> the data structure header file.
>
> I also checked all the code related to this data structure. The inputs 
> for this data structure are CpuS3DataDxe and RegisterCpuFeaturesLib.
> Both these two drivers not support CPU hot plug feature, so the real 
> inputs for mAcpiCpuData.NumberOfCpus is the enabled CPU number in this 
> system. So before and after your code change, the CPU values are same.
> But the data structure comments said it can support CPU hot plug, so I 
> agree your code change.
>
> Reviewed-by: Eric Dong <eric.d...@intel.com>

while merging this patch, the edk2 CI system reported the following build 
warning (in the "Windows VS2019 PR" check):

https://github.com/tianocore/edk2/pull/416/checks?check_run_id=484688972
https://dev.azure.com/tianocore/edk2-ci/_build/results?buildId=4568&view=logs&j=898a5c7a-7a49-5be1-c417-92a6761a8039&t=7c50f5c2-8a2c-50f9-5007-e26f12377af8&l=64

> 2020-03-04T11:06:29.7838532Z ERROR - Compiler #2220 from 
> d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(624):   the following warning is 
> treated as an error
> 2020-03-04T11:06:29.7839570Z WARNING - Compiler #4244 from 
> d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(624):   '=': conversion from 
> 'UINTN' to 'volatile UINT32', possible loss of data
> 2020-03-04T11:06:29.7841177Z WARNING - Compiler #4244 from 
> d:\a\1\s\UefiCpuPkg\PiSmmCpuDxeSmm\CpuS3.c(659):   '=': conversion from 
> 'UINTN' to 'volatile UINT32', possible loss of data

I've suppressed that by squashing the following hunks into the patch:

> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index 1e0840119724..29e9ba92b453 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -621,7 +621,7 @@ InitializeCpuBeforeRebase (
>    } else {
>      ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);
>    }
> -  mNumberToFinish = mNumberOfCpus - 1;
> +  mNumberToFinish = (UINT32)(mNumberOfCpus - 1);
>    mExchangeInfo->ApFunction  = (VOID *) (UINTN) InitializeAp;
>
>    //
> @@ -656,7 +656,7 @@ InitializeCpuAfterRebase (
>    } else {
>      ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);
>    }
> -  mNumberToFinish = mNumberOfCpus - 1;
> +  mNumberToFinish = (UINT32)(mNumberOfCpus - 1);
>
>    //
>    // Signal that SMM base relocation is complete and to continue 
> initialization for all APs.

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55415): https://edk2.groups.io/g/devel/message/55415
Mute This Topic: https://groups.io/mt/71575170/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to