On February 26, 2013 21:03:46 Jan Kiszka wrote: > On 2013-02-26 20:48, Mark Asselstine wrote: > > Commit 0e21e12bb311c4c1095d0269dc2ef81196ccb60a [Don't route PIC > > interrupts through the local APIC if the local APIC config says so.] > > missed a check to ensure the local APIC is enabled. Since if the local > > APIC is disabled it doesn't matter what the local APIC config says. > > > > If this check isn't done and the guest has disabled the local APIC the > > guest will receive a general protection fault, similar to what is seen > > here: > > > > https://lists.gnu.org/archive/html/qemu-devel/2012-12/msg02304.html > > > > The GPF is caused by an attempt to service interrupt 0xffffffff. This > > comes about since cpu_get_pic_interrupt() calls apic_accept_pic_intr() > > (with the local APIC disabled apic_get_interrupt() returns -1). > > apic_accept_pic_intr() returns 0 and thus the interrupt number which > > is returned from cpu_get_pic_interrupt(), and which is attempted to be > > serviced, is -1. > > > > Signed-off-by: Mark Asselstine <mark.asselst...@windriver.com> > > --- > > The GPF only happens on occasion when you shutdown a linux guest > > using 'poweroff -f' but I am able to get it to reproduce nearly > > 100% of the time by attaching gdb to the Qemu backend, breaking > > at 'lapic_shutdown' and stepping through the remainder of the > > shutdown. > > > > Mark > > > > hw/apic.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/hw/apic.c b/hw/apic.c > > index fd14b73..051bd3e 100644 > > --- a/hw/apic.c > > +++ b/hw/apic.c > > @@ -591,6 +591,8 @@ int apic_accept_pic_intr(DeviceState *d) > > > > if (!s) > > > > return -1; > > > > + if (!(s->spurious_vec & APIC_SV_ENABLE)) > > + return -1; > > > > lvt0 = s->lvt[APIC_LVT_LINT0]; > > The check is correct, but please adjust the coding style to QEMU standard: > > if (x) { > ... > } >
I was just mirroring the existing code in apic_get_interrupt() but agreed, the one liner reads better. I will send an V2 with the change shortly. Thanks, Mark > And you can merge your check with the one above. > > Thanks, > Jan
