Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU

[Jan Kiszka <jan.kiszka@web.de>]

On 2016-04-19 10:38, Peter Xu wrote:
> Jan, Michael,
> 
> Still haven't got your response from v3 review comments, but I
> decided to move on to provide a workable version first (v3 is
> missing the first patch, so it is not working). Also, misc issues
> are addressed from Radim. Anyway, I am always ready for v5 and
> further. :)
> 
> Github branch for your reference in case needed:
> 
>   https://github.com/xzpeter/qemu vtd-intr-v4
> 
> Thanks,
> 
> v4 changes (all patch number corresponds to v3):
> - add one patch at the start of v3 series: I missed to send the
>   first patch in v3. adding it in. [Jan]
> - patch 9: add support for compatible mode (no reason not to support
>   it, if not, we will get some warnings when using split irqchip)
> - patch 11: further simplify ioapic_update_kvm_routes() using the
>   helper function.
> - patch 12: tweak on kvm_arch_fixup_msi_route() rather than
>   ioapic_update_kvm_routes() only. [Radim]
> - add patch 15: introduce IEC (Interrupt Entry Cache) invalidation
>   notifier list. We can register to this list if we want to be
>   notified when we got IR invalidation requests [Radim]
> - add patch 16: let IOAPIC the first consumer for the above IEC
>   notifier list. [Radim]
> - several other trivial fixes (like moving some defines from .c to
>   .h, moving several lines of changes from one patch to another to
>   make it make more sense, etc.)
> 
> v3 changes (all patch numbers corresponds to v2):
> - patch 1 (-> v3 patch 13)
>   - move to the end of series [Alex]
> - patch 10 (dropped)
>   - drop this one, since re-worked on IOAPIC support, so we do not
>     need this any more.
> - patch 12 (-> v3 patch 10)
>   - leverage MSI path for IOAPIC IR [Jan]
> - patch 13 (v3 -> patch 9)
>   - remove vtd_interrupt_remap_msi() declaration by reordering the
>     functions [mst]
>   - vtd_generate_msi_message(): init msg using {}, remove FIXME
>     [mst]
> - new patches
>   - v3 patch 11: introduce ioapic_entry_parse() helper function
>   - v3 patch 12: add support for kernel-irqchip=split. This needs
>     more reviews, logically this should enable lots of things:
> 	splitted irqchip, irqfd, vhost, and irqfd support for
> 	passthrough devices (not tested). Please refer to the patch for
> 	more information.
> 
> v2 changes:
> - patch 1
>   - rename "int_remap" to "intr" in several places [Marcel]
>   - remove "Intel" specific words in desc or commit message, prepare
>     itself with further AMD support [Marcel]
>   - avoid using object_property_get_bool() [Marcel]
> - patch 5
>   - use PCI bus number 0xff rather than 0xf0 for the IOAPIC scope
>     definition. (please let me know if anyone knows how I can avoid
> 	user using PCI bus number 0xff... TIA)
> - patch 11
>   - fix comments [Marcel]
> - all
>   - remove intr_supported variable [Marcel]
> 
> This patchset provide very basic functionalities for interrupt
> remapping (IR) support of the emulated Intel IOMMU device.
> 
> By default, IR is disabled to be better compatible with current
> QEMU. To enable IR, we can using the following command to boot a
> IR-supported VM with virtio-net device with vhost (still do not
> support kvm-ioapic, so we need to specify kernel-irqchip={split|off}
> here):
> 
> $ qemu-system-x86_64 -M q35,iommu=on,intr=on,kernel-irqchip=split \

"intr" sounds a bit too much like "interrupt", not "interrupt
remapping". Why not use the kernel's form, "intremap"?

>      -enable-kvm -m 1024 \
> 	 -netdev tap,id=net0,vhost=on \
>      -device virtio-net-pci,netdev=user.0 \
>      -monitor telnet::3333,server,nowait \
> 	 /var/lib/libvirt/images/vm1.qcow2
> 
> When guest boots, we can verify whether IR enabled by grepping the
> dmesg like:
> 
> [root@localhost ~]# journalctl -k | grep "DMAR-IR"
> Feb 19 11:21:23 localhost.localdomain kernel: DMAR-IR: IOAPIC id 0 under DRHD base  0xfed90000 IOMMU 0
> Feb 19 11:21:23 localhost.localdomain kernel: DMAR-IR: Enabled IRQ remapping in xapic mode
> 
> Currently supported devices:
> 
> - Emulated/Splitted irqchip
> - Generic PCI Devices
> - vhost devices
> - pass through device support? Not tested, but suppose it should work.

I've tested this series against my Jailhouse setup, and it works pretty
well! Actually considering to move my test setup over this branch.

However, split irqchip still has some issues: When I boot a q35 machine
with Linux, the e1000 network adapter only gets a single IRQ delivered.
Interestingly, other IOAPIC IRQs like the keyboard work all the time. I
didn't debug this in details yet.

> 
> TODO List:
> 
> - kvm-ioapic support (?)
> - EIM support

That should be fairly easy, I already played with it (hack in EIM cap,
change vtd_remap_irq_get, assuming EIME would be set). However, it
depends on split irqchip to work properly (there is no x2apic in
userspace APIC), and that is not yet the case.

> - IR fault reporting

Would be welcome! I found a "test case" yesterday: misconfigured IOAPIC
ID blocked its IRQs under Jailhouse, and I first had to enable tracing
to realize it ;).

Jan

> - source-id validation for IRTE
> - IRTE cache and better queued invalidation
> - migration support (for IOMMU as general?)
> - more?
> 
> Peter Xu (16):
>   acpi: enable INTR for DMAR report structure
>   intel_iommu: allow queued invalidation for IR
>   intel_iommu: set IR bit for ECAP register
>   acpi: add DMAR scope definition for root IOAPIC
>   intel_iommu: define interrupt remap table addr register
>   intel_iommu: handle interrupt remap enable
>   intel_iommu: define several structs for IOMMU IR
>   intel_iommu: provide helper function vtd_get_iommu
>   intel_iommu: add IR translation faults defines
>   intel_iommu: Add support for PCI MSI remap
>   q35: ioapic: add support for emulated IOAPIC IR
>   ioapic: introduce ioapic_entry_parse() helper
>   intel_iommu: add support for split irqchip
>   q35: add "int-remap" flag to enable intr
>   intel_iommu: introduce IEC notifiers
>   ioapic: register VT-d IEC invalidate notifier
> 
>  hw/core/machine.c                 |  22 +++
>  hw/i386/acpi-build.c              |  36 ++--
>  hw/i386/intel_iommu.c             | 376 +++++++++++++++++++++++++++++++++++++-
>  hw/i386/intel_iommu_internal.h    |  47 ++++-
>  hw/i386/pc.c                      |   3 +
>  hw/intc/ioapic.c                  | 125 ++++++++-----
>  hw/pci-host/q35.c                 |   4 +
>  include/hw/acpi/acpi-defs.h       |  15 ++
>  include/hw/boards.h               |   1 +
>  include/hw/i386/apic-msidef.h     |   1 +
>  include/hw/i386/intel_iommu.h     | 145 +++++++++++++++
>  include/hw/i386/ioapic_internal.h |   1 +
>  include/hw/i386/pc.h              |   4 +
>  include/hw/pci-host/q35.h         |   9 +
>  target-i386/kvm.c                 |  24 +++
>  15 files changed, 754 insertions(+), 59 deletions(-)
>