From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtgEe-0001cP-JX for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:23:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtgEb-0005eG-TG for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:23:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46174) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtgEb-0005dq-LI for qemu-devel@nongnu.org; Tue, 03 Nov 2015 13:23:05 -0500 References: <1445616339-28414-1-git-send-email-lersek@redhat.com> From: Laszlo Ersek Message-ID: <5638FB85.8070801@redhat.com> Date: Tue, 3 Nov 2015 19:23:01 +0100 MIME-Version: 1.0 In-Reply-To: <1445616339-28414-1-git-send-email-lersek@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: "Michael S. Tsirkin" , Jordan Justen , Kevin O'Connor , Gerd Hoffmann , Michael Kinney , Paolo Bonzini On 10/23/15 18:05, Laszlo Ersek wrote: > 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 problem with (1a) and (2) is that the BSP and AP synchronization is > slow. The above taskset + efibootmgr command takes more than 30 seconds to > complete on TCG, for example, because efibootmgr accesses non-volatile > UEFI variables intensively. > > Therefore introduce a special APM_STS value (0x51) 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. > > SeaBIOS uses values 0x00 and 0x01 for APM_STS (called PORT_SMI_STATUS in > the SeaBIOS code), so this change should be transparent to it. > > While commit 3c23402d4032 targeted correctness, this one aims at better > performance only. > > Cc: Paolo Bonzini > Cc: Gerd Hoffmann > Cc: Jordan Justen > Cc: Michael Kinney > Cc: "Kevin O'Connor" > Cc: "Michael S. Tsirkin" > Signed-off-by: Laszlo Ersek > --- > hw/isa/lpc_ich9.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 1ffc803..117baff 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -380,6 +380,8 @@ void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled, bool enable_tco) > > /* APM */ > > +#define QEMU_ICH9_APM_STS_BROADCAST_SMI 'Q' > + > static void ich9_apm_ctrl_changed(uint32_t val, void *arg) > { > ICH9LPCState *lpc = arg; > @@ -394,7 +396,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->apm.apms == QEMU_ICH9_APM_STS_BROADCAST_SMI) { > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + cpu_interrupt(cs, CPU_INTERRUPT_SMI); > + } > + } else { > + cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI); > + } > } > } > > I'm withdrawing this patch for now. Thanks Laszlo