[PATCH,RFC] quirks for VIA VT8237 southbridge

[PATCH,RFC] quirks for VIA VT8237 southbridge

[Karsten Wiese]

Hi,

this fixes the 'doubled ioapic level interrupt rate' issue I've been
seeing on a K8T800/AMD64 mainboard.
It also switches off quirk_via_irq() for the VT8237 southbridge.

As both changes are uncritical fixes, I'll post a real patch to -mm
in a week or so.

   Karsten


--- ../linux-2.6.13-rc6/drivers/pci/quirks.c    2005-08-08 11:46:05.000000000 +0200
+++ drivers/pci/quirks.c        2005-08-13 16:52:19.000000000 +0200
@@ -403,6 +403,25 @@ static void __devinit quirk_via_ioapic(s
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA,     PCI_DEVICE_ID_VIA_82C686,       quirk_via_ioapic );

 /*
+ * VIA 8237: Some BIOSs don't set the 'Bypass APIC De-Assert Message' Bit.
+ * This leads to doubled level interrupt rates.
+ * Set this bit to get rid of cycle wastage.
+ * Otherwise uncritical.
+ */
+static void __devinit quirk_via_vt8237_bypass_apic_deassert(struct pci_dev *dev)
+{
+       u8 misc_control2;
+#define BYPASS_APIC_DEASSERT 8
+
+       pci_read_config_byte(dev, 0x5B, &misc_control2);
+       if (!(misc_control2 & BYPASS_APIC_DEASSERT)) {
+               printk(KERN_INFO "PCI: Bypassing VIA 8237 APIC De-Assert Message\n");
+               pci_write_config_byte(dev, 0x5B, misc_control2|BYPASS_APIC_DEASSERT);
+       }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA,     PCI_DEVICE_ID_VIA_8237,         quirk_via_vt8237_bypass_apic_deassert);
+
+/*
  * The AMD io apic can hang the box when an apic irq is masked.
  * We check all revs >= B0 (yet not in the pre production!) as the bug
  * is currently marked NoFix
@@ -488,6 +507,47 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_VIA,    PCI_DEVICE_ID_VIA_82C686_4,     quirk_via_acpi );

 /*
+ * Devices part of the VIA VT8237 don't need quirk_via_irq().
+ * They also don't get confused by it, but dmesg gets quiter
+ * with this 'anti'-quirk.
+ * Here we are overly paranoic:
+ * we assume there might also exist via devices not part of the VT8237
+ * needing quirk_via_irq().
+ * This might never be the case in reality, when there is a VT8237.
+ */
+static unsigned int vt8237_devfn[] = {
+       PCI_DEVFN(15, 0),
+       PCI_DEVFN(15, 1),
+       PCI_DEVFN(16, 0),
+       PCI_DEVFN(16, 1),
+       PCI_DEVFN(16, 2),
+       PCI_DEVFN(16, 3),
+       PCI_DEVFN(16, 4),
+       PCI_DEVFN(16, 5),
+       PCI_DEVFN(17, 5),
+       PCI_DEVFN(17, 6),
+       PCI_DEVFN(18, 0)
+};
+static struct pci_dev *quirk_via_irq_not[ARRAY_SIZE(vt8237_devfn)];
+static void quirk_via_irq_not_for_8237(struct pci_dev *dev)
+{
+       // Make sure we do this only once
+       if (quirk_via_irq_not[0] != NULL)
+               return;
+
+       if (dev->devfn == PCI_DEVFN(0x11, 0)) {
+               int i, j;
+               for (i = 0, j = 0; i < ARRAY_SIZE(vt8237_devfn); i++) {
+                       struct pci_dev * d;
+                       d = pci_find_slot(dev->bus->number, vt8237_devfn[i]);
+                       if (d != NULL)
+                               quirk_via_irq_not[j++] = d;
+               }
+       }
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237, quirk_via_irq_not_for_8237);
+
+/*
  * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
  * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
  * when written, it makes an internal connection to the PIC.
@@ -499,8 +559,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
  */
 static void quirk_via_irq(struct pci_dev *dev)
 {
+       int i;
        u8 irq, new_irq;

+       for (i = 0; i < ARRAY_SIZE(vt8237_devfn); i++)
+               if (quirk_via_irq_not[i] == dev)
+                       return;
+
+
        new_irq = dev->irq & 0xf;
        pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
        if (new_irq != irq) {
_

	

	
		
___________________________________________________________ 
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Grant Coady]

On Sat, 13 Aug 2005 17:10:38 +0200, Karsten Wiese <annabellesgarden@yahoo.de> wrote:

>Hi,
>
>this fixes the 'doubled ioapic level interrupt rate' issue I've been
>seeing on a K8T800/AMD64 mainboard.
>It also switches off quirk_via_irq() for the VT8237 southbridge.

I'm tracking a dataloss on box with this chip, finding it difficult 
to nail a configuration that reliably produces dataloss, sometimes 
only one bit (e.g. 'c' --> 'C') of unpacking kernel source tree gets 
changed.

Relevant?  This is on a KM400 with Skt A Sempron + Seagate SATA HDD.
http://bugsplatter.mine.nu/test/linux-2.6/sempro/

Thanks,
Grant.

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Karsten Wiese]

Am Samstag, 13. August 2005 18:04 schrieb Grant Coady:
> 
> I'm tracking a dataloss on box with this chip, finding it difficult 
> to nail a configuration that reliably produces dataloss, sometimes 
> only one bit (e.g. 'c' --> 'C') of unpacking kernel source tree gets 
> changed.
> 
> Relevant?  This is on a KM400 with Skt A Sempron + Seagate SATA HDD.
> http://bugsplatter.mine.nu/test/linux-2.6/sempro/

Very unlikely. Rare bitwise errors like yours are more likely caused by 
i.e. broken IDE cable or too fast an IDE-controller setting for the
cable / drive. Or memory error. Or an itch on the mainboard. or++

Interrupt errors would cause errors blockwisely.
like whole sectors of data missing, device not working....

   Karsten  

	

	
		
___________________________________________________________ 
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Jeff Garzik]

Grant Coady wrote:
> I'm tracking a dataloss on box with this chip, finding it difficult 
> to nail a configuration that reliably produces dataloss, sometimes 
> only one bit (e.g. 'c' --> 'C') of unpacking kernel source tree gets 
> changed.


I've been watching this thread in the background.

Just to eliminate one possibility, I would definitely switch out SATA 
cables, which are notoriously crappy.

	Jeff

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Grant Coady]

On Sat, 13 Aug 2005 12:41:33 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:

>Grant Coady wrote:
>> I'm tracking a dataloss on box with this chip, finding it difficult 
>> to nail a configuration that reliably produces dataloss, sometimes 
>> only one bit (e.g. 'c' --> 'C') of unpacking kernel source tree gets 
>> changed.
...
>Just to eliminate one possibility, I would definitely switch out SATA 
>cables, which are notoriously crappy.

Since I have a spare new cable, done so.  But just now check SMART (in 
windows) and have zero UltraATA CRC Error rate (cable errors) for HDD.

Thanks,
Grant.

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Bjorn Helgaas]

On Saturday 13 August 2005 9:10 am, Karsten Wiese wrote:
> this fixes the 'doubled ioapic level interrupt rate' issue I've been
> seeing on a K8T800/AMD64 mainboard.
> It also switches off quirk_via_irq() for the VT8237 southbridge.

These patches seem unrelated except that they both contain the
text "via", so I think you should at least split them.

> + * Devices part of the VIA VT8237 don't need quirk_via_irq().
> + * They also don't get confused by it, but dmesg gets quiter
> + * with this 'anti'-quirk.
> + * Here we are overly paranoic:
> + * we assume there might also exist via devices not part of the VT8237
> + * needing quirk_via_irq().
> + * This might never be the case in reality, when there is a VT8237.

This quirk_via_irq() change seems like an awful lot of work to
get rid of a few log messages.  In my opinion, it's just not
worth it, because it's so hard to debug problems in that area
already.

> +static unsigned int vt8237_devfn[] = {
> +       PCI_DEVFN(15, 0),
> +       PCI_DEVFN(15, 1),
> +       PCI_DEVFN(16, 0),
> +       PCI_DEVFN(16, 1),
> +       PCI_DEVFN(16, 2),
> +       PCI_DEVFN(16, 3),
> +       PCI_DEVFN(16, 4),
> +       PCI_DEVFN(16, 5),
> +       PCI_DEVFN(17, 5),
> +       PCI_DEVFN(17, 6),
> +       PCI_DEVFN(18, 0)
> +};
> +static struct pci_dev *quirk_via_irq_not[ARRAY_SIZE(vt8237_devfn)];
> +static void quirk_via_irq_not_for_8237(struct pci_dev *dev)
> +{
> +       // Make sure we do this only once
> +       if (quirk_via_irq_not[0] != NULL)
> +               return;
> +
> +       if (dev->devfn == PCI_DEVFN(0x11, 0)) {
> +               int i, j;
> +               for (i = 0, j = 0; i < ARRAY_SIZE(vt8237_devfn); i++) {
> +                       struct pci_dev * d;
> +                       d = pci_find_slot(dev->bus->number, vt8237_devfn[i]);
> +                       if (d != NULL)
> +                               quirk_via_irq_not[j++] = d;
> +               }
> +       }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237, quirk_via_irq_not_for_8237);
> +
> +/*
>   * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
>   * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
>   * when written, it makes an internal connection to the PIC.
> @@ -499,8 +559,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
>   */
>  static void quirk_via_irq(struct pci_dev *dev)
>  {
> +       int i;
>         u8 irq, new_irq;
> 
> +       for (i = 0; i < ARRAY_SIZE(vt8237_devfn); i++)
> +               if (quirk_via_irq_not[i] == dev)
> +                       return;
> +
> +
>         new_irq = dev->irq & 0xf;
>         pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
>         if (new_irq != irq) {

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Karsten Wiese]

Am Dienstag, 16. August 2005 00:31 schrieb Bjorn Helgaas:
> These patches seem unrelated except that they both contain the
> text "via", so I think you should at least split them.

Will do.

> This quirk_via_irq() change seems like an awful lot of work to
> get rid of a few log messages.  In my opinion, it's just not
> worth it, because it's so hard to debug problems in that area
> already.

What about the following version?
quirk_via_686irq() only works on pci_devs that are part of the 686.
Sooner or later there'll be more VIA southbridge types, which will
not need quirk_via_irq() like the 8237.

------
/*
 * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
 * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
 * when written, it makes an internal connection to the PIC.
 * For these devices, this register is defined to be 4 bits wide.
 * Normally this is fine.  However for IO-APIC motherboards, or
 * non-x86 architectures (yes Via exists on PPC among other places),
 * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
 * interrupts delivered properly.
 */
static void quirk_via_686irq(struct pci_dev *dev)
{
	u8 irq, new_irq;
		/*
		 * The ISA bridge is used to identify the 686.
		 * It shares it's PCI slot and bus with the device under test
		 * and is assigned to function 0.
		 */
	unsigned int isa_bridge_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
	struct pci_dev * isa_bridge = pci_find_slot(dev->bus->number, isa_bridge_devfn);
		
	if (isa_bridge == NULL || isa_bridge->vendor != PCI_VENDOR_ID_VIA ||
	    isa_bridge->device != PCI_DEVICE_ID_VIA_82C686)
		return;		/* We are not on a 686. Lets leave */

	new_irq = dev->irq & 0xf;
	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
	if (new_irq != irq) {
		printk(KERN_INFO "PCI: Via IRQ fixup for %s, from %d to %d\n",
			pci_name(dev), irq, new_irq);
		udelay(15);	/* unknown if delay really needed */
		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, new_irq);
	} 
}
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_1, quirk_via_686irq);
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_2, quirk_via_686irq);
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_via_686irq);
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_5, quirk_via_686irq);
DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_6, quirk_via_686irq);
------

We could also add a "pci_dev * housed_in;" to "struct pci_dev"
to get rid of search done above under pci_find_slot().
To fill in "pci_dev * housed_in;", we'd need another quirk though.

Side note 1: IIRC via82xxx IDE code could also make use of such a "pci_dev * housed_in;"

Side note 2: The members
./include/linux/pci.h:542:	unsigned short vendor_compatible[DEVICE_COUNT_COMPATIBLE];
./include/linux/pci.h:543:	unsigned short device_compatible[DEVICE_COUNT_COMPATIBLE];
of "struct pci_dev" are unused in the current kernel and can vanish.
So a new "pci_dev * housed_in;" in "struct pci_dev" wouldn't add code in the sum.

   Karsten

	

	
		
___________________________________________________________ 
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Bjorn Helgaas]

On Tuesday 16 August 2005 9:26 am, Karsten Wiese wrote:
> What about the following version?
> quirk_via_686irq() only works on pci_devs that are part of the 686.
> Sooner or later there'll be more VIA southbridge types, which will
> not need quirk_via_irq() like the 8237.

Do you have VIA spec references to support this?  I had a quick
look, but couldn't find anything specific about which VIA devices
need the quirk and which don't.

As I understand it, you're not making a functional change here,
only getting rid of some messages in the log.  If that's true,
I'm hesitant about making more changes without information from
VIA.

> > ------ 
> /*
>  * Via 686A/B:  The PCI_INTERRUPT_LINE register for the on-chip
>  * devices, USB0/1, AC97, MC97, and ACPI, has an unusual feature:
>  * when written, it makes an internal connection to the PIC.
>  * For these devices, this register is defined to be 4 bits wide.
>  * Normally this is fine.  However for IO-APIC motherboards, or
>  * non-x86 architectures (yes Via exists on PPC among other places),
>  * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
>  * interrupts delivered properly.
>  */
> static void quirk_via_686irq(struct pci_dev *dev)
> {
> 	u8 irq, new_irq;
> 		/*
> 		 * The ISA bridge is used to identify the 686.
> 		 * It shares it's PCI slot and bus with the device under test
> 		 * and is assigned to function 0.
> 		 */
> 	unsigned int isa_bridge_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
> 	struct pci_dev * isa_bridge = pci_find_slot(dev->bus->number, isa_bridge_devfn);
> 		
> 	if (isa_bridge == NULL || isa_bridge->vendor != PCI_VENDOR_ID_VIA ||
> 	    isa_bridge->device != PCI_DEVICE_ID_VIA_82C686)
> 		return;		/* We are not on a 686. Lets leave */
> 
> 	new_irq = dev->irq & 0xf;
> 	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
> 	if (new_irq != irq) {
> 		printk(KERN_INFO "PCI: Via IRQ fixup for %s, from %d to %d\n",
> 			pci_name(dev), irq, new_irq);
> 		udelay(15);	/* unknown if delay really needed */
> 		pci_write_config_byte(dev, PCI_INTERRUPT_LINE, new_irq);
> 	} 
> }
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_1, quirk_via_686irq);
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C586_2, quirk_via_686irq);
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_4, quirk_via_686irq);
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_5, quirk_via_686irq);
> DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_82C686_6, quirk_via_686irq);

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Alan Cox]

On Maw, 2005-08-16 at 09:49 -0600, Bjorn Helgaas wrote:
> On Tuesday 16 August 2005 9:26 am, Karsten Wiese wrote:
> > What about the following version?
> > quirk_via_686irq() only works on pci_devs that are part of the 686.
> > Sooner or later there'll be more VIA southbridge types, which will
> > not need quirk_via_irq() like the 8237.
> 
> Do you have VIA spec references to support this?  I had a quick
> look, but couldn't find anything specific about which VIA devices
> need the quirk and which don't.

PCI interrupt line routing is used for all V-Bus devices. Thats all
devices in any way on an internal bus of the north or south bridge. The
quirk is harmless when applied to other devices.

Note the description of the quirk is still wrong


> >  * For these devices, this register is defined to be 4 bits wide.
> >  * Normally this is fine.  However for IO-APIC motherboards, or
> >  * non-x86 architectures (yes Via exists on PPC among other places),
> >  * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
> >  * interrupts delivered properly.

The interrupt line field is used to indicate the ISA type IRQ or the
APIC pin used for the IRQ delivery. See the data sheet.

The & 0x0F is the old hack used to get the ISA type IRQ line in use from
the IRQ number. 

Alan

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Karsten Wiese]

Am Dienstag, 16. August 2005 19:20 schrieb Alan Cox:
> 
> PCI interrupt line routing is used for all V-Bus devices. Thats all
> devices in any way on an internal bus of the north or south bridge. The
> quirk is harmless when applied to other devices.
> 
> Note the description of the quirk is still wrong
> 
> 
> > >  * For these devices, this register is defined to be 4 bits wide.
> > >  * Normally this is fine.  However for IO-APIC motherboards, or
> > >  * non-x86 architectures (yes Via exists on PPC among other places),
> > >  * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
> > >  * interrupts delivered properly.
> 
> The interrupt line field is used to indicate the ISA type IRQ or the
> APIC pin used for the IRQ delivery. See the data sheet.
> 
> The & 0x0F is the old hack used to get the ISA type IRQ line in use from
> the IRQ number. 
> 

Thanks, Bjorn and Alan!
After thinking again here is a 3rd version to change quirk_via_irq().

Motivation is to cleanup the IOAPIC-case:
Currently as of 2.6.13-rc6 new_irq is wrong then,
as dev->irq isn't the PIN number.

Solutions: either calculate correct new_irq (= PIN-Number & 0x0F)
 or don't apply likely wrong value.

Following diff takes the 2nd way.

Well, VT8237 ignores the wrong new_irq in IOAPIC-Mode,
but its irritating to see dmesg print out nonsense then. 

------
--- linux-2.6.13-rc6/drivers/pci/quirks.c	2005-08-08 11:46:05.000000000 +0200
+++ linux-2.6.13/drivers/pci/quirks.c	2005-08-18 11:55:50.000000000 +0200
@@ -497,9 +516,27 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_V
  * we must mask the PCI_INTERRUPT_LINE value versus 0xf to get
  * interrupts delivered properly.
  */
+#ifdef CONFIG_X86_IO_APIC
+	extern struct hw_interrupt_type ioapic_edge_type;
+	extern struct hw_interrupt_type ioapic_level_type;
+#include <linux/irq.h>
+#endif
 static void quirk_via_irq(struct pci_dev *dev)
 {
 	u8 irq, new_irq;
+#ifdef CONFIG_X86_IO_APIC
+	{
+		irq_desc_t *desc = irq_desc + dev->irq;
+		if (desc &&
+		    (desc->handler == &ioapic_edge_type ||
+		     desc->handler == &ioapic_level_type)
+			)
+			return;	/* We cannot guess the right
+				 * new_irq with the simple hack below.
+				 * lets just leave.
+				 */
+	}
+#endif
 
 	new_irq = dev->irq & 0xf;
 	pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &irq);
------

What do you think?

   Karsten

	

	
		
___________________________________________________________ 
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de

Re: [PATCH,RFC] quirks for VIA VT8237 southbridge

[Alan Cox]

On Iau, 2005-08-18 at 14:36 +0200, Karsten Wiese wrote:
> Solutions: either calculate correct new_irq (= PIN-Number & 0x0F)
>  or don't apply likely wrong value.
> 
> Following diff takes the 2nd way.
> 
> Well, VT8237 ignores the wrong new_irq in IOAPIC-Mode,
> but its irritating to see dmesg print out nonsense then. 

The docs and my poking around with a later board seem to imply you need
to set the IRQ value > 15 to get it to the IO-APIC. The data sheet
doesn't seem clear if you need to set it all up by hand or if ACPI does
it.