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" <ke...@koconnor.net> Cc: "Michael S. Tsirkin" <m...@redhat.com> Cc: Gerd Hoffmann <kra...@redhat.com> Cc: Paolo Bonzini <pbonz...@redhat.com> Also-suggested-by: Paolo Bonzini <pbonz...@redhat.com> Signed-off-by: Laszlo Ersek <ler...@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