[PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Gerd Hoffmann]

In case the number of CPUs can in increase beyond 255
due to CPU hotplug choose x2apic mode.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index e5dc852ed95f..d73b95001263 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -526,7 +526,9 @@ CollectProcessorCount (
   //
   // Enable x2APIC mode if
   //  1. Number of CPU is greater than 255; or
-  //  2. There are any logical processors reporting an Initial APIC ID of 255 or greater.
+  //  2. The platform exposed the exact *boot* CPU count to us in advance, and
+  //     more than 255 logical processors are possible later, with hotplug; or
+  //  3. There are any logical processors reporting an Initial APIC ID of 255 or greater.
   //
   X2Apic = FALSE;
   if (CpuMpData->CpuCount > 255) {
@@ -534,6 +536,10 @@ CollectProcessorCount (
     // If there are more than 255 processor found, force to enable X2APIC
     //
     X2Apic = TRUE;
+  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
+             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
+  {
+    X2Apic = TRUE;
   } else {
     CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
     for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-- 
2.39.2

Re: [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Gerd Hoffmann]

On Tue, Mar 07, 2023 at 01:20:37PM +0100, Gerd Hoffmann wrote:
> In case the number of CPUs can in increase beyond 255
> due to CPU hotplug choose x2apic mode.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Ping.  Any comments on this patch?

thanks,
  Gerd

> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index e5dc852ed95f..d73b95001263 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -526,7 +526,9 @@ CollectProcessorCount (
>    //
>    // Enable x2APIC mode if
>    //  1. Number of CPU is greater than 255; or
> -  //  2. There are any logical processors reporting an Initial APIC ID of 255 or greater.
> +  //  2. The platform exposed the exact *boot* CPU count to us in advance, and
> +  //     more than 255 logical processors are possible later, with hotplug; or
> +  //  3. There are any logical processors reporting an Initial APIC ID of 255 or greater.
>    //
>    X2Apic = FALSE;
>    if (CpuMpData->CpuCount > 255) {
> @@ -534,6 +536,10 @@ CollectProcessorCount (
>      // If there are more than 255 processor found, force to enable X2APIC
>      //
>      X2Apic = TRUE;
> +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> +  {
> +    X2Apic = TRUE;
>    } else {
>      CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
>      for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
> -- 
> 2.39.2
> 

-- 

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Ni, Ray]

> +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> +  {
> +    X2Apic = TRUE;

Gerd,
I agree with your needs that want X2 APIC even the actual processor number in BIOS phase <= 255.

Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2 APIC when the maximum CPU count > 255?

I am thinking about adding a new PCD to tell MP code switch to x2 apic in the first time AP wakes up. Possible timeline for the code change is
about within 1 month. Do you think it can meet your needs?

Thanks,
Ray

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Gerd Hoffmann]

On Tue, Mar 21, 2023 at 07:28:44AM +0000, Ni, Ray wrote:
> > +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> > +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> > +  {
> > +    X2Apic = TRUE;
> 
> Gerd,
> I agree with your needs that want X2 APIC even the actual processor number in BIOS phase <= 255.
> 
> Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2 APIC when the maximum CPU count > 255?

Linux doesn't do that.  In case x2apic mode is not active at boot the
number of CPUs will be limited to 255.

> I am thinking about adding a new PCD to tell MP code switch to x2 apic
> in the first time AP wakes up. Possible timeline for the code change
> is about within 1 month. Do you think it can meet your needs?

So the idea being that OVMF simply sets that PCD if needed (probably at
the same place where PcdCpuMaxLogicalProcessorNumber is set)?  That
would work too.

take care,
  Gerd

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Ni, Ray]

Thanks for confirming that.

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, March 21, 2023 7:54 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Pawel Polawski <ppolawsk@redhat.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Oliver Steffen <osteffen@redhat.com>;
> Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode
> for cpu hotplug
> 
> On Tue, Mar 21, 2023 at 07:28:44AM +0000, Ni, Ray wrote:
> > > +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> > > +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> > > +  {
> > > +    X2Apic = TRUE;
> >
> > Gerd,
> > I agree with your needs that want X2 APIC even the actual processor
> number in BIOS phase <= 255.
> >
> > Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2
> APIC when the maximum CPU count > 255?
> 
> Linux doesn't do that.  In case x2apic mode is not active at boot the
> number of CPUs will be limited to 255.
> 
> > I am thinking about adding a new PCD to tell MP code switch to x2 apic
> > in the first time AP wakes up. Possible timeline for the code change
> > is about within 1 month. Do you think it can meet your needs?
> 
> So the idea being that OVMF simply sets that PCD if needed (probably at
> the same place where PcdCpuMaxLogicalProcessorNumber is set)?  That
> would work too.
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Gerd Hoffmann]

On Tue, Mar 21, 2023 at 07:28:44AM +0000, Ni, Ray wrote:
> > +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> > +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> > +  {
> > +    X2Apic = TRUE;
> 
> Gerd,
> I agree with your needs that want X2 APIC even the actual processor number in BIOS phase <= 255.
> 
> Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2 APIC when the maximum CPU count > 255?
> 
> I am thinking about adding a new PCD to tell MP code switch to x2 apic in the first time AP wakes up. Possible timeline for the code change is
> about within 1 month. Do you think it can meet your needs?

Ping.  What is the status here?  I'd like to see that fixed for OVMF
for the next stable release, and freeze is coming closer ...

take care,
  Gerd

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Ni, Ray]

Gerd,
I was busy on something else. The patch was ready and I am testing it before sending them out.

Thanks,
Ray


> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Wednesday, May 3, 2023 3:24 PM
> To: Ni, Ray <ray.ni@intel.com>
> Cc: devel@edk2.groups.io; Pawel Polawski <ppolawsk@redhat.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Oliver Steffen <osteffen@redhat.com>;
> Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode
> for cpu hotplug
> 
> On Tue, Mar 21, 2023 at 07:28:44AM +0000, Ni, Ray wrote:
> > > +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> > > +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> > > +  {
> > > +    X2Apic = TRUE;
> >
> > Gerd,
> > I agree with your needs that want X2 APIC even the actual processor number
> in BIOS phase <= 255.
> >
> > Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2
> APIC when the maximum CPU count > 255?
> >
> > I am thinking about adding a new PCD to tell MP code switch to x2 apic in the
> first time AP wakes up. Possible timeline for the code change is
> > about within 1 month. Do you think it can meet your needs?
> 
> Ping.  What is the status here?  I'd like to see that fixed for OVMF
> for the next stable release, and freeze is coming closer ...
> 
> take care,
>   Gerd

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode for cpu hotplug

[Gerd Hoffmann]

On Wed, May 24, 2023 at 12:32:24PM +0000, Ni, Ray wrote:
> Gerd,
> I was busy on something else. The patch was ready and I am testing it before sending them out.

Ping.  Any update?

take care,
  Gerd

> 
> Thanks,
> Ray
> 
> 
> > -----Original Message-----
> > From: kraxel@redhat.com <kraxel@redhat.com>
> > Sent: Wednesday, May 3, 2023 3:24 PM
> > To: Ni, Ray <ray.ni@intel.com>
> > Cc: devel@edk2.groups.io; Pawel Polawski <ppolawsk@redhat.com>; Kumar,
> > Rahul R <rahul.r.kumar@intel.com>; Oliver Steffen <osteffen@redhat.com>;
> > Dong, Eric <eric.dong@intel.com>
> > Subject: Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg/MpInitLib: fix apic mode
> > for cpu hotplug
> > 
> > On Tue, Mar 21, 2023 at 07:28:44AM +0000, Ni, Ray wrote:
> > > > +  } else if ((PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) &&
> > > > +             (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) > 255))
> > > > +  {
> > > > +    X2Apic = TRUE;
> > >
> > > Gerd,
> > > I agree with your needs that want X2 APIC even the actual processor number
> > in BIOS phase <= 255.
> > >
> > > Question: Is it possible that BIOS stays at XAPIC, and later OS switches to X2
> > APIC when the maximum CPU count > 255?
> > >
> > > I am thinking about adding a new PCD to tell MP code switch to x2 apic in the
> > first time AP wakes up. Possible timeline for the code change is
> > > about within 1 month. Do you think it can meet your needs?
> > 
> > Ping.  What is the status here?  I'd like to see that fixed for OVMF
> > for the next stable release, and freeze is coming closer ...
> > 
> > take care,
> >   Gerd
> 

--