Re: [PATCH v1] [PATCH v1] UefiCpuPkg: Check SMM Delayed/Blocked AP Count to decide all CPUs in SMI or not

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Thanks Ray. I added the comments in patch v2 (https://edk2.groups.io/g/devel/message/96722) to clarify the expected value of "SmmRegSmmEnable" & "SmmRegSmmDelayed" & "SmmRegSmmBlocked" returned from SmmCpuFeaturesLib. That can answer the questions.

Due to more core count increasement, it's hard to reflect all APs
state via AP bitvector support in the register. Actually, SMM CPU
driver doesn't need to check each AP state to know all CPUs in SMI
or not, one alternative method is to check the SMM Delayed & Blocked
AP Count number:

APs in SMI + Blocked Count + Disabled Count >= All supported Aps
(code comments explained why can be > All supported Aps)

With above change, the returned value of "SmmRegSmmEnable" &
"SmmRegSmmDelayed" & "SmmRegSmmBlocked" from SmmCpuFeaturesLib
should be the AP count number within the existing CPU package.

For register that return the bitvector state, require
SmmCpuFeaturesGetSmmRegister() returns count number of all bit per
logical processor within the same package.

For register that return the AP count, require
SmmCpuFeaturesGetSmmRegister() returns the register value directly. 

Thanks,
Jiaxin 

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, November 29, 2022 5:11 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>
> Subject: RE: [PATCH v1] [PATCH v1] UefiCpuPkg: Check SMM
> Delayed/Blocked AP Count to decide all CPUs in SMI or not
> 
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4173
> >
> > The blocked register might return the counter instead of bitvector. This
> > request
> > is to update the code to handle the case by checking SMM
> Delayed/Blocked
> > AP Count
> > to decide all CPUs in SMI or not.
> 
> The code change actually assumes the SmmRegSmmBlocked,
> SmmRegSmmDelayed
> and SmmRegSmmEnable registers return the number of threads that are in
> blocked, delayed or disabled state.
> Can you provide suggestions regarding how to change those close source
> CpuSmmFeaturesLib
> implementation which still returns the bitmap value instead of count in the
> commit message?
> 
> 
> > +UINT32                       *mPackageBspInfo = NULL;
> 
> The variable name is confusing. Can you rename it to
> "mPackageFirstThreadIndex"?