[PATCH 0/4] Drop uninformative messages from bootlog

[PATCH 0/4] Drop uninformative messages from bootlog

[Punit Agrawal]

Hi,

While booting v4.13-rc1 on an arm64 system, I noticed some messages
that clutter the boot log without providing any useful
information. This series trims some of the obvious offenders.

The patches are spread across a few sub-systems - arm64/mm, mailbox,
APEI and xen, and can be independently merged.

Thanks,
Punit


Punit Agrawal (4):
  arm64/numa: Drop duplicate message
  mailbox: pcc: Drop uninformative output during boot
  ACPI / APEI: Drop uninformative messages during boot
  xen: Drop un-informative message during boot

 arch/arm64/mm/numa.c      | 7 +------
 drivers/acpi/apei/ghes.c  | 9 +--------
 drivers/mailbox/pcc.c     | 4 +---
 drivers/xen/xenfs/super.c | 1 -
 4 files changed, 3 insertions(+), 18 deletions(-)

-- 
2.11.0

[PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Punit Agrawal]

When booting an ACPI enabled system that does not provide the hardware
error source table (HEST), the ghes driver prints the following message
in the kernel log -

[    3.460067] GHES: HEST is not enabled!

which is not helpful.

The message is also output when HEST is explicitly disabled using kernel
command line parameter.

Drop this message. While we are touching this code, also drop similar
message when GHES is disabled using the module parameter.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Borislav Petkov <bp@suse.de>
---
 drivers/acpi/apei/ghes.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d661d452b238..3ddd1bd714fc 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1265,15 +1265,8 @@ static int __init ghes_init(void)
 	if (acpi_disabled)
 		return -ENODEV;
 
-	if (hest_disable) {
-		pr_info(GHES_PFX "HEST is not enabled!\n");
+	if (hest_disable || ghes_disable)
 		return -EINVAL;
-	}
-
-	if (ghes_disable) {
-		pr_info(GHES_PFX "GHES is not enabled!\n");
-		return -EINVAL;
-	}
 
 	ghes_nmi_init_cxt();
 
-- 
2.11.0

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Borislav Petkov]

On Thu, Jul 20, 2017 at 12:04:01PM +0100, Punit Agrawal wrote:
> When booting an ACPI enabled system that does not provide the hardware
> error source table (HEST), the ghes driver prints the following message
> in the kernel log -
> 
> [    3.460067] GHES: HEST is not enabled!
> 
> which is not helpful.
> 
> The message is also output when HEST is explicitly disabled using kernel
> command line parameter.
> 
> Drop this message. While we are touching this code, also drop similar
> message when GHES is disabled using the module parameter.

No.

To the contrary, we want to know *why* GHES/HEST doesn't get enabled when
booting. See

  dba648300e89 ("ACPI / einj: Make error paths more talkative")

for a good example why.

If anything, you should do the total opposite patch and add more
printks to the error paths so that it is obvious why GHES startup fails.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Punit Agrawal]

Hi Borislav,

Borislav Petkov <bp@suse.de> writes:

> On Thu, Jul 20, 2017 at 12:04:01PM +0100, Punit Agrawal wrote:
>> When booting an ACPI enabled system that does not provide the hardware
>> error source table (HEST), the ghes driver prints the following message
>> in the kernel log -
>> 
>> [    3.460067] GHES: HEST is not enabled!
>> 
>> which is not helpful.
>> 
>> The message is also output when HEST is explicitly disabled using kernel
>> command line parameter.
>> 
>> Drop this message. While we are touching this code, also drop similar
>> message when GHES is disabled using the module parameter.
>
> No.
>
> To the contrary, we want to know *why* GHES/HEST doesn't get enabled when
> booting. See
>
>   dba648300e89 ("ACPI / einj: Make error paths more talkative")
>
> for a good example why.

einj verbosity can't be used as a model here. einj by it's definition is
for development and testing. It can also be loaded as a module.

>
> If anything, you should do the total opposite patch and add more
>printks to the error paths so that it is obvious why GHES startup
>fails.

As mentioned in the commit log, the platform doesn't provide the HEST
(or any of the APEI tables for that matter). So it's not useful to see a
message complaining that HEST is not enabled. For such platforms, this
message at best adds no value and at worse gives the impression of
something gone wrong during boot.

I agree that where there is a genuine problem, relevant messages can
help to diagnose the problem. But what's printed now doesn't fit the
criteria.

Hope that makes sense.

Thanks,
Punit

>
> -- Regards/Gruss, Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Borislav Petkov]

On Thu, Jul 20, 2017 at 01:29:17PM +0100, Punit Agrawal wrote:
> I agree that where there is a genuine problem, relevant messages can
> help to diagnose the problem. But what's printed now doesn't fit the
> criteria.

So make it fit the criteria. Change the code to not issue that message
when the platform doesn't have those tables. But keep it in the
remaining cases, when the tables are there.

You can't be removing useful error messages just because your platform
doesn't have the tables.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Punit Agrawal]

Borislav Petkov <bp@suse.de> writes:

> On Thu, Jul 20, 2017 at 01:29:17PM +0100, Punit Agrawal wrote:
>> I agree that where there is a genuine problem, relevant messages can
>> help to diagnose the problem. But what's printed now doesn't fit the
>> criteria.
>
> So make it fit the criteria. Change the code to not issue that message
> when the platform doesn't have those tables. But keep it in the
> remaining cases, when the tables are there.
>
> You can't be removing useful error messages just because your platform
> doesn't have the tables.

Fair point! I'll focus on quiescing the messages when the tables are not
present.

Inspired by your suggestion, I tried the following diff instead of
$SUBJECT.

diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index 456b488eb1df..f7784d9514e1 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -233,8 +233,9 @@ 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)) {
+         return;
+
+ if (ACPI_FAILURE(status)) {
                const char *msg = acpi_format_exception(status);
                pr_err(HEST_PFX "Failed to get table, %s\n", msg);
                rc = -EINVAL;

This landed me a new message not seen before -

[    3.232940] GHES: Failed to enable APEI firmware first mode.

On further digging, this message is printed on platforms not supporting
the legacy WHEA stuff, when the APEI Support bit is not set in the
platform wide _OSC capabilities.

To make this message a bit less alarming, I can modify it to something
like -

"Firmware does not support APEI firmware first mode"

Thoughts?

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Borislav Petkov]

On Thu, Jul 20, 2017 at 06:50:51PM +0100, Punit Agrawal wrote:
> "Firmware does not support APEI firmware first mode"
> 
> Thoughts?

I guess the simplest would be to add a third state to that hest_disable
to denote "HEST table not found" and then exit ghes_init() early, based on
checking it.

Otherwise ghes_init() inits a bunch of things which you probably don't
want on a platform which doesn't support APEI.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Punit Agrawal]

Hi Boris,

Apologies for the delayed response. I somehow managed to lose updates on
this thread.

Borislav Petkov <bp@suse.de> writes:

> On Thu, Jul 20, 2017 at 06:50:51PM +0100, Punit Agrawal wrote:
>> "Firmware does not support APEI firmware first mode"
>> 
>> Thoughts?
>
> I guess the simplest would be to add a third state to that hest_disable
> to denote "HEST table not found" and then exit ghes_init() early, based on
> checking it.

Although simple, won't it make the already convoluted code flow more
so. Instead in addition to not setting hest_disable when the table is
not found ...

>
> Otherwise ghes_init() inits a bunch of things which you probably don't
> want on a platform which doesn't support APEI.

... would you be open to a patch re-working the ghes driver
initialisation to only do the platform driver registration. The the rest
of the initialisation (including the apei_osc_setup and related
messages) can be performed when the first ghes device gets probed.

Does that sound like a better alternative?

>
> -- 
> Regards/Gruss,
>     Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Borislav Petkov]

On Thu, Jul 27, 2017 at 10:35:45AM +0100, Punit Agrawal wrote:
> ... would you be open to a patch re-working the ghes driver
> initialisation to only do the platform driver registration. The the rest
> of the initialisation (including the apei_osc_setup and related
> messages) can be performed when the first ghes device gets probed.
> 
> Does that sound like a better alternative?

If you split it by doing one logical change per patch so that it is
obvious what's going on and you don't break existing usage, I don't see
why not.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

Re: [PATCH 3/4] ACPI / APEI: Drop uninformative messages during boot

[Punit Agrawal]

Borislav Petkov <bp@suse.de> writes:

> On Thu, Jul 27, 2017 at 10:35:45AM +0100, Punit Agrawal wrote:
>> ... would you be open to a patch re-working the ghes driver
>> initialisation to only do the platform driver registration. The the rest
>> of the initialisation (including the apei_osc_setup and related
>> messages) can be performed when the first ghes device gets probed.
>> 
>> Does that sound like a better alternative?
>
> If you split it by doing one logical change per patch so that it is
> obvious what's going on and you don't break existing usage, I don't see
> why not.

Good! I'll try and get the patches out in the next few days.

Thanks.

>
> -- 
> Regards/Gruss,
>     Boris.
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)