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
