Re: Question about SEA handling process happened in user space

[Xiaofei Tan <tanxiaofei@huawei.com>]

Hi James，

Thanks for kindly reply.

On 2020/3/31 0:49, James Morse wrote:
> Hi Xiaofei,
> 
> On 3/30/20 2:10 PM, Xiaofei Tan wrote:
>> I'm a little confused about the handling process of SEA happened in user space.
> 
>> Following the description of FnV bit of register ESR_ELx in ARMv8.4 SPEC,FAR is
>> valid only for synchronous External abort on a translation table walk.
> 
>> But for this FAR valid scenario(attached code from line 684 to 687),
>> we send signal SIGKILL to kill process. For some other scenario, such as line 680,
>> FAR is not valid, but we send SIGBUS and transfer error address to process to try to do some recovery.
> 
> 'FAR is not valid': its optional. The ESR_EL1.FnV bit can be set for the 'catch
> all' external abort fault-status-code. This lets the CPU tell us that it doesn't
> know what the faulting virtual address is.
> 
> do_sea() checks for this:
> |	if (esr & ESR_ELx_FnV)
> |		siaddr = NULL;
> |	else
> |		siaddr  = (void __user *)addr;
> 
> If we can't know the address, there isn't much we can do.
> (This check is really making sure we don't pass junk to user-space when FnV is set)
> 
> 

OK. So even if FAR is not valid, we still send SIGBUS for SEA, not on translation table walk, but
set the addr to NULL here.

>> should it be an problem ?
> 
> I'm not quite sure what your question is.
> 
> If the CPU doesn't tell us the address, we can't tell user-space what it is. The
> alternative is to upgrade to SIGKILL in that case.
> 

Got it. May be the description of FnV bit of register ESR_ELx is not quite exactly. Because
following the code, CPU may still have an chance to tell the address for SEA, not on translation table walk.

> 
> If you see this instead of the address provided via firmware-first, there is a
> series to improve that here:
> https://lore.kernel.org/linux-acpi/20200228174817.74278-1-james.morse@arm.com/
> 
> (We skip this signal code of APEI promises it did all the work. This lets you
> take the signal from memory_failure() instead, which may have better information.)
> 

This should be an great direction.
I have two concerns.
1.memory_failure() is only called for "memory error section" record. Then
should we use this memory record for ghes sea report? Our platform is
using "ARM processor error section".
2.Should we define an error source structure for each cpu core in HEST table?
If not, there may be conflict if more than one cpu core fall into SEA.

> 
> If its the SIGKILL entries: these are for the translation table walk.
> There is no point telling user-space about corruption in its page tables as it
> can't do anything about it. The kernel's handling of this is to kill the
> process. (page tables make up a very small amount of memory, so this should be
> rarer than the regular 'external abort' case)
> 

Hmm, then it is useless that CPU record address for this entries.

> 
> Thanks,
> 
> James
> 
> 
> 
>> 680         { do_sea,               SIGBUS,  BUS_OBJERR,    "synchronous external abort"    },
>> 684         { do_sea,               SIGKILL, SI_KERNEL,     "level 0 (translation table walk)"      },
>> 685         { do_sea,               SIGKILL, SI_KERNEL,     "level 1 (translation table walk)"      },
>> 686         { do_sea,               SIGKILL, SI_KERNEL,     "level 2 (translation table walk)"      },
>> 687         { do_sea,               SIGKILL, SI_KERNEL,     "level 3 (translation table walk)"      },
>> 688         { do_sea,               SIGBUS,  BUS_OBJERR,    "synchronous parity or ECC error" },    // Reserved when RAS is implemented
>> 692         { do_sea,               SIGKILL, SI_KERNEL,     "level 0 synchronous parity error (translation table walk)"     },      // Reserved when RAS is implemented
>> 693         { do_sea,               SIGKILL, SI_KERNEL,     "level 1 synchronous parity error (translation table walk)"     },      // Reserved when RAS is implemented
>> 694         { do_sea,               SIGKILL, SI_KERNEL,     "level 2 synchronous parity error (translation table walk)"     },      // Reserved when RAS is implemented
>> 695         { do_sea,               SIGKILL, SI_KERNEL,     "level 3 synchronous parity error (translation table walk)"     },      // Reserved when RAS is implemented
>> 696         { do_bad,               SIGKILL, SI_KERNEL,     "unknown 32"                    },
> 
> .
> 

-- 
 thanks
tanxiaofei


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel