[PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Borislav Petkov]

From: Borislav Petkov <borislav.petkov@amd.com>

Correctable errors are considered something rather normal on modern
hardware these days. Even more importantly, correctable errors mean
exactly that - they've been corrected by the hardware - and there's no
need to taint the kernel since execution hasn't been compromised so far.

Suggested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/cpu/mcheck/mce.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 3385ea2..68e2303 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -590,7 +590,6 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		if (!(flags & MCP_DONTLOG) && !mce_dont_log_ce) {
 			mce_log(&m);
 			atomic_notifier_call_chain(&x86_mce_decoder_chain, 0, &m);
-			add_taint(TAINT_MACHINE_CHECK);
 		}
 
 		/*
-- 
1.7.4.rc2

RE: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Chumbalkar, Nagananda]

>-----Original Message-----
>From: linux-edac-owner@vger.kernel.org [mailto:linux-edac-
>owner@vger.kernel.org] On Behalf Of Borislav Petkov
>Sent: Friday, April 15, 2011 3:46 AM
>To: H. Peter Anvin; Ingo Molnar; Thomas Gleixner
>Cc: Prarit Bhargava; Tony Luck; Russ Anderson; LKML; EDAC devel;
>Borislav Petkov
>Subject: [PATCH 1/2] x86, MCE: Do not taint when handling correctable
>errors
>
>From: Borislav Petkov <borislav.petkov@amd.com>
>
>Correctable errors are considered something rather normal on modern
>hardware these days. Even more importantly, correctable errors mean
>exactly that - they've been corrected by the hardware - and there's no
>need to taint the kernel since execution hasn't been compromised so far.
>

Would it be okay to extend this reasoning and also remove the tainting 
caused by TM1/TM2 thermal events:

diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
b/arch/x86/kernel/cpu/mcheck/therm_throt.c
index 6f8c5e9..9f3b5ae 100644
--- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
@@ -188,7 +188,6 @@ static int therm_throt_process(bool new_event, int event,
int level)
                                level == CORE_LEVEL ? "Core" : "Package",
                                state->count);

-               add_taint(TAINT_MACHINE_CHECK);
                return 1;
        }
        if (old_event) {
@@ -393,7 +392,6 @@ static void unexpected_thermal_interrupt(void)
{
        printk(KERN_ERR "CPU%d: Unexpected LVT thermal interrupt!\n",
                        smp_processor_id());
-       add_taint(TAINT_MACHINE_CHECK);
}

static void (*smp_thermal_vector)(void) = unexpected_thermal_interrupt;


- naga -


>Suggested-by: Tony Luck <tony.luck@intel.com>
>Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
>---
> arch/x86/kernel/cpu/mcheck/mce.c |    1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
>diff --git a/arch/x86/kernel/cpu/mcheck/mce.c
>b/arch/x86/kernel/cpu/mcheck/mce.c
>index 3385ea2..68e2303 100644
>--- a/arch/x86/kernel/cpu/mcheck/mce.c
>+++ b/arch/x86/kernel/cpu/mcheck/mce.c
>@@ -590,7 +590,6 @@ void machine_check_poll(enum mcp_flags flags,
>mce_banks_t *b)
> 		if (!(flags & MCP_DONTLOG) && !mce_dont_log_ce) {
> 			mce_log(&m);
> 			atomic_notifier_call_chain(&x86_mce_decoder_chain, 0,
>&m);
>-			add_taint(TAINT_MACHINE_CHECK);
> 		}
>
> 		/*
>--
>1.7.4.rc2
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-edac" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Luck, Tony]

> Would it be okay to extend this reasoning and also remove the tainting 
> caused by TM1/TM2 thermal events:

Looks good to me - crossing a thermal threshold doesn't sound (by
itself) worthy of a TAINT.  Nothing has been corrupted.

Acked-by: Tony Luck <tony.luck@intel.com>

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Ingo Molnar]

* Chumbalkar, Nagananda <Nagananda.Chumbalkar@hp.com> wrote:

> Would it be okay to extend this reasoning and also remove the tainting 
> caused by TM1/TM2 thermal events:
> 
> diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> index 6f8c5e9..9f3b5ae 100644
> --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> @@ -188,7 +188,6 @@ static int therm_throt_process(bool new_event, int event,
> int level)
>                                 level == CORE_LEVEL ? "Core" : "Package",
>                                 state->count);
> 
> -               add_taint(TAINT_MACHINE_CHECK);
>                 return 1;
>         }
>         if (old_event) {
> @@ -393,7 +392,6 @@ static void unexpected_thermal_interrupt(void)
> {
>         printk(KERN_ERR "CPU%d: Unexpected LVT thermal interrupt!\n",
>                         smp_processor_id());
> -       add_taint(TAINT_MACHINE_CHECK);
> }

Mind sending a proper patch, with changelog, signoff, acks, etc?

Thanks,

	Ingo

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Borislav Petkov]

On Thu, Apr 21, 2011 at 05:35:54AM -0400, Ingo Molnar wrote:
> 
> * Chumbalkar, Nagananda <Nagananda.Chumbalkar@hp.com> wrote:
> 
> > Would it be okay to extend this reasoning and also remove the tainting 
> > caused by TM1/TM2 thermal events:
> > 
> > diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > index 6f8c5e9..9f3b5ae 100644
> > --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > @@ -188,7 +188,6 @@ static int therm_throt_process(bool new_event, int event,
> > int level)
> >                                 level == CORE_LEVEL ? "Core" : "Package",
> >                                 state->count);
> > 
> > -               add_taint(TAINT_MACHINE_CHECK);
> >                 return 1;
> >         }
> >         if (old_event) {
> > @@ -393,7 +392,6 @@ static void unexpected_thermal_interrupt(void)
> > {
> >         printk(KERN_ERR "CPU%d: Unexpected LVT thermal interrupt!\n",
> >                         smp_processor_id());
> > -       add_taint(TAINT_MACHINE_CHECK);
> > }
> 
> Mind sending a proper patch, with changelog, signoff, acks, etc?

No need since this is part of 7b70bd3441437b7bc04fc9d321e17c8ed0e8f958
now which is in tip/x86/mce.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> On Thu, Apr 21, 2011 at 05:35:54AM -0400, Ingo Molnar wrote:
> > 
> > * Chumbalkar, Nagananda <Nagananda.Chumbalkar@hp.com> wrote:
> > 
> > > Would it be okay to extend this reasoning and also remove the tainting 
> > > caused by TM1/TM2 thermal events:
> > > 
> > > diff --git a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > > b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > > index 6f8c5e9..9f3b5ae 100644
> > > --- a/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > > +++ b/arch/x86/kernel/cpu/mcheck/therm_throt.c
> > > @@ -188,7 +188,6 @@ static int therm_throt_process(bool new_event, int event,
> > > int level)
> > >                                 level == CORE_LEVEL ? "Core" : "Package",
> > >                                 state->count);
> > > 
> > > -               add_taint(TAINT_MACHINE_CHECK);
> > >                 return 1;
> > >         }
> > >         if (old_event) {
> > > @@ -393,7 +392,6 @@ static void unexpected_thermal_interrupt(void)
> > > {
> > >         printk(KERN_ERR "CPU%d: Unexpected LVT thermal interrupt!\n",
> > >                         smp_processor_id());
> > > -       add_taint(TAINT_MACHINE_CHECK);
> > > }
> > 
> > Mind sending a proper patch, with changelog, signoff, acks, etc?
> 
> No need since this is part of 7b70bd3441437b7bc04fc9d321e17c8ed0e8f958
> now which is in tip/x86/mce.

Ok, indeed. Also, in the future, if you take patches from others please also 
credit them in the changelog. Something like this would have been good in the 
current case:

  Also, this patch includes a change from Nagananda Chumbalkar as well, which 
  drops tainting in the therma throttling code for a similar reason: crossing a
  thermal threshold does not mean corruption.

Nagananda's Acked-by is there so there's at least partial credit - but we 
generally try to aim for at least 100% credit where credit is due :-)

Thanks,

	Ingo

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Borislav Petkov]

On Thu, Apr 21, 2011 at 05:58:48AM -0400, Ingo Molnar wrote:
> Ok, indeed. Also, in the future, if you take patches from others please also 
> credit them in the changelog. Something like this would have been good in the 
> current case:
> 
>   Also, this patch includes a change from Nagananda Chumbalkar as well, which 
>   drops tainting in the therma throttling code for a similar reason: crossing a
>   thermal threshold does not mean corruption.
> 
> Nagananda's Acked-by is there so there's at least partial credit - but we 
> generally try to aim for at least 100% credit where credit is due :-)

Absolutely, and in the light of recent events :) I'm still not sure how
to do that though in a straight-forward manner so that it is visible at
a first glance. Sure, adding freeform text to the commit message is one
way. Using a SOB chain might work too - even the Acked-by tag - but all
those have another main purpose and are being repurposed for annotating
the fact that a patch is the result of more than one author's thought
process.

IOW, in case I'm not missing anything, we don't really have a way to
denote a multiple authorship, correct? And we should...

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Borislav Petkov]

On Thu, Apr 21, 2011 at 06:06:39AM -0400, Borislav Petkov wrote:
> On Thu, Apr 21, 2011 at 05:58:48AM -0400, Ingo Molnar wrote:
> > Ok, indeed. Also, in the future, if you take patches from others please also 
> > credit them in the changelog. Something like this would have been good in the 
> > current case:
> > 
> >   Also, this patch includes a change from Nagananda Chumbalkar as well, which 
> >   drops tainting in the therma throttling code for a similar reason: crossing a
> >   thermal threshold does not mean corruption.
> > 
> > Nagananda's Acked-by is there so there's at least partial credit - but we 
> > generally try to aim for at least 100% credit where credit is due :-)
> 
> Absolutely, and in the light of recent events :) I'm still not sure how
> to do that though in a straight-forward manner so that it is visible at
> a first glance. Sure, adding freeform text to the commit message is one
> way. Using a SOB chain might work too - even the Acked-by tag - but all
> those have another main purpose and are being repurposed for annotating
> the fact that a patch is the result of more than one author's thought
> process.
> 
> IOW, in case I'm not missing anything, we don't really have a way to
> denote a multiple authorship, correct? And we should...

Ok, after RTFMing <Documentation/SubmittingPatches> here's how:

From: Original Author <author@example.com>

	^ this is the original author who sent the initial patch

Signed-off-by: Original Author <author@example.com>
Signed-off-by: Additional Author <author2@example.com>
Signed-off-by: Third Author <author3@example.com>

and the SOBs following after the 1st one are denoting additional authors.

However, additional SOBs mean also subsystem maintainers on the delivery
path of the patch. I guess this is ok though.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH 1/2] x86, MCE: Do not taint when handling correctable errors

[Ingo Molnar]

* Borislav Petkov <bp@amd64.org> wrote:

> On Thu, Apr 21, 2011 at 06:06:39AM -0400, Borislav Petkov wrote:
> > On Thu, Apr 21, 2011 at 05:58:48AM -0400, Ingo Molnar wrote:
> > > Ok, indeed. Also, in the future, if you take patches from others please also 
> > > credit them in the changelog. Something like this would have been good in the 
> > > current case:
> > > 
> > >   Also, this patch includes a change from Nagananda Chumbalkar as well, which 
> > >   drops tainting in the therma throttling code for a similar reason: crossing a
> > >   thermal threshold does not mean corruption.
> > > 
> > > Nagananda's Acked-by is there so there's at least partial credit - but we 
> > > generally try to aim for at least 100% credit where credit is due :-)
> > 
> > Absolutely, and in the light of recent events :) I'm still not sure how
> > to do that though in a straight-forward manner so that it is visible at
> > a first glance. Sure, adding freeform text to the commit message is one
> > way. Using a SOB chain might work too - even the Acked-by tag - but all
> > those have another main purpose and are being repurposed for annotating
> > the fact that a patch is the result of more than one author's thought
> > process.
> > 
> > IOW, in case I'm not missing anything, we don't really have a way to
> > denote a multiple authorship, correct? And we should...
> 
> Ok, after RTFMing <Documentation/SubmittingPatches> here's how:
> 
> From: Original Author <author@example.com>
> 
> 	^ this is the original author who sent the initial patch
> 
> Signed-off-by: Original Author <author@example.com>
> Signed-off-by: Additional Author <author2@example.com>
> Signed-off-by: Third Author <author3@example.com>

The easiest solution is to create a separate patch for the TM1/TM2 change and 
ask for a SOB :-)

Thanks,

	Ingo