| Message ID | 20170720110402.15313-4-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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.
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) -- 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
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.
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();
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(-)