>>> On 14.06.11 at 23:20, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
>> Let's see what our friends at Intel think...
>
> IOMMU_WAIT_OP() has a timeout value of 1 second. When it times out, it will
> cause a system panic.
Sort of bad when we're already bringing down the system possibly
because of a panic.
Jan
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxxxx]
> Sent: Tuesday, June 14, 2011 2:02 AM
> To: Andrew Cooper
> Cc: Kay, Allen M; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 5 of 7] IOMMU VTD BUG: disable Extended
> Interrupt Mode when disabling Interupt Remapping
>
>>>> On 13.06.11 at 19:02, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>> Experimental evidence shows that Extended Interrupt Mode remains in
>> effect even after Interrupt Remapping is disabled in each DMAR Global
>> Command Register. A consiquence of this is that when we switch from
>> x2apic mode back to xapic mode, and disable interrupt remapping for
>> the kdump kernel, interrupts passing through the IO APICs are in
>> x2apic format as opposed xapic. This causes a triple fault in the
>> kexec kernel.
>>
>> As EIM is explicitly set up each time Interrup Remapping is enabled,
>> it is safe for us to clobber this when taring down.
>>
>> Also, change the header definition of IRTA_REG_EIME_SHIFT. It caused
>> verbose and error-prone code, and was only used in 1 place before. We
>> now have IRTA_EIME which is the specific bit in the register.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>
>> diff -r 615db56671fa -r 2635774346c4 xen/drivers/passthrough/vtd/intremap.c
>> --- a/xen/drivers/passthrough/vtd/intremap.c Mon Jun 13 17:45:43 2011 +0100
>> +++ b/xen/drivers/passthrough/vtd/intremap.c Mon Jun 13 17:45:43 2011 +0100
>> @@ -776,8 +776,7 @@ int enable_intremap(struct iommu *iommu,
>>
>> #ifdef CONFIG_X86
>> /* set extended interrupt mode bit */
>> - ir_ctrl->iremap_maddr |=
>> - eim ? (1 << IRTA_REG_EIME_SHIFT) : 0;
>> + ir_ctrl->iremap_maddr |= eim ? IRTA_EIME : 0;
>> #endif
>> spin_lock_irqsave(&iommu->register_lock, flags);
>>
>> @@ -817,6 +816,22 @@ void disable_intremap(struct iommu *iomm
>> if ( !ecap_intr_remap(iommu->ecap) )
>> return;
>>
>> + /* If we are disabling Interrupt Remapping, make sure we dont stay in
>> + * Extended Interrupt Mode, as this is unaffected by the Interrupt
>> + * Remapping flag in each DMAR Global Control Register.
>> + * Specifically, local apics in xapic mode do not like interrupts
>> delivered
>> + * in x2apic mode. Any code turning interrupt remapping back on will
>> set
>> + * EIME back correctly.
>> + */
>> + if ( iommu_supports_eim() )
>> + {
>> + u64 irta;
>> + irta = dmar_readl(iommu->reg, DMAR_IRTA_REG);
>> + dmar_writel(iommu->reg, DMAR_IRTA_REG, irta & ~IRTA_EIME);
>> + IOMMU_WAIT_OP(iommu, DMAR_IRTA_REG, dmar_readl,
>> + !(irta & IRTA_EIME), irta);
>
> Hmm, this of course can be problematic in the context of a crash:
> I realize that you want it cleared, but does system state guarantee
> that this WAIT_OP will always be able to complete? You'd get a
> hung system instead if it won't. Admittedly it's not clear which one
> is better, but a best effort approach would probably skip the wait
> loop. Let's see what our friends at Intel think...
>
> Jan
>
>> + }
>> +
>> spin_lock_irqsave(&iommu->register_lock, flags);
>> sts = dmar_readl(iommu->reg, DMAR_GSTS_REG);
>> if ( !(sts & DMA_GSTS_IRES) )
>> diff -r 615db56671fa -r 2635774346c4 xen/drivers/passthrough/vtd/iommu.h
>> --- a/xen/drivers/passthrough/vtd/iommu.h Mon Jun 13 17:45:43 2011 +0100
>> +++ b/xen/drivers/passthrough/vtd/iommu.h Mon Jun 13 17:45:43 2011 +0100
>> @@ -471,7 +471,7 @@ struct qinval_entry {
>>
>> #define IEC_GLOBAL_INVL 0
>> #define IEC_INDEX_INVL 1
>> -#define IRTA_REG_EIME_SHIFT 11
>> +#define IRTA_EIME (1 << 11)
>>
>> /* 2^(IRTA_REG_TABLE_SIZE + 1) = IREMAP_ENTRY_NR */
>> #define IRTA_REG_TABLE_SIZE ( IREMAP_PAGE_ORDER + 7 )
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|