Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1690666 > unrolled thread

Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 23:30 +0200
    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:00 +0200
      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 18:20 +0200
        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 18:30 +0200
          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 19:00 +0200
            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:20 +0200
              Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 16:50 +0200
                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 17:10 +0200
                  RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-20 19:00 +0200
                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 19:10 +0200
                      RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-20 19:20 +0200
                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-20 20:20 +0200
          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Aristeu Rozanski <aris@redhat.com> - 2017-07-19 21:00 +0200
            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 22:20 +0200
            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:30 +0200

#1690666 — Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac

On Tue, 2017-07-18 at 10:08 +0200, Borislav Petkov wrote:
> On Tue, Jul 18, 2017 at 08:00:07AM +0200, Borislav Petkov wrote:
> > And I think we should try this first: have the firmware disable
> > detection methods so that the platform drivers don't load.
> 
> Btw, in looking at this more, what about the firmware-first thing?
> 
> I.e., the firmware-first detection with apei_osc_setup() at the end
> of ghes_init().
> 
> Can we make ghes_edac loading dependent on that? I mean, that was
> *the* predicate for exactly that - to have the firmware look at the
> errors first. No need for platform whitelisting and so on.

I agree that 'osc_sb_apei_support_acked' should be checked when
enabling ghes_edac.  I do not know the details of existing issues, but
it sounds unlikely that this will address all of them since bugs can be
everywhere.  For instance, ghes_edac relies on DMI/SMBIOS info, unlike
other EDAC drivers, which can be buggy regardless of this _OSC info.

> I'd still decouple ghes_edac loading from ghes_probe() even though
> loading the platform driver should've been done *after* the
> firmware-first detection regardless.
> 
> So what we could do is make ghes_edac a normal module and have the
> relevant x86 EDAC modules query FF mode and if enabled, fail loading.

I agree that making ghes_edac as a normal module is a good thing, but I
do not think it's going to solve this issue.

Thanks,
-Toshi

[toc] | [next] | [standalone]

#1690998

On Tue, Jul 18, 2017 at 09:20:44PM +0000, Kani, Toshimitsu wrote:
> I agree that 'osc_sb_apei_support_acked' should be checked when
> enabling ghes_edac.  I do not know the details of existing issues, but
> it sounds unlikely that this will address all of them since bugs can be
> everywhere.

No, see below.

> For instance, ghes_edac relies on DMI/SMBIOS info, unlike
> other EDAC drivers, which can be buggy regardless of this _OSC info.

That's the problem with firmware. You can't really fix it and it is
buggy as hell.

> I agree that making ghes_edac as a normal module is a good thing, but I
> do not think it's going to solve this issue.

Of course it will - if the firmware says it wants to look at the errors
first, then it gets to do so. This is the whole handling of hardware
errors in the firmware deal. I admit, sometimes it makes sense because
the firmware has the most intimate knowledge of the platform and, in
a perfect world, we won't ever need to have platform-specific EDAC
drivers.

But, we don't live in a perfect world. And the vendor execution of the
whole firmware-error-handling deal is an abomination at best.

So, if we realize that the firmware is buggy, we can use a platform list
to blacklist it (^hint hint^) and have a parameter to disable ghes_edac
from loading.

But we'll deal with that when we get to cross that bridge. Right now,
I'd like to do the loading spec-conform and not fiddle with white-,
black-, or any-other-color lists.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1691998

On Wed, 2017-07-19 at 07:52 +0200, Borislav Petkov wrote:
> On Tue, Jul 18, 2017 at 09:20:44PM +0000, Kani, Toshimitsu wrote:
> > I agree that 'osc_sb_apei_support_acked' should be checked when
> > enabling ghes_edac.  I do not know the details of existing issues,
> > but it sounds unlikely that this will address all of them since
> > bugs can be everywhere.
> 
> No, see below.
> 
> > For instance, ghes_edac relies on DMI/SMBIOS info, unlike
> > other EDAC drivers, which can be buggy regardless of this _OSC
> > info.
> 
> That's the problem with firmware. You can't really fix it and it is
> buggy as hell.

Right, and that's what I was told as an issue for ghes_edac.  This is
why this patch introduces a white-list to preclude all buggy firmwares
that are unknown to us...

> > I agree that making ghes_edac as a normal module is a good thing,
> > but I do not think it's going to solve this issue.
> 
> Of course it will - if the firmware says it wants to look at the
> errors first, then it gets to do so. This is the whole handling of
> hardware errors in the firmware deal. I admit, sometimes it makes
> sense because the firmware has the most intimate knowledge of the
> platform and, in a perfect world, we won't ever need to have
> platform-specific EDAC drivers.
>
> But, we don't live in a perfect world. And the vendor execution of
> the whole firmware-error-handling deal is an abomination at best.
> 
> So, if we realize that the firmware is buggy, we can use a platform
> list to blacklist it (^hint hint^) and have a parameter to disable
> ghes_edac from loading.

Setting blacklist needs us to enable ghes_edac and find all buggy
firmwares to date.  I think this is too disturbing for people who are
happily using regular edac drivers today even though their platforms
have GHES.

> But we'll deal with that when we get to cross that bridge. Right now,
> I'd like to do the loading spec-conform and not fiddle with white-,
> black-, or any-other-color lists.

I do prefer to avoid any white / black listing.  But I do not see how
it solves the buggy DMI/SMBIOS info as an example of firmware bugs we
may have to deal with.

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]

#1692014

On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote:
> I do prefer to avoid any white / black listing.  But I do not see how
> it solves the buggy DMI/SMBIOS info as an example of firmware bugs we
> may have to deal with.

So how do you want to deal with this?

Maintain an evergrowing whitelist of platforms which are OK and then the
moment a new platform comes along, you send a patch to add it to that
whitelist?

I'm sure you can see the problems with that approach.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1692029

On Wed, 2017-07-19 at 18:22 +0200, Borislav Petkov wrote:
> On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote:
> > I do prefer to avoid any white / black listing.  But I do not see
> > how it solves the buggy DMI/SMBIOS info as an example of firmware
> > bugs we may have to deal with.
> 
> So how do you want to deal with this?
> 
> Maintain an evergrowing whitelist of platforms which are OK and then
> the moment a new platform comes along, you send a patch to add it to
> that whitelist?
> 
> I'm sure you can see the problems with that approach.

Since ghes_edac has not been used for a long time, I have a feeling
that not so many vendors want to use it.  In the case of HPE, we do not
need to update with each platform since "HPE" "Server" will cover all
platforms we need.

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]

#1692396

On Wed, Jul 19, 2017 at 04:56:17PM +0000, Kani, Toshimitsu wrote:
> Since ghes_edac has not been used for a long time, I have a feeling
> that not so many vendors want to use it.  In the case of HPE, we do not
> need to update with each platform since "HPE" "Server" will cover all
> platforms we need.

Does the apei_osc_setup() detection with the uuid work on HP systems?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1692994

On Thu, 2017-07-20 at 06:16 +0200, Borislav Petkov wrote:
> On Wed, Jul 19, 2017 at 04:56:17PM +0000, Kani, Toshimitsu wrote:
> > Since ghes_edac has not been used for a long time, I have a feeling
> > that not so many vendors want to use it.  In the case of HPE, we do
> > not need to update with each platform since "HPE" "Server" will
> > cover all platforms we need.
> 
> Does the apei_osc_setup() detection with the uuid work on HP systems?

Yes, the following message is shown on HP systems.  Please note that
WHEA is a Windows-defined interface.

"GHES: APEI firmware first mode is enabled by APEI bit and WHEA _OSC."

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]

#1693010

On Thu, Jul 20, 2017 at 02:42:25PM +0000, Kani, Toshimitsu wrote:
> Yes, the following message is shown on HP systems.  Please note that
> WHEA is a Windows-defined interface.

Ok, so let's couple ghes_edac loading to that and see how far we could
go. I guess we should add checks for that to the major x86 EDAC drivers
to not load and this way ghes_edac will be the only driver loading.

Tony, how does that sound?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1693095

>> Yes, the following message is shown on HP systems.  Please note that
>> WHEA is a Windows-defined interface.
>
> Ok, so let's couple ghes_edac loading to that and see how far we could
> go. I guess we should add checks for that to the major x86 EDAC drivers
> to not load and this way ghes_edac will be the only driver loading.
>
> Tony, how does that sound?

Add a module parameter to those edac drivers that can override the check
and let them load anyway.  I'm not paranoid, I just assume that there is a BIOS
out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs.

-Tony

[toc] | [prev] | [next] | [standalone]

#1693097

On Thu, Jul 20, 2017 at 04:55:59PM +0000, Luck, Tony wrote:
> Add a module parameter to those edac drivers that can override the check
> and let them load anyway.  I'm not paranoid, I just assume that there is a BIOS
> out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs.

Or add that parameter to edac_core.ko and let it control which EDAC
driver gets loaded? Something like

edac=ignore_ghes

or so. And then the other EDAC drivers query it.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]

#1693102

> Or add that parameter to edac_core.ko and let it control which EDAC
> driver gets loaded? Something like
>
> edac=ignore_ghes
>
> or so. And then the other EDAC drivers query it.

Sure ... one central place is better than adding code to each
driver.

-Tony

[toc] | [prev] | [next] | [standalone]

#1693155

Em Thu, 20 Jul 2017 19:05:04 +0200
Borislav Petkov <bp@alien8.de> escreveu:

> On Thu, Jul 20, 2017 at 04:55:59PM +0000, Luck, Tony wrote:
> > Add a module parameter to those edac drivers that can override the check
> > and let them load anyway.  I'm not paranoid, I just assume that there is a BIOS
> > out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs.  
> 
> Or add that parameter to edac_core.ko and let it control which EDAC
> driver gets loaded? Something like
> 
> edac=ignore_ghes
> 
> or so. And then the other EDAC drivers query it.

Works for me.

Thanks,
Mauro

[toc] | [prev] | [next] | [standalone]

#1692110

On Wed, Jul 19, 2017 at 06:22:04PM +0200, Borislav Petkov wrote:
> On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote:
> > I do prefer to avoid any white / black listing.  But I do not see how
> > it solves the buggy DMI/SMBIOS info as an example of firmware bugs we
> > may have to deal with.
> 
> So how do you want to deal with this?
> 
> Maintain an evergrowing whitelist of platforms which are OK and then the
> moment a new platform comes along, you send a patch to add it to that
> whitelist?

That would also need to keep an eye on versions. A newer version of BIOS
on a whitelisted platform might be broken.

-- 
Aristeu

[toc] | [prev] | [next] | [standalone]

#1692170

On Wed, 2017-07-19 at 14:55 -0400, Aristeu Rozanski wrote:
> On Wed, Jul 19, 2017 at 06:22:04PM +0200, Borislav Petkov wrote:
> > On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote:
> > > I do prefer to avoid any white / black listing.  But I do not see
> > > how
> > > it solves the buggy DMI/SMBIOS info as an example of firmware
> > > bugs we
> > > may have to deal with.
> > 
> > So how do you want to deal with this?
> > 
> > Maintain an evergrowing whitelist of platforms which are OK and
> > then the moment a new platform comes along, you send a patch to add
> > it to that whitelist?
> 
> That would also need to keep an eye on versions. A newer version of
> BIOS on a whitelisted platform might be broken.

Right.  I think a question comes to who broke a running system -- OS
update or BIOS update.  This whitelist attempts to protect the former
case by not introducing ghes_edac on arbitrary platforms.  The latter
case should be vendor's responsibility.

Thanks,
-Toshi

[toc] | [prev] | [next] | [standalone]

#1692398

On Wed, Jul 19, 2017 at 02:55:08PM -0400, Aristeu Rozanski wrote:
> That would also need to keep an eye on versions. A newer version of BIOS
> on a whitelisted platform might be broken.

Yeah, that would be a nasty, back-stabbing SNAFU.

So I'm thinking of adding a bunch of FW_ERR sanity checks to that whole
ghes_edac and ghes init code to hopefully catch issues during platform
validation. I.e., early enough for them to get fixed.

But that's the same problem as with UEFI - vendors need to try to boot
Linux on their platforms early enough.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web