From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41832) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SpBUN-0005s5-CU for qemu-devel@nongnu.org; Thu, 12 Jul 2012 00:58:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SpBUM-0003K3-2l for qemu-devel@nongnu.org; Thu, 12 Jul 2012 00:58:55 -0400 Received: from mail-gh0-f173.google.com ([209.85.160.173]:46835) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SpBUL-0003Jk-UH for qemu-devel@nongnu.org; Thu, 12 Jul 2012 00:58:54 -0400 Received: by ghrr14 with SMTP id r14so2167012ghr.4 for ; Wed, 11 Jul 2012 21:58:53 -0700 (PDT) Message-ID: <4FFE5987.7050904@ozlabs.ru> Date: Thu, 12 Jul 2012 14:58:47 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 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> <1342068222.2229.79.camel@bling.home> In-Reply-To: <1342068222.2229.79.camel@bling.home> Content-Type: text/plain; charset=KOI8-R 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: Alex Williamson Cc: qemu-devel@nongnu.org, Alexander Graf , qemu-ppc@nongnu.org, Jan Kiszka , David Gibson On 12/07/12 14:43, Alex Williamson wrote: > 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. There are 2 level of intrusion. 1. Fix PCIINTxRoute to return the GSI's qemu_irq as well. 2. Add add_eoi_notifier to all levels including PCI. As a part of this, we will have to add this callback to all pci_register_bus() calls to reach global interrupts via platform-specific PCI bus. I would stay with 1). Is that bad? > A callback on QEMUMachine seems completely > random. Thanks, True :) -- Alexey