[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 12/13] q35: ioapic: add support for split irq
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 12/13] q35: ioapic: add support for split irqchip and irqfd |
Date: |
Sun, 17 Apr 2016 12:45:03 +0300 |
On Sat, Apr 16, 2016 at 07:44:12PM -0700, Jan Kiszka wrote:
> On 2016-04-14 20:31, Peter Xu wrote:
> > This patch allows Intel IR work with splitted irqchip. Two more fields
> > are added to IOAPICCommonState to support the translation process (For
> > future AMD IR support, we will need to provide another AMD-specific
> > callback for int_remap()). In split irqchip mode, IOAPIC is working in
> > user space, only update kernel irq routes when entry changed. When IR is
> > enabled, we directly update the kernel with translated messages. It
> > works just like a kernel cache for the remapping entries.
> >
> > Since KVM irqfd is using kernel gsi routes to deliver interrupts, as
> > long as we can support split irqchip, we will support irqfd as
> > well. Also, since kernel gsi routes will cache translated interrupts,
> > irqfd delivery will not suffer from any performance impact due to IR.
> >
> > And, since we supported irqfd, vhost devices will be able to work
> > seamlessly with IR now. Logically this should contain both vhost-net and
> > vhost-user case.
> >
> > Here we avoided capturing IOMMU IR invalidation, based on the assumption
> > that, guest kernel will always first update IR entry, then IOAPIC
> > entry. As long as guest follows this order to update IOAPIC entries, we
> > should be safe.
> >
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> > hw/i386/intel_iommu.c | 9 +++++++--
> > hw/intc/ioapic.c | 39
> > ++++++++++++++-------------------------
> > hw/intc/ioapic_common.c | 4 ++++
> > include/hw/i386/intel_iommu.h | 2 ++
> > include/hw/i386/ioapic_internal.h | 5 +++++
> > 5 files changed, 32 insertions(+), 27 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 68ebc1e..104afeb 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2077,9 +2077,9 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
> > *iommu,
> > uint16_t index = 0;
> > VTDIrq irq = {0};
> >
> > - assert(iommu && origin && translated);
> > + assert(origin && translated);
> >
> > - if (!iommu->intr_enabled) {
> > + if (!iommu || !iommu->intr_enabled) {
> > memcpy(translated, origin, sizeof(*origin));
> > return 0;
> > }
> > @@ -2143,6 +2143,11 @@ static int vtd_interrupt_remap_msi(IntelIOMMUState
> > *iommu,
> > return 0;
> > }
> >
> > +int vtd_int_remap(void *iommu, MSIMessage *src, MSIMessage *dst)
> > +{
> > + return vtd_interrupt_remap_msi(iommu, src, dst);
> > +}
> > +
> > static uint64_t vtd_mem_ir_read(void *opaque, hwaddr addr, unsigned size)
> > {
> > uint64_t data = 0;
> > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> > index 84e8948..b993bd0 100644
> > --- a/hw/intc/ioapic.c
> > +++ b/hw/intc/ioapic.c
> > @@ -182,34 +182,23 @@ static void
> > ioapic_update_kvm_routes(IOAPICCommonState *s)
> > {
> > #ifdef CONFIG_KVM
> > int i;
> > + int ret;
> >
> > if (kvm_irqchip_is_split()) {
> > for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > - uint64_t entry = s->ioredtbl[i];
> > - uint8_t trig_mode;
> > - uint8_t delivery_mode;
> > - uint8_t dest;
> > - uint8_t dest_mode;
> > - uint64_t pin_polarity;
> > - MSIMessage msg;
> > -
> > - trig_mode = ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1);
> > - dest = entry >> IOAPIC_LVT_DEST_SHIFT;
> > - dest_mode = (entry >> IOAPIC_LVT_DEST_MODE_SHIFT) & 1;
> > - pin_polarity = (entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1;
> > - delivery_mode =
> > - (entry >> IOAPIC_LVT_DELIV_MODE_SHIFT) & IOAPIC_DM_MASK;
> > -
> > - msg.address = APIC_DEFAULT_ADDRESS;
> > - msg.address |= dest_mode << 2;
> > - msg.address |= dest << 12;
> > -
> > - msg.data = entry & IOAPIC_VECTOR_MASK;
> > - msg.data |= delivery_mode << APIC_DELIVERY_MODE_SHIFT;
> > - msg.data |= pin_polarity << APIC_POLARITY_SHIFT;
> > - msg.data |= trig_mode << APIC_TRIG_MODE_SHIFT;
> > -
> > - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL);
> > + MSIMessage src, dst;
> > + struct ioapic_entry_info info;
> > + ioapic_entry_parse(s->ioredtbl[i], &info);
> > + src.address = info.addr;
> > + src.data = info.data;
> > + /* We update kernel irqchip routes with translated
> > + * results. */
> > + ret = s->int_remap(s->iommu, &src, &dst);
> > + if (ret) {
> > + DPRINTF("Int remap failed: %d, drop interrupt\n", ret);
> > + continue;
> > + }
> > + kvm_irqchip_update_msi_route(kvm_state, i, dst, NULL);
>
> The need to hook here makes me wonder if we can't inject IOAPIC
> interrupts via KVM_SIGNAL_MSI (abstracted by kvm_irqchip_send_msi, but
> that will pick the fast-path on kernels supporting split irqchip)
> instead of open-coding the route changes. If we translated the IOAPIC
> outputs always into MSIs, the need for special-casing split irqchip
> would be gone, and the need for hooking here for IR just as well.
>
> Jan
Will work for irqfd as well.
--
MST
- [Qemu-devel] [PATCH v3 06/13] intel_iommu: define several structs for IOMMU IR, (continued)
- [Qemu-devel] [PATCH v3 06/13] intel_iommu: define several structs for IOMMU IR, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 07/13] intel_iommu: provide helper function vtd_get_iommu, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 08/13] intel_iommu: add IR translation faults defines, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 09/13] intel_iommu: Add support for PCI MSI remap, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 10/13] q35: ioapic: add support for emulated IOAPIC IR, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 11/13] ioapic: introduce ioapic_entry_parse() helper, Peter Xu, 2016/04/14
- [Qemu-devel] [PATCH v3 12/13] q35: ioapic: add support for split irqchip and irqfd, Peter Xu, 2016/04/14
[Qemu-devel] [PATCH v3 13/13] q35: add "int-remap" flag to enable intr, Peter Xu, 2016/04/14
Re: [Qemu-devel] [PATCH v3 00/13] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/16