From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60437 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PlL8Y-0001eh-Db for qemu-devel@nongnu.org; Fri, 04 Feb 2011 07:51:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PlL8X-0008TV-3a for qemu-devel@nongnu.org; Fri, 04 Feb 2011 07:51:42 -0500 Received: from mail-vw0-f45.google.com ([209.85.212.45]:46350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PlL8W-0008MR-W5 for qemu-devel@nongnu.org; Fri, 04 Feb 2011 07:51:41 -0500 Received: by mail-vw0-f45.google.com with SMTP id 12so1393605vws.4 for ; Fri, 04 Feb 2011 04:51:40 -0800 (PST) Message-ID: <4D4BF656.8060203@codemonkey.ws> Date: Fri, 04 Feb 2011 06:51:34 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [0.14?][PATCH v3 1/4] ioapic: Implement EOI handling for level-triggered IRQs References: <268c736763ed0488f0c29a17121c298e682c88f1.1296770049.git.jan.kiszka@web.de> In-Reply-To: <268c736763ed0488f0c29a17121c298e682c88f1.1296770049.git.jan.kiszka@web.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Anthony Liguori , Gleb Natapov , kvm@vger.kernel.org, Marcelo Tosatti , Alexander Graf , qemu-devel@nongnu.org, Blue Swirl , Avi Kivity On 02/03/2011 03:54 PM, Jan Kiszka wrote: > From: Jan Kiszka > > Add the missing EOI broadcast from local APIC to the IOAPICs on > completion of level-triggered IRQs. This ensures that a still asserted > IRQ source properly re-triggers an APIC IRQ. > > Signed-off-by: Jan Kiszka > Applied to master, Thanks. Regards, Anthony Liguori > --- > hw/apic.c | 9 ++++++--- > hw/ioapic.c | 43 +++++++++++++++++++++++++++++++++++++++++-- > hw/ioapic.h | 20 ++++++++++++++++++++ > 3 files changed, 67 insertions(+), 5 deletions(-) > create mode 100644 hw/ioapic.h > > diff --git a/hw/apic.c b/hw/apic.c > index ff581f0..2f8376a 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -18,6 +18,7 @@ > */ > #include "hw.h" > #include "apic.h" > +#include "ioapic.h" > #include "qemu-timer.h" > #include "host-utils.h" > #include "sysbus.h" > @@ -57,7 +58,8 @@ > > #define ESR_ILLEGAL_ADDRESS (1<< 7) > > -#define APIC_SV_ENABLE (1<< 8) > +#define APIC_SV_DIRECTED_IO (1<<12) > +#define APIC_SV_ENABLE (1<<8) > > #define MAX_APICS 255 > #define MAX_APIC_WORDS 8 > @@ -420,8 +422,9 @@ static void apic_eoi(APICState *s) > if (isrv< 0) > return; > reset_bit(s->isr, isrv); > - /* XXX: send the EOI packet to the APIC bus to allow the I/O APIC to > - set the remote IRR bit for level triggered interrupts. */ > + if (!(s->spurious_vec& APIC_SV_DIRECTED_IO)&& get_bit(s->tmr, isrv)) { > + ioapic_eoi_broadcast(isrv); > + } > apic_update_irq(s); > } > > diff --git a/hw/ioapic.c b/hw/ioapic.c > index 2109568..8bc31f7 100644 > --- a/hw/ioapic.c > +++ b/hw/ioapic.c > @@ -23,6 +23,7 @@ > #include "hw.h" > #include "pc.h" > #include "apic.h" > +#include "ioapic.h" > #include "qemu-timer.h" > #include "host-utils.h" > #include "sysbus.h" > @@ -36,7 +37,10 @@ > #define DPRINTF(fmt, ...) > #endif > > -#define IOAPIC_LVT_MASKED (1<<16) > +#define MAX_IOAPICS 1 > + > +#define IOAPIC_LVT_MASKED (1<< 16) > +#define IOAPIC_LVT_REMOTE_IRR (1<< 14) > > #define IOAPIC_TRIGGER_EDGE 0 > #define IOAPIC_TRIGGER_LEVEL 1 > @@ -61,6 +65,8 @@ struct IOAPICState { > uint64_t ioredtbl[IOAPIC_NUM_PINS]; > }; > > +static IOAPICState *ioapics[MAX_IOAPICS]; > + > static void ioapic_service(IOAPICState *s) > { > uint8_t i; > @@ -83,8 +89,11 @@ static void ioapic_service(IOAPICState *s) > dest_mode = (entry>> 11)& 1; > delivery_mode = (entry>> 8)& 7; > polarity = (entry>> 13)& 1; > - if (trig_mode == IOAPIC_TRIGGER_EDGE) > + if (trig_mode == IOAPIC_TRIGGER_EDGE) { > s->irr&= ~mask; > + } else { > + s->ioredtbl[i] |= IOAPIC_LVT_REMOTE_IRR; > + } > if (delivery_mode == IOAPIC_DM_EXTINT) > vector = pic_read_irq(isa_pic); > else > @@ -131,6 +140,29 @@ static void ioapic_set_irq(void *opaque, int vector, int level) > } > } > > +void ioapic_eoi_broadcast(int vector) > +{ > + IOAPICState *s; > + uint64_t entry; > + int i, n; > + > + for (i = 0; i< MAX_IOAPICS; i++) { > + s = ioapics[i]; > + if (!s) { > + continue; > + } > + for (n = 0; n< IOAPIC_NUM_PINS; n++) { > + entry = s->ioredtbl[n]; > + if ((entry& IOAPIC_LVT_REMOTE_IRR)&& (entry& 0xff) == vector) { > + s->ioredtbl[n] = entry& ~IOAPIC_LVT_REMOTE_IRR; > + if (!(entry& IOAPIC_LVT_MASKED)&& (s->irr& (1<< n))) { > + ioapic_service(s); > + } > + } > + } > + } > +} > + > static uint32_t ioapic_mem_readl(void *opaque, target_phys_addr_t addr) > { > IOAPICState *s = opaque; > @@ -240,6 +272,11 @@ static int ioapic_init1(SysBusDevice *dev) > { > IOAPICState *s = FROM_SYSBUS(IOAPICState, dev); > int io_memory; > + static int ioapic_no; > + > + if (ioapic_no>= MAX_IOAPICS) { > + return -1; > + } > > io_memory = cpu_register_io_memory(ioapic_mem_read, > ioapic_mem_write, s, > @@ -248,6 +285,8 @@ static int ioapic_init1(SysBusDevice *dev) > > qdev_init_gpio_in(&dev->qdev, ioapic_set_irq, IOAPIC_NUM_PINS); > > + ioapics[ioapic_no++] = s; > + > return 0; > } > > diff --git a/hw/ioapic.h b/hw/ioapic.h > new file mode 100644 > index 0000000..cb2642a > --- /dev/null > +++ b/hw/ioapic.h > @@ -0,0 +1,20 @@ > +/* > + * ioapic.c IOAPIC emulation logic > + * > + * Copyright (c) 2011 Jan Kiszka, Siemens AG > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see. > + */ > + > +void ioapic_eoi_broadcast(int vector); >