[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 16/26] intel_iommu: add support for split ir

From: Jan Kiszka
Subject: Re: [Qemu-devel] [PATCH v10 16/26] intel_iommu: add support for split irqchip
Date: Sat, 25 Jun 2016 17:18:40 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv: Gecko/20080226 SUSE/ Thunderbird/ Mnenhy/

On 2016-06-25 15:18, Peter Xu wrote:
> On Sat, Jun 25, 2016 at 10:08:10AM +0200, Jan Kiszka wrote:
> [...]
>> For successful remappings, this is fine - it just caches the result in
>> an interrupt route. But what will happen with invalid interrupts?
>> My current understanding is that, because the translation happens on
>> activation of that interrupt source, not on actual signalling, the IOMMU
>> will report an error too early and none when the interrupt is actually
>> sent. That will lead to unwanted results, in the worst case
>> false-positiv IR error reports to the guest, no?
>> I think we need to do this:
>> - silently remap broken sources to an error sink
>> - hook up the error sink with the actual IOMMU model (Intel or AMD)
>> - when that source actually fires, let the sink report an IR
>>   translation error to the guest
>> Am I right?
> Right. I totally missed this one. :(
> Currently when split irqchip is specified, IOAPIC interrupts are
> cached in kernel with type KVM_IRQ_ROUTING_MSI (which is the same as
> irqfds). When guest specify a fault interrupt entry, it is possible
> that we silently fail the update, and all further interrupts are still
> the old and correct one.
> I agree with your solution on this. First of all we update the
> interrupt even if it's faulty, but we should mark it out. After that,
> we should fire QEMU from kernel side when the fault interrupt is
> triggered, so that QEMU IOMMU can still generate corresponding fault
> report interrupt to guest (though for Intel IOMMU IR, we still haven't
> handled any fault report yet, but we should be prepared for it).
> So it seems that finally we cannot avoid touching KVM this time.
> I have a thought on how to implement the "sink" you have mentioned:
> First of all, in KVM, we provide a new KVM_IRQ_ROUTING_* type, maybe
> called:

Not really, because all sources are either using eventfds, which you can
also terminate in user space (already done for vhost and vfio in certain
scenarios - IIRC) or originate there anyway (IOAPIC).

> When KVM got this kind of interrupt, KVM does not trigger any real
> interrupt to guest. Instead, it just do eventfd_signal() to a
> pre-defined fd (maybe also with some data along with the notification,
> so that we can put the error inside?), which is set during
> After that, QEMU register all fault interrupts using this new
> KVM_IRQ_ROUTING_EVENTFD type (rather than original
> KVM_IRQ_ROUTING_MSI), assign a specific handler to handle the events
> from these interrupts, and trigger IOMMU fault report path in that
> handler.
> (Here I used KVM_IRQ_ROUTING_EVENTFD rather than something like
>  KVM_IRQ_ROUTING_FAULT_MSI to make the API a more general one, in case
>  we can leverage it in other cases in the future)
> Do you think the above workable?
> No matter which solution we will have, I would still suggest we add
> this as an "enhancement" after this series, since:
> - there are works that depend on this series, so I would appreciate if
>   this series can be merged first, so that other people can have a
>   good basement (Radim's x2apic, David's AMD IOMMU). Though this is
>   based on the assumption that the basic design of this series is
>   workable...

I understand, and it is probably safe...

> - this problem will only exist for guest driver developers and should
>   not happen for generic users (right?), so only a small subset of
>   users might be affected.

...provided there is only little risk that some guest programs some
half-backed or stale message that would be rejected prematurely. But
that risk is most likely low.


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]