Re: [PATCH 02/11] ACPI / APEI: Generalise the estatus queue's add/remove and notify code

[James Morse <james.morse@arm.com>]

Hi Borislav, Punit,

On 01/03/18 22:35, Borislav Petkov wrote:
> On Thu, Mar 01, 2018 at 06:06:59PM +0000, Punit Agrawal wrote:
>> The 64-bit support lives in arch/arm64 and the die() there doesn't
>> contain an oops_begin()/oops_end(). But the lack of oops_begin() on
>> arm64 doesn't really matter here.

>> One issue I see with calling die() is that it is defined in different
>> includes across various architectures, (e.g., include/asm/kdebug.h for
>> x86, include/asm/system_misc.h in arm64, etc.)
> 
> I don't think that's insurmountable.

I don't think die() helps us, its not quite the same as oops_begin()/panic(),
which means we're interpreting the APEI notification's severity differently,
depending on when we took it.


> The more important question is, can we do the same set of calls when
> panic severity on all architectures which support APEI or should we have
> arch-specific ghes_panic() callbacks or so.

I think the purpose of this oops_begin() is to ensure two CPUs calling
oops_begin() at the same time don't have their traces interleaved, unblanks the
screen and 'busts' any spinlocks printk() may need (console etc).

This code is called in_nmi(), printk() now supports this so it doesn't need its
locks busting.
When called in_nmi(), printk batches the messages into its per-cpu
printk_safe_seq_buf, which in our case is dumped by panic() using
printk_safe_flush_on_panic(). So provided we call panic(), the in_nmi() messages
from ghes.c are already batched, and printed behind panic()'s atomic_cmpxchg()
exclusion thing.

If your arm64 system has one of these futuristic 'screens', they get unblanked
when panic() calls console_verbose() and bust_spinlocks(1).


> As it is now, it would turn into a mess if we start with the ifdeffery
> and the different requirements architectures might have...

Today its just x86 and arm64. arm64 doesn't have a hook to do this. I'm happy to
add an empty declaration or leave it under an ifdef until someone complains
about any behaviour I missed!


Thanks,

James