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


Groups > linux.kernel > #1577467

Re: [PATCH V8 06/10] acpi: apei: panic OS with fatal error status block

From James Morse <james.morse@arm.com>
Newsgroups linux.kernel
Subject Re: [PATCH V8 06/10] acpi: apei: panic OS with fatal error status block
Date 2017-02-09 12:00 +0100
Message-ID <t8UCd-56L-9@gated-at.bofh.it> (permalink)
References <t66Jz-iP-5@gated-at.bofh.it> <t66Jz-iP-15@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Jonathan, Tyler,

On 01/02/17 17:16, Tyler Baicar wrote:
> From: "Jonathan (Zhixiong) Zhang" <zjzhang@codeaurora.org>
> 
> Even if an error status block's severity is fatal, the kernel does not
> honor the severity level and panic.
> 
> With the firmware first model, the platform could inform the OS about a
> fatal hardware error through the non-NMI GHES notification type. The OS
> should panic when a hardware error record is received with this
> severity.
> 
> Call panic() after CPER data in error status block is printed if
> severity is fatal, before each error section is handled.

> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 8756172..86c1f15 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -687,6 +689,13 @@ static int ghes_ack_error(struct acpi_hest_generic_v2 *generic_v2)
>  	return rc;
>  }
>  
> +static void __ghes_call_panic(void)
> +{
> +	if (panic_timeout == 0)
> +		panic_timeout = ghes_panic_timeout;
> +	panic("Fatal hardware error!");
> +}
> +

__ghes_panic() also has:
>	__ghes_print_estatus(KERN_EMERG, ghes->generic, ghes->estatus);

Which prints this estatus regardless of rate limiting and cache-ing.

[ ... ]

> @@ -698,6 +707,10 @@ static int ghes_proc(struct ghes *ghes)
>  		if (ghes_print_estatus(NULL, ghes->generic, ghes->estatus))

ghes_print_estatus() uses some custom rate limiting '2 messages every 5
seconds', GHES_SEV_PANIC shares the same limit as GHES_SEV_RECOVERABLE.

I think its possible to get 2 recoverable messages, then a panic in a 5 second
window. The rate limit will kick in to stop the panic estatus block being
printed, but we still go on to call panic() without the real reason being printed...

(the caching thing only seems to consider identical messages, given we would
never see two panic messages, I don't think that will cause any problems.)

>  			ghes_estatus_cache_add(ghes->generic, ghes->estatus);
>  	}
> +	if (ghes_severity(ghes->estatus->error_severity) >= GHES_SEV_PANIC) {
> +		__ghes_call_panic();
> +	}
> +

I think this ghes_severity() then panic() should go above the:
>	if (!ghes_estatus_cached(ghes->estatus)) {
and we should call __ghes_print_estatus() here too, to make sure the message
definitely got out!


With that,
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH V8 00/10] Add UEFI 2.6 and ACPI 6.1 updates for RAS on ARM64 Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
  [PATCH V8 01/10] acpi: apei: read ack upon ghes record consumption Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
  [PATCH V8 06/10] acpi: apei: panic OS with fatal error status block Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
    Re: [PATCH V8 06/10] acpi: apei: panic OS with fatal error status  block James Morse <james.morse@arm.com> - 2017-02-09 12:00 +0100
  [PATCH V8 03/10] efi: parse ARM processor error Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
  [PATCH V8 08/10] ras: acpi / apei: generate trace event for unrecognized CPER section Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
    Re: [PATCH V8 08/10] ras: acpi / apei: generate trace event for  unrecognized CPER section kbuild test robot <lkp@intel.com> - 2017-02-02 00:30 +0100
  [PATCH V8 09/10] trace, ras: add ARM processor error trace event Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
    Re: [PATCH V8 09/10] trace, ras: add ARM processor error trace event kbuild test robot <lkp@intel.com> - 2017-02-02 03:40 +0100
    Re: [PATCH V8 09/10] trace, ras: add ARM processor error trace  event Steven Rostedt <rostedt@goodmis.org> - 2017-02-02 04:20 +0100
      Re: [PATCH V8 09/10] trace, ras: add ARM processor error trace event "Baicar, Tyler" <tbaicar@codeaurora.org> - 2017-02-03 21:20 +0100
  [PATCH V8 04/10] arm64: exception: handle Synchronous External Abort Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:20 +0100
    Re: [PATCH V8 04/10] arm64: exception: handle Synchronous External  Abort James Morse <james.morse@arm.com> - 2017-02-03 17:10 +0100
      Re: [PATCH V8 04/10] arm64: exception: handle Synchronous External  Abort "Baicar, Tyler" <tbaicar@codeaurora.org> - 2017-02-03 21:30 +0100
  [PATCH V8 02/10] ras: acpi/apei: cper: generic error data entry v3 per ACPI 6.1 Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:30 +0100
  [PATCH V8 07/10] efi: print unrecognized CPER section Tyler Baicar <tbaicar@codeaurora.org> - 2017-02-01 18:30 +0100

csiph-web