diff mbox

irq: add quirk for broken interrupt remapping on 55XX chipsets

Message ID 1362158276-4901-1-git-send-email-nhorman@tuxdriver.com
State Superseded
Headers show

Commit Message

Neil Horman March 1, 2013, 5:17 p.m. UTC
A few years back intel published a spec update:
http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf

For the 5520 and 5500 chipsets which contained an errata (specificially errata
53), which noted that these chipsets can't properly do interrupt remapping, and
as a result the recommend that interrupt remapping be disabled in bios.  While
many vendors have a bios update to do exactly that, not all do, and of course
not all users update their bios to a level that corrects the problem.  As a
result, occasionally interrupts can arrive at a cpu even after affinity for that
interrupt has be moved, leading to lost or spurrious interrupts (usually
characterized by the message:
kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)

There have been several incidents recently of people seeing this error, and
investigation has shown that they have system for which their BIOS level is such
that this feature was not properly turned off.  As such, it would be good to
give them a reminder that their systems are vulnurable to this problem.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: Don Zickus <dzickus@redhat.com>
CC: Don Dutile <ddutile@redhat.com>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Asit Mallick <asit.k.mallick@intel.com>
CC: linux-pci@vger.kernel.org
---
 drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
 include/linux/pci_ids.h             |  2 ++
 2 files changed, 22 insertions(+)

Comments

Yinghai Lu March 1, 2013, 6:20 p.m. UTC | #1
On Fri, Mar 1, 2013 at 9:17 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> A few years back intel published a spec update:
> http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf
>
> For the 5520 and 5500 chipsets which contained an errata (specificially errata
> 53), which noted that these chipsets can't properly do interrupt remapping, and
> as a result the recommend that interrupt remapping be disabled in bios.  While
> many vendors have a bios update to do exactly that, not all do, and of course
> not all users update their bios to a level that corrects the problem.  As a
> result, occasionally interrupts can arrive at a cpu even after affinity for that
> interrupt has be moved, leading to lost or spurrious interrupts (usually
> characterized by the message:
> kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)
>
> There have been several incidents recently of people seeing this error, and
> investigation has shown that they have system for which their BIOS level is such
> that this feature was not properly turned off.  As such, it would be good to
> give them a reminder that their systems are vulnurable to this problem.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: Don Zickus <dzickus@redhat.com>
> CC: Don Dutile <ddutile@redhat.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Asit Mallick <asit.k.mallick@intel.com>
> CC: linux-pci@vger.kernel.org
> ---
>  drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
>  include/linux/pci_ids.h             |  2 ++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index f3b8f23..9bfb6c2 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -1113,3 +1113,23 @@ struct irq_remap_ops intel_irq_remap_ops = {
>         .msi_setup_irq          = intel_msi_setup_irq,
>         .setup_hpet_msi         = intel_setup_hpet_msi,
>  };
> +
> +
> +static void intel_remapping_check(struct pci_dev *dev)
> +{
> +       u8 revision;
> +
> +       pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
> +
> +       if ((revision == 0x13) && irq_remapping_enabled) {
> +               pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
> +                       "This system BIOS has enabled interrupt remapping\n"
> +                       "on a chipset that contains an errata making that\n"
> +                       "feature unstable.  Please reboot with nointremap\n"
> +                       "added to the kernel command line and contact\n"
> +                       "your BIOS vendor for an update");
> +       }
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5520_IOHUB, intel_remapping_check);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5500_IOHUB, intel_remapping_check);

only for x86 platform?
If so, you can check that in arch/x86/kernel/early-quirks.c::early_quirks()
and set one flag and later print warning and skip there if someone
need to enable intr-remap.
So users will not need to reboot the system...

Thanks

Yinghai

> +
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 31717bd..54027a6 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2732,6 +2732,8 @@
>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_RANK_REV2  0x2db2
>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_TC_REV2    0x2db3
>  #define PCI_DEVICE_ID_INTEL_82855PM_HB 0x3340
> +#define PCI_DEVICE_ID_INTEL_5500_IOHUB 0x3403
> +#define PCI_DEVICE_ID_INTEL_5520_IOHUB 0x3406
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG4  0x3429
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG5  0x342a
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG6  0x342b
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Neil Horman March 1, 2013, 7:29 p.m. UTC | #2
On Fri, Mar 01, 2013 at 10:20:35AM -0800, Yinghai Lu wrote:
> On Fri, Mar 1, 2013 at 9:17 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > A few years back intel published a spec update:
> > http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf
> >
> > For the 5520 and 5500 chipsets which contained an errata (specificially errata
> > 53), which noted that these chipsets can't properly do interrupt remapping, and
> > as a result the recommend that interrupt remapping be disabled in bios.  While
> > many vendors have a bios update to do exactly that, not all do, and of course
> > not all users update their bios to a level that corrects the problem.  As a
> > result, occasionally interrupts can arrive at a cpu even after affinity for that
> > interrupt has be moved, leading to lost or spurrious interrupts (usually
> > characterized by the message:
> > kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)
> >
> > There have been several incidents recently of people seeing this error, and
> > investigation has shown that they have system for which their BIOS level is such
> > that this feature was not properly turned off.  As such, it would be good to
> > give them a reminder that their systems are vulnurable to this problem.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Prarit Bhargava <prarit@redhat.com>
> > CC: Don Zickus <dzickus@redhat.com>
> > CC: Don Dutile <ddutile@redhat.com>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Asit Mallick <asit.k.mallick@intel.com>
> > CC: linux-pci@vger.kernel.org
> > ---
> >  drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
> >  include/linux/pci_ids.h             |  2 ++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> > index f3b8f23..9bfb6c2 100644
> > --- a/drivers/iommu/intel_irq_remapping.c
> > +++ b/drivers/iommu/intel_irq_remapping.c
> > @@ -1113,3 +1113,23 @@ struct irq_remap_ops intel_irq_remap_ops = {
> >         .msi_setup_irq          = intel_msi_setup_irq,
> >         .setup_hpet_msi         = intel_setup_hpet_msi,
> >  };
> > +
> > +
> > +static void intel_remapping_check(struct pci_dev *dev)
> > +{
> > +       u8 revision;
> > +
> > +       pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
> > +
> > +       if ((revision == 0x13) && irq_remapping_enabled) {
> > +               pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
> > +                       "This system BIOS has enabled interrupt remapping\n"
> > +                       "on a chipset that contains an errata making that\n"
> > +                       "feature unstable.  Please reboot with nointremap\n"
> > +                       "added to the kernel command line and contact\n"
> > +                       "your BIOS vendor for an update");
> > +       }
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5520_IOHUB, intel_remapping_check);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5500_IOHUB, intel_remapping_check);
> 
> only for x86 platform?
> If so, you can check that in arch/x86/kernel/early-quirks.c::early_quirks()
> and set one flag and later print warning and skip there if someone
> need to enable intr-remap.
> So users will not need to reboot the system...
> 
> Thanks
> 
I was under the impression that BIOS might have enabled irq remapping prior to
the OS booting, at which point a reboot was requried anyway.

Neil

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiang Liu March 2, 2013, 2:28 a.m. UTC | #3
On 03/02/2013 02:20 AM, Yinghai Lu wrote:
> On Fri, Mar 1, 2013 at 9:17 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>> A few years back intel published a spec update:
>> http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf
>>
>> For the 5520 and 5500 chipsets which contained an errata (specificially errata
>> 53), which noted that these chipsets can't properly do interrupt remapping, and
>> as a result the recommend that interrupt remapping be disabled in bios.  While
>> many vendors have a bios update to do exactly that, not all do, and of course
>> not all users update their bios to a level that corrects the problem.  As a
>> result, occasionally interrupts can arrive at a cpu even after affinity for that
>> interrupt has be moved, leading to lost or spurrious interrupts (usually
>> characterized by the message:
>> kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)
>>
>> There have been several incidents recently of people seeing this error, and
>> investigation has shown that they have system for which their BIOS level is such
>> that this feature was not properly turned off.  As such, it would be good to
>> give them a reminder that their systems are vulnurable to this problem.
>>
>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>> CC: Prarit Bhargava <prarit@redhat.com>
>> CC: Don Zickus <dzickus@redhat.com>
>> CC: Don Dutile <ddutile@redhat.com>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Asit Mallick <asit.k.mallick@intel.com>
>> CC: linux-pci@vger.kernel.org
>> ---
>>  drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
>>  include/linux/pci_ids.h             |  2 ++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
>> index f3b8f23..9bfb6c2 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -1113,3 +1113,23 @@ struct irq_remap_ops intel_irq_remap_ops = {
>>         .msi_setup_irq          = intel_msi_setup_irq,
>>         .setup_hpet_msi         = intel_setup_hpet_msi,
>>  };
>> +
>> +
>> +static void intel_remapping_check(struct pci_dev *dev)
>> +{
>> +       u8 revision;
>> +
>> +       pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
>> +
>> +       if ((revision == 0x13) && irq_remapping_enabled) {
>> +               pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
>> +                       "This system BIOS has enabled interrupt remapping\n"
>> +                       "on a chipset that contains an errata making that\n"
>> +                       "feature unstable.  Please reboot with nointremap\n"
>> +                       "added to the kernel command line and contact\n"
>> +                       "your BIOS vendor for an update");
>> +       }
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5520_IOHUB, intel_remapping_check);
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5500_IOHUB, intel_remapping_check);
> 
> only for x86 platform?
> If so, you can check that in arch/x86/kernel/early-quirks.c::early_quirks()
> and set one flag and later print warning and skip there if someone
> need to enable intr-remap.
> So users will not need to reboot the system...
We have just struggled with this issue when doing kvm restarting stress tests,
and finally found it's a chipset errata. Thanks for fix it.
And I think 5520/5500 is for x86 only, so could move it to x86 arch subdirectory.

Regards!
Gerry

> 
> Thanks
> 
> Yinghai
> 
>> +
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 31717bd..54027a6 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -2732,6 +2732,8 @@
>>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_RANK_REV2  0x2db2
>>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_TC_REV2    0x2db3
>>  #define PCI_DEVICE_ID_INTEL_82855PM_HB 0x3340
>> +#define PCI_DEVICE_ID_INTEL_5500_IOHUB 0x3403
>> +#define PCI_DEVICE_ID_INTEL_5520_IOHUB 0x3406
>>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG4  0x3429
>>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG5  0x342a
>>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG6  0x342b
>> --
>> 1.7.11.7
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Mohr March 2, 2013, 3:59 p.m. UTC | #4
Hi,

> if ((revision == 0x13) && irq_remapping_enabled) {
> +		pr_warn("WARNING WARNING WARNING WARNING WARNING
> WARNING\n"
> +			"This system BIOS has enabled interrupt
> remapping\n"
> +			"on a chipset that contains an errata making
> that\n"
> +			"feature unstable.  Please reboot with
> nointremap\n"
> +			"added to the kernel command line and contact\n"
> +			"your BIOS vendor for an update");
> +	}

Forgive me, but ISTR that there's a special BIOS firmware quirk bug annotating
logger warning message mechanism (have I managed to hit all keywords yet? ;)
in the kernel which might be useful in this case.


OK, found something (but I don't think it was the mechanism
that ISTR - perhaps it got modernized?):


include/linux/printk.h:

/*
 * FW_BUG
 * Add this to a message where you are sure the firmware is buggy or
 * behaves
 * really stupid or out of spec. Be aware that the responsible BIOS
 * developer
 * should be able to fix this issue or at least get a concrete idea of
 * the
 * problem by reading your message without the need of looking at the
 * kernel
 * code.
 *
 * Use it for definite and high priority BIOS bugs.
 *
 * FW_WARN
 * Use it for not that clear (e.g. could the kernel messed up things
 * already?)
 * and medium priority BIOS bugs.
 *
 * FW_INFO
 * Use this one if you want to tell the user or vendor about something
 * suspicious, but generally harmless related to the firmware.
 *
 * Use it for information or very low priority BIOS bugs.
 */


So perhaps it is appropriate to be used here.


HTH,

Andreas Mohr
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Prarit Bhargava March 2, 2013, 4:21 p.m. UTC | #5
On 03/01/2013 12:17 PM, Neil Horman wrote:
> A few years back intel published a spec update:
> http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf
> 
> For the 5520 and 5500 chipsets which contained an errata (specificially errata
> 53), which noted that these chipsets can't properly do interrupt remapping, and
> as a result the recommend that interrupt remapping be disabled in bios.  While
> many vendors have a bios update to do exactly that, not all do, and of course
> not all users update their bios to a level that corrects the problem.  As a
> result, occasionally interrupts can arrive at a cpu even after affinity for that
> interrupt has be moved, leading to lost or spurrious interrupts (usually
> characterized by the message:
> kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)
> 
> There have been several incidents recently of people seeing this error, and
> investigation has shown that they have system for which their BIOS level is such
> that this feature was not properly turned off.  As such, it would be good to
> give them a reminder that their systems are vulnurable to this problem.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: Don Zickus <dzickus@redhat.com>
> CC: Don Dutile <ddutile@redhat.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Asit Mallick <asit.k.mallick@intel.com>
> CC: linux-pci@vger.kernel.org
> ---
>  drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
>  include/linux/pci_ids.h             |  2 ++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index f3b8f23..9bfb6c2 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -1113,3 +1113,23 @@ struct irq_remap_ops intel_irq_remap_ops = {
>  	.msi_setup_irq		= intel_msi_setup_irq,
>  	.setup_hpet_msi		= intel_setup_hpet_msi,
>  };
> +
> +
> +static void intel_remapping_check(struct pci_dev *dev)
> +{
> +	u8 revision;
> +
> +	pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
> +
> +	if ((revision == 0x13) && irq_remapping_enabled) {
> +		pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
> +			"This system BIOS has enabled interrupt remapping\n"
> +			"on a chipset that contains an errata making that\n"
> +			"feature unstable.  Please reboot with nointremap\n"
> +			"added to the kernel command line and contact\n"
> +			"your BIOS vendor for an update");

Make this one line?  Might be too long but I believe the preferred policy is now
to keep the output on one line so that it is easy to find in the kernel source.

Also, IMO, remove the WARNING WARNING stuff.

You also should probably use HW_ERR here too.

P.

> +	}
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5520_IOHUB, intel_remapping_check);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5500_IOHUB, intel_remapping_check);
> +
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 31717bd..54027a6 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2732,6 +2732,8 @@
>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_RANK_REV2  0x2db2
>  #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_TC_REV2    0x2db3
>  #define PCI_DEVICE_ID_INTEL_82855PM_HB	0x3340
> +#define PCI_DEVICE_ID_INTEL_5500_IOHUB	0x3403
> +#define PCI_DEVICE_ID_INTEL_5520_IOHUB	0x3406
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG4	0x3429
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG5	0x342a
>  #define PCI_DEVICE_ID_INTEL_IOAT_TBG6	0x342b

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Neil Horman March 2, 2013, 8:13 p.m. UTC | #6
On Sat, Mar 02, 2013 at 11:21:29AM -0500, Prarit Bhargava wrote:
> On 03/01/2013 12:17 PM, Neil Horman wrote:
> > A few years back intel published a spec update:
> > http://www.intel.com/content/dam/doc/specification-update/5520-and-5500-chipset-ioh-specification-update.pdf
> > 
> > For the 5520 and 5500 chipsets which contained an errata (specificially errata
> > 53), which noted that these chipsets can't properly do interrupt remapping, and
> > as a result the recommend that interrupt remapping be disabled in bios.  While
> > many vendors have a bios update to do exactly that, not all do, and of course
> > not all users update their bios to a level that corrects the problem.  As a
> > result, occasionally interrupts can arrive at a cpu even after affinity for that
> > interrupt has be moved, leading to lost or spurrious interrupts (usually
> > characterized by the message:
> > kernel: do_IRQ: 7.71 No irq handler for vector (irq -1)
> > 
> > There have been several incidents recently of people seeing this error, and
> > investigation has shown that they have system for which their BIOS level is such
> > that this feature was not properly turned off.  As such, it would be good to
> > give them a reminder that their systems are vulnurable to this problem.
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Prarit Bhargava <prarit@redhat.com>
> > CC: Don Zickus <dzickus@redhat.com>
> > CC: Don Dutile <ddutile@redhat.com>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Asit Mallick <asit.k.mallick@intel.com>
> > CC: linux-pci@vger.kernel.org
> > ---
> >  drivers/iommu/intel_irq_remapping.c | 20 ++++++++++++++++++++
> >  include/linux/pci_ids.h             |  2 ++
> >  2 files changed, 22 insertions(+)
> > 
> > diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> > index f3b8f23..9bfb6c2 100644
> > --- a/drivers/iommu/intel_irq_remapping.c
> > +++ b/drivers/iommu/intel_irq_remapping.c
> > @@ -1113,3 +1113,23 @@ struct irq_remap_ops intel_irq_remap_ops = {
> >  	.msi_setup_irq		= intel_msi_setup_irq,
> >  	.setup_hpet_msi		= intel_setup_hpet_msi,
> >  };
> > +
> > +
> > +static void intel_remapping_check(struct pci_dev *dev)
> > +{
> > +	u8 revision;
> > +
> > +	pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
> > +
> > +	if ((revision == 0x13) && irq_remapping_enabled) {
> > +		pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
> > +			"This system BIOS has enabled interrupt remapping\n"
> > +			"on a chipset that contains an errata making that\n"
> > +			"feature unstable.  Please reboot with nointremap\n"
> > +			"added to the kernel command line and contact\n"
> > +			"your BIOS vendor for an update");
> 
> Make this one line?  Might be too long but I believe the preferred policy is now
> to keep the output on one line so that it is easy to find in the kernel source.
> 
> Also, IMO, remove the WARNING WARNING stuff.
> 
> You also should probably use HW_ERR here too.
> 
> P.
> 
I can do that, I'll post version 2 on monday.
Neil

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Don Dutile March 4, 2013, 1:24 p.m. UTC | #7
On 03/02/2013 10:59 AM, Andreas Mohr wrote:
> Hi,
>
>> if ((revision == 0x13)&&  irq_remapping_enabled) {
>> +		pr_warn("WARNING WARNING WARNING WARNING WARNING
>> WARNING\n"
>> +			"This system BIOS has enabled interrupt
>> remapping\n"
>> +			"on a chipset that contains an errata making
>> that\n"
>> +			"feature unstable.  Please reboot with
>> nointremap\n"
>> +			"added to the kernel command line and contact\n"
>> +			"your BIOS vendor for an update");
>> +	}
>
> Forgive me, but ISTR that there's a special BIOS firmware quirk bug annotating
> logger warning message mechanism (have I managed to hit all keywords yet? ;)
> in the kernel which might be useful in this case.
>
>
> OK, found something (but I don't think it was the mechanism
> that ISTR - perhaps it got modernized?):
>
>
> include/linux/printk.h:
>
> /*
>   * FW_BUG
>   * Add this to a message where you are sure the firmware is buggy or
>   * behaves
>   * really stupid or out of spec. Be aware that the responsible BIOS
>   * developer
>   * should be able to fix this issue or at least get a concrete idea of
>   * the
>   * problem by reading your message without the need of looking at the
>   * kernel
>   * code.
>   *
>   * Use it for definite and high priority BIOS bugs.
>   *
>   * FW_WARN
>   * Use it for not that clear (e.g. could the kernel messed up things
>   * already?)
>   * and medium priority BIOS bugs.
>   *
>   * FW_INFO
>   * Use this one if you want to tell the user or vendor about something
>   * suspicious, but generally harmless related to the firmware.
>   *
>   * Use it for information or very low priority BIOS bugs.
>   */
>

It is not a firmware/BIOS bug. Prarit's comment to annotate it as
a HW_ERR is more accurate.  A software patch is being tested now
to see if it can do set-affinity in a manner that avoids this race
and enables IR to stay on for all these systems.  It requires
more testing to ensure the logic is valid.  This patch was
recommended as a necessary short-term fix, and to highlight to
others this possible state -- which Gerry mentioned he had.

Note: the race condition is a very small window, and could not
        be duplicated on upstream kernel on known-failing-hw b/c
        the interrupt rate dropped by 80%.

The errata's recommendation (to disable IR in the BIOS) makes
the option of a workaround impossible, since it will hide the
IR hw (no DMAR table for it).  The BIOS 'update' may be avoided,
and corrected, if testing yields positive results, and after
posting, others don't see a flaw in the logic that was missed
by other reviewers.

>
> So perhaps it is appropriate to be used here.
>
>
> HTH,
>
> Andreas Mohr
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Prarit Bhargava March 10, 2013, 1:11 a.m. UTC | #8
On 03/04/2013 08:24 AM, Don Dutile wrote:
> On 03/02/2013 10:59 AM, Andreas Mohr wrote:
>> Hi,
>>
>>> if ((revision == 0x13)&&  irq_remapping_enabled) {
>>> +        pr_warn("WARNING WARNING WARNING WARNING WARNING
>>> WARNING\n"
>>> +            "This system BIOS has enabled interrupt
>>> remapping\n"
>>> +            "on a chipset that contains an errata making
>>> that\n"
>>> +            "feature unstable.  Please reboot with
>>> nointremap\n"
>>> +            "added to the kernel command line and contact\n"
>>> +            "your BIOS vendor for an update");
>>> +    }
>>
>> Forgive me, but ISTR that there's a special BIOS firmware quirk bug annotating
>> logger warning message mechanism (have I managed to hit all keywords yet? ;)
>> in the kernel which might be useful in this case.
>>
>>
>> OK, found something (but I don't think it was the mechanism
>> that ISTR - perhaps it got modernized?):
>>
>>
>> include/linux/printk.h:
>>
>> /*
>>   * FW_BUG
>>   * Add this to a message where you are sure the firmware is buggy or
>>   * behaves
>>   * really stupid or out of spec. Be aware that the responsible BIOS
>>   * developer
>>   * should be able to fix this issue or at least get a concrete idea of
>>   * the
>>   * problem by reading your message without the need of looking at the
>>   * kernel
>>   * code.
>>   *
>>   * Use it for definite and high priority BIOS bugs.
>>   *
>>   * FW_WARN
>>   * Use it for not that clear (e.g. could the kernel messed up things
>>   * already?)
>>   * and medium priority BIOS bugs.
>>   *
>>   * FW_INFO
>>   * Use this one if you want to tell the user or vendor about something
>>   * suspicious, but generally harmless related to the firmware.
>>   *
>>   * Use it for information or very low priority BIOS bugs.
>>   */
>>
> 
> It is not a firmware/BIOS bug. 

Correct.  This is a hardware bug that *may be* resolved through a BIOS update.
But there is no guarantee that a BIOS update is available.  Labelling it a FW
bug would be a mistake.

Prarit's comment to annotate it as
> a HW_ERR is more accurate.  A software patch is being tested now
> to see if it can do set-affinity in a manner that avoids this race
> and enables IR to stay on for all these systems.  It requires
> more testing to ensure the logic is valid.  This patch was
> recommended as a necessary short-term fix, and to highlight to
> others this possible state -- which Gerry mentioned he had.

Yup -- as mstowe asked ... should we even consider this patch then, or should we
wait for the possible real fix?

Having said that ... I'm nervous about playing around with the set-affinity path
for this HW problem.  We're basically changing good/reliable code for broken-ass
hardware.  :/  That doesn't seem a like a good choice to me.

I can understand if we all feel that the code is broken, or it can be made
better -- but to change it because of bad HW  just doesn't seem like the right
thing to do.

IMO.

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index f3b8f23..9bfb6c2 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -1113,3 +1113,23 @@  struct irq_remap_ops intel_irq_remap_ops = {
 	.msi_setup_irq		= intel_msi_setup_irq,
 	.setup_hpet_msi		= intel_setup_hpet_msi,
 };
+
+
+static void intel_remapping_check(struct pci_dev *dev)
+{
+	u8 revision;
+
+	pci_read_config_byte(dev, PCI_REVISION_ID, &revision);
+
+	if ((revision == 0x13) && irq_remapping_enabled) {
+		pr_warn("WARNING WARNING WARNING WARNING WARNING WARNING\n"
+			"This system BIOS has enabled interrupt remapping\n"
+			"on a chipset that contains an errata making that\n"
+			"feature unstable.  Please reboot with nointremap\n"
+			"added to the kernel command line and contact\n"
+			"your BIOS vendor for an update");
+	}
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5520_IOHUB, intel_remapping_check);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_5500_IOHUB, intel_remapping_check);
+
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 31717bd..54027a6 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2732,6 +2732,8 @@ 
 #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_RANK_REV2  0x2db2
 #define PCI_DEVICE_ID_INTEL_LYNNFIELD_MC_CH2_TC_REV2    0x2db3
 #define PCI_DEVICE_ID_INTEL_82855PM_HB	0x3340
+#define PCI_DEVICE_ID_INTEL_5500_IOHUB	0x3403
+#define PCI_DEVICE_ID_INTEL_5520_IOHUB	0x3406
 #define PCI_DEVICE_ID_INTEL_IOAT_TBG4	0x3429
 #define PCI_DEVICE_ID_INTEL_IOAT_TBG5	0x342a
 #define PCI_DEVICE_ID_INTEL_IOAT_TBG6	0x342b