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: > > KVM_IRQ_ROUTING_EVENTFD
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 > 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... 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. Jan
signature.asc
Description: OpenPGP digital signature
