diff mbox

setup: Move unmask of async interrupts after possible earlycon setup

Message ID 1409079338-14001-1-git-send-email-jcm@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jon Masters Aug. 26, 2014, 6:55 p.m. UTC
The kernel wants to enable reporting of asynchronous interrupts (i.e.
System Errors) as early as possible. But if this happens too early
then a pending System Error on initial entry into the kernel will
never be reported where a user can see it (instead it will remain
in the kernel ring buffer and be visible only via hardware debug).
Therefore, move the enabling of asynchronous interrupts to after
parsing any possible earlycon parameters setting up earlycon.

Signed-off-by: Jon Masters <jcm@redhat.com>
---
 arch/arm64/kernel/setup.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Grant Likely Aug. 26, 2014, 8:01 p.m. UTC | #1
On Tue, Aug 26, 2014 at 7:55 PM, Jon Masters <jcm@redhat.com> wrote:
> The kernel wants to enable reporting of asynchronous interrupts (i.e.
> System Errors) as early as possible. But if this happens too early
> then a pending System Error on initial entry into the kernel will
> never be reported where a user can see it (instead it will remain
> in the kernel ring buffer and be visible only via hardware debug).
> Therefore, move the enabling of asynchronous interrupts to after
> parsing any possible earlycon parameters setting up earlycon.
>
> Signed-off-by: Jon Masters <jcm@redhat.com>

That sounds wrong. Why aren't early log messages getting flushed out
when the console shows up?

g.

> ---
>  arch/arm64/kernel/setup.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index f6f0ccf..f849b88 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -362,11 +362,6 @@ u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
>
>  void __init setup_arch(char **cmdline_p)
>  {
> -       /*
> -        * Unmask asynchronous aborts early to catch possible system errors.
> -        */
> -       local_async_enable();
> -
>         setup_processor();
>
>         setup_machine_fdt(__fdt_pointer);
> @@ -382,6 +377,12 @@ void __init setup_arch(char **cmdline_p)
>
>         parse_early_param();
>
> +       /*
> +        *  Unmask asynchronous aborts after bringing up possible earlycon.
> +        * (Report possible System Errors once we can report this occurred)
> +        */
> +       local_async_enable();
> +
>         efi_init();
>         arm64_memblock_init();
>
> --
> 1.8.3.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Jon Masters Aug. 26, 2014, 8:05 p.m. UTC | #2
Hi Grant,

On 08/26/2014 04:01 PM, Grant Likely wrote:
> On Tue, Aug 26, 2014 at 7:55 PM, Jon Masters <jcm@redhat.com> wrote:
>> The kernel wants to enable reporting of asynchronous interrupts (i.e.
>> System Errors) as early as possible. But if this happens too early
>> then a pending System Error on initial entry into the kernel will
>> never be reported where a user can see it (instead it will remain
>> in the kernel ring buffer and be visible only via hardware debug).
>> Therefore, move the enabling of asynchronous interrupts to after
>> parsing any possible earlycon parameters setting up earlycon.
>>
>> Signed-off-by: Jon Masters <jcm@redhat.com>
> 
> That sounds wrong. Why aren't early log messages getting flushed out
> when the console shows up?

The problem is that when such an error occurs, we immediately panic. And
we've not setup the earlycon yet so there's no way to ever see it. I
wasted a bunch of time last week so it's worth saving someone else.

Jon.
Grant Likely Aug. 26, 2014, 8:09 p.m. UTC | #3
On Tue, Aug 26, 2014 at 9:05 PM, Jon Masters <jcm@redhat.com> wrote:
> Hi Grant,
>
> On 08/26/2014 04:01 PM, Grant Likely wrote:
>> On Tue, Aug 26, 2014 at 7:55 PM, Jon Masters <jcm@redhat.com> wrote:
>>> The kernel wants to enable reporting of asynchronous interrupts (i.e.
>>> System Errors) as early as possible. But if this happens too early
>>> then a pending System Error on initial entry into the kernel will
>>> never be reported where a user can see it (instead it will remain
>>> in the kernel ring buffer and be visible only via hardware debug).
>>> Therefore, move the enabling of asynchronous interrupts to after
>>> parsing any possible earlycon parameters setting up earlycon.
>>>
>>> Signed-off-by: Jon Masters <jcm@redhat.com>
>>
>> That sounds wrong. Why aren't early log messages getting flushed out
>> when the console shows up?
>
> The problem is that when such an error occurs, we immediately panic. And
> we've not setup the earlycon yet so there's no way to ever see it. I
> wasted a bunch of time last week so it's worth saving someone else.

Reasonable reason. Perhaps put that in the patch description.

g.
Jon Masters Aug. 26, 2014, 8:12 p.m. UTC | #4
On 08/26/2014 04:05 PM, Jon Masters wrote:
> Hi Grant,
> 
> On 08/26/2014 04:01 PM, Grant Likely wrote:
>> On Tue, Aug 26, 2014 at 7:55 PM, Jon Masters <jcm@redhat.com> wrote:
>>> The kernel wants to enable reporting of asynchronous interrupts (i.e.
>>> System Errors) as early as possible. But if this happens too early
>>> then a pending System Error on initial entry into the kernel will
>>> never be reported where a user can see it (instead it will remain
>>> in the kernel ring buffer and be visible only via hardware debug).
>>> Therefore, move the enabling of asynchronous interrupts to after
>>> parsing any possible earlycon parameters setting up earlycon.
>>>
>>> Signed-off-by: Jon Masters <jcm@redhat.com>
>>
>> That sounds wrong. Why aren't early log messages getting flushed out
>> when the console shows up?
> 
> The problem is that when such an error occurs, we immediately panic. And
> we've not setup the earlycon yet so there's no way to ever see it. I
> wasted a bunch of time last week so it's worth saving someone else.

Admittedly this is more likely to be an issue for Enterprise vendor
kernels, where the default config (should be always) is PANIC_ON_OOPS.

Jon.
Jon Masters Aug. 26, 2014, 8:52 p.m. UTC | #5
On 08/26/2014 04:09 PM, Grant Likely wrote:
> On Tue, Aug 26, 2014 at 9:05 PM, Jon Masters <jcm@redhat.com> wrote:
>> Hi Grant,
>>
>> On 08/26/2014 04:01 PM, Grant Likely wrote:
>>> On Tue, Aug 26, 2014 at 7:55 PM, Jon Masters <jcm@redhat.com> wrote:
>>>> The kernel wants to enable reporting of asynchronous interrupts (i.e.
>>>> System Errors) as early as possible. But if this happens too early
>>>> then a pending System Error on initial entry into the kernel will
>>>> never be reported where a user can see it (instead it will remain
>>>> in the kernel ring buffer and be visible only via hardware debug).
>>>> Therefore, move the enabling of asynchronous interrupts to after
>>>> parsing any possible earlycon parameters setting up earlycon.
>>>>
>>>> Signed-off-by: Jon Masters <jcm@redhat.com>
>>>
>>> That sounds wrong. Why aren't early log messages getting flushed out
>>> when the console shows up?
>>
>> The problem is that when such an error occurs, we immediately panic. And
>> we've not setup the earlycon yet so there's no way to ever see it. I
>> wasted a bunch of time last week so it's worth saving someone else.
> 
> Reasonable reason. Perhaps put that in the patch description.

Sent a V2 with that fixed. Forgot to copy you, apologies.

Jon.
diff mbox

Patch

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index f6f0ccf..f849b88 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -362,11 +362,6 @@  u64 __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = INVALID_HWID };
 
 void __init setup_arch(char **cmdline_p)
 {
-	/*
-	 * Unmask asynchronous aborts early to catch possible system errors.
-	 */
-	local_async_enable();
-
 	setup_processor();
 
 	setup_machine_fdt(__fdt_pointer);
@@ -382,6 +377,12 @@  void __init setup_arch(char **cmdline_p)
 
 	parse_early_param();
 
+	/*
+	 *  Unmask asynchronous aborts after bringing up possible earlycon.
+	 * (Report possible System Errors once we can report this occurred)
+	 */
+	local_async_enable();
+
 	efi_init();
 	arm64_memblock_init();