Message-ID: <aGfFyktcuRV73Bl8@gmail.com>
Date: Fri, 4 Jul 2025 13:15:06 +0100
From: Breno Leitao <leitao@...ian.org>
To: Borislav Petkov <bp@...en8.de>
Cc: Len Brown <lenb@...nel.org>, James Morse <james.morse@....com>,
	Jonathan Corbet <corbet@....net>, tony.luck@...el.com,
	rafael@...nel.org, Alexei Starovoitov <ast@...nel.org>,
	kbusch@...nel.org, rmikey@...a.com, kuba@...nel.org,
	linux-edac@...r.kernel.org, mchehab@...nel.org,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCH 0/2] panic: taint flag for recoverable hardware errors

Hello Borislav,

First of all, thanks for spending time in this one.

On Fri, Jul 04, 2025 at 01:19:54PM +0200, Borislav Petkov wrote:
> On Fri, Jul 04, 2025 at 03:55:18AM -0700, Breno Leitao wrote:
> > Add a new taint flag to the kernel (HW_ERROR_RECOVERED - for the lack of
> > a better name) that gets set whenever the kernel detects and recovers
> > from hardware errors.
> > 
> > The taint provides additional context during crash investigation *without*
> > implying that crashes are necessarily caused by hardware failures
> > (similar to how PROPRIETARY_MODULE taint works). It is just an extra
> > information that will provide more context about that machine.
> 
> Dunno, looks like a hack to me to serve your purpose only.
> 
> Because when this goes up, then people will start wanting to taint the kernel
> for *every* *single* correctable error.
> 
> So even if an error got corrected, the kernel will be tainted.
> 
> Then users will say, oh oh, my kernel is tainted, I need to replace my hw
> because broken. Even if it isn't broken in the very least.

The information is not there to show correlation of broken hardware,
but, to flag that this kernel is running on a hardware that has
recovered from an error. It doesn't not mean that the problem is in the
hardware.  During my investigations, most of the time, the kernel was
buggy when recovering from hardware issues, mainly PCI re-plugs.

Anyway, the taints are not to tell you the root cause of the problem,
but, to give you an indication that would help the investigations. For
instance:

TAINT_PROPRIETARY_MODULE: 
   - It doesn't mean that the machine crashed because of the proprietary module.

TAINT_FIRMWARE_WORKAROUND:
  - It doesn't tell you that crashes came because of the workaround,
    but, it tells you of this workaround.

Same for TAINT_LIVEPATCH, TAINT_FORCED_RMMOD and most of the taints. It
helps the users, it doesn't not tell you the root cause. For that we
have AITM. :-P

> Basically what we're doing with drivers/ras/cec.c will be undone.
> 
> All because you want to put a bit of information somewhere that the machine
> had a recoverable error.
> 
> Well, that bit of information is in your own RAS logs, no? I presume you log
> hw errors in a big fleet and then you analyze those logs when the machine
> bombs. So a mere look at those logs will tell you that you had hw errors.

True, but, this argument would apply for every taint flag above. You can
look at the logs and find LIVEPATCHES, PROPRIETARY_MODULES,
FIRMWARE_WORKAROUND, etc.

Those information could be somewhere else, but, being somewhere easy to
read proved to be useful.

For instance, reading from `cat /proc/sys/kernel/tainted` might be
*way easier* than parsing *thousands* different RAS tools logs for you
to find what is going on. 

> And mind you, that proposed solution does not help people who want to know
> what the errors were: "Oh look, my kernel got tainted because of hw errors. Now
> where are those errors?"

Agree and that is the intention. Whoever look a crash/warning knows that
the machine recovered from a hardware error, and this help the user in
two ways:

1) Know that the kernel executed a path that is not frequently executed.
2) Look at the RAS logs if you think this is hardware related

Maybe these two things doesn't mean much, but, it is like a heads-up
flag for whoever is looking at this issue.
 
> So I think this is just adding redundant information which we already have
> somewhere else and also actively can mislead users.
> 
> IOW, no need to taint - you want to simply put a bit of info in the kdump blob
> which gets dumped by the second kernel that the first kernel experienced hw
> errors. That is, if you don't log hw errors. But you should...!

Sure, saving this information somewhere will solve the problem as well.

I thought that adding a taint would be easier for few reasons:

1) you can easily read from userspace (/proc/sys/kernel/tainted), so, it
is easy to scan the fleet for hardware error, and query the RAS logs
only for those.

2) it is shown at crash time already, so, this information will be
"free" mostly.

3) taint is consumed by kdump/kexec already, so, nothing would change.

Anyway, I am happy to add this information somewhere else if you think
that taint is not the right place.

Thanks for your ideas and suggestions,
--breno

Powered by blists - more mailing lists