Hi James,
Sorry for reply late, and thanks for your comments.
On 2017/4/1 0:20, James Morse wrote:
> Hi Xie XiuQi,
>
> On 30/03/17 11:31, Xie XiuQi wrote:
>> ARM APEI extension proposal added SEI (asynchronous SError interrupt)
>> notification type for ARMv8.
>>
>> Add a new GHES error source handling function for SEI. In firmware
>> first mode, if an error source's notification type is SEI. Then GHES
>> could parse and report the detail error information.
>
> The APEI additions are unsafe until patch 4 as SEA can interrupt SEI and
> deadlock while trying to take the same set of locks. This patch needs to be
> after that interaction is fixed/prevented, or we should prevent it by adding a
> depends-on-not to the Kconfig to prevent SEI and SEA being registered at the
> same time. (as a short term fix).
Will fix later.
>
> (more comments on this on that later patch)
>
>
>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>> index e52be6a..53710a2 100644
>> --- a/arch/arm64/kernel/traps.c
>> +++ b/arch/arm64/kernel/traps.c
>
>> @@ -625,6 +627,14 @@ asmlinkage void bad_mode(struct pt_regs *regs, int reason, unsigned int esr)
>
> bad_mode() is called in other scenarios too, for example executing an undefined
> instruction at EL1. You split the SError path out of the vectors in patch 7, I
> think we should do that here.
>
>
>> handler[reason], smp_processor_id(), esr,
>> esr_get_class_string(esr));
>>
>> + /*
>> + * In firmware first mode, we could assume firmware will only generate one
>> + * of cper records at a time. There is no risk for one cpu to parse ghes table.
>> + */
>
> I don't follow this comment, is this saying SError can't interrupt SError? We
> already get this guarantee as the CPU masks SError when it takes an exception.
>
> Firmware can generate multiple CPER records for a single 'event'. The CPER
> records are the 'Data' in ACPI:Table 18-343 Generic Error Data Entry, and there
> are 'zero or more' of these with a 'Generic Error Status Block' header that
> describes the overall event. (Table 18-342).
>
> I don't think we need this comment.
Thanks for your explanation, OK, I'll remove this comment.
>
>
>> + if (IS_ENABLED(CONFIG_ACPI_APEI_SEI) && ESR_ELx_EC(esr) == ESR_ELx_EC_SERROR) {
>> + ghes_notify_sei();
>> + }
>
>> die("Oops - bad mode", regs, 0);
>> local_irq_disable();
>> panic("bad mode");
>
> Thanks,
>
> James
>
>
> .
>
--
Thanks,
Xie XiuQi
