[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU |
Date: |
Tue, 26 Apr 2016 19:40:51 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Apr 26, 2016 at 12:51:44PM +0200, Jan Kiszka wrote:
> On 2016-04-26 12:38, Peter Xu wrote:
> >>>> Hi, Jan,
> >>>>
> >>>> The above issue should be caused by EOI missing of level-triggered
> >>>> interrupts. Before that, I was always using edge-triggered
> >>>> interrupts for test, so didn't encounter this one. Would you please
> >>>> help try below patch? It can be applied directly onto the series,
> >>>> and should solve the issue (it works on my test vm, and I'll take it
> >>>> in v5 as well if it also works for you):
> >>>>
> >>>
> >>> Works here as well. I even made EIM working with some hack, though
> >>> Jailhouse spits out strange warnings, despite it works fine (x2apic
> >>> mode, split irqchip).
> >>
> >> Corrections: the warnings are issued by qemu, not Jailhouse, e.g.
> >>
> >> qemu-system-x86_64: VT-d Failed to remap interrupt for gsi 22.
> >>
> >> I suspect that comes from the hand-over phase of Jailhouse, when it
> >> mutes all interrupts in the system while reconfiguring IR and IOAPIC.
> >>
> >> Please convert this error (in kvm_arch_fixup_msi_route) into a trace
> >> point. It shall not annoy the host. Also check if you have more of such
> >> guest-triggerable error messages.
> >
> > Okay. This should be the only one. I can use trace instead.
> >
> > Meanwhile, I still suppose we should not seen it even with
> > error_report().. Would this happen when boot e.g. generic kernels?
>
> No, this is - most probably, still need to validate in details - an
> effect due to the special case with Jailhouse that interrupt and VT-d
> management is handed over from a running OS to a hypervisor.
>
> Jan
>
> PS: Current quick hack for EIM support (lacks compat mode blocking at
> least):
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 1082ab5..709a92c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -907,6 +907,7 @@ static void
> vtd_interrupt_remap_table_setup(IntelIOMMUState *s)
> value = vtd_get_quad_raw(s, DMAR_IRTA_REG);
> s->intr_size = 1UL << ((value & VTD_IRTA_SIZE_MASK) + 1);
> s->intr_root = value & VTD_IRTA_ADDR_MASK;
> + s->intr_eime = value & VTD_IRTA_EIME;
>
> QLIST_FOREACH(notifier, &s->iec_notifiers, list) {
> if (notifier->iec_notify) {
> @@ -2052,11 +2053,13 @@ static int vtd_remap_irq_get(IntelIOMMUState *iommu,
> uint16_t index, VTDIrq *irq
> irq->trigger_mode = irte.trigger_mode;
> irq->vector = irte.vector;
> irq->delivery_mode = irte.delivery_mode;
> - /* Not support EIM yet: please refer to vt-d 9.10 DST bits */
> + irq->dest = irte.dest_id;
> + if (!iommu->intr_eime) {
> #define VTD_IR_APIC_DEST_MASK (0xff00ULL)
> #define VTD_IR_APIC_DEST_SHIFT (8)
> - irq->dest = (irte.dest_id & VTD_IR_APIC_DEST_MASK) >> \
> - VTD_IR_APIC_DEST_SHIFT;
> + irq->dest = (irq->dest & VTD_IR_APIC_DEST_MASK) >>
> + VTD_IR_APIC_DEST_SHIFT;
> + }
> irq->dest_mode = irte.dest_mode;
> irq->redir_hint = irte.redir_hint;
>
> @@ -2316,7 +2319,7 @@ static void vtd_init(IntelIOMMUState *s)
> s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
>
> if (ms->iommu_intr) {
> - s->ecap |= VTD_ECAP_IR;
> + s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM;
> }
>
> vtd_reset_context_cache(s);
> @@ -2370,10 +2373,9 @@ static void vtd_init(IntelIOMMUState *s)
> vtd_define_quad(s, DMAR_FRCD_REG_0_2, 0, 0, 0x8000000000000000ULL);
>
> /*
> - * Interrupt remapping registers, not support extended interrupt
> - * mode for now.
> + * Interrupt remapping registers.
> */
> - vtd_define_quad(s, DMAR_IRTA_REG, 0, 0xfffffffffffff00fULL, 0);
> + vtd_define_quad(s, DMAR_IRTA_REG, 0, 0xfffffffffffff80fULL, 0);
> }
>
> /* Should not reset address_spaces when reset because devices will still use
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 10c20fe..72b0114 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -176,6 +176,7 @@
>
> /* IRTA_REG */
> #define VTD_IRTA_ADDR_MASK (VTD_HAW_MASK ^ 0xfffULL)
> +#define VTD_IRTA_EIME (1ULL << 11)
> #define VTD_IRTA_SIZE_MASK (0xfULL)
>
> /* ECAP_REG */
> @@ -184,6 +185,7 @@
> #define VTD_ECAP_QI (1ULL << 1)
> /* Interrupt Remapping support */
> #define VTD_ECAP_IR (1ULL << 3)
> +#define VTD_ECAP_EIM (1ULL << 4)
>
> /* CAP_REG */
> /* (offset >> 4) << 24 */
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index d7613af..c6c53c6 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -261,6 +261,7 @@ struct IntelIOMMUState {
> bool intr_enabled; /* Whether guest enabled IR */
> dma_addr_t intr_root; /* Interrupt remapping table pointer */
> uint32_t intr_size; /* Number of IR table entries */
> + bool intr_eime; /* Extended interrupt mode enabled */
> QLIST_HEAD(, VTD_IEC_Notifier) iec_notifiers; /* IEC notify list */
> };
>
Currently, all the interrupts will be translated into one MSI in
vtd_generate_msi_message(), in which only 8 bits of dest_id is used
(msg.dest = irq->dest). We may possibly need to use the high 32 bits
of MSI address to store the rest dest[31:8]? Don't know whether this
would be enough though.
Thanks.
-- peterx
- [Qemu-devel] [PATCH v4 15/16] intel_iommu: introduce IEC notifiers, (continued)
- [Qemu-devel] [PATCH v4 15/16] intel_iommu: introduce IEC notifiers, Peter Xu, 2016/04/19
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/25
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Peter Xu, 2016/04/25
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/25
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/25
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Peter Xu, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Peter Xu, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU,
Peter Xu <=
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Jan Kiszka, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/26
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Peter Xu, 2016/04/27
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/27
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Peter Xu, 2016/04/28
- Re: [Qemu-devel] [PATCH v4 00/16] IOMMU: Enable interrupt remapping for Intel IOMMU, Radim Krčmář, 2016/04/28