[Qemu-devel] [PATCH v3 for-2.9 3/3] hw/isa/lpc_ich9: ICH9_APM_STS_F_BROADCAST_SMI: inject SMI on all VCPUs

[Laszlo Ersek <lersek@redhat.com>]

The generic edk2 SMM infrastructure prefers
EFI_SMM_CONTROL2_PROTOCOL.Trigger() to inject an SMI on each processor. If
Trigger() only brings the current processor into SMM, then edk2 handles it
in the following ways:

(1) If Trigger() is executed by the BSP (which is guaranteed before
    ExitBootServices(), but is not necessarily true at runtime), then:

    (a) If edk2 has been configured for "traditional" SMM synchronization,
        then the BSP sends directed SMIs to the APs with APIC delivery,
        bringing them into SMM individually. Then the BSP runs the SMI
        handler / dispatcher.

    (b) If edk2 has been configured for "relaxed" SMM synchronization,
        then the APs that are not already in SMM are not brought in, and
        the BSP runs the SMI handler / dispatcher.

(2) If Trigger() is executed by an AP (which is possible after
    ExitBootServices(), and can be forced e.g. by "taskset -c 1
    efibootmgr"), then the AP in question brings in the BSP with a
    directed SMI, and the BSP runs the SMI handler / dispatcher.

The smaller problem with (1a) and (2) is that the BSP and AP
synchronization is slow. For example, the "taskset -c 1 efibootmgr"
command from (2) can take more than 3 seconds to complete, because
efibootmgr accesses non-volatile UEFI variables intensively.

The larger problem is that QEMU's current behavior diverges from the
behavior usually seen on physical hardware, and that keeps exposing
obscure corner cases, race conditions and other instabilities in edk2,
which generally expects / prefers a software SMI to affect all CPUs at
once.

Therefore introduce the "broadcast SMI" feature
(ICH9_APM_STS_F_BROADCAST_SMI) that causes QEMU to inject the SMI on all
VCPUs. OVMF's EFI_SMM_CONTROL2_PROTOCOL.Trigger() can utilize this to
accommodate edk2's preference about "broadcast" SMI.

While the original posting of this patch
<http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg05658.html>
only intended to speed up (2), based on our recent "stress testing" of SMM
this patch actually provides functional improvements.

Cc: "Kevin O'Connor" <kevin@koconnor.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Also-suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - key the broadcast SMI off of ICH9_APM_STS_F_BROADCAST_SMI, if it was
      negotiated [Paolo, Michael]

 docs/specs/q35-apm-sts.txt | 15 ++++++++++++---
 include/hw/i386/ich9.h     |  3 ++-
 hw/isa/lpc_ich9.c          | 10 +++++++++-
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/docs/specs/q35-apm-sts.txt b/docs/specs/q35-apm-sts.txt
index cdffb6834380..201baec52e9d 100644
--- a/docs/specs/q35-apm-sts.txt
+++ b/docs/specs/q35-apm-sts.txt
@@ -27,7 +27,9 @@ The following describes the (non-standard) bit definitions in APM_STS.
     |   |   |   |   |   |   |
     |   |   |   |   |   |   Feature negotiation bit.
     |   |   |   |   |   |
-    Feature bits. All reserved at the moment.
+    |   |   |   |   |   Broadcast SMI feature bit.
+    |   |   |   |   |
+    Reserved feature bits.
 
 Feature negotiation
 -------------------
@@ -37,8 +39,8 @@ negotiation bit first (clearing all other bits), then read back the APM_STS
 register. If the feature negotiation bit is set in the result, then QEMU lacks
 the feature negotiation feature, and APM_STS is entirely transparent. Otherwise
 (i.e., the feature negotiation bit is clear in the result), the more
-significant bits (the feature bits) expose the features supported by QEMU. At
-the moment, no features are defined, and all feature bits read as zero.
+significant bits (the feature bits) expose the features supported by QEMU.
+Reserved and unsupported feature bits read as zero.
 
 Once firmware confirms feature negotiation is available, it shall set (select)
 a subset of the advertised feature bits, and clear the feature negotiation bit,
@@ -51,6 +53,13 @@ dependencies, for example). Regardless of the feature negotiation bit in the
 read back value, the higher order bits (i.e., the individual feature bits) are
 always zero in that value.
 
+The broadcast SMI feature
+-------------------------
+
+Negotiating the broadcast SMI feature causes QEMU to raise the SMI on all VCPUs
+in response to subsequent SMI Command Port (APM_CNT) writes. By default QEMU
+raises the SMI only on the VCPU that writes to the SMI Command Port (APM_CNT).
+
 SeaBIOS compatibility
 ---------------------
 
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 8304396a487f..f14b747ff207 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -214,7 +214,8 @@ Object *ich9_lpc_find(void);
 /* non-standard bits for the APM_STS register */
 #define ICH9_APM_STS_TRANSPARENT_MASK          0x01
 #define ICH9_APM_STS_GET_SET_FEATURES          0x02
-#define ICH9_APM_STS_KNOWN_FEATURES            0x00
+#define ICH9_APM_STS_F_BROADCAST_SMI           0x04
+#define ICH9_APM_STS_KNOWN_FEATURES            0x04
 #define ICH9_APM_STS_FEATURE_MASK              0xfc
 
 /* D31:F3 SMBus controller */
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index a50c4a15b6d1..d8332f16e704 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -386,7 +386,15 @@ static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
 
     /* SMI_EN = PMBASE + 30. SMI control and enable register */
     if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
-        cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+        if (lpc->smi_features & ICH9_APM_STS_F_BROADCAST_SMI) {
+            CPUState *cs;
+
+            CPU_FOREACH(cs) {
+                cpu_interrupt(cs, CPU_INTERRUPT_SMI);
+            }
+        } else {
+            cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+        }
     }
 }
 
-- 
2.9.2