| 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
| ||
|
Message-ID: <alpine.LFD.1.10.0806031213360.3235@apollo.tec.linutronix.de>
Date: Tue, 3 Jun 2008 12:37:58 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Olaf Dabrunz <od@...e.de>
cc: Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
Jon Masters <jonathan@...masters.org>,
LKML <linux-kernel@...r.kernel.org>,
Stefan Assmann <sassmann@...e.de>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Jesse Barnes <jbarnes@...tuousgeek.org>
Subject: Re: [PATCH 2/7] reroute PCI interrupt to legacy boot interrupt
equivalent
On Mon, 2 Jun 2008, Olaf Dabrunz wrote:
> From: Stefan Assmann <sassmann@...e.de>
>
> Some chipsets (e.g. intel 6700PXH) generate a legacy INTx when the
> IRQ entry in the chipset's IO-APIC is masked (as, e.g. the RT kernel
> does during interrupt handling). On chipsets where this INTx generation
> cannot be disabled, we reroute the valid interrupts to their legacy
> equivalent to get rid of spurious interrupts that might otherwise bring
> down (vital) interrupt lines through spurious interrupt detection in
> note_interrupt().
>
> The patch should eventually simply use another bitfield in the pci_dev
> structure for the rerouting variant. This was in another variant of the patch
> by Daniel Gollub. It is the cleanest and most simple approach.
>
> This patch benefited from discussions with Alexander Graf, Torsten Duwe,
> Ihno Krumreich, Daniel Gollub, Hannes Reinecke. The conclusions we drew
> and the patch itself are the authors' responsibility alone.
>
> Signed-off-by: Stefan Assmann <sassmann@...e.de>
> Signed-off-by: Olaf Dabrunz <od@...e.de>
> ---
> drivers/acpi/pci_irq.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/quirks.c | 51 ++++++++++++++++++++++++++++++++++-
> include/linux/pci.h | 2 +
> include/linux/pci_ids.h | 4 +++
> include/linux/pci_quirks.h | 28 +++++++++++++++++++
> 5 files changed, 147 insertions(+), 1 deletions(-)
> create mode 100644 include/linux/pci_quirks.h
>
> diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
> index 89022a7..ebdfbfe 100644
> --- a/drivers/acpi/pci_irq.c
> +++ b/drivers/acpi/pci_irq.c
> @@ -384,6 +384,34 @@ acpi_pci_free_irq(struct acpi_prt_entry *entry,
> return irq;
> }
>
> +#ifdef CONFIG_X86_IO_APIC
> +static int
> +bridge_has_boot_interrupt_variant(struct pci_bus *bus)
one line
> +{
> + struct pci_bus *bus_it;
> + int i;
> +
> + for (bus_it = bus ; bus_it ; bus_it = bus_it->parent) {
> + if (!bus_it->self)
> + return 0;
> +
> + printk(KERN_INFO "vendor=%04x device=%04x\n", bus_it->self->vendor,
> + bus_it->self->device);
> + for (i = 0; i < MAX_QUIRK_BOOTIRQ_REROUTE_DEVS; i++) {
> + if (quirk_bootirq_reroute_devs[i].vendor == 0)
> + return 0;
> +
> + if (quirk_bootirq_reroute_devs[i].vendor == bus_it->self->vendor &&
> + quirk_bootirq_reroute_devs[i].device == bus_it->self->device)
> + return quirk_bootirq_reroute_devs[i].quirk_variant;
> + }
> + }
> + return 0;
> +}
> +
> +extern int reroute_to_boot_interrupts;
declarations need to be in a header file.
> +#endif /* CONFIG_X86_IO_APIC */
> +
> /*
> * acpi_pci_irq_lookup
> * success: return IRQ >= 0
> @@ -413,6 +441,41 @@ acpi_pci_irq_lookup(struct pci_bus *bus,
> }
>
> ret = func(entry, triggering, polarity, link);
> +
> +#ifdef CONFIG_X86_IO_APIC
> + /*
> + * Some chipsets (e.g. intel 6700PXH) generate a legacy INTx when the
> + * IRQ entry in the chipset's IO-APIC is masked (as, e.g. the RT kernel
> + * does during interrupt handling). When this INTx generation cannot be
> + * disabled, we reroute these interrupts to their legacy equivalent to
> + * get rid of spurious interrupts.
> + */
> + if (reroute_to_boot_interrupts) {
> + switch (bridge_has_boot_interrupt_variant(bus)) {
> + case 0:
> + /* no rerouting necessary */
> + break;
> +
> + case VARIANT_INTEL_BUS_INT_REMAPPING:
> + /*
> + * Remap according to INTx routing table in 6700PXH
> + * specs, intel order number 302628-002, section
> + * 2.15.2. Other chipsets (80332, ...) have the same
> + * mapping and are handled here as well.
> + */
They generate INTA,B,C,D messages. What makes sure, that those are
always routed to IRQ 16,17,18,19 ?
IIRC we have seen those spurious interrupts on IRQ 9 and 12 as well.
> + printk(KERN_INFO "pci irq %d -> rerouted to legacy "
> + "irq %d\n", ret, (ret % 4) + 16);
> + ret = (ret % 4) + 16;
> + break;
> +
> + default:
> + printk(KERN_INFO "not rerouting irq %d to legacy irq: "
> + "unknown mapping\n", ret);
> + break;
> + }
> + }
> +#endif /* CONFIG_X86_IO_APIC */
> +
> return ret;
> }
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6245486..6173be5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1375,8 +1375,57 @@ int nobootirqquirk_setup(char *str)
> return 1;
> }
> __setup("nobootirqquirk", nobootirqquirk_setup);
> -#endif /* CONFIG_X86_IO_APIC */
>
> +/*
> + * Boot interrupts on some chipsets cannot be turned off. For these chipsets,
> + * remap the original interrupt in the linux kernel to the boot interrupt, so
> + * that a PCI device's interrupt handler is installed on the boot interrupt
> + * line instead.
> + */
> +int reroute_to_boot_interrupts;
> +EXPORT_SYMBOL(reroute_to_boot_interrupts);
> +
> +struct quirk_bootirq_reroute_dev
> + quirk_bootirq_reroute_devs[MAX_QUIRK_BOOTIRQ_REROUTE_DEVS];
> +EXPORT_SYMBOL(quirk_bootirq_reroute_devs);
Please use a consistent name space. This is about ioapic quirks so the
whole stuff should use ioapic_quirk_XXX or something like this.
Also why dont we put this information into the pci_dev structure
instead of having these tables and exports ?
> +static void quirk_reroute_to_boot_interrupts_intel(struct pci_dev *dev)
> +{
> + int i;
> +
> + if (nobootirqquirk)
> + return;
Default should be off.
> + for (i = 0; i < MAX_QUIRK_BOOTIRQ_REROUTE_DEVS; i++) {
> + if (quirk_bootirq_reroute_devs[i].vendor == 0)
> + break;
> +
> + if (quirk_bootirq_reroute_devs[i].vendor == dev->vendor &&
> + quirk_bootirq_reroute_devs[i].device == dev->device)
> + return; /* already on list */
> + }
> +
> + quirk_bootirq_reroute_devs[i].vendor = dev->vendor;
> + quirk_bootirq_reroute_devs[i].device = dev->device;
> +
> + quirk_bootirq_reroute_devs[i].quirk_variant =
> + VARIANT_INTEL_BUS_INT_REMAPPING;
> +
> + reroute_to_boot_interrupts = 1;
> +
> + printk(KERN_INFO "PCI quirk: reroute interrupts for 0x%04x:0x%04x\n",
> + dev->vendor, dev->device);
> + return;
> +}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d18b1dd..6c46cad 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1070,5 +1070,7 @@ static inline void pci_mmcfg_early_init(void) { }
> static inline void pci_mmcfg_late_init(void) { }
> #endif
>
> +#include <linux/pci_quirks.h>
> +
> #endif /* __KERNEL__ */
> #endif /* LINUX_PCI_H */
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 9b940e6..c675399 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2233,6 +2233,10 @@
> #define PCI_DEVICE_ID_INTEL_PXH_0 0x0329
> #define PCI_DEVICE_ID_INTEL_PXH_1 0x032A
> #define PCI_DEVICE_ID_INTEL_PXHV 0x032C
> +#define PCI_DEVICE_ID_INTEL_80332_0 0x0330
> +#define PCI_DEVICE_ID_INTEL_80332_1 0x0332
> +#define PCI_DEVICE_ID_INTEL_80333_0 0x0370
> +#define PCI_DEVICE_ID_INTEL_80333_1 0x0372
> #define PCI_DEVICE_ID_INTEL_82375 0x0482
> #define PCI_DEVICE_ID_INTEL_82424 0x0483
> #define PCI_DEVICE_ID_INTEL_82378 0x0484
Please move the PCI ids out into a separate patch instead of adding
them gradually.
> diff --git a/include/linux/pci_quirks.h b/include/linux/pci_quirks.h
> new file mode 100644
Do we really need a separate header for this ?
> index 0000000..d875b87
> --- /dev/null
> +++ b/include/linux/pci_quirks.h
> @@ -0,0 +1,28 @@
> +/*
> + * pci_quirks.h
> + *
> + * PCI quirks defines
> + * Copyright 2008, Olaf Dabrunz
> + */
> +
> +#ifndef LINUX_PCI_QUIRKS_H
> +#define LINUX_PCI_QUIRKS_H
> +
> +#ifdef CONFIG_X86_IO_APIC
> +#define MAX_QUIRK_BOOTIRQ_REROUTE_VARIANTS 32
> +#define VARIANT_INTEL_BUS_INT_REMAPPING 1
Aside of the horrible names, if there are multiple variants, please
use an enum.
> +
> +struct quirk_bootirq_reroute_dev {
> + unsigned short vendor;
> + unsigned short device;
> + unsigned short quirk_variant;
> +};
> +
> +/* max number of different quirky devices */
> +#define MAX_QUIRK_BOOTIRQ_REROUTE_DEVS 32
> +
> +extern struct quirk_bootirq_reroute_dev
> + quirk_bootirq_reroute_devs[];
> +#endif /* CONFIG_X86_IO_APIC */
> +
> +#endif /* LINUX_PCI_QUIRKS_H */
> --
> 1.5.2.4
>
> --
> Olaf Dabrunz (od/odabrunz), SUSE Linux Products GmbH, N??rnberg
>
--
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/
Powered by blists - more mailing lists