Re: [PATCH] iommu/vt-d: Expand interrupt remapping quirk to coverx58 chipset

[Neil Horman]

On Thu, Jul 11, 2013 at 06:59:52AM -0400, Neil Horman wrote:
> On Wed, Jul 10, 2013 at 01:31:36PM -0400, Don Dutile wrote:
> > On 07/09/2013 03:11 PM, Neil Horman wrote:
> > >Recently we added an early quirk to detect 5500/5520 chipsets with early
> > >revisions that had problems with irq draining with interrupt remapping enabled:
> > >
> > >commit 03bbcb2e7e292838bb0244f5a7816d194c911d62
> > >Author: Neil Horman<nhorman@xxxxxxxxxxxxx>
> > >Date:   Tue Apr 16 16:38:32 2013 -0400
> > >
> > >     iommu/vt-d: add quirk for broken interrupt remapping on 55XX chipsets
> > >
> > >It turns out this same problem is present in the intel X58 chipset as well.  See
> > >errata 69 here:
> > >http://www.intel.com/content/www/us/en/chipsets/x58-express-specification-update.html
> > >
> > >This patch extends the pci early quirk so that the chip devices/revisions
> > >specified in the above update are also covered in the same way:
> > >
> > >Signed-off-by: Neil Horman<nhorman@xxxxxxxxxxxxx>
> > >Reviewed-by: Jan Beulich<jbeulich@xxxxxxxx>
> > >CC: Jan Beulich<jbeulich@xxxxxxxx>
> > >CC: Joerg Roedel<joro@xxxxxxxxxx>
> > >CC: Andrew Cooper<andrew.cooper3@xxxxxxxxxx>
> > >CC: Malcolm Crossley<malcolm.crossley@xxxxxxxxxx>
> > >CC: Prarit Bhargava<prarit@xxxxxxxxxx>
> > >CC: Don Zickus<dzickus@xxxxxxxxxx>
> > >CC: Don Dutile<ddutile@xxxxxxxxxx>
> > >CC: Thomas Gleixner<tglx@xxxxxxxxxxxxx>
> > >CC: Ingo Molnar<mingo@xxxxxxxxxx>
> > >CC: "H. Peter Anvin"<hpa@xxxxxxxxx>
> > >CC: x86@xxxxxxxxxx (maintainer:X86 ARCHITECTURE...)
> > >CC: stable@xxxxxxxxxxxxxxx
> > >---
> > >  arch/x86/kernel/early-quirks.c | 15 +++++++++++++--
> > >  1 file changed, 13 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
> > >index 94ab6b9..743d583 100644
> > >--- a/arch/x86/kernel/early-quirks.c
> > >+++ b/arch/x86/kernel/early-quirks.c
> > >@@ -196,15 +196,23 @@ static void __init ati_bugs_contd(int num, int slot, int func)
> > >  static void __init intel_remapping_check(int num, int slot, int func)
> > >  {
> > >  	u8 revision;
> > >+	u16 device;
> > >
> > >+	device = read_pci_config_16(num, slot, func, PCI_DEVICE_ID);
> > >  	revision = read_pci_config_byte(num, slot, func, PCI_REVISION_ID);
> > >
> > >  	/*
> > >-	 * Revision 0x13 of this chipset supports irq remapping
> > >-	 * but has an erratum that breaks its behavior, flag it as such
> > >+ 	 * Revision 13 of all triggering devices id in this quirk have
> > >+	 * a problem draining interrupts when irq remapping is enabled,
> > >+	 * and should be flagged as broken.  Additionally revisions 0x12
> > >+	 * and 0x22 of device id 0x3405 has this problem.
> > >  	 */
> > >  	if (revision == 0x13)
> > >  		set_irq_remapping_broken();
> > >+	else if ((device == 0x3405)&&
> > >+	    ((revision == 0x12) ||
> > >+	     (revision == 0x22)))
> > >+		set_irq_remapping_broken();
> > >
> > >  }
> > >
> > When discussing the original-seen errata w/Intel on 55xx chips, the
> > statements made were any chip with rev C1(revision = 0x21) or
> > greater had the correct
> > hw implementation for the intr-pending flush.
> > We knew the bug existed in the A3 (rev=0x13) rev of the chip, but the
> > true check should be:
> > 	revision < 0x21
> > 
> > I suspect there were multiple revs of the x58, of which B2(0x12) & C2(0x22)
> > were shipped to oem's, system vendors, etc.
> > But, in case there were any chip revisions in between these well-known values
> > out there, I suggest the 0x3405 check be changed to:
> > 	revision < 0x22
> > 
> > Since it's unlikely that hw degressed in design over revisions, it seems
> > more correct to check for revs less than a rev-value having an errata,
> > or conversely, a chip value >= rev-value do not have the errata.
> > IOW, an equal check may not provide sufficient.
> > 
> Don and I discussed this offline.  Given that his comments make good sense
> to me, I'm hesitant to apply the quirk to anything other than what the spec
> update says, given that its clear.  Don is attempting to contact people at Intel
> who will be able (we hope) to give us a definitive answer on this, please hold
> on this patch until we have resolution on the issue.
> Neil
> 
Don, do you have any updates here from Intel?  I'd like to get this put to bed.
Neil

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/