diff mbox

[v2,3/7] ACPI / APEI: add OSC APEI bit check for ghes_edac

Message ID 20170803215753.30553-4-toshi.kani@hpe.com (mailing list archive)
State Rejected, archived
Headers show

Commit Message

Kani, Toshi Aug. 3, 2017, 9:57 p.m. UTC
When 'osc_sb_apei_support_acked' is set, it indicates that
the platform supports APEI, firmware-first mode, as ACPI _OSC
capability bit 4, APEI Support, was set in query.  While _OSC
is an optional method, platforms with APEI support should
implement it to inform its capability to the OS properly.

Add check to 'osc_sb_apei_support_acked' before calling
ghes_edac_register() to restrict that ghes_edac is enabled
on platforms with APEI support capability set in _OSC.

Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
---
 drivers/acpi/apei/ghes.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Borislav Petkov Aug. 4, 2017, 3:44 a.m. UTC | #1
On Thu, Aug 03, 2017 at 03:57:49PM -0600, Toshi Kani wrote:
> When 'osc_sb_apei_support_acked' is set, it indicates that
> the platform supports APEI, firmware-first mode, as ACPI _OSC
> capability bit 4, APEI Support, was set in query.  While _OSC
> is an optional method, platforms with APEI support should
> implement it to inform its capability to the OS properly.
> 
> Add check to 'osc_sb_apei_support_acked' before calling
> ghes_edac_register() to restrict that ghes_edac is enabled
> on platforms with APEI support capability set in _OSC.
> 
> Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Tony Luck <tony.luck@intel.com>
> ---
>  drivers/acpi/apei/ghes.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index d661d45..49d75eb 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1139,9 +1139,11 @@ static int ghes_probe(struct platform_device *ghes_dev)
>  		goto err;
>  	}
>  
> -	rc = ghes_edac_register(ghes, &ghes_dev->dev);
> -	if (rc < 0)
> -		goto err;
> +	if (osc_sb_apei_support_acked) {

What for when we have the whitelist?
Kani, Toshi Aug. 4, 2017, 8:49 p.m. UTC | #2
On Fri, 2017-08-04 at 05:44 +0200, Borislav Petkov wrote:
> On Thu, Aug 03, 2017 at 03:57:49PM -0600, Toshi Kani wrote:

> > When 'osc_sb_apei_support_acked' is set, it indicates that

> > the platform supports APEI, firmware-first mode, as ACPI _OSC

> > capability bit 4, APEI Support, was set in query.  While _OSC

> > is an optional method, platforms with APEI support should

> > implement it to inform its capability to the OS properly.

> > 

> > Add check to 'osc_sb_apei_support_acked' before calling

> > ghes_edac_register() to restrict that ghes_edac is enabled

> > on platforms with APEI support capability set in _OSC.

> > 

> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com>

> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>

> > Cc: Borislav Petkov <bp@alien8.de>

> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>

> > Cc: Tony Luck <tony.luck@intel.com>

> > ---

> >  drivers/acpi/apei/ghes.c |    8 +++++---

> >  1 file changed, 5 insertions(+), 3 deletions(-)

> > 

> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c

> > index d661d45..49d75eb 100644

> > --- a/drivers/acpi/apei/ghes.c

> > +++ b/drivers/acpi/apei/ghes.c

> > @@ -1139,9 +1139,11 @@ static int ghes_probe(struct platform_device

> > *ghes_dev)

> >  		goto err;

> >  	}

> >  

> > -	rc = ghes_edac_register(ghes, &ghes_dev->dev);

> > -	if (rc < 0)

> > -		goto err;

> > +	if (osc_sb_apei_support_acked) {

> 

> What for when we have the whitelist?


Some firmware features can be enabled / disabled in BIOS.  While HPE
firmware does not allow to disable FF, it's possible that other vendors
might allow such and still have the same OEM ID info.  So, this check
makes sure that FF is in fact enabled in firmware.

Thanks,
-Toshi
Borislav Petkov Aug. 5, 2017, 5:14 a.m. UTC | #3
On Fri, Aug 04, 2017 at 08:49:51PM +0000, Kani, Toshimitsu wrote:
> Some firmware features can be enabled / disabled in BIOS.  While HPE
> firmware does not allow to disable FF, it's possible that other vendors
> might allow such and still have the same OEM ID info.

Yeah, we don't protect ourselves against "might allow". Either you have
an *actual* example for such platform or you don't and then you don't
need this check.
Kani, Toshi Aug. 7, 2017, 2:50 p.m. UTC | #4
On Sat, 2017-08-05 at 07:14 +0200, Borislav Petkov wrote:
> On Fri, Aug 04, 2017 at 08:49:51PM +0000, Kani, Toshimitsu wrote:

> > Some firmware features can be enabled / disabled in BIOS.  While

> > HPE firmware does not allow to disable FF, it's possible that other

> > vendors might allow such and still have the same OEM ID info.

> 

> Yeah, we don't protect ourselves against "might allow". Either you

> have an *actual* example for such platform or you don't and then you

> don't need this check.


No, I do not any actual example.  Will drop this patch.

Thanks,
-Toshi
diff mbox

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d661d45..49d75eb 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1139,9 +1139,11 @@  static int ghes_probe(struct platform_device *ghes_dev)
 		goto err;
 	}
 
-	rc = ghes_edac_register(ghes, &ghes_dev->dev);
-	if (rc < 0)
-		goto err;
+	if (osc_sb_apei_support_acked) {
+		rc = ghes_edac_register(ghes, &ghes_dev->dev);
+		if (rc < 0)
+			goto err;
+	}
 
 	switch (generic->notify.type) {
 	case ACPI_HEST_NOTIFY_POLLED: