Re: [Qemu-devel] [PATCH] hw/isa/lpc_ich9: inject the SMI on the VCPU that is writing to APM_CNT

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Oct 21, 2015 at 11:49:13AM +0200, Paolo Bonzini wrote:
> 
> 
> On 20/10/2015 20:14, Laszlo Ersek wrote:
> > Commit 4d00636e97b7 ("ich9: Add the lpc chip", Nov 14 2012) added the
> > ich9_apm_ctrl_changed() ioport write callback function such that it would
> > inject the SMI, in response to a write to the APM_CNT register, on the
> > first CPU, invariably.
> > 
> > Since this register is used by guest code to trigger an SMI synchronously,
> > the interrupt should be injected on the VCPU that is performing the write.
> > 
> > apm_ioport_writeb() is the .write callback of the "apm_ops"
> > MemoryRegionOps [hw/isa/apm.c]; it is parametrized to call
> > ich9_apm_ctrl_changed() by ich9_lpc_init() [hw/isa/lpc_ich9.c], via
> > apm_init(). Therefore this change affects no other board.
> > 
> > ich9_generate_smi() is an unrelated function that is called by the TCO
> > watchdog; a watchdog is likely in its right to (asynchronously) inject
> > interrupts on the first CPU only.
> > 
> > This patch allows the combined edk2/OVMF SMM driver stack to work with
> > multiple VCPUs on TCG, using both qemu-system-i386 and qemu-system-x86_64.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> >  hw/isa/lpc_ich9.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> > index 360699f..1ffc803 100644
> > --- a/hw/isa/lpc_ich9.c
> > +++ b/hw/isa/lpc_ich9.c
> > @@ -394,7 +394,7 @@ 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(first_cpu, CPU_INTERRUPT_SMI);
> > +        cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
> >      }
> >  }
> >  
> > 
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Sorry, only saw this now, and it's already on its way upstream.

> The same issue happens on PIIX4.  I can fix it as the change is a bit
> more involved (it goes through a qemu_irq).
> 
> Paolo

Thanks!

-- 
MST