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: KVM_IRQ_ROUTING_EVENTFD 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 KVM_SET_GSI_ROUTING ioctl(). 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... - 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. Thanks, -- peterx