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

Groups > linux.kernel > #1715398 > unrolled thread

[PATCH v3 0/5] enable ghes_edac on selected platforms

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/5] enable ghes_edac on selected platforms Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
    [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
      Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() Borislav Petkov <bp@alien8.de> - 2017-08-21 13:30 +0200
        Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-08-21 14:30 +0200
          [PATCH] ACPICA: Check whether ACPI is disabled before getting a table Borislav Petkov <bp@alien8.de> - 2017-08-21 15:30 +0200
            Re: [PATCH] ACPICA: Check whether ACPI is disabled before getting a table "Rafael J. Wysocki" <rafael@kernel.org> - 2017-08-21 15:40 +0200
              Re: [PATCH] ACPICA: Check whether ACPI is disabled before getting a  table Borislav Petkov <bp@alien8.de> - 2017-08-21 17:40 +0200
        Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-08-21 18:50 +0200
          Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() Borislav Petkov <bp@alien8.de> - 2017-08-21 19:10 +0200
            Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-08-21 19:30 +0200
              Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() Borislav Petkov <bp@alien8.de> - 2017-08-21 19:40 +0200
                Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-08-21 22:40 +0200
                  Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-08-21 23:10 +0200
                    Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-08-21 23:50 +0200
                      Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-08-22 00:30 +0200
                        Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-08-22 00:30 +0200
    [PATCH v3 4/5] EDAC: add edac_get_owner() to check MC owner Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
    [PATCH v3 5/5] edac drivers: add MC owner check in init Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
    [PATCH v3 2/5] intel_pstate: convert to use acpi_match_platform_list() Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
      Re: [PATCH v3 2/5] intel_pstate: convert to use  acpi_match_platform_list() Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2017-08-21 20:00 +0200
      Re: [PATCH v3 2/5] intel_pstate: convert to use  acpi_match_platform_list() Borislav Petkov <bp@alien8.de> - 2017-08-23 17:50 +0200
        Re: [PATCH v3 2/5] intel_pstate: convert to use  acpi_match_platform_list() "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-08-23 18:00 +0200
    [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac Toshi Kani <toshi.kani@hpe.com> - 2017-08-18 22:00 +0200
      Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-08-23 18:30 +0200
        Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-08-23 23:00 +0200
          Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-08-24 10:00 +0200

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

#1718467 — Re: [PATCH v3 2/5] intel_pstate: convert to use acpi_match_platform_list()

On Fri, Aug 18, 2017 at 01:46:41PM -0600, Toshi Kani wrote:
> Convert to use acpi_match_platform_list() for the platform check.
> There is no change in functionality.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Borislav Petkov <bp@alien8.de>
> ---
>  drivers/cpufreq/intel_pstate.c |   64 ++++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 65ee4fc..ad713cd 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2466,39 +2466,31 @@ enum {
>  	PPC,
>  };
>  
> -struct hw_vendor_info {
> -	u16  valid;
> -	char oem_id[ACPI_OEM_ID_SIZE];
> -	char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
> -	int  oem_pwr_table;
> -};
> -
>  /* Hardware vendor-specific info that has its own power management modes */
> -static struct hw_vendor_info vendor_info[] __initdata = {
> -	{1, "HP    ", "ProLiant", PSS},
> -	{1, "ORACLE", "X4-2    ", PPC},
> -	{1, "ORACLE", "X4-2L   ", PPC},
> -	{1, "ORACLE", "X4-2B   ", PPC},
> -	{1, "ORACLE", "X3-2    ", PPC},
> -	{1, "ORACLE", "X3-2L   ", PPC},
> -	{1, "ORACLE", "X3-2B   ", PPC},
> -	{1, "ORACLE", "X4470M2 ", PPC},
> -	{1, "ORACLE", "X4270M3 ", PPC},
> -	{1, "ORACLE", "X4270M2 ", PPC},
> -	{1, "ORACLE", "X4170M2 ", PPC},
> -	{1, "ORACLE", "X4170 M3", PPC},
> -	{1, "ORACLE", "X4275 M3", PPC},
> -	{1, "ORACLE", "X6-2    ", PPC},
> -	{1, "ORACLE", "Sudbury ", PPC},
> -	{0, "", ""},
> +static struct acpi_platform_list plat_info[] __initdata = {
> +	{"HP    ", "ProLiant", 0, ACPI_SIG_FADT, all_versions, 0, PSS},

Btw, why is that ACPI_SIG_FADT's description not "FADT" ?

#define ACPI_SIG_FADT           "FACP"  /* Fixed ACPI Description Table */

More ACPI fun? I don't think I can take any more fun.

Oh well,

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

#1718476 — Re: [PATCH v3 2/5] intel_pstate: convert to use acpi_match_platform_list()

On Wed, 2017-08-23 at 17:46 +0200, Borislav Petkov wrote:
> On Fri, Aug 18, 2017 at 01:46:41PM -0600, Toshi Kani wrote:
> > Convert to use acpi_match_platform_list() for the platform check.
> > There is no change in functionality.
> > 
 :
> 
> Btw, why is that ACPI_SIG_FADT's description not "FADT" ?
> 
> #define ACPI_SIG_FADT           "FACP"  /* Fixed ACPI Description
> Table */
> 
> More ACPI fun? I don't think I can take any more fun.

Yes, more ACPI fun. :-)  According to the spec:

‘FACP’. Signature for the Fixed ACPI Description Table. (This signature
predates ACPI 1.0, explaining the mismatch with this table's name.)

> Oh well,
> 
> Reviewed-by: Borislav Petkov <bp@suse.de>

Thanks!
-Toshi

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

#1715420 — [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac

The ghes_edac driver was introduced in 2013 [1], but it has not
been enabled by any distro yet.  This driver obtains error info
from firmware interfaces, which are not properly implemented on
many platforms, as the driver always emits the messages below:

 This EDAC driver relies on BIOS to enumerate memory and get error reports.
 Unfortunately, not all BIOSes reflect the memory layout correctly
 So, the end result of using this driver varies from vendor to vendor
 If you find incorrect reports, please contact your hardware vendor
 to correct its BIOS.

To get out from this situation, add a platform check to selectively
enable the driver on the platforms that are known to have proper
firmware implementation.  Platform vendors can add their platforms
to the list when they support ghes_edac.

"ghes_edac.force_load=1" skips this platform check.

[1]: https://lwn.net/Articles/538438/
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
---
 drivers/edac/ghes_edac.c |   28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 4e61a62..367e106 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -34,6 +34,9 @@ static LIST_HEAD(ghes_reglist);
 static DEFINE_MUTEX(ghes_edac_lock);
 static int ghes_edac_mc_num;
 
+/* Set 1 to skip the platform check */
+static bool __read_mostly ghes_edac_force_load;
+module_param_named(force_load, ghes_edac_force_load, bool, 0);
 
 /* Memory Device - Type 17 of SMBIOS spec */
 struct memdev_dmi_entry {
@@ -405,6 +408,15 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
 }
 EXPORT_SYMBOL_GPL(ghes_edac_report_mem_error);
 
+/*
+ * Known systems that are safe to enable this module.
+ * "ghes_edac.force_load=1" skips this check if necessary.
+ */
+static struct acpi_platform_list plat_list[] = {
+	{"HPE   ", "Server  ", 0, ACPI_SIG_FADT, all_versions},
+	{ } /* End */
+};
+
 int ghes_edac_register(struct ghes *ghes, struct device *dev)
 {
 	bool fake = false;
@@ -413,6 +425,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
 	struct edac_mc_layer layers[1];
 	struct ghes_edac_pvt *pvt;
 	struct ghes_edac_dimm_fill dimm_fill;
+	int idx;
+
+	/* Check if safe to enable on this system */
+	idx = acpi_match_platform_list(plat_list);
+	if (!ghes_edac_force_load && idx < 0)
+		return 0;
 
 	/* Get the number of DIMMs */
 	dmi_walk(ghes_edac_count_dimms, &num_dimm);
@@ -456,7 +474,11 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
 	mci->dev_name = "ghes";
 
 	if (!ghes_edac_mc_num) {
-		if (!fake) {
+		if (fake) {
+			pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
+			pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
+			pr_info("work on such system. Use this driver with caution\n");
+		} else if (idx < 0) {
 			pr_info("This EDAC driver relies on BIOS to enumerate memory and get error reports.\n");
 			pr_info("Unfortunately, not all BIOSes reflect the memory layout correctly.\n");
 			pr_info("So, the end result of using this driver varies from vendor to vendor.\n");
@@ -464,10 +486,6 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
 			pr_info("to correct its BIOS.\n");
 			pr_info("This system has %d DIMM sockets.\n",
 				num_dimm);
-		} else {
-			pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
-			pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
-			pr_info("work on such system. Use this driver with caution\n");
 		}
 	}
 

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

#1718499 — Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac

On Fri, Aug 18, 2017 at 01:46:42PM -0600, Toshi Kani wrote:
> The ghes_edac driver was introduced in 2013 [1], but it has not
> been enabled by any distro yet.  This driver obtains error info
> from firmware interfaces, which are not properly implemented on
> many platforms, as the driver always emits the messages below:
> 
>  This EDAC driver relies on BIOS to enumerate memory and get error reports.
>  Unfortunately, not all BIOSes reflect the memory layout correctly
>  So, the end result of using this driver varies from vendor to vendor
>  If you find incorrect reports, please contact your hardware vendor
>  to correct its BIOS.
> 
> To get out from this situation, add a platform check to selectively
> enable the driver on the platforms that are known to have proper
> firmware implementation.  Platform vendors can add their platforms
> to the list when they support ghes_edac.
> 
> "ghes_edac.force_load=1" skips this platform check.
> 
> [1]: https://lwn.net/Articles/538438/
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Tony Luck <tony.luck@intel.com>
> ---
>  drivers/edac/ghes_edac.c |   28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)

Ok, for the remaining three, I've updated my "ghes" branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=ghes

Please, redo them ontop.

@Rafael: how do you want to handle this?

The first two are ACPI patches and the remaining three are EDAC. It
would be probably easier if you acked the ACPI ones (but wait until
Toshi's next version) and took them all through the EDAC tree as I have
two more reworking that ghes_edac driver.

Alternatively, they could all go through the ACPI tree but you'll have
to pick them all up together. That shouldn't be a problem either as all
changes are solely to drivers/edac/ghes_edac.c and there's one other
patch in my EDAC pile which touches ghes_edac.c:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/diff/drivers/edac/ghes_edac.c?h=for-next&id=c54182ec0e157988f0cafd1e8d37b68ab4210f87

That's why I say, it'll be easier if I carried them all. :)

But I'm sure we can work something out.

Thanks.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

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

#1718646 — Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac

On Wednesday, August 23, 2017 6:20:53 PM CEST Borislav Petkov wrote:
> On Fri, Aug 18, 2017 at 01:46:42PM -0600, Toshi Kani wrote:
> > The ghes_edac driver was introduced in 2013 [1], but it has not
> > been enabled by any distro yet.  This driver obtains error info
> > from firmware interfaces, which are not properly implemented on
> > many platforms, as the driver always emits the messages below:
> > 
> >  This EDAC driver relies on BIOS to enumerate memory and get error reports.
> >  Unfortunately, not all BIOSes reflect the memory layout correctly
> >  So, the end result of using this driver varies from vendor to vendor
> >  If you find incorrect reports, please contact your hardware vendor
> >  to correct its BIOS.
> > 
> > To get out from this situation, add a platform check to selectively
> > enable the driver on the platforms that are known to have proper
> > firmware implementation.  Platform vendors can add their platforms
> > to the list when they support ghes_edac.
> > 
> > "ghes_edac.force_load=1" skips this platform check.
> > 
> > [1]: https://lwn.net/Articles/538438/
> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: Tony Luck <tony.luck@intel.com>
> > ---
> >  drivers/edac/ghes_edac.c |   28 +++++++++++++++++++++++-----
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> Ok, for the remaining three, I've updated my "ghes" branch here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=ghes
> 
> Please, redo them ontop.
> 
> @Rafael: how do you want to handle this?
> 
> The first two are ACPI patches and the remaining three are EDAC. It
> would be probably easier if you acked the ACPI ones (but wait until
> Toshi's next version) and took them all through the EDAC tree as I have
> two more reworking that ghes_edac driver.

So I have some pending intel_pstate and the intel_pstate changes are likely
to conflict with it.

> Alternatively, they could all go through the ACPI tree but you'll have
> to pick them all up together.

That can be done. :-)

> That shouldn't be a problem either as all
> changes are solely to drivers/edac/ghes_edac.c and there's one other
> patch in my EDAC pile which touches ghes_edac.c:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/diff/drivers/edac/ghes_edac.c?h=for-next&id=c54182ec0e157988f0cafd1e8d37b68ab4210f87
> 
> That's why I say, it'll be easier if I carried them all. :)
> 
> But I'm sure we can work something out.

I can expose a branch with this series for you to merge if that helps.

Thanks,
Rafael

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

#1718940 — Re: [PATCH v3 3/5] ghes_edac: add platform check to enable ghes_edac

On Wed, Aug 23, 2017 at 10:46:42PM +0200, Rafael J. Wysocki wrote:
> I can expose a branch with this series for you to merge if that helps.

I think that'll be the best solution. So yes, pls lemme know when you
have it so that I can pick up the rest.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [standalone]

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

Back to top | Article view | linux.kernel

csiph-web