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

Groups > linux.kernel > #1689523 > unrolled thread

[PATCH 0/3] enable ghes_edac on selected platforms

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] enable ghes_edac on selected platforms Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
    [PATCH 2/3] intel_pstate: convert to use acpi_match_oemlist() Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
    [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-18 08:10 +0200
        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-18 10:10 +0200
          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
        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 22:00 +0200
          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-18 23:20 +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 "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 17:20 +0200
                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 18:00 +0200
                  RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 20:10 +0200
            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 18:50 +0200
              Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:40 +0200
                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 22:00 +0200
                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-20 22:20 +0200
                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 23:10 +0200
                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 15:40 +0200
                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 15:50 +0200
                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 17:20 +0200
                        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 17:40 +0200
                          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 17:50 +0200
                            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 18:50 +0200
                              Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 19:10 +0200
                                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 19:30 +0200
                                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 20:50 +0200
                                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-22 08:30 +0200
                                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 17:00 +0200
                                        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 17:10 +0200
                                          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 17:30 +0200
                                            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 17:40 +0200
                                              Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 18:00 +0200
                                                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 18:40 +0200
                                                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 19:50 +0200
                                                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Boris Petkov <bp@alien8.de> - 2017-07-24 20:00 +0200
                                                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 20:00 +0200
                                                        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 20:20 +0200
                                                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 20:00 +0200
                                                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 20:20 +0200
                                              Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 18:10 +0200
                                                Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 18:50 +0200
                                                  Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 20:20 +0200
                                                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 20:40 +0200
                                                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-26 01:10 +0200
                          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 18:00 +0200
                            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 18:40 +0200
                      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 17:20 +0200
                    Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 15:50 +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 "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 00:20 +0200
          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:10 +0200
      Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Jeffrey Hugo <jhugo@codeaurora.org> - 2017-07-18 16:50 +0200
        Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 17:40 +0200
          Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Jeffrey Hugo <jhugo@codeaurora.org> - 2017-07-18 18:30 +0200
            Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 18:50 +0200
    [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
      Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 07:40 +0200
        Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 17:50 +0200
          Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 18:50 +0200
            Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 19:30 +0200
              Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 19:50 +0200
                Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 21:00 +0200
                  Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 21:40 +0200
                    Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 22:20 +0200

Page 4 of 4 — ← Prev page 1 2 3 [4]

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

On Fri, 2017-07-21 at 15:47 +0200, Borislav Petkov wrote:
> On Fri, Jul 21, 2017 at 10:40:01AM -0300, Mauro Carvalho Chehab
> wrote:
> > What happens when the error can be corrected? Does it still report
> > it to userspace, or just silently hide the error?
> > 
> > If I remember well about a past discussion with some vendor, I was
> > told that the firmware can hide some errors from being reported. Is
> > it still the case?
> 
> I've heard the same thing but I have no idea what they're actually
> doing. But it would make sense because the intention is not to worry
> users unnecessarily if it can hide the error and if there are no
> adverse consequences from it.

Yes, that is correct.  Corrected errors are reported to the OS when
they exceeded the platform's threshold.

Thanks,
-Toshi

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

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

Em Fri, 21 Jul 2017 15:34:41 +0200
Borislav Petkov <bp@alien8.de> escreveu:

> On Thu, Jul 20, 2017 at 07:50:03PM +0000, Kani, Toshimitsu wrote:
> > GHES / firmware-first still requires OS recovery actions when an error
> > cannot be corrected by the platform.  They are handled by ghes_proc(),
> > and ghes_edac remains its error-reporting wrapper. 

What happens when the error can be corrected? Does it still report it to
userspace, or just silently hide the error?

If I remember well about a past discussion with some vendor, I was told
that the firmware can hide some errors from being reported. Is it
still the case?


Thanks,
Mauro

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

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

On Tue, Jul 18, 2017 at 07:58:54PM +0000, Kani, Toshimitsu wrote:
> I have HPE Haswell and Skylake test systems with GHES, but they do not
> hide IMCs from the OS.  So, the sb_edac and skx_edac drivers get
> attached on these systems when ghes_edac is disabled.

That's how it is supposed to work. The platform drivers are the
fallback, practically.

But this is the important piece of info I was looking for - having GHES
enabled in the firmware does not prevent the platform drivers from
loading. But I think we have a better solution, the FF thing.

> Hmm... what's the platform name of this box?  I can look into this case
> if you need.

You can but that's not addressing the issue as a whole so it'll be a
waste of time.

-- 
Regards/Gruss,
    Boris.

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

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

#1690760 — RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac

> The question is: does the platform do this disabling now?
>
> Tony, I'm looking at sb_edac and there we don't do something like that
> or maybe I'm missing it.

Historically we've had complaints that sb_edac won't load that have been
tracked to BIOS hiding one of the (many) PCI devices that it needs.  But
device hiding is orthogonal to providing GHES error records.  A BIOS might
do that, but I don't know that anyone intentionally does so.

-Tony

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

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

On Tue, Jul 18, 2017 at 10:13:42PM +0000, Luck, Tony wrote:
> Historically we've had complaints that sb_edac won't load that have been
> tracked to BIOS hiding one of the (many) PCI devices that it needs.  But
> device hiding is orthogonal to providing GHES error records.  A BIOS might
> do that, but I don't know that anyone intentionally does so.

Yeah, the hiding-devices path doesn't look like the optimal one. I think we
should look at the firmware-first setting and load ghes if FF is being done by
the firmware.

-- 
Regards/Gruss,
    Boris.

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

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

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

On 7/17/2017 3:59 PM, Toshi Kani wrote:
> The ghes_edac driver was introduced in 2013 [1], but it has not
> been enabled by any distro yet.   

Ubuntu is expected to enable this soon.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

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

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

On Tue, 2017-07-18 at 08:39 -0600, Jeffrey Hugo wrote:
> On 7/17/2017 3:59 PM, Toshi Kani wrote:
> > The ghes_edac driver was introduced in 2013 [1], but it has not
> > been enabled by any distro yet.   
> 
> Ubuntu is expected to enable this soon.

Interesting.  I was told from other distro that there were many buggy
firmwares out there that prevented to enable ghes_edac.  Do you know if
Ubuntu has any plan to address such issue?  Or do they not see such
issue?  I do not test with other vendors' platforms, so I cannot tell
exactly what those bugs are...

Thanks, 
-Toshi

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

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

On 7/18/2017 9:36 AM, Kani, Toshimitsu wrote:
> On Tue, 2017-07-18 at 08:39 -0600, Jeffrey Hugo wrote:
>> On 7/17/2017 3:59 PM, Toshi Kani wrote:
>>> The ghes_edac driver was introduced in 2013 [1], but it has not
>>> been enabled by any distro yet.
>>
>> Ubuntu is expected to enable this soon.
> 
> Interesting.  I was told from other distro that there were many buggy
> firmwares out there that prevented to enable ghes_edac.  Do you know if
> Ubuntu has any plan to address such issue?  Or do they not see such
> issue?  I do not test with other vendors' platforms, so I cannot tell
> exactly what those bugs are...
> 

I do not know if Ubuntu intends to address any "known issues".  I know a 
request was made to Canonical to enable the option, and it appears the 
request is being considered, although the option may be limited to 
ARM64, depending on how Canonical's evaluation goes.  I am not aware of 
any particular issues, so I cannot say what the side effects are, or 
what platforms are considered to exhibit such issues.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

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

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

On Tue, 2017-07-18 at 10:24 -0600, Jeffrey Hugo wrote:
> On 7/18/2017 9:36 AM, Kani, Toshimitsu wrote:
> > On Tue, 2017-07-18 at 08:39 -0600, Jeffrey Hugo wrote:
> > > On 7/17/2017 3:59 PM, Toshi Kani wrote:
> > > > The ghes_edac driver was introduced in 2013 [1], but it has not
> > > > been enabled by any distro yet.
> > > 
> > > Ubuntu is expected to enable this soon.
> > 
> > Interesting.  I was told from other distro that there were many
> > buggy firmwares out there that prevented to enable ghes_edac.  Do
> > you know if Ubuntu has any plan to address such issue?  Or do they
> > not see such issue?  I do not test with other vendors' platforms,
> > so I cannot tell exactly what those bugs are...
> > 
> 
> I do not know if Ubuntu intends to address any "known issues".  I
> know a request was made to Canonical to enable the option, and it
> appears the request is being considered, although the option may be
> limited to ARM64, depending on how Canonical's evaluation goes.  I am
> not aware of any particular issues, so I cannot say what the side
> effects are, or what platforms are considered to exhibit such issues.

I see. Thanks for the info! I hope someone from Canonical is on the
list.
-Toshi

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

#1689530 — [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

ACPI OEM ID / OEM Table ID / Revision can be used to identify
platform type based on ACPI firmware.  acpi_blacklisted(),
intel_pstate_platform_pwr_mgmt_exists() and some other funcs
have been using this type of check to detect a list of platforms
that require special handlings.

Move the platform type check in acpi_blacklisted() to a common
utility function, acpi_match_oemlist(), so that other drivers
do not have to implement their own.

There is no change in functionality.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/blacklist.c |   84 ++++++++--------------------------------------
 drivers/acpi/utils.c     |   40 ++++++++++++++++++++++
 include/linux/acpi.h     |   19 ++++++++++
 3 files changed, 74 insertions(+), 69 deletions(-)

diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
index bb542ac..288fe4d 100644
--- a/drivers/acpi/blacklist.c
+++ b/drivers/acpi/blacklist.c
@@ -30,30 +30,13 @@
 
 #include "internal.h"
 
-enum acpi_blacklist_predicates {
-	all_versions,
-	less_than_or_equal,
-	equal,
-	greater_than_or_equal,
-};
-
-struct acpi_blacklist_item {
-	char oem_id[7];
-	char oem_table_id[9];
-	u32 oem_revision;
-	char *table;
-	enum acpi_blacklist_predicates oem_revision_predicate;
-	char *reason;
-	u32 is_critical_error;
-};
-
 static struct dmi_system_id acpi_rev_dmi_table[] __initdata;
 
 /*
  * POLICY: If *anything* doesn't work, put it on the blacklist.
  *	   If they are critical errors, mark it critical, and abort driver load.
  */
-static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
+static struct acpi_oemlist acpi_blacklist[] __initdata = {
 	/* Compaq Presario 1700 */
 	{"PTLTD ", "  DSDT  ", 0x06040000, ACPI_SIG_DSDT, less_than_or_equal,
 	 "Multiple problems", 1},
@@ -67,65 +50,28 @@ static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
 	{"IBM   ", "TP600E  ", 0x00000105, ACPI_SIG_DSDT, less_than_or_equal,
 	 "Incorrect _ADR", 1},
 
-	{""}
+	{ }
 };
 
 int __init acpi_blacklisted(void)
 {
-	int i = 0;
+	int i;
 	int blacklisted = 0;
-	struct acpi_table_header table_header;
-
-	while (acpi_blacklist[i].oem_id[0] != '\0') {
-		if (acpi_get_table_header(acpi_blacklist[i].table, 0, &table_header)) {
-			i++;
-			continue;
-		}
-
-		if (strncmp(acpi_blacklist[i].oem_id, table_header.oem_id, 6)) {
-			i++;
-			continue;
-		}
-
-		if (strncmp
-		    (acpi_blacklist[i].oem_table_id, table_header.oem_table_id,
-		     8)) {
-			i++;
-			continue;
-		}
-
-		if ((acpi_blacklist[i].oem_revision_predicate == all_versions)
-		    || (acpi_blacklist[i].oem_revision_predicate ==
-			less_than_or_equal
-			&& table_header.oem_revision <=
-			acpi_blacklist[i].oem_revision)
-		    || (acpi_blacklist[i].oem_revision_predicate ==
-			greater_than_or_equal
-			&& table_header.oem_revision >=
-			acpi_blacklist[i].oem_revision)
-		    || (acpi_blacklist[i].oem_revision_predicate == equal
-			&& table_header.oem_revision ==
-			acpi_blacklist[i].oem_revision)) {
 
-			printk(KERN_ERR PREFIX
-			       "Vendor \"%6.6s\" System \"%8.8s\" "
-			       "Revision 0x%x has a known ACPI BIOS problem.\n",
-			       acpi_blacklist[i].oem_id,
-			       acpi_blacklist[i].oem_table_id,
-			       acpi_blacklist[i].oem_revision);
+	i = acpi_match_oemlist(acpi_blacklist);
+	if (i >= 0) {
+		pr_err(PREFIX "Vendor \"%6.6s\" System \"%8.8s\" "
+		       "Revision 0x%x has a known ACPI BIOS problem.\n",
+		       acpi_blacklist[i].oem_id,
+		       acpi_blacklist[i].oem_table_id,
+		       acpi_blacklist[i].oem_revision);
 
-			printk(KERN_ERR PREFIX
-			       "Reason: %s. This is a %s error\n",
-			       acpi_blacklist[i].reason,
-			       (acpi_blacklist[i].
-				is_critical_error ? "non-recoverable" :
-				"recoverable"));
+		pr_err(PREFIX "Reason: %s. This is a %s error\n",
+		       acpi_blacklist[i].reason,
+		       (acpi_blacklist[i].data ?
+			"non-recoverable" : "recoverable"));
 
-			blacklisted = acpi_blacklist[i].is_critical_error;
-			break;
-		} else {
-			i++;
-		}
+		blacklisted = acpi_blacklist[i].data;
 	}
 
 	(void)early_acpi_osi_init();
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index b9d956c..e5909d5 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -816,3 +816,43 @@ static int __init acpi_backlight(char *str)
 	return 1;
 }
 __setup("acpi_backlight=", acpi_backlight);
+
+/**
+ * acpi_match_oemlist - Check if the system matches with an oem list
+ * @oem: pointer to acpi_oemlist table terminated by a NULL entry
+ *
+ * Return the matched index if the system is found in the oem list.
+ * Otherwise, return a negative error code.
+ */
+int acpi_match_oemlist(const struct acpi_oemlist *oem)
+{
+	struct acpi_table_header hdr;
+	int idx = 0;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	for (; oem->oem_id[0]; oem++, idx++) {
+		if (ACPI_FAILURE(acpi_get_table_header(oem->table, 0, &hdr)))
+			continue;
+
+		if (strncmp(oem->oem_id, hdr.oem_id, ACPI_OEM_ID_SIZE))
+			continue;
+
+		if (strncmp(oem->oem_table_id, hdr.oem_table_id,
+			    ACPI_OEM_TABLE_ID_SIZE))
+			continue;
+
+		if ((oem->oem_revision_predicate == all_versions) ||
+		    (oem->oem_revision_predicate == less_than_or_equal
+			&& hdr.oem_revision <= oem->oem_revision) ||
+		    (oem->oem_revision_predicate == greater_than_or_equal
+			&& hdr.oem_revision >= oem->oem_revision) ||
+		    (oem->oem_revision_predicate == equal
+			&& hdr.oem_revision == oem->oem_revision))
+			return idx;
+	}
+
+	return -ENODEV;
+}
+EXPORT_SYMBOL(acpi_match_oemlist);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index c749eef..86479b5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -556,6 +556,25 @@ extern acpi_status acpi_pci_osc_control_set(acpi_handle handle,
 #define ACPI_OST_SC_DRIVER_LOAD_FAILURE		0x81
 #define ACPI_OST_SC_INSERT_NOT_SUPPORTED	0x82
 
+enum acpi_oemlist_predicates {
+	all_versions,
+	less_than_or_equal,
+	equal,
+	greater_than_or_equal,
+};
+
+/* Table must be terminted by a NULL entry */
+struct acpi_oemlist {
+	char	oem_id[ACPI_OEM_ID_SIZE];
+	char	oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
+	u32	oem_revision;
+	char	*table;
+	enum	acpi_oemlist_predicates oem_revision_predicate;
+	char	*reason;
+	u32	data;
+};
+int acpi_match_oemlist(const struct acpi_oemlist *oem);
+
 extern void acpi_early_init(void);
 extern void acpi_subsystem_init(void);
 

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

#1689726 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Mon, Jul 17, 2017 at 03:59:10PM -0600, Toshi Kani wrote:
> ACPI OEM ID / OEM Table ID / Revision can be used to identify
> platform type based on ACPI firmware.  acpi_blacklisted(),
> intel_pstate_platform_pwr_mgmt_exists() and some other funcs
> have been using this type of check to detect a list of platforms
> that require special handlings.
> 
> Move the platform type check in acpi_blacklisted() to a common
> utility function, acpi_match_oemlist(), so that other drivers
> do not have to implement their own.
> 
> There is no change in functionality.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/acpi/blacklist.c |   84 ++++++++--------------------------------------
>  drivers/acpi/utils.c     |   40 ++++++++++++++++++++++
>  include/linux/acpi.h     |   19 ++++++++++
>  3 files changed, 74 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
> index bb542ac..288fe4d 100644
> --- a/drivers/acpi/blacklist.c
> +++ b/drivers/acpi/blacklist.c
> @@ -30,30 +30,13 @@
>  
>  #include "internal.h"
>  
> -enum acpi_blacklist_predicates {
> -	all_versions,
> -	less_than_or_equal,
> -	equal,
> -	greater_than_or_equal,
> -};
> -
> -struct acpi_blacklist_item {
> -	char oem_id[7];
> -	char oem_table_id[9];
> -	u32 oem_revision;
> -	char *table;
> -	enum acpi_blacklist_predicates oem_revision_predicate;
> -	char *reason;
> -	u32 is_critical_error;
> -};
> -
>  static struct dmi_system_id acpi_rev_dmi_table[] __initdata;
>  
>  /*
>   * POLICY: If *anything* doesn't work, put it on the blacklist.
>   *	   If they are critical errors, mark it critical, and abort driver load.
>   */
> -static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
> +static struct acpi_oemlist acpi_blacklist[] __initdata = {

Why the arbitrary rename?

If anything, you should shorten that

	enum acpi_blacklist_predicates oem_revision_predicate;

unreadable insanity.

>  	/* Compaq Presario 1700 */
>  	{"PTLTD ", "  DSDT  ", 0x06040000, ACPI_SIG_DSDT, less_than_or_equal,
>  	 "Multiple problems", 1},
> @@ -67,65 +50,28 @@ static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
>  	{"IBM   ", "TP600E  ", 0x00000105, ACPI_SIG_DSDT, less_than_or_equal,
>  	 "Incorrect _ADR", 1},
>  
> -	{""}
> +	{ }
>  };
>  
>  int __init acpi_blacklisted(void)
>  {
> -	int i = 0;
> +	int i;
>  	int blacklisted = 0;
> -	struct acpi_table_header table_header;
> -
> -	while (acpi_blacklist[i].oem_id[0] != '\0') {
> -		if (acpi_get_table_header(acpi_blacklist[i].table, 0, &table_header)) {
> -			i++;
> -			continue;
> -		}
> -
> -		if (strncmp(acpi_blacklist[i].oem_id, table_header.oem_id, 6)) {
> -			i++;
> -			continue;
> -		}
> -
> -		if (strncmp
> -		    (acpi_blacklist[i].oem_table_id, table_header.oem_table_id,
> -		     8)) {
> -			i++;
> -			continue;
> -		}
> -
> -		if ((acpi_blacklist[i].oem_revision_predicate == all_versions)
> -		    || (acpi_blacklist[i].oem_revision_predicate ==
> -			less_than_or_equal
> -			&& table_header.oem_revision <=
> -			acpi_blacklist[i].oem_revision)
> -		    || (acpi_blacklist[i].oem_revision_predicate ==
> -			greater_than_or_equal
> -			&& table_header.oem_revision >=
> -			acpi_blacklist[i].oem_revision)
> -		    || (acpi_blacklist[i].oem_revision_predicate == equal
> -			&& table_header.oem_revision ==
> -			acpi_blacklist[i].oem_revision)) {
>  
> -			printk(KERN_ERR PREFIX
> -			       "Vendor \"%6.6s\" System \"%8.8s\" "
> -			       "Revision 0x%x has a known ACPI BIOS problem.\n",
> -			       acpi_blacklist[i].oem_id,
> -			       acpi_blacklist[i].oem_table_id,
> -			       acpi_blacklist[i].oem_revision);
> +	i = acpi_match_oemlist(acpi_blacklist);
> +	if (i >= 0) {
> +		pr_err(PREFIX "Vendor \"%6.6s\" System \"%8.8s\" "
> +		       "Revision 0x%x has a known ACPI BIOS problem.\n",

Put that string on a single line for grepping. checkpatch catches that
error, didn't you see it?

> +		       acpi_blacklist[i].oem_id,
> +		       acpi_blacklist[i].oem_table_id,
> +		       acpi_blacklist[i].oem_revision);
>  
> -			printk(KERN_ERR PREFIX
> -			       "Reason: %s. This is a %s error\n",
> -			       acpi_blacklist[i].reason,
> -			       (acpi_blacklist[i].
> -				is_critical_error ? "non-recoverable" :
> -				"recoverable"));
> +		pr_err(PREFIX "Reason: %s. This is a %s error\n",
> +		       acpi_blacklist[i].reason,
> +		       (acpi_blacklist[i].data ?
> +			"non-recoverable" : "recoverable"));
>  
> -			blacklisted = acpi_blacklist[i].is_critical_error;
> -			break;
> -		} else {
> -			i++;
> -		}
> +		blacklisted = acpi_blacklist[i].data;
>  	}
>  
>  	(void)early_acpi_osi_init();

-- 
Regards/Gruss,
    Boris.

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

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

#1690378 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, 2017-07-18 at 07:34 +0200, Borislav Petkov wrote:
> On Mon, Jul 17, 2017 at 03:59:10PM -0600, Toshi Kani wrote:
> > ACPI OEM ID / OEM Table ID / Revision can be used to identify
> > platform type based on ACPI firmware.  acpi_blacklisted(),
> > intel_pstate_platform_pwr_mgmt_exists() and some other funcs
> > have been using this type of check to detect a list of platforms
> > that require special handlings.
> > 
> > Move the platform type check in acpi_blacklisted() to a common
> > utility function, acpi_match_oemlist(), so that other drivers
> > do not have to implement their own.
> > 
> > There is no change in functionality.
 :
> >  /*
> >   * POLICY: If *anything* doesn't work, put it on the blacklist.
> >   *	   If they are critical errors, mark it critical, and
> > abort driver load.
> >   */
> > -static struct acpi_blacklist_item acpi_blacklist[] __initdata = {
> > +static struct acpi_oemlist acpi_blacklist[] __initdata = {
> 
> Why the arbitrary rename?

This patch defines 'struct acpi_oemlist' in "include/linux/acpi.h" as a
common structure, and replaces this specific 'struct acpi_blacklist'.

> If anything, you should shorten that
> 
> 	enum acpi_blacklist_predicates oem_revision_predicate;
> 
> unreadable insanity.

Agreed.  Will change to a shorter name like below. 

	enum acpi_oemlist_pred predicate;

+	i = acpi_match_oemlist(acpi_blacklist);
> > +	if (i >= 0) {
> > +		pr_err(PREFIX "Vendor \"%6.6s\" System \"%8.8s\" "
> > +		       "Revision 0x%x has a known ACPI BIOS
> > problem.\n",
> 
> Put that string on a single line for grepping. checkpatch catches
> that error, didn't you see it?

Will do.

Thanks!
-Toshi

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

#1690440 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, Jul 18, 2017 at 03:48:54PM +0000, Kani, Toshimitsu wrote:
> This patch defines 'struct acpi_oemlist' in "include/linux/acpi.h" as a

I see that.

> common structure, and replaces this specific 'struct acpi_blacklist'.

And what makes acpi_oemlist "common" and acpi_blacklist "specific"?

So let me save you some time - "oemlist" is more specific than
"blacklist" and I can imagine a blacklist item not always being
oem-specific.

What I'm hinting at is, don't change that name. acpi_blacklist is just
fine.

> Agreed.  Will change to a shorter name like below. 
> 
> 	enum acpi_oemlist_pred predicate;

	enum acpi_predicate pred;

is even better.

-- 
Regards/Gruss,
    Boris.

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

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

#1690507 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, 2017-07-18 at 18:43 +0200, Borislav Petkov wrote:
> On Tue, Jul 18, 2017 at 03:48:54PM +0000, Kani, Toshimitsu wrote:
> > This patch defines 'struct acpi_oemlist' in "include/linux/acpi.h"
> > as a
> 
> I see that.
> 
> > common structure, and replaces this specific 'struct
> > acpi_blacklist'.
> 
> And what makes acpi_oemlist "common" and acpi_blacklist "specific"?
> 
> So let me save you some time - "oemlist" is more specific than
> "blacklist" and I can imagine a blacklist item not always being
> oem-specific.
> 
> What I'm hinting at is, don't change that name. acpi_blacklist is
> just fine.

Well, a list does not need to be a black-list.  It can be a white-list
or anything that matters.  The caller defines the usage of a list.  So,
I tried to avoid putting any usage to the structure name.

> > Agreed.  Will change to a shorter name like below. 
> > 
> > 	enum acpi_oemlist_pred predicate;
> 
> 	enum acpi_predicate pred;
> 
> is even better.

Sounds good.

Thanks,
-Toshi

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

#1690529 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, Jul 18, 2017 at 05:24:50PM +0000, Kani, Toshimitsu wrote:
> Well, a list does not need to be a black-list.

But this one *is* a blacklist.

> So, I tried to avoid putting any usage to the structure name.

So OEM is a usage. The moment you need to use it for something else
besides an OEM, it is not an OEM list anymore - it is a generic
blacklist which blacklists OEMs too.

-- 
Regards/Gruss,
    Boris.

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

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

#1690570 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, 2017-07-18 at 19:42 +0200, Borislav Petkov wrote:
> On Tue, Jul 18, 2017 at 05:24:50PM +0000, Kani, Toshimitsu wrote:
> > Well, a list does not need to be a black-list.
> 
> But this one *is* a blacklist.

Right.  Hence, acpi_backlisted() still declares the list as
'acpi_blacklist[]'.

> > So, I tried to avoid putting any usage to the structure name.
> 
> So OEM is a usage. The moment you need to use it for something else
> besides an OEM, it is not an OEM list anymore - it is a generic
> blacklist which blacklists OEMs too.

The term "oem" represents data types of the structure, oem_id[],
oem_table_id[], and oem_revision, which are defined by the ACPI spec.  

ghes_edac uses this structure as a while-list, so the term backlist is
misleading.  intel_pstate also uses it to list the platforms that do
not need OS control.

Thanks,
-Toshi

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

#1690592 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, Jul 18, 2017 at 06:49:51PM +0000, Kani, Toshimitsu wrote:
> ghes_edac uses this structure as a while-list, so the term backlist is
> misleading.

So this matching function gets both blacklists and whitelists. No wonder
it is confusing. Now I finally understand what you wanna do: you want to
call all those lists something agnostic as platform_list or so because
they contain exactly that: platforms - not OEMs.

And then you want to match *platforms*. *Not* OEMs.

*Now* I understand what you're trying to tell me.

-- 
Regards/Gruss,
    Boris.

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

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

#1690627 — Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface

On Tue, 2017-07-18 at 21:32 +0200, Borislav Petkov wrote:
> On Tue, Jul 18, 2017 at 06:49:51PM +0000, Kani, Toshimitsu wrote:
> > ghes_edac uses this structure as a while-list, so the term backlist
> > is misleading.
> 
> So this matching function gets both blacklists and whitelists. No
> wonder it is confusing. Now I finally understand what you wanna do:
> you want to call all those lists something agnostic as platform_list
> or so because they contain exactly that: platforms - not OEMs.

Right.

> And then you want to match *platforms*. *Not* OEMs.

True, there is some stretch to use OEMIDs for detecting platforms.  But
we do not have other standard interfaces better than this one.

> *Now* I understand what you're trying to tell me.

:-)

Thanks,
-Toshi

[toc] | [prev] | [standalone]

Page 4 of 4 — ← Prev page 1 2 3 [4]

Back to top | Article view | linux.kernel

csiph-web