From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47976) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SpBFp-0003ph-4D for qemu-devel@nongnu.org; Thu, 12 Jul 2012 00:43:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SpBFn-0008OC-Ux for qemu-devel@nongnu.org; Thu, 12 Jul 2012 00:43:53 -0400 Message-ID: <1342068222.2229.79.camel@bling.home> From: Alex Williamson Date: Wed, 11 Jul 2012 22:43:42 -0600 In-Reply-To: <4FFE54AA.5070905@ozlabs.ru> References: <1341899497-23265-1-git-send-email-aik@ozlabs.ru> <1341939458.2428.107.camel@bling.home> <4FFCE423.1020505@ozlabs.ru> <1342061686.2229.34.camel@bling.home> <4FFE4F96.6050604@ozlabs.ru> <1342067466.2229.76.camel@bling.home> <4FFE54AA.5070905@ozlabs.ru> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/2] RFC: powerpc-vfio: adding support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-devel@nongnu.org, Alexander Graf , qemu-ppc@nongnu.org, Jan Kiszka , David Gibson On Thu, 2012-07-12 at 14:38 +1000, Alexey Kardashevskiy wrote: > On 12/07/12 14:31, Alex Williamson wrote: > > On Thu, 2012-07-12 at 14:16 +1000, Alexey Kardashevskiy wrote: > >> On 12/07/12 12:54, Alex Williamson wrote: > >>> On Wed, 2012-07-11 at 12:25 +1000, Alexey Kardashevskiy wrote: > >>>> On 11/07/12 02:57, Alex Williamson wrote: > >>>>> On Tue, 2012-07-10 at 15:51 +1000, Alexey Kardashevskiy wrote: > >>>>>> The two patches in this set are supposed to add VFIO support for POWER. > >>>>>> > >>>>>> The first one adds one more step in the initalizaion sequence which I am not > >>>>>> sure is correct. > >>>>>> > >>>>>> The second patch adds actual VFIO support. It is not ready to submit but > >>>>>> ready to discuss. I would like to get rid of all #ifdef TARGET_PPC64 in patch #2 > >>>>>> and I wonder if there is any plan to implement some generic EOI support code, etc. > >>>>> > >>>>> A generic EOI notifier is on my todo list, but I have no idea what it's > >>>>> going to look like. As you know, I've got an ioapic specific notifier > >>>>> in my tree, you add a spapr specific one. I welcome ideas on how to > >>>>> create something generic that has a chance of being accepted. Thanks, > >>>> > >>>> > >>>> So far the only platform specific call is xxxx_add_gsi_eoi_notifier. The > >>>> xxxx_remove_gsi_eoi_notifier only calls notifier_remove, you've got to fix yours > >>>> ioapic_remove_gsi_eoi_notifier() as it does too much :) > >>>> > >>>> > >>>> The only place for placing "add_eoi" callback I can see right now is QEMUMachine as there is no > >>>> unified machine interrupt controller - IOAPIC has its own type TYPE_IOAPIC_COMMON and XICS is not > >>>> even a SysBusDevice. And the callback is not specific for any kind of bus so it cannot go to PCIBus. > >>>> > >>>> Does it sound reasonable? > >>> > >>> I suspect we'd need to somehow tie it into qemu_irq where both handlers > >>> and notifiers are allocated so we don't really care the underlying > >>> implementation. Something like qemu_add_irq_eoi_notifier(qemu_irq > >>> irq, ...). It's another mess like adding the PCIBus interrupt line to > >>> gsi effort though. Thanks, > >> > >> > >> Tried. Added add_eoi_notifier() callback to qemu_irq, new IRQ allocator: > >> qemu_irq *qemu_allocate_irqs2(qemu_irq_handler handler, void *opaque, int n, > >> qemu_eoi_add_notifier add_notifier); > >> and called it from the XICS initialization code. > >> > >> It could work out if pci_get_irq() or pci_route_irq_fn() returned qemu_irq but no, they just return > >> a global IRQ number (pure or embedded in a struct) and there is no common way to resolve qemu_irq > >> (and then add_eoi_notifier()) from that number within vfio_pci. > > > > Well GSI and qemu_irq are different address spaces. We still need GSI > > for any kind of qemu bypass case. > > No, that is ok, we also need GSI because XICS and IOAPIC need it in the end. > > >> May be we could add the callback pointer into PCIINTxRoute? > > > > Maybe, but why is this PCI specific? Can't we call it as > > qemu_add_irq_eoi_notifier(pdev->irq[0], Notifier)? That would work much > > like qemu_set_irq, extracting the irq number from the IRQState and > > passing it through to the add_notifier callback for IRQState until it > > got to the ioapic/pic/xics. > > > > int qemu_add_irq_eoi_notifier(qemu_irq *irq, Notifier notifier) > > { > > if (!irq || !irq->add_eoi_notifier) > > return -1; > > > > return irq->add_eoi_notifier(irq->opaque, irq->n, notifier); > > } > > > > Then we will have to entirely replace qemu_allocate_irqs() with qemu_allocate_irqs2() and pass some > non-zero add_eoi_notifier() on every level, at least for PCI for now. I would like to avoid that if > possible - hard to get accepted :) Yep, that's why I said it was the same kind of mess as the PCIBus intx routing. It's intrusive, but qemu_irq is the common interrupt model so we need to make use of it. A callback on QEMUMachine seems completely random. Thanks, Alex