linux-kernel - Re: [PATCH] iommu/vt-d: Expand interrupt remapping quirk to cover x58 chipset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Jul 2013 06:59:52 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Don Dutile <ddutile@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Jan Beulich <jbeulich@...e.com>,
	Joerg Roedel <joro@...tes.org>,
	Andrew Cooper <andrew.cooper3@...rix.com>,
	Malcolm Crossley <malcolm.crossley@...rix.com>,
	Prarit Bhargava <prarit@...hat.com>,
	Don Zickus <dzickus@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"maintainer:X86 ARCHITECTURE..." <x86@...nel.org>,
	stable@...r.kernel.org
Subject: Re: [PATCH] iommu/vt-d: Expand interrupt remapping quirk to cover
 x58 chipset

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@...driver.com>
> >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@...driver.com>
> >Reviewed-by: Jan Beulich<jbeulich@...e.com>
> >CC: Jan Beulich<jbeulich@...e.com>
> >CC: Joerg Roedel<joro@...tes.org>
> >CC: Andrew Cooper<andrew.cooper3@...rix.com>
> >CC: Malcolm Crossley<malcolm.crossley@...rix.com>
> >CC: Prarit Bhargava<prarit@...hat.com>
> >CC: Don Zickus<dzickus@...hat.com>
> >CC: Don Dutile<ddutile@...hat.com>
> >CC: Thomas Gleixner<tglx@...utronix.de>
> >CC: Ingo Molnar<mingo@...hat.com>
> >CC: "H. Peter Anvin"<hpa@...or.com>
> >CC: x86@...nel.org (maintainer:X86 ARCHITECTURE...)
> >CC: stable@...r.kernel.org
> >---
> >  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

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