subject:"\[edk2\-devel\] \[PATCH v8 04\/10\] OvmfPkg\/CpuHotplugSmm\: introduce UnplugCpus\(\)"

Re: [edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

2021-02-22 Thread Ankur Arora


On 2021-02-22 4:39 a.m., Laszlo Ersek wrote:

On 02/22/21 08:19, Ankur Arora wrote:

Introduce UnplugCpus() which maps each APIC ID being unplugged
onto the hardware ID of the processor and informs PiSmmCpuDxeSmm
of removal by calling EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().

With this change we handle the first phase of unplug where we collect
the CPUs that need to be unplugged and mark them for removal in SMM
data structures.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
 Addresses these review comments from v6:
  (1) Drop the empty line in the comment block around UnplugCpus().
  (2) Make the "did not find APIC ID" DEBUG_VERBOSE instead of DEBUG_INFO.
  (3) Un-Indented ("Outdented") the line following the comment "Ignore the
   unplug if APIC ID.
  (4) Remove the empty line between Status assignment and check.
  (5) Drop the "goto Fatal" logic and just return Status directly.
  (6) Handle both Plugging and Unplugging of CPUs in one go.
  (7) Also nest the EFI_STATUS check.

  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 84 ++
  1 file changed, 84 insertions(+)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 3192bfea1f15..f07b5072749a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -188,6 +188,83 @@ RevokeNewSlot:
  }
  
  /**

+  Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
+
+  For each such CPU, report the CPU to PiSmmCpuDxeSmm via
+  EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
+  unknown, skip it silently.
+
+  @param[in] ToUnplugApicIdsThe APIC IDs of the CPUs that are about to be
+hot-unplugged.
+
+  @param[in] ToUnplugCount  The number of filled-in APIC IDs in
+ToUnplugApicIds.
+
+  @retval EFI_SUCCESS   Known APIC IDs have been removed from SMM data
+structures.
+
+  @return   Error codes propagated from
+mMmCpuService->RemoveProcessor().
+**/
+STATIC
+EFI_STATUS
+UnplugCpus (
+  IN APIC_ID  *ToUnplugApicIds,
+  IN UINT32   ToUnplugCount
+  )
+{
+  EFI_STATUS Status;
+  UINT32 ToUnplugIdx;
+  UINTN  ProcessorNum;
+
+  ToUnplugIdx = 0;
+  while (ToUnplugIdx < ToUnplugCount) {
+APIC_IDRemoveApicId;
+
+RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+
+//
+// mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
+// the ProcessorNum for the APIC ID to be removed.
+//
+for (ProcessorNum = 0;
+ ProcessorNum < mCpuHotPlugData->ArrayLength;
+ ProcessorNum++) {
+  if (mCpuHotPlugData->ApicId[ProcessorNum] == RemoveApicId) {
+break;
+  }
+}
+
+//
+// Ignore the unplug if APIC ID not found
+//
+if (ProcessorNum == mCpuHotPlugData->ArrayLength) {
+  DEBUG ((DEBUG_VERBOSE, "%a: did not find APIC ID " FMT_APIC_ID
+" to unplug\n", __FUNCTION__, RemoveApicId));
+  ToUnplugIdx++;
+  continue;
+}
+
+//
+// Mark ProcessorNum for removal from SMM data structures
+//
+Status = mMmCpuService->RemoveProcessor (mMmCpuService, ProcessorNum);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: RemoveProcessor(" FMT_APIC_ID "): %r\n",
+__FUNCTION__, RemoveApicId, Status));
+  return Status;
+}
+
+ToUnplugIdx++;
+  }
+
+  //
+  // We've removed this set of APIC IDs from SMM data structures.
+  //
+  return EFI_SUCCESS;
+}
+
+/**
CPU Hotplug MMI handler function.
  
This is a root MMI handler.

@@ -309,6 +386,13 @@ CpuHotplugMmi (
  }
}
  
+  if (ToUnplugCount > 0) {

+Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+if (EFI_ERROR (Status)) {
+  goto Fatal;
+}
+  }
+
//
// We've handled this MMI.
//



Reviewed-by: Laszlo Ersek 


Thanks for the review!

Ankur


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72003): https://edk2.groups.io/g/devel/message/72003
Mute This Topic: https://groups.io/mt/80819859/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

2021-02-22 Thread Laszlo Ersek

On 02/22/21 08:19, Ankur Arora wrote:
> Introduce UnplugCpus() which maps each APIC ID being unplugged
> onto the hardware ID of the processor and informs PiSmmCpuDxeSmm
> of removal by calling EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().
> 
> With this change we handle the first phase of unplug where we collect
> the CPUs that need to be unplugged and mark them for removal in SMM
> data structures.
> 
> Cc: Laszlo Ersek 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Igor Mammedov 
> Cc: Boris Ostrovsky 
> Cc: Aaron Young 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
> Signed-off-by: Ankur Arora 
> ---
> 
> Notes:
> Addresses these review comments from v6:
>  (1) Drop the empty line in the comment block around UnplugCpus().
>  (2) Make the "did not find APIC ID" DEBUG_VERBOSE instead of DEBUG_INFO.
>  (3) Un-Indented ("Outdented") the line following the comment "Ignore the
>   unplug if APIC ID.
>  (4) Remove the empty line between Status assignment and check.
>  (5) Drop the "goto Fatal" logic and just return Status directly.
>  (6) Handle both Plugging and Unplugging of CPUs in one go.
>  (7) Also nest the EFI_STATUS check.
> 
>  OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 84 
> ++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
> b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> index 3192bfea1f15..f07b5072749a 100644
> --- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> +++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
> @@ -188,6 +188,83 @@ RevokeNewSlot:
>  }
>  
>  /**
> +  Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
> +
> +  For each such CPU, report the CPU to PiSmmCpuDxeSmm via
> +  EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
> +  unknown, skip it silently.
> +
> +  @param[in] ToUnplugApicIdsThe APIC IDs of the CPUs that are about to be
> +hot-unplugged.
> +
> +  @param[in] ToUnplugCount  The number of filled-in APIC IDs in
> +ToUnplugApicIds.
> +
> +  @retval EFI_SUCCESS   Known APIC IDs have been removed from SMM 
> data
> +structures.
> +
> +  @return   Error codes propagated from
> +mMmCpuService->RemoveProcessor().
> +**/
> +STATIC
> +EFI_STATUS
> +UnplugCpus (
> +  IN APIC_ID  *ToUnplugApicIds,
> +  IN UINT32   ToUnplugCount
> +  )
> +{
> +  EFI_STATUS Status;
> +  UINT32 ToUnplugIdx;
> +  UINTN  ProcessorNum;
> +
> +  ToUnplugIdx = 0;
> +  while (ToUnplugIdx < ToUnplugCount) {
> +APIC_IDRemoveApicId;
> +
> +RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
> +
> +//
> +// mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
> +// the ProcessorNum for the APIC ID to be removed.
> +//
> +for (ProcessorNum = 0;
> + ProcessorNum < mCpuHotPlugData->ArrayLength;
> + ProcessorNum++) {
> +  if (mCpuHotPlugData->ApicId[ProcessorNum] == RemoveApicId) {
> +break;
> +  }
> +}
> +
> +//
> +// Ignore the unplug if APIC ID not found
> +//
> +if (ProcessorNum == mCpuHotPlugData->ArrayLength) {
> +  DEBUG ((DEBUG_VERBOSE, "%a: did not find APIC ID " FMT_APIC_ID
> +" to unplug\n", __FUNCTION__, RemoveApicId));
> +  ToUnplugIdx++;
> +  continue;
> +}
> +
> +//
> +// Mark ProcessorNum for removal from SMM data structures
> +//
> +Status = mMmCpuService->RemoveProcessor (mMmCpuService, ProcessorNum);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR, "%a: RemoveProcessor(" FMT_APIC_ID "): %r\n",
> +__FUNCTION__, RemoveApicId, Status));
> +  return Status;
> +}
> +
> +ToUnplugIdx++;
> +  }
> +
> +  //
> +  // We've removed this set of APIC IDs from SMM data structures.
> +  //
> +  return EFI_SUCCESS;
> +}
> +
> +/**
>CPU Hotplug MMI handler function.
>  
>This is a root MMI handler.
> @@ -309,6 +386,13 @@ CpuHotplugMmi (
>  }
>}
>  
> +  if (ToUnplugCount > 0) {
> +Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
> +if (EFI_ERROR (Status)) {
> +  goto Fatal;
> +}
> +  }
> +
>//
>// We've handled this MMI.
>//
> 

Reviewed-by: Laszlo Ersek 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71949): https://edk2.groups.io/g/devel/message/71949
Mute This Topic: https://groups.io/mt/80819859/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

2021-02-21 Thread Ankur Arora

Introduce UnplugCpus() which maps each APIC ID being unplugged
onto the hardware ID of the processor and informs PiSmmCpuDxeSmm
of removal by calling EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().

With this change we handle the first phase of unplug where we collect
the CPUs that need to be unplugged and mark them for removal in SMM
data structures.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses these review comments from v6:
 (1) Drop the empty line in the comment block around UnplugCpus().
 (2) Make the "did not find APIC ID" DEBUG_VERBOSE instead of DEBUG_INFO.
 (3) Un-Indented ("Outdented") the line following the comment "Ignore the
  unplug if APIC ID.
 (4) Remove the empty line between Status assignment and check.
 (5) Drop the "goto Fatal" logic and just return Status directly.
 (6) Handle both Plugging and Unplugging of CPUs in one go.
 (7) Also nest the EFI_STATUS check.

 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 84 ++
 1 file changed, 84 insertions(+)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 3192bfea1f15..f07b5072749a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -188,6 +188,83 @@ RevokeNewSlot:
 }
 
 /**
+  Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
+
+  For each such CPU, report the CPU to PiSmmCpuDxeSmm via
+  EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
+  unknown, skip it silently.
+
+  @param[in] ToUnplugApicIdsThe APIC IDs of the CPUs that are about to be
+hot-unplugged.
+
+  @param[in] ToUnplugCount  The number of filled-in APIC IDs in
+ToUnplugApicIds.
+
+  @retval EFI_SUCCESS   Known APIC IDs have been removed from SMM data
+structures.
+
+  @return   Error codes propagated from
+mMmCpuService->RemoveProcessor().
+**/
+STATIC
+EFI_STATUS
+UnplugCpus (
+  IN APIC_ID  *ToUnplugApicIds,
+  IN UINT32   ToUnplugCount
+  )
+{
+  EFI_STATUS Status;
+  UINT32 ToUnplugIdx;
+  UINTN  ProcessorNum;
+
+  ToUnplugIdx = 0;
+  while (ToUnplugIdx < ToUnplugCount) {
+APIC_IDRemoveApicId;
+
+RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+
+//
+// mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
+// the ProcessorNum for the APIC ID to be removed.
+//
+for (ProcessorNum = 0;
+ ProcessorNum < mCpuHotPlugData->ArrayLength;
+ ProcessorNum++) {
+  if (mCpuHotPlugData->ApicId[ProcessorNum] == RemoveApicId) {
+break;
+  }
+}
+
+//
+// Ignore the unplug if APIC ID not found
+//
+if (ProcessorNum == mCpuHotPlugData->ArrayLength) {
+  DEBUG ((DEBUG_VERBOSE, "%a: did not find APIC ID " FMT_APIC_ID
+" to unplug\n", __FUNCTION__, RemoveApicId));
+  ToUnplugIdx++;
+  continue;
+}
+
+//
+// Mark ProcessorNum for removal from SMM data structures
+//
+Status = mMmCpuService->RemoveProcessor (mMmCpuService, ProcessorNum);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: RemoveProcessor(" FMT_APIC_ID "): %r\n",
+__FUNCTION__, RemoveApicId, Status));
+  return Status;
+}
+
+ToUnplugIdx++;
+  }
+
+  //
+  // We've removed this set of APIC IDs from SMM data structures.
+  //
+  return EFI_SUCCESS;
+}
+
+/**
   CPU Hotplug MMI handler function.
 
   This is a root MMI handler.
@@ -309,6 +386,13 @@ CpuHotplugMmi (
 }
   }
 
+  if (ToUnplugCount > 0) {
+Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+if (EFI_ERROR (Status)) {
+  goto Fatal;
+}
+  }
+
   //
   // We've handled this MMI.
   //
-- 
2.9.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71915): https://edk2.groups.io/g/devel/message/71915
Mute This Topic: https://groups.io/mt/80819859/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

Re: [edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

[edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

3 matches

Site Navigation

Mail list logo

Footer information