* [edk2-devel] [PATCH 0/2] Change the usage of input parameter ProcessorNumber in MpInitLibGetProcessorInfo() of MpInitLibUp @ 2024-01-04 7:32 duntan 2024-01-04 7:32 ` [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION duntan 2024-01-04 7:32 ` [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber duntan 0 siblings, 2 replies; 11+ messages in thread From: duntan @ 2024-01-04 7:32 UTC (permalink / raw) To: devel In this patch set, the usage of input parameter ProcessorNumber in MpInitLibGetProcessorInfo() of MpInitLibUp instance is changed. 1.Retrive EXTENDED_PROCESSOR_INFORMATION in the API MpInitLibGetProcessorInfo() of MpInitLibUp instance when the BIT24 of input ProcessorNumber is set. It's to align with the behavior in PEI/DXE MpInitLib. 2.Also, check lower 24 bits of ProcessorNumber instead of the value of ProcessorNumber in the API MpInitLibGetProcessorInfo() of MpInitLibUp instance. The BIT24 of input ProcessorNumber might be set to indicate if the EXTENDED_PROCESSOR_INFORMATION will be retrived. Dun Tan (2): UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION UefiCpuPkg: Check lower 24 bits of ProcessorNumber UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 17 ++++++++++++++++- 3 files changed, 20 insertions(+), 1 deletion(-) -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113137): https://edk2.groups.io/g/devel/message/113137 Mute This Topic: https://groups.io/mt/103518740/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION 2024-01-04 7:32 [edk2-devel] [PATCH 0/2] Change the usage of input parameter ProcessorNumber in MpInitLibGetProcessorInfo() of MpInitLibUp duntan @ 2024-01-04 7:32 ` duntan 2024-01-04 14:53 ` Laszlo Ersek 2024-01-04 7:32 ` [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber duntan 1 sibling, 1 reply; 11+ messages in thread From: duntan @ 2024-01-04 7:32 UTC (permalink / raw) To: devel; +Cc: Ray Ni, Laszlo Ersek, Rahul Kumar, Gerd Hoffmann, Min Xu Retrive EXTENDED_PROCESSOR_INFORMATION in the API MpInitLibGetProcessorInfo() of MpInitLibUp instance when the BIT24 of input ProcessorNumber is set. It's to align with the behavior in PEI/DXE MpInitLib Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Min Xu <min.m.xu@intel.com> --- UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 12 ++++++++++++ 3 files changed, 16 insertions(+) diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h b/UefiCpuPkg/Include/Library/MpInitLib.h index 1853c46415..842c6f7ff9 100644 --- a/UefiCpuPkg/Include/Library/MpInitLib.h +++ b/UefiCpuPkg/Include/Library/MpInitLib.h @@ -63,6 +63,8 @@ MpInitLibGetNumberOfProcessors ( instant this call is made. This service may only be called from the BSP. @param[in] ProcessorNumber The handle number of processor. + Lower 24 bits contains the actual processor number. + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. @param[out] ProcessorInfoBuffer A pointer to the buffer where information for the requested processor is deposited. @param[out] HealthData Return processor health data. diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index a359906923..cdfb570e61 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -2333,6 +2333,8 @@ MpInitLibInitialize ( instant this call is made. This service may only be called from the BSP. @param[in] ProcessorNumber The handle number of processor. + Lower 24 bits contains the actual processor number. + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. @param[out] ProcessorInfoBuffer A pointer to the buffer where information for the requested processor is deposited. @param[out] HealthData Return processor health data. diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c index 86f9fbf903..3af4911d4b 100644 --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c @@ -77,6 +77,8 @@ MpInitLibGetNumberOfProcessors ( instant this call is made. This service may only be called from the BSP. @param[in] ProcessorNumber The handle number of processor. + Lower 24 bits contains the actual processor number. + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. @param[out] ProcessorInfoBuffer A pointer to the buffer where information for the requested processor is deposited. @param[out] HealthData Return processor health data. @@ -115,6 +117,16 @@ MpInitLibGetProcessorInfo ( ProcessorInfoBuffer->Location.Package = 0; ProcessorInfoBuffer->Location.Core = 0; ProcessorInfoBuffer->Location.Thread = 0; + + if ((ProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) { + ProcessorInfoBuffer->ExtendedInformation.Location2.Package = 0; + ProcessorInfoBuffer->ExtendedInformation.Location2.Die = 0; + ProcessorInfoBuffer->ExtendedInformation.Location2.Tile = 0; + ProcessorInfoBuffer->ExtendedInformation.Location2.Module = 0; + ProcessorInfoBuffer->ExtendedInformation.Location2.Core = 0; + ProcessorInfoBuffer->ExtendedInformation.Location2.Thread = 0; + } + if (HealthData != NULL) { GuidHob = GetFirstGuidHob (&gEfiSecPlatformInformationPpiGuid); if (GuidHob != NULL) { -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113138): https://edk2.groups.io/g/devel/message/113138 Mute This Topic: https://groups.io/mt/103518742/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION 2024-01-04 7:32 ` [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION duntan @ 2024-01-04 14:53 ` Laszlo Ersek 2024-01-05 9:24 ` duntan 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2024-01-04 14:53 UTC (permalink / raw) To: devel, dun.tan; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Min Xu On 1/4/24 08:32, duntan wrote: > Retrive EXTENDED_PROCESSOR_INFORMATION in the API > MpInitLibGetProcessorInfo() of MpInitLibUp instance > when the BIT24 of input ProcessorNumber is set. > It's to align with the behavior in PEI/DXE MpInitLib > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Min Xu <min.m.xu@intel.com> > --- > UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ > UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 12 ++++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h b/UefiCpuPkg/Include/Library/MpInitLib.h > index 1853c46415..842c6f7ff9 100644 > --- a/UefiCpuPkg/Include/Library/MpInitLib.h > +++ b/UefiCpuPkg/Include/Library/MpInitLib.h > @@ -63,6 +63,8 @@ MpInitLibGetNumberOfProcessors ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index a359906923..cdfb570e61 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -2333,6 +2333,8 @@ MpInitLibInitialize ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > index 86f9fbf903..3af4911d4b 100644 > --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > @@ -77,6 +77,8 @@ MpInitLibGetNumberOfProcessors ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > @@ -115,6 +117,16 @@ MpInitLibGetProcessorInfo ( > ProcessorInfoBuffer->Location.Package = 0; > ProcessorInfoBuffer->Location.Core = 0; > ProcessorInfoBuffer->Location.Thread = 0; > + > + if ((ProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) { > + ProcessorInfoBuffer->ExtendedInformation.Location2.Package = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Die = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Tile = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Module = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Core = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Thread = 0; > + } > + > if (HealthData != NULL) { > GuidHob = GetFirstGuidHob (&gEfiSecPlatformInformationPpiGuid); > if (GuidHob != NULL) { (1) For the UP implementation of MpInitLibGetProcessorInfo(): How about, for a *complete* solution (covering both pre-patch and post-patch functionality): ZeroMem (ProcessorInfoBuffer, sizeof *ProcessorInfoBuffer); ProcessorInfoBuffer->StatusFlag = PROCESSOR_AS_BSP_BIT | PROCESSOR_ENABLED_BIT | PROCESSOR_HEALTH_STATUS_BIT; This approach is not slow (most of the time I expect the platform will have an optimized ZeroMem() implementation), it is frugal with code (replaces a bunch of manual zeroing of fields), and it is relatively future-proof (the next time EFI_PROCESSOR_INFORMATION is extended, you likely won't have to touch up the code again, because the ZeroMem() will cover the new fields automatically). Also, this approach will zero out ProcessorInfoBuffer->ExtendedInformation *regardless* of BIT24 in the input, which I kind of consider an advantage! (No garbage in the output structure.) Again, I don't think the zeroing is wasteful, regarding CPU cycles. I do agree that the leading function comments should mention BIT24 Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113179): https://edk2.groups.io/g/devel/message/113179 Mute This Topic: https://groups.io/mt/103518742/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION 2024-01-04 14:53 ` Laszlo Ersek @ 2024-01-05 9:24 ` duntan 2024-01-05 12:56 ` Ni, Ray 0 siblings, 1 reply; 11+ messages in thread From: duntan @ 2024-01-05 9:24 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io Cc: Ni, Ray, Kumar, Rahul R, Gerd Hoffmann, Xu, Min M Hi Laszlo, Thanks for your comments. I agree with your solution. It seems simpler and clearer. Will change the code and keep the additional function comments in next version patch set. Thanks, Dun -----Original Message----- From: Laszlo Ersek <lersek@redhat.com> Sent: Thursday, January 4, 2024 10:53 PM To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> Subject: Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION On 1/4/24 08:32, duntan wrote: > Retrive EXTENDED_PROCESSOR_INFORMATION in the API > MpInitLibGetProcessorInfo() of MpInitLibUp instance when the BIT24 of > input ProcessorNumber is set. > It's to align with the behavior in PEI/DXE MpInitLib > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Min Xu <min.m.xu@intel.com> > --- > UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ > UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ > UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 12 ++++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h > b/UefiCpuPkg/Include/Library/MpInitLib.h > index 1853c46415..842c6f7ff9 100644 > --- a/UefiCpuPkg/Include/Library/MpInitLib.h > +++ b/UefiCpuPkg/Include/Library/MpInitLib.h > @@ -63,6 +63,8 @@ MpInitLibGetNumberOfProcessors ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index a359906923..cdfb570e61 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -2333,6 +2333,8 @@ MpInitLibInitialize ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > index 86f9fbf903..3af4911d4b 100644 > --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > @@ -77,6 +77,8 @@ MpInitLibGetNumberOfProcessors ( > instant this call is made. This service may only be called from the BSP. > > @param[in] ProcessorNumber The handle number of processor. > + Lower 24 bits contains the actual processor number. > + BIT24 indicates if the EXTENDED_PROCESSOR_INFORMATION will be retrived. > @param[out] ProcessorInfoBuffer A pointer to the buffer where information for > the requested processor is deposited. > @param[out] HealthData Return processor health data. > @@ -115,6 +117,16 @@ MpInitLibGetProcessorInfo ( > ProcessorInfoBuffer->Location.Package = 0; > ProcessorInfoBuffer->Location.Core = 0; > ProcessorInfoBuffer->Location.Thread = 0; > + > + if ((ProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) { > + ProcessorInfoBuffer->ExtendedInformation.Location2.Package = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Die = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Tile = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Module = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Core = 0; > + ProcessorInfoBuffer->ExtendedInformation.Location2.Thread = 0; > + } > + > if (HealthData != NULL) { > GuidHob = GetFirstGuidHob (&gEfiSecPlatformInformationPpiGuid); > if (GuidHob != NULL) { (1) For the UP implementation of MpInitLibGetProcessorInfo(): How about, for a *complete* solution (covering both pre-patch and post-patch functionality): ZeroMem (ProcessorInfoBuffer, sizeof *ProcessorInfoBuffer); ProcessorInfoBuffer->StatusFlag = PROCESSOR_AS_BSP_BIT | PROCESSOR_ENABLED_BIT | PROCESSOR_HEALTH_STATUS_BIT; This approach is not slow (most of the time I expect the platform will have an optimized ZeroMem() implementation), it is frugal with code (replaces a bunch of manual zeroing of fields), and it is relatively future-proof (the next time EFI_PROCESSOR_INFORMATION is extended, you likely won't have to touch up the code again, because the ZeroMem() will cover the new fields automatically). Also, this approach will zero out ProcessorInfoBuffer->ExtendedInformation *regardless* of BIT24 in the input, which I kind of consider an advantage! (No garbage in the output structure.) Again, I don't think the zeroing is wasteful, regarding CPU cycles. I do agree that the leading function comments should mention BIT24 Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113233): https://edk2.groups.io/g/devel/message/113233 Mute This Topic: https://groups.io/mt/103518742/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION 2024-01-05 9:24 ` duntan @ 2024-01-05 12:56 ` Ni, Ray 2024-01-05 13:59 ` Laszlo Ersek 0 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2024-01-05 12:56 UTC (permalink / raw) To: Tan, Dun, Laszlo Ersek, devel@edk2.groups.io Cc: Kumar, Rahul R, Gerd Hoffmann, Xu, Min M Laszlo, Good suggestion. Your solution will not work if in future some extra fields might require to be set to non-zero. But future is not coming yet. I agree with your approach. Thanks, Ray > -----Original Message----- > From: Tan, Dun <dun.tan@intel.com> > Sent: Friday, January 5, 2024 5:25 PM > To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io > Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; > Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> > Subject: RE: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive > EXTENDED_PROCESSOR_INFORMATION > > Hi Laszlo, > > Thanks for your comments. I agree with your solution. It seems simpler and > clearer. Will change the code and keep the additional function comments in > next version patch set. > > Thanks, > Dun > > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Thursday, January 4, 2024 10:53 PM > To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com> > Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; > Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> > Subject: Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive > EXTENDED_PROCESSOR_INFORMATION > > On 1/4/24 08:32, duntan wrote: > > Retrive EXTENDED_PROCESSOR_INFORMATION in the API > > MpInitLibGetProcessorInfo() of MpInitLibUp instance when the BIT24 of > > input ProcessorNumber is set. > > It's to align with the behavior in PEI/DXE MpInitLib > > > > Signed-off-by: Dun Tan <dun.tan@intel.com> > > Cc: Ray Ni <ray.ni@intel.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Rahul Kumar <rahul1.kumar@intel.com> > > Cc: Gerd Hoffmann <kraxel@redhat.com> > > Cc: Min Xu <min.m.xu@intel.com> > > --- > > UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ > > UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ > > UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 12 ++++++++++++ > > 3 files changed, 16 insertions(+) > > > > diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h > > b/UefiCpuPkg/Include/Library/MpInitLib.h > > index 1853c46415..842c6f7ff9 100644 > > --- a/UefiCpuPkg/Include/Library/MpInitLib.h > > +++ b/UefiCpuPkg/Include/Library/MpInitLib.h > > @@ -63,6 +63,8 @@ MpInitLibGetNumberOfProcessors ( > > instant this call is made. This service may only be called from the BSP. > > > > @param[in] ProcessorNumber The handle number of processor. > > + Lower 24 bits contains the actual processor number. > > + BIT24 indicates if the > EXTENDED_PROCESSOR_INFORMATION will be retrived. > > @param[out] ProcessorInfoBuffer A pointer to the buffer where > information for > > the requested processor is deposited. > > @param[out] HealthData Return processor health data. > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > index a359906923..cdfb570e61 100644 > > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > > @@ -2333,6 +2333,8 @@ MpInitLibInitialize ( > > instant this call is made. This service may only be called from the BSP. > > > > @param[in] ProcessorNumber The handle number of processor. > > + Lower 24 bits contains the actual processor number. > > + BIT24 indicates if the > EXTENDED_PROCESSOR_INFORMATION will be retrived. > > @param[out] ProcessorInfoBuffer A pointer to the buffer where > information for > > the requested processor is deposited. > > @param[out] HealthData Return processor health data. > > diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > > b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > > index 86f9fbf903..3af4911d4b 100644 > > --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > > +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > > @@ -77,6 +77,8 @@ MpInitLibGetNumberOfProcessors ( > > instant this call is made. This service may only be called from the BSP. > > > > @param[in] ProcessorNumber The handle number of processor. > > + Lower 24 bits contains the actual processor number. > > + BIT24 indicates if the > EXTENDED_PROCESSOR_INFORMATION will be retrived. > > @param[out] ProcessorInfoBuffer A pointer to the buffer where > information for > > the requested processor is deposited. > > @param[out] HealthData Return processor health data. > > @@ -115,6 +117,16 @@ MpInitLibGetProcessorInfo ( > > ProcessorInfoBuffer->Location.Package = 0; > > ProcessorInfoBuffer->Location.Core = 0; > > ProcessorInfoBuffer->Location.Thread = 0; > > + > > + if ((ProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) { > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Package = 0; > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Die = 0; > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Tile = 0; > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Module = 0; > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Core = 0; > > + ProcessorInfoBuffer->ExtendedInformation.Location2.Thread = 0; > > + } > > + > > if (HealthData != NULL) { > > GuidHob = GetFirstGuidHob (&gEfiSecPlatformInformationPpiGuid); > > if (GuidHob != NULL) { > > (1) For the UP implementation of MpInitLibGetProcessorInfo(): > > How about, for a *complete* solution (covering both pre-patch and post- > patch functionality): > > ZeroMem (ProcessorInfoBuffer, sizeof *ProcessorInfoBuffer); > ProcessorInfoBuffer->StatusFlag = PROCESSOR_AS_BSP_BIT | > PROCESSOR_ENABLED_BIT | > PROCESSOR_HEALTH_STATUS_BIT; > > This approach is not slow (most of the time I expect the platform will have an > optimized ZeroMem() implementation), it is frugal with code (replaces a > bunch of manual zeroing of fields), and it is relatively future-proof (the next > time EFI_PROCESSOR_INFORMATION is extended, you likely won't have to > touch up the code again, because the ZeroMem() will cover the new fields > automatically). > > Also, this approach will zero out > ProcessorInfoBuffer->ExtendedInformation *regardless* of BIT24 in the > input, which I kind of consider an advantage! (No garbage in the output > structure.) Again, I don't think the zeroing is wasteful, regarding CPU cycles. > > I do agree that the leading function comments should mention BIT24 > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113282): https://edk2.groups.io/g/devel/message/113282 Mute This Topic: https://groups.io/mt/103518742/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION 2024-01-05 12:56 ` Ni, Ray @ 2024-01-05 13:59 ` Laszlo Ersek 0 siblings, 0 replies; 11+ messages in thread From: Laszlo Ersek @ 2024-01-05 13:59 UTC (permalink / raw) To: Ni, Ray, Tan, Dun, devel@edk2.groups.io Cc: Kumar, Rahul R, Gerd Hoffmann, Xu, Min M On 1/5/24 13:56, Ni, Ray wrote: > Laszlo, > Good suggestion. > > Your solution will not work if in future some extra fields might require to be set to non-zero. > But future is not coming yet. I agree with your approach. Well, if we need to set some fields to nonzero, manual assignments will become necessary either way, with or without the ZeroMem(). With the ZeroMem(), we just overwrite the zero values later. I certainly agree that there is a tipping point. Like, if we have 5 fields, and we need to set 4 of them to nonzero values, then an initial ZeroMem is wasteful. Right now the ZeroMem() looks much more frugal, and a bit more future-proof too. Thanks! Laszlo > > Thanks, > Ray >> -----Original Message----- >> From: Tan, Dun <dun.tan@intel.com> >> Sent: Friday, January 5, 2024 5:25 PM >> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io >> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; >> Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> >> Subject: RE: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive >> EXTENDED_PROCESSOR_INFORMATION >> >> Hi Laszlo, >> >> Thanks for your comments. I agree with your solution. It seems simpler and >> clearer. Will change the code and keep the additional function comments in >> next version patch set. >> >> Thanks, >> Dun >> >> -----Original Message----- >> From: Laszlo Ersek <lersek@redhat.com> >> Sent: Thursday, January 4, 2024 10:53 PM >> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com> >> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; >> Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> >> Subject: Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive >> EXTENDED_PROCESSOR_INFORMATION >> >> On 1/4/24 08:32, duntan wrote: >>> Retrive EXTENDED_PROCESSOR_INFORMATION in the API >>> MpInitLibGetProcessorInfo() of MpInitLibUp instance when the BIT24 of >>> input ProcessorNumber is set. >>> It's to align with the behavior in PEI/DXE MpInitLib >>> >>> Signed-off-by: Dun Tan <dun.tan@intel.com> >>> Cc: Ray Ni <ray.ni@intel.com> >>> Cc: Laszlo Ersek <lersek@redhat.com> >>> Cc: Rahul Kumar <rahul1.kumar@intel.com> >>> Cc: Gerd Hoffmann <kraxel@redhat.com> >>> Cc: Min Xu <min.m.xu@intel.com> >>> --- >>> UefiCpuPkg/Include/Library/MpInitLib.h | 2 ++ >>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ >>> UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 12 ++++++++++++ >>> 3 files changed, 16 insertions(+) >>> >>> diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h >>> b/UefiCpuPkg/Include/Library/MpInitLib.h >>> index 1853c46415..842c6f7ff9 100644 >>> --- a/UefiCpuPkg/Include/Library/MpInitLib.h >>> +++ b/UefiCpuPkg/Include/Library/MpInitLib.h >>> @@ -63,6 +63,8 @@ MpInitLibGetNumberOfProcessors ( >>> instant this call is made. This service may only be called from the BSP. >>> >>> @param[in] ProcessorNumber The handle number of processor. >>> + Lower 24 bits contains the actual processor number. >>> + BIT24 indicates if the >> EXTENDED_PROCESSOR_INFORMATION will be retrived. >>> @param[out] ProcessorInfoBuffer A pointer to the buffer where >> information for >>> the requested processor is deposited. >>> @param[out] HealthData Return processor health data. >>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> index a359906923..cdfb570e61 100644 >>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >>> @@ -2333,6 +2333,8 @@ MpInitLibInitialize ( >>> instant this call is made. This service may only be called from the BSP. >>> >>> @param[in] ProcessorNumber The handle number of processor. >>> + Lower 24 bits contains the actual processor number. >>> + BIT24 indicates if the >> EXTENDED_PROCESSOR_INFORMATION will be retrived. >>> @param[out] ProcessorInfoBuffer A pointer to the buffer where >> information for >>> the requested processor is deposited. >>> @param[out] HealthData Return processor health data. >>> diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c >>> b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c >>> index 86f9fbf903..3af4911d4b 100644 >>> --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c >>> +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c >>> @@ -77,6 +77,8 @@ MpInitLibGetNumberOfProcessors ( >>> instant this call is made. This service may only be called from the BSP. >>> >>> @param[in] ProcessorNumber The handle number of processor. >>> + Lower 24 bits contains the actual processor number. >>> + BIT24 indicates if the >> EXTENDED_PROCESSOR_INFORMATION will be retrived. >>> @param[out] ProcessorInfoBuffer A pointer to the buffer where >> information for >>> the requested processor is deposited. >>> @param[out] HealthData Return processor health data. >>> @@ -115,6 +117,16 @@ MpInitLibGetProcessorInfo ( >>> ProcessorInfoBuffer->Location.Package = 0; >>> ProcessorInfoBuffer->Location.Core = 0; >>> ProcessorInfoBuffer->Location.Thread = 0; >>> + >>> + if ((ProcessorNumber & CPU_V2_EXTENDED_TOPOLOGY) != 0) { >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Package = 0; >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Die = 0; >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Tile = 0; >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Module = 0; >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Core = 0; >>> + ProcessorInfoBuffer->ExtendedInformation.Location2.Thread = 0; >>> + } >>> + >>> if (HealthData != NULL) { >>> GuidHob = GetFirstGuidHob (&gEfiSecPlatformInformationPpiGuid); >>> if (GuidHob != NULL) { >> >> (1) For the UP implementation of MpInitLibGetProcessorInfo(): >> >> How about, for a *complete* solution (covering both pre-patch and post- >> patch functionality): >> >> ZeroMem (ProcessorInfoBuffer, sizeof *ProcessorInfoBuffer); >> ProcessorInfoBuffer->StatusFlag = PROCESSOR_AS_BSP_BIT | >> PROCESSOR_ENABLED_BIT | >> PROCESSOR_HEALTH_STATUS_BIT; >> >> This approach is not slow (most of the time I expect the platform will have an >> optimized ZeroMem() implementation), it is frugal with code (replaces a >> bunch of manual zeroing of fields), and it is relatively future-proof (the next >> time EFI_PROCESSOR_INFORMATION is extended, you likely won't have to >> touch up the code again, because the ZeroMem() will cover the new fields >> automatically). >> >> Also, this approach will zero out >> ProcessorInfoBuffer->ExtendedInformation *regardless* of BIT24 in the >> input, which I kind of consider an advantage! (No garbage in the output >> structure.) Again, I don't think the zeroing is wasteful, regarding CPU cycles. >> >> I do agree that the leading function comments should mention BIT24 >> >> Thanks >> Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113288): https://edk2.groups.io/g/devel/message/113288 Mute This Topic: https://groups.io/mt/103518742/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber 2024-01-04 7:32 [edk2-devel] [PATCH 0/2] Change the usage of input parameter ProcessorNumber in MpInitLibGetProcessorInfo() of MpInitLibUp duntan 2024-01-04 7:32 ` [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION duntan @ 2024-01-04 7:32 ` duntan 2024-01-04 14:43 ` Laszlo Ersek 1 sibling, 1 reply; 11+ messages in thread From: duntan @ 2024-01-04 7:32 UTC (permalink / raw) To: devel; +Cc: Ray Ni, Laszlo Ersek, Rahul Kumar, Gerd Hoffmann, Min Xu Check lower 24 bits of ProcessorNumber instead of the value of ProcessorNumber in the API MpInitLibGetProcessorInfo() of MpInitLibUp instance. Lower 24 bits of ProcessorNumber contains the actual processor number. The BIT24 of input ProcessorNumber might be set to indicate if the EXTENDED_PROCESSOR_INFORMATION will be retrived. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Min Xu <min.m.xu@intel.com> --- UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c index 3af4911d4b..b804e400e6 100644 --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c @@ -106,7 +106,10 @@ MpInitLibGetProcessorInfo ( return EFI_INVALID_PARAMETER; } - if (ProcessorNumber != 0) { + // + // Lower 24 bits contains the actual processor number. + // + if ((ProcessorNumber & (CPU_V2_EXTENDED_TOPOLOGY - 1)) != 0) { return EFI_NOT_FOUND; } -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113139): https://edk2.groups.io/g/devel/message/113139 Mute This Topic: https://groups.io/mt/103518743/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber 2024-01-04 7:32 ` [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber duntan @ 2024-01-04 14:43 ` Laszlo Ersek 2024-01-05 12:55 ` Ni, Ray 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2024-01-04 14:43 UTC (permalink / raw) To: devel, dun.tan; +Cc: Ray Ni, Rahul Kumar, Gerd Hoffmann, Min Xu On 1/4/24 08:32, duntan wrote: > Check lower 24 bits of ProcessorNumber instead of > the value of ProcessorNumber in the API > MpInitLibGetProcessorInfo() of MpInitLibUp instance. > Lower 24 bits of ProcessorNumber contains the actual > processor number. > The BIT24 of input ProcessorNumber might be set to > indicate if the EXTENDED_PROCESSOR_INFORMATION will > be retrived. > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Min Xu <min.m.xu@intel.com> > --- > UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > index 3af4911d4b..b804e400e6 100644 > --- a/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > +++ b/UefiCpuPkg/Library/MpInitLibUp/MpInitLibUp.c > @@ -106,7 +106,10 @@ MpInitLibGetProcessorInfo ( > return EFI_INVALID_PARAMETER; > } > > - if (ProcessorNumber != 0) { > + // > + // Lower 24 bits contains the actual processor number. > + // > + if ((ProcessorNumber & (CPU_V2_EXTENDED_TOPOLOGY - 1)) != 0) { > return EFI_NOT_FOUND; > } > Reviewed-by: Laszlo Ersek <lersek@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113178): https://edk2.groups.io/g/devel/message/113178 Mute This Topic: https://groups.io/mt/103518743/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber 2024-01-04 14:43 ` Laszlo Ersek @ 2024-01-05 12:55 ` Ni, Ray 2024-01-05 13:56 ` Laszlo Ersek 0 siblings, 1 reply; 11+ messages in thread From: Ni, Ray @ 2024-01-05 12:55 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io, Tan, Dun Cc: Kumar, Rahul R, Gerd Hoffmann, Xu, Min M > > - if (ProcessorNumber != 0) { > > + // > > + // Lower 24 bits contains the actual processor number. > > + // > > + if ((ProcessorNumber & (CPU_V2_EXTENDED_TOPOLOGY - 1)) != 0) { I suggest we explicitly use BIT24 instead of CPU_V2_EXTENDED_TOPOLOGY. Using BIT24 clearly tells that processor number only occupies the lower 24 bits. > > return EFI_NOT_FOUND; > > } > > > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113281): https://edk2.groups.io/g/devel/message/113281 Mute This Topic: https://groups.io/mt/103518743/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber 2024-01-05 12:55 ` Ni, Ray @ 2024-01-05 13:56 ` Laszlo Ersek 2024-01-08 3:57 ` duntan 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2024-01-05 13:56 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io, Tan, Dun Cc: Kumar, Rahul R, Gerd Hoffmann, Xu, Min M On 1/5/24 13:55, Ni, Ray wrote: >>> - if (ProcessorNumber != 0) { >>> + // >>> + // Lower 24 bits contains the actual processor number. >>> + // >>> + if ((ProcessorNumber & (CPU_V2_EXTENDED_TOPOLOGY - 1)) != 0) { > I suggest we explicitly use BIT24 instead of CPU_V2_EXTENDED_TOPOLOGY. > Using BIT24 clearly tells that processor number only occupies the lower 24 bits. Yes, I've noticed this discrepancy too; I agree BIT24 is clearer here! > > >>> return EFI_NOT_FOUND; >>> } >>> >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113287): https://edk2.groups.io/g/devel/message/113287 Mute This Topic: https://groups.io/mt/103518743/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber 2024-01-05 13:56 ` Laszlo Ersek @ 2024-01-08 3:57 ` duntan 0 siblings, 0 replies; 11+ messages in thread From: duntan @ 2024-01-08 3:57 UTC (permalink / raw) To: Laszlo Ersek, Ni, Ray, devel@edk2.groups.io Cc: Kumar, Rahul R, Gerd Hoffmann, Xu, Min M Thanks for the comments. The code has been updated in the V2 patch set. Thanks, Dun -----Original Message----- From: Laszlo Ersek <lersek@redhat.com> Sent: Friday, January 5, 2024 9:56 PM To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com> Cc: Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Xu, Min M <min.m.xu@intel.com> Subject: Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber On 1/5/24 13:55, Ni, Ray wrote: >>> - if (ProcessorNumber != 0) { >>> + // >>> + // Lower 24 bits contains the actual processor number. >>> + // >>> + if ((ProcessorNumber & (CPU_V2_EXTENDED_TOPOLOGY - 1)) != 0) { > I suggest we explicitly use BIT24 instead of CPU_V2_EXTENDED_TOPOLOGY. > Using BIT24 clearly tells that processor number only occupies the lower 24 bits. Yes, I've noticed this discrepancy too; I agree BIT24 is clearer here! > > >>> return EFI_NOT_FOUND; >>> } >>> >> >> Reviewed-by: Laszlo Ersek <lersek@redhat.com> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113352): https://edk2.groups.io/g/devel/message/113352 Mute This Topic: https://groups.io/mt/103518743/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-01-08 3:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-04 7:32 [edk2-devel] [PATCH 0/2] Change the usage of input parameter ProcessorNumber in MpInitLibGetProcessorInfo() of MpInitLibUp duntan 2024-01-04 7:32 ` [edk2-devel] [PATCH 1/2] UefiCpuPkg: Retrive EXTENDED_PROCESSOR_INFORMATION duntan 2024-01-04 14:53 ` Laszlo Ersek 2024-01-05 9:24 ` duntan 2024-01-05 12:56 ` Ni, Ray 2024-01-05 13:59 ` Laszlo Ersek 2024-01-04 7:32 ` [edk2-devel] [PATCH 2/2] UefiCpuPkg: Check lower 24 bits of ProcessorNumber duntan 2024-01-04 14:43 ` Laszlo Ersek 2024-01-05 12:55 ` Ni, Ray 2024-01-05 13:56 ` Laszlo Ersek 2024-01-08 3:57 ` duntan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox