From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49895 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PClU7-0006cm-5E for qemu-devel@nongnu.org; Sun, 31 Oct 2010 23:55:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PClU1-0003C9-S7 for qemu-devel@nongnu.org; Sun, 31 Oct 2010 23:55:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48512) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PClU1-0003C4-JP for qemu-devel@nongnu.org; Sun, 31 Oct 2010 23:54:57 -0400 From: Alex Williamson In-Reply-To: <4CCB1E31.8020006@codemonkey.ws> References: <20101029175434.25281.70647.stgit@s20.home> <4CCB1E31.8020006@codemonkey.ws> Content-Type: text/plain; charset="UTF-8" Date: Sun, 31 Oct 2010 21:54:51 -0600 Message-ID: <1288583691.20007.5.camel@x201> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] APIC/IOAPIC EOI callback List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: chrisw@redhat.com, ddutile@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, mst@redhat.com On Fri, 2010-10-29 at 14:19 -0500, Anthony Liguori wrote: > On 10/29/2010 12:56 PM, Alex Williamson wrote: > > For device assignment, we need to know when the VM writes an end > > of interrupt to the APIC, which allows us to re-enable interrupts > > on the physical device. Add a new wrapper for ioapic generated > > interrupts with a callback on eoi and create an interface for > > drivers to be notified on eoi. > > > > Signed-off-by: Alex Williamson > > --- > > > > Note that the notifier and notifier_enabled eoi_client fields aren't > > used here yet. I'll send an RFC patch showing how we make use of > > these with the proposed KVM_EOI_EVENTFD patches. > > > > hw/apic.c | 18 ++++++++++++++++-- > > hw/apic.h | 4 ++++ > > hw/ioapic.c | 38 ++++++++++++++++++++++++++++++++++++-- > > hw/pc.h | 16 +++++++++++++++- > > 4 files changed, 71 insertions(+), 5 deletions(-) > > > > diff --git a/hw/apic.c b/hw/apic.c > > index 63d62c7..a24117b 100644 > > --- a/hw/apic.c > > +++ b/hw/apic.c > > @@ -22,6 +22,7 @@ > > #include "host-utils.h" > > #include "sysbus.h" > > #include "trace.h" > > +#include "pc.h" > > > > /* APIC Local Vector Table */ > > #define APIC_LVT_TIMER 0 > > @@ -103,6 +104,7 @@ struct APICState { > > int wait_for_sipi; > > }; > > > > +static uint8_t vector_to_gsi_map[256] = { 0xff }; > > static APICState *local_apics[MAX_APICS + 1]; > > static int apic_irq_delivered; > > > > @@ -292,6 +294,15 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, > > trigger_mode); > > } > > > > +void apic_deliver_ioapic_irq(uint8_t dest, uint8_t dest_mode, > > + uint8_t delivery_mode, uint8_t vector_num, > > + uint8_t polarity, uint8_t trigger_mode, int gsi) > > +{ > > + vector_to_gsi_map[vector_num] = gsi; > > + apic_deliver_irq(dest, dest_mode, delivery_mode, > > + vector_num, polarity, trigger_mode); > > +} > > + > > void cpu_set_apic_base(DeviceState *d, uint64_t val) > > { > > APICState *s = DO_UPCAST(APICState, busdev.qdev, d); > > @@ -420,8 +431,11 @@ 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 (vector_to_gsi_map[isrv] != 0xff) { > > + ioapic_eoi(vector_to_gsi_map[isrv]); > > + vector_to_gsi_map[isrv] = 0xff; > > + } > > apic_update_irq(s); > > } > > > > diff --git a/hw/apic.h b/hw/apic.h > > index 8a0c9d0..59d0e37 100644 > > --- a/hw/apic.h > > +++ b/hw/apic.h > > @@ -8,6 +8,10 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, > > uint8_t delivery_mode, > > uint8_t vector_num, uint8_t polarity, > > uint8_t trigger_mode); > > +void apic_deliver_ioapic_irq(uint8_t dest, uint8_t dest_mode, > > + uint8_t delivery_mode, > > + uint8_t vector_num, uint8_t polarity, > > + uint8_t trigger_mode, int gsi); > > int apic_accept_pic_intr(DeviceState *s); > > void apic_deliver_pic_intr(DeviceState *s, int level); > > int apic_get_interrupt(DeviceState *s); > > diff --git a/hw/ioapic.c b/hw/ioapic.c > > index 5ae21e9..ffd1c92 100644 > > --- a/hw/ioapic.c > > +++ b/hw/ioapic.c > > @@ -26,6 +26,7 @@ > > #include "qemu-timer.h" > > #include "host-utils.h" > > #include "sysbus.h" > > +#include "qlist.h" > > > > //#define DEBUG_IOAPIC > > > > @@ -61,6 +62,39 @@ struct IOAPICState { > > uint64_t ioredtbl[IOAPIC_NUM_PINS]; > > }; > > > > +static QLIST_HEAD(ioapic_eoi_client_list, > > + ioapic_eoi_client) ioapic_eoi_client_list = > > + QLIST_HEAD_INITIALIZER(ioapic_eoi_client_list); > > + > > +int ioapic_register_eoi_client(ioapic_eoi_client *client) > > +{ > > + QLIST_INSERT_HEAD(&ioapic_eoi_client_list, client, list); > > + return 0; > > +} > > + > > +void ioapic_unregister_eoi_client(ioapic_eoi_client *client) > > +{ > > + QLIST_REMOVE(client, list); > > +} > > + > > +int ioapic_eoi_client_get_fd(ioapic_eoi_client *client) > > +{ > > + if (!client->notifier_enabled) { > > + return -ENODEV; > > + } > > + return event_notifier_get_fd(&client->notifier); > > +} > > + > > +void ioapic_eoi(int gsi) > > +{ > > + ioapic_eoi_client *client; > > + QLIST_FOREACH(client,&ioapic_eoi_client_list, list) { > > + if (client->irq == gsi) { > > + client->eoi(client); > > + } > > + } > > +} > > > > I think this all goes away with a NotifierList. Are you thinking of an array of 256 NotifierLists, one per irq? I'm not sure how else I could keep track of which Notifier is associated with which irq. That seems a little excessive unless we start getting a lot more users of this callback. Thanks, Alex