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 (#55408): https://edk2.groups.io/g/devel/message/55408 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] -=-=-=-=-=-=-=-=-=-=-=-
