* [PATCH v2] ACPI / APEI: Suppress message if HEST not present
@ 2017-08-16 11:27 Punit Agrawal
2017-08-29 9:03 ` Punit Agrawal
2017-08-29 12:38 ` Borislav Petkov
0 siblings, 2 replies; 4+ messages in thread
From: Punit Agrawal @ 2017-08-16 11:27 UTC (permalink / raw)
To: linux-acpi
Cc: Punit Agrawal, lorenzo.pieralisi, sudeep.holla, linux-kernel,
Borislav Petkov, Rafael J . Wysocki, James Morse
According to the ACPI specification, firmware is not required to provide
the Hardware Error Source Table (HEST). When HEST is not present, the
following superfluous message is printed to the kernel boot log -
[ 3.460067] GHES: HEST is not enabled!
Extend hest_disable variable to track whether the firmware provides this
table and if it is not present skip any log output. The existing
behaviour is preserved in all other cases.
Suggested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: James Morse <james.morse@arm.com>
---
drivers/acpi/apei/ghes.c | 4 ++--
drivers/acpi/apei/hest.c | 13 +++++++------
include/acpi/apei.h | 8 +++++++-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d661d452b238..f8685bcbeff2 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1262,10 +1262,10 @@ static int __init ghes_init(void)
{
int rc;
- if (acpi_disabled)
+ if (acpi_disabled || hest_disable == HEST_NOT_FOUND)
return -ENODEV;
- if (hest_disable) {
+ if (hest_disable == HEST_DISABLED) {
pr_info(GHES_PFX "HEST is not enabled!\n");
return -EINVAL;
}
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 456b488eb1df..9cb74115a43d 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -37,7 +37,7 @@
#define HEST_PFX "HEST: "
-bool hest_disable;
+int hest_disable;
EXPORT_SYMBOL_GPL(hest_disable);
/* HEST table parsing */
@@ -213,7 +213,7 @@ static int __init hest_ghes_dev_register(unsigned int ghes_count)
static int __init setup_hest_disable(char *str)
{
- hest_disable = 1;
+ hest_disable = HEST_DISABLED;
return 0;
}
@@ -232,9 +232,10 @@ void __init acpi_hest_init(void)
status = acpi_get_table(ACPI_SIG_HEST, 0,
(struct acpi_table_header **)&hest_tab);
- if (status == AE_NOT_FOUND)
- goto err;
- else if (ACPI_FAILURE(status)) {
+ if (status == AE_NOT_FOUND) {
+ hest_disable = HEST_NOT_FOUND;
+ return;
+ } else if (ACPI_FAILURE(status)) {
const char *msg = acpi_format_exception(status);
pr_err(HEST_PFX "Failed to get table, %s\n", msg);
rc = -EINVAL;
@@ -257,5 +258,5 @@ void __init acpi_hest_init(void)
pr_info(HEST_PFX "Table parsing has been initialized.\n");
return;
err:
- hest_disable = 1;
+ hest_disable = HEST_DISABLED;
}
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index 76284bb560a6..c46694abea28 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -16,7 +16,13 @@
#ifdef __KERNEL__
-extern bool hest_disable;
+enum hest_status {
+ HEST_ENABLED,
+ HEST_DISABLED,
+ HEST_NOT_FOUND,
+};
+
+extern int hest_disable;
extern int erst_disable;
#ifdef CONFIG_ACPI_APEI_GHES
extern bool ghes_disable;
--
2.13.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] ACPI / APEI: Suppress message if HEST not present
2017-08-16 11:27 [PATCH v2] ACPI / APEI: Suppress message if HEST not present Punit Agrawal
@ 2017-08-29 9:03 ` Punit Agrawal
2017-08-29 12:38 ` Borislav Petkov
1 sibling, 0 replies; 4+ messages in thread
From: Punit Agrawal @ 2017-08-29 9:03 UTC (permalink / raw)
To: Borislav Petkov
Cc: lorenzo.pieralisi, sudeep.holla, linux-kernel,
Rafael J . Wysocki, James Morse, linux-acpi
Punit Agrawal <punit.agrawal@arm.com> writes:
> According to the ACPI specification, firmware is not required to provide
> the Hardware Error Source Table (HEST). When HEST is not present, the
> following superfluous message is printed to the kernel boot log -
>
> [ 3.460067] GHES: HEST is not enabled!
>
> Extend hest_disable variable to track whether the firmware provides this
> table and if it is not present skip any log output. The existing
> behaviour is preserved in all other cases.
>
> Suggested-by: Borislav Petkov <bp@suse.de>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: James Morse <james.morse@arm.com>
> ---
> drivers/acpi/apei/ghes.c | 4 ++--
> drivers/acpi/apei/hest.c | 13 +++++++------
> include/acpi/apei.h | 8 +++++++-
> 3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index d661d452b238..f8685bcbeff2 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1262,10 +1262,10 @@ static int __init ghes_init(void)
> {
> int rc;
>
> - if (acpi_disabled)
> + if (acpi_disabled || hest_disable == HEST_NOT_FOUND)
> return -ENODEV;
>
> - if (hest_disable) {
> + if (hest_disable == HEST_DISABLED) {
> pr_info(GHES_PFX "HEST is not enabled!\n");
> return -EINVAL;
> }
> diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
> index 456b488eb1df..9cb74115a43d 100644
> --- a/drivers/acpi/apei/hest.c
> +++ b/drivers/acpi/apei/hest.c
> @@ -37,7 +37,7 @@
>
> #define HEST_PFX "HEST: "
>
> -bool hest_disable;
> +int hest_disable;
> EXPORT_SYMBOL_GPL(hest_disable);
>
> /* HEST table parsing */
> @@ -213,7 +213,7 @@ static int __init hest_ghes_dev_register(unsigned int ghes_count)
>
> static int __init setup_hest_disable(char *str)
> {
> - hest_disable = 1;
> + hest_disable = HEST_DISABLED;
> return 0;
> }
>
> @@ -232,9 +232,10 @@ void __init acpi_hest_init(void)
>
> status = acpi_get_table(ACPI_SIG_HEST, 0,
> (struct acpi_table_header **)&hest_tab);
> - if (status == AE_NOT_FOUND)
> - goto err;
> - else if (ACPI_FAILURE(status)) {
> + if (status == AE_NOT_FOUND) {
> + hest_disable = HEST_NOT_FOUND;
> + return;
> + } else if (ACPI_FAILURE(status)) {
> const char *msg = acpi_format_exception(status);
> pr_err(HEST_PFX "Failed to get table, %s\n", msg);
> rc = -EINVAL;
> @@ -257,5 +258,5 @@ void __init acpi_hest_init(void)
> pr_info(HEST_PFX "Table parsing has been initialized.\n");
> return;
> err:
> - hest_disable = 1;
> + hest_disable = HEST_DISABLED;
> }
> diff --git a/include/acpi/apei.h b/include/acpi/apei.h
> index 76284bb560a6..c46694abea28 100644
> --- a/include/acpi/apei.h
> +++ b/include/acpi/apei.h
> @@ -16,7 +16,13 @@
>
> #ifdef __KERNEL__
>
> -extern bool hest_disable;
> +enum hest_status {
> + HEST_ENABLED,
> + HEST_DISABLED,
> + HEST_NOT_FOUND,
> +};
> +
> +extern int hest_disable;
> extern int erst_disable;
> #ifdef CONFIG_ACPI_APEI_GHES
> extern bool ghes_disable;
Ping!
Looks like this one has slipped through the cracks? Any objections
Boris?
Thanks,
Punit
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] ACPI / APEI: Suppress message if HEST not present
2017-08-16 11:27 [PATCH v2] ACPI / APEI: Suppress message if HEST not present Punit Agrawal
2017-08-29 9:03 ` Punit Agrawal
@ 2017-08-29 12:38 ` Borislav Petkov
2017-08-29 13:17 ` Punit Agrawal
1 sibling, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2017-08-29 12:38 UTC (permalink / raw)
To: Punit Agrawal
Cc: linux-acpi, lorenzo.pieralisi, sudeep.holla, linux-kernel,
Rafael J . Wysocki, James Morse
On Wed, Aug 16, 2017 at 12:27:53PM +0100, Punit Agrawal wrote:
> According to the ACPI specification, firmware is not required to provide
> the Hardware Error Source Table (HEST). When HEST is not present, the
> following superfluous message is printed to the kernel boot log -
>
> [ 3.460067] GHES: HEST is not enabled!
>
> Extend hest_disable variable to track whether the firmware provides this
> table and if it is not present skip any log output. The existing
> behaviour is preserved in all other cases.
>
> Suggested-by: Borislav Petkov <bp@suse.de>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: James Morse <james.morse@arm.com>
> ---
> drivers/acpi/apei/ghes.c | 4 ++--
> drivers/acpi/apei/hest.c | 13 +++++++------
> include/acpi/apei.h | 8 +++++++-
> 3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index d661d452b238..f8685bcbeff2 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1262,10 +1262,10 @@ static int __init ghes_init(void)
> {
> int rc;
>
> - if (acpi_disabled)
> + if (acpi_disabled || hest_disable == HEST_NOT_FOUND)
> return -ENODEV;
>
> - if (hest_disable) {
> + if (hest_disable == HEST_DISABLED) {
> pr_info(GHES_PFX "HEST is not enabled!\n");
> return -EINVAL;
> }
Yap, looks good.
Just a minor nitpick: I'd additionally group the hest_disable checking
in one switch-case, so that the code flow is obvious at a quick glance:
if (acpi_disabled)
return -ENODEV;
switch (hest_disable) {
case HEST_NOT_FOUND:
return -ENODEV;
case HEST_DISABLED:
pr_info(GHES_PFX "HEST is not enabled!\n");
return -EINVAL;
default:
break;
}
Thx.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] ACPI / APEI: Suppress message if HEST not present
2017-08-29 12:38 ` Borislav Petkov
@ 2017-08-29 13:17 ` Punit Agrawal
0 siblings, 0 replies; 4+ messages in thread
From: Punit Agrawal @ 2017-08-29 13:17 UTC (permalink / raw)
To: Borislav Petkov
Cc: linux-acpi, lorenzo.pieralisi, sudeep.holla, linux-kernel,
Rafael J . Wysocki, James Morse
Borislav Petkov <bp@suse.de> writes:
> On Wed, Aug 16, 2017 at 12:27:53PM +0100, Punit Agrawal wrote:
>> According to the ACPI specification, firmware is not required to provide
>> the Hardware Error Source Table (HEST). When HEST is not present, the
>> following superfluous message is printed to the kernel boot log -
>>
>> [ 3.460067] GHES: HEST is not enabled!
>>
>> Extend hest_disable variable to track whether the firmware provides this
>> table and if it is not present skip any log output. The existing
>> behaviour is preserved in all other cases.
>>
>> Suggested-by: Borislav Petkov <bp@suse.de>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: James Morse <james.morse@arm.com>
>> ---
>> drivers/acpi/apei/ghes.c | 4 ++--
>> drivers/acpi/apei/hest.c | 13 +++++++------
>> include/acpi/apei.h | 8 +++++++-
>> 3 files changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index d661d452b238..f8685bcbeff2 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -1262,10 +1262,10 @@ static int __init ghes_init(void)
>> {
>> int rc;
>>
>> - if (acpi_disabled)
>> + if (acpi_disabled || hest_disable == HEST_NOT_FOUND)
>> return -ENODEV;
>>
>> - if (hest_disable) {
>> + if (hest_disable == HEST_DISABLED) {
>> pr_info(GHES_PFX "HEST is not enabled!\n");
>> return -EINVAL;
>> }
>
> Yap, looks good.
>
> Just a minor nitpick: I'd additionally group the hest_disable checking
> in one switch-case, so that the code flow is obvious at a quick glance:
>
> if (acpi_disabled)
> return -ENODEV;
>
> switch (hest_disable) {
> case HEST_NOT_FOUND:
> return -ENODEV;
> case HEST_DISABLED:
> pr_info(GHES_PFX "HEST is not enabled!\n");
> return -EINVAL;
> default:
> break;
> }
I've picked this change and will send out an update shortly.
Thanks,
Punit
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-29 13:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-16 11:27 [PATCH v2] ACPI / APEI: Suppress message if HEST not present Punit Agrawal
2017-08-29 9:03 ` Punit Agrawal
2017-08-29 12:38 ` Borislav Petkov
2017-08-29 13:17 ` Punit Agrawal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.