Re: [edk2-devel] [PATCH v8 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

2021-02-23 Thread Laszlo Ersek
On 02/22/21 08:19, Ankur Arora wrote:
> Advertise OVMF support for CPU hot-unplug and negotiate it
> if QEMU requests the feature.
> 
> 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 the following review comments:
> (1,3) s/hot unplug/hot-unplug/
> (2) Get rid of the reference to the made up ICH9_APM_CNT_CPU_HOT_UNPLUG
> (4,6) Remove the artificial tie in between
>  ICH9_LPC_SMI_F_CPU_HOTPLUG, ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
> (5) Fully spell out "SMI on CPU hot-unplug".
> (7) Emit separate messages on negotiation (or not) of
>  ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
> 
>  OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c 
> b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
> index c9d875543205..b1d59a559dae 100644
> --- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
> +++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
> @@ -29,6 +29,12 @@
>  //
>  #define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
>  
> +// The following bit value stands for "enable CPU hot-unplug, and inject an 
> SMI
> +// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
> +// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg 
> files.
> +//
> +#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
> +

(1) The comment formatting is inconsistent with the rest. The line right
after the (extant) "ICH9_LPC_SMI_F_CPU_HOTPLUG" macro definition should
be "//", and not an empty line.

In fact, this new hunk should not introduce any empty line.


>  //
>  // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
>  // for the S3 boot script fragment to write to and read from.
> @@ -112,7 +118,8 @@ NegotiateSmiFeatures (
>QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);
>  
>//
> -  // We want broadcast SMI, SMI on CPU hotplug, and nothing else.
> +  // We want broadcast SMI, SMI on CPU hotplug, SMI on CPU hot-unplug
> +  // and nothing else.
>//
>RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST;
>if (!MemEncryptSevIsEnabled ()) {
> @@ -120,8 +127,10 @@ NegotiateSmiFeatures (
>  // For now, we only support hotplug with SEV disabled.
>  //
>  RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG;
> +RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG;
>}
>mSmiFeatures &= RequestedFeaturesMask;
> +

(2) Spurious empty line addition (it's likely left over from removing
the earlier attempt to "be smarter than QEMU").

The rest seems fine.

Thanks!
Laszlo


>QemuFwCfgSelectItem (mRequestedFeaturesItem);
>QemuFwCfgWriteBytes (sizeof mSmiFeatures, &mSmiFeatures);
>  
> @@ -166,6 +175,13 @@ NegotiateSmiFeatures (
>__FUNCTION__));
>}
>  
> +  if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) == 0) {
> +DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug not negotiated\n", 
> __FUNCTION__));
> +  } else {
> +DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug with SMI negotiated\n",
> +  __FUNCTION__));
> +  }
> +
>//
>// Negotiation successful (although we may not have gotten the optimal
>// feature set).
> 



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




[edk2-devel] [PATCH v8 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

2021-02-21 Thread Ankur Arora
Advertise OVMF support for CPU hot-unplug and negotiate it
if QEMU requests the feature.

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 the following review comments:
(1,3) s/hot unplug/hot-unplug/
(2) Get rid of the reference to the made up ICH9_APM_CNT_CPU_HOT_UNPLUG
(4,6) Remove the artificial tie in between
 ICH9_LPC_SMI_F_CPU_HOTPLUG, ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
(5) Fully spell out "SMI on CPU hot-unplug".
(7) Emit separate messages on negotiation (or not) of
 ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.

 OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c 
b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index c9d875543205..b1d59a559dae 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -29,6 +29,12 @@
 //
 #define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
 
+// The following bit value stands for "enable CPU hot-unplug, and inject an SMI
+// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
+// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
+//
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
+
 //
 // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
 // for the S3 boot script fragment to write to and read from.
@@ -112,7 +118,8 @@ NegotiateSmiFeatures (
   QemuFwCfgReadBytes (sizeof mSmiFeatures, &mSmiFeatures);
 
   //
-  // We want broadcast SMI, SMI on CPU hotplug, and nothing else.
+  // We want broadcast SMI, SMI on CPU hotplug, SMI on CPU hot-unplug
+  // and nothing else.
   //
   RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST;
   if (!MemEncryptSevIsEnabled ()) {
@@ -120,8 +127,10 @@ NegotiateSmiFeatures (
 // For now, we only support hotplug with SEV disabled.
 //
 RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG;
+RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG;
   }
   mSmiFeatures &= RequestedFeaturesMask;
+
   QemuFwCfgSelectItem (mRequestedFeaturesItem);
   QemuFwCfgWriteBytes (sizeof mSmiFeatures, &mSmiFeatures);
 
@@ -166,6 +175,13 @@ NegotiateSmiFeatures (
   __FUNCTION__));
   }
 
+  if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) == 0) {
+DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug not negotiated\n", __FUNCTION__));
+  } else {
+DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug with SMI negotiated\n",
+  __FUNCTION__));
+  }
+
   //
   // Negotiation successful (although we may not have gotten the optimal
   // feature set).
-- 
2.9.3



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