* [PATCH] iommu: Remove stack trace from broken irq remapping warning
@ 2013-09-27 16:53 Neil Horman
2013-09-27 19:24 ` Andy Lutomirski
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Neil Horman @ 2013-09-27 16:53 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pci, Neil Horman, Joerg Roedel, Bjorn Helgaas,
Andy Lutomirski, Konrad Rzeszutek Wilk, Sebastian Andrzej Siewior
The warning for the irq remapping broken check in intel_irq_remapping.c is
pretty pointless. We need the warning, but we know where its comming from, the
stack trace will always be the same, and it needlessly triggers things like
Abrt. This changes the warning to just print a text warning about BIOS being
broken, without the stack trace, then sets the appropriate taint bit. Since we
automatically disable irq remapping, theres no need to contiue making Abrt jump
at this problem
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Joerg Roedel <joro@8bytes.org>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
drivers/iommu/intel_irq_remapping.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index f71673d..b97d70b 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -525,12 +525,13 @@ static int __init intel_irq_remapping_supported(void)
if (disable_irq_remap)
return 0;
if (irq_remap_broken) {
- WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
- "This system BIOS has enabled interrupt remapping\n"
- "on a chipset that contains an erratum making that\n"
- "feature unstable. To maintain system stability\n"
- "interrupt remapping is being disabled. Please\n"
- "contact your BIOS vendor for an update\n");
+ printk(KERN_WARNING
+ "This system BIOS has enabled interrupt remapping\n"
+ "on a chipset that contains an erratum making that\n"
+ "feature unstable. To maintain system stability\n"
+ "interrupt remapping is being disabled. Please\n"
+ "contact your BIOS vendor for an update\n");
+ add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
disable_irq_remap = 1;
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-09-27 16:53 [PATCH] iommu: Remove stack trace from broken irq remapping warning Neil Horman
@ 2013-09-27 19:24 ` Andy Lutomirski
2013-09-27 19:41 ` Neil Horman
2013-10-03 17:21 ` Neil Horman
2013-10-04 14:32 ` Joerg Roedel
2 siblings, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2013-09-27 19:24 UTC (permalink / raw)
To: Neil Horman
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Joerg Roedel, Bjorn Helgaas, Konrad Rzeszutek Wilk,
Sebastian Andrzej Siewior
On Fri, Sep 27, 2013 at 9:53 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> The warning for the irq remapping broken check in intel_irq_remapping.c is
> pretty pointless. We need the warning, but we know where its comming from, the
> stack trace will always be the same, and it needlessly triggers things like
> Abrt. This changes the warning to just print a text warning about BIOS being
> broken, without the stack trace, then sets the appropriate taint bit. Since we
> automatically disable irq remapping, theres no need to contiue making Abrt jump
> at this problem
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Joerg Roedel <joro@8bytes.org>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> ---
> drivers/iommu/intel_irq_remapping.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index f71673d..b97d70b 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -525,12 +525,13 @@ static int __init intel_irq_remapping_supported(void)
> if (disable_irq_remap)
> return 0;
> if (irq_remap_broken) {
> - WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
> - "This system BIOS has enabled interrupt remapping\n"
> - "on a chipset that contains an erratum making that\n"
> - "feature unstable. To maintain system stability\n"
> - "interrupt remapping is being disabled. Please\n"
> - "contact your BIOS vendor for an update\n");
> + printk(KERN_WARNING
> + "This system BIOS has enabled interrupt remapping\n"
> + "on a chipset that contains an erratum making that\n"
> + "feature unstable. To maintain system stability\n"
> + "interrupt remapping is being disabled. Please\n"
> + "contact your BIOS vendor for an update\n");
> + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
Is the taint bit actually useful? It seems like functionality will be
missing if this workaround happens, but everything should be stable.
--Andy
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-09-27 19:24 ` Andy Lutomirski
@ 2013-09-27 19:41 ` Neil Horman
0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2013-09-27 19:41 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Joerg Roedel, Bjorn Helgaas, Konrad Rzeszutek Wilk,
Sebastian Andrzej Siewior
On Fri, Sep 27, 2013 at 12:24:10PM -0700, Andy Lutomirski wrote:
> On Fri, Sep 27, 2013 at 9:53 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > The warning for the irq remapping broken check in intel_irq_remapping.c is
> > pretty pointless. We need the warning, but we know where its comming from, the
> > stack trace will always be the same, and it needlessly triggers things like
> > Abrt. This changes the warning to just print a text warning about BIOS being
> > broken, without the stack trace, then sets the appropriate taint bit. Since we
> > automatically disable irq remapping, theres no need to contiue making Abrt jump
> > at this problem
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Joerg Roedel <joro@8bytes.org>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Andy Lutomirski <luto@amacapital.net>
> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> > ---
> > drivers/iommu/intel_irq_remapping.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> > index f71673d..b97d70b 100644
> > --- a/drivers/iommu/intel_irq_remapping.c
> > +++ b/drivers/iommu/intel_irq_remapping.c
> > @@ -525,12 +525,13 @@ static int __init intel_irq_remapping_supported(void)
> > if (disable_irq_remap)
> > return 0;
> > if (irq_remap_broken) {
> > - WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
> > - "This system BIOS has enabled interrupt remapping\n"
> > - "on a chipset that contains an erratum making that\n"
> > - "feature unstable. To maintain system stability\n"
> > - "interrupt remapping is being disabled. Please\n"
> > - "contact your BIOS vendor for an update\n");
> > + printk(KERN_WARNING
> > + "This system BIOS has enabled interrupt remapping\n"
> > + "on a chipset that contains an erratum making that\n"
> > + "feature unstable. To maintain system stability\n"
> > + "interrupt remapping is being disabled. Please\n"
> > + "contact your BIOS vendor for an update\n");
> > + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
>
> Is the taint bit actually useful? It seems like functionality will be
> missing if this workaround happens, but everything should be stable.
>
I think its useful yes. The system will be stable, an in fact should run
exactly as it did before, but since the errata indicates this should be fixed in
BIOS, its a reminder to the admin that you should investigate an update, or take
action in manually disabling iommu on the command line
Its also in keeping with the way this was structured previously.
Neil
> --Andy
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-09-27 16:53 [PATCH] iommu: Remove stack trace from broken irq remapping warning Neil Horman
2013-09-27 19:24 ` Andy Lutomirski
@ 2013-10-03 17:21 ` Neil Horman
2013-10-03 19:21 ` Bjorn Helgaas
2013-10-03 20:08 ` Joerg Roedel
2013-10-04 14:32 ` Joerg Roedel
2 siblings, 2 replies; 8+ messages in thread
From: Neil Horman @ 2013-10-03 17:21 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pci, Joerg Roedel, Bjorn Helgaas, Andy Lutomirski,
Konrad Rzeszutek Wilk, Sebastian Andrzej Siewior
On Fri, Sep 27, 2013 at 12:53:35PM -0400, Neil Horman wrote:
> The warning for the irq remapping broken check in intel_irq_remapping.c is
> pretty pointless. We need the warning, but we know where its comming from, the
> stack trace will always be the same, and it needlessly triggers things like
> Abrt. This changes the warning to just print a text warning about BIOS being
> broken, without the stack trace, then sets the appropriate taint bit. Since we
> automatically disable irq remapping, theres no need to contiue making Abrt jump
> at this problem
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Joerg Roedel <joro@8bytes.org>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> ---
> drivers/iommu/intel_irq_remapping.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
> index f71673d..b97d70b 100644
> --- a/drivers/iommu/intel_irq_remapping.c
> +++ b/drivers/iommu/intel_irq_remapping.c
> @@ -525,12 +525,13 @@ static int __init intel_irq_remapping_supported(void)
> if (disable_irq_remap)
> return 0;
> if (irq_remap_broken) {
> - WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
> - "This system BIOS has enabled interrupt remapping\n"
> - "on a chipset that contains an erratum making that\n"
> - "feature unstable. To maintain system stability\n"
> - "interrupt remapping is being disabled. Please\n"
> - "contact your BIOS vendor for an update\n");
> + printk(KERN_WARNING
> + "This system BIOS has enabled interrupt remapping\n"
> + "on a chipset that contains an erratum making that\n"
> + "feature unstable. To maintain system stability\n"
> + "interrupt remapping is being disabled. Please\n"
> + "contact your BIOS vendor for an update\n");
> + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
> disable_irq_remap = 1;
> return 0;
> }
> --
> 1.8.3.1
>
>
Ping Bjorn, Jeorg, any thoughts here?
Neil
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-10-03 17:21 ` Neil Horman
@ 2013-10-03 19:21 ` Bjorn Helgaas
2013-10-03 20:08 ` Joerg Roedel
1 sibling, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2013-10-03 19:21 UTC (permalink / raw)
To: Neil Horman
Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Joerg Roedel, Andy Lutomirski, Konrad Rzeszutek Wilk,
Sebastian Andrzej Siewior
On Thu, Oct 3, 2013 at 11:21 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Fri, Sep 27, 2013 at 12:53:35PM -0400, Neil Horman wrote:
>> The warning for the irq remapping broken check in intel_irq_remapping.c is
>> pretty pointless. We need the warning, but we know where its comming from, the
>> stack trace will always be the same, and it needlessly triggers things like
>> Abrt. This changes the warning to just print a text warning about BIOS being
>> broken, without the stack trace, then sets the appropriate taint bit. Since we
>> automatically disable irq remapping, theres no need to contiue making Abrt jump
>> at this problem
>>
>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>> CC: Joerg Roedel <joro@8bytes.org>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Andy Lutomirski <luto@amacapital.net>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
>> ---
>> drivers/iommu/intel_irq_remapping.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
>> index f71673d..b97d70b 100644
>> --- a/drivers/iommu/intel_irq_remapping.c
>> +++ b/drivers/iommu/intel_irq_remapping.c
>> @@ -525,12 +525,13 @@ static int __init intel_irq_remapping_supported(void)
>> if (disable_irq_remap)
>> return 0;
>> if (irq_remap_broken) {
>> - WARN_TAINT(1, TAINT_FIRMWARE_WORKAROUND,
>> - "This system BIOS has enabled interrupt remapping\n"
>> - "on a chipset that contains an erratum making that\n"
>> - "feature unstable. To maintain system stability\n"
>> - "interrupt remapping is being disabled. Please\n"
>> - "contact your BIOS vendor for an update\n");
>> + printk(KERN_WARNING
>> + "This system BIOS has enabled interrupt remapping\n"
>> + "on a chipset that contains an erratum making that\n"
>> + "feature unstable. To maintain system stability\n"
>> + "interrupt remapping is being disabled. Please\n"
>> + "contact your BIOS vendor for an update\n");
>> + add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
>> disable_irq_remap = 1;
>> return 0;
>> }
>> --
>> 1.8.3.1
>>
>>
>
> Ping Bjorn, Jeorg, any thoughts here?
This is in drivers/iommu, so I'd prefer that Joerg handle this.
My opinion is that this patch does the right thing in dropping the
backtrace and keeping the taint.
I tend to agree with Andy that we should also consider a second patch
that drops the taint, because this is basically just a quirk that
avoids broken chipset functionality, but as far as I can tell, there's
no reason to fear an undebuggable problem related to disabling
interrupt remapping. We don't taint the kernel for other similar
quirks, so I don't see why we should here.
Bjorn
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-10-03 17:21 ` Neil Horman
2013-10-03 19:21 ` Bjorn Helgaas
@ 2013-10-03 20:08 ` Joerg Roedel
2013-10-03 20:26 ` Neil Horman
1 sibling, 1 reply; 8+ messages in thread
From: Joerg Roedel @ 2013-10-03 20:08 UTC (permalink / raw)
To: Neil Horman
Cc: linux-kernel, linux-pci, Bjorn Helgaas, Andy Lutomirski,
Konrad Rzeszutek Wilk, Sebastian Andrzej Siewior
On Thu, Oct 03, 2013 at 01:21:42PM -0400, Neil Horman wrote:
> On Fri, Sep 27, 2013 at 12:53:35PM -0400, Neil Horman wrote:
> > The warning for the irq remapping broken check in intel_irq_remapping.c is
> > pretty pointless. We need the warning, but we know where its comming from, the
> > stack trace will always be the same, and it needlessly triggers things like
> > Abrt. This changes the warning to just print a text warning about BIOS being
> > broken, without the stack trace, then sets the appropriate taint bit. Since we
> > automatically disable irq remapping, theres no need to contiue making Abrt jump
> > at this problem
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Joerg Roedel <joro@8bytes.org>
> > CC: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Andy Lutomirski <luto@amacapital.net>
> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
>
> Ping Bjorn, Jeorg, any thoughts here?
Yes, the patch is doing the right thing. I have it already on my list
and will merge it soon.
Joerg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-10-03 20:08 ` Joerg Roedel
@ 2013-10-03 20:26 ` Neil Horman
0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2013-10-03 20:26 UTC (permalink / raw)
To: Joerg Roedel
Cc: linux-kernel, linux-pci, Bjorn Helgaas, Andy Lutomirski,
Konrad Rzeszutek Wilk, Sebastian Andrzej Siewior
On Thu, Oct 03, 2013 at 10:08:24PM +0200, Joerg Roedel wrote:
> On Thu, Oct 03, 2013 at 01:21:42PM -0400, Neil Horman wrote:
> > On Fri, Sep 27, 2013 at 12:53:35PM -0400, Neil Horman wrote:
> > > The warning for the irq remapping broken check in intel_irq_remapping.c is
> > > pretty pointless. We need the warning, but we know where its comming from, the
> > > stack trace will always be the same, and it needlessly triggers things like
> > > Abrt. This changes the warning to just print a text warning about BIOS being
> > > broken, without the stack trace, then sets the appropriate taint bit. Since we
> > > automatically disable irq remapping, theres no need to contiue making Abrt jump
> > > at this problem
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: Joerg Roedel <joro@8bytes.org>
> > > CC: Bjorn Helgaas <bhelgaas@google.com>
> > > CC: Andy Lutomirski <luto@amacapital.net>
> > > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > CC: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> >
> > Ping Bjorn, Jeorg, any thoughts here?
>
> Yes, the patch is doing the right thing. I have it already on my list
> and will merge it soon.
>
Awesome, thanks guys. Regarding the taint, I'll propose something for that
early next week.
Regards
Neil
>
> Joerg
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu: Remove stack trace from broken irq remapping warning
2013-09-27 16:53 [PATCH] iommu: Remove stack trace from broken irq remapping warning Neil Horman
2013-09-27 19:24 ` Andy Lutomirski
2013-10-03 17:21 ` Neil Horman
@ 2013-10-04 14:32 ` Joerg Roedel
2 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2013-10-04 14:32 UTC (permalink / raw)
To: Neil Horman
Cc: linux-kernel, linux-pci, Bjorn Helgaas, Andy Lutomirski,
Konrad Rzeszutek Wilk, Sebastian Andrzej Siewior
On Fri, Sep 27, 2013 at 12:53:35PM -0400, Neil Horman wrote:
> The warning for the irq remapping broken check in intel_irq_remapping.c is
> pretty pointless. We need the warning, but we know where its comming from, the
> stack trace will always be the same, and it needlessly triggers things like
> Abrt. This changes the warning to just print a text warning about BIOS being
> broken, without the stack trace, then sets the appropriate taint bit. Since we
> automatically disable irq remapping, theres no need to contiue making Abrt jump
> at this problem
Applied to x86/vt-d, thanks Neil.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-04 14:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 16:53 [PATCH] iommu: Remove stack trace from broken irq remapping warning Neil Horman
2013-09-27 19:24 ` Andy Lutomirski
2013-09-27 19:41 ` Neil Horman
2013-10-03 17:21 ` Neil Horman
2013-10-03 19:21 ` Bjorn Helgaas
2013-10-03 20:08 ` Joerg Roedel
2013-10-03 20:26 ` Neil Horman
2013-10-04 14:32 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).