Re: [PATCH 3/9] arm64: mm: install SError abort handler

Mark Rutland
2017-03-24T15:17:34Z

On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
> This commit adds support for minimal handling of SError aborts and
> allows them to be hooked by a driver or other part of the kernel to
> install a custom SError abort handler. The hook function returns
> the previously registered handler so that handlers may be chained if
> desired.
>
> The handler should return the value 0 if the error has been handled,
> otherwise the handler should either call the next handler in the
> chain or return a non-zero value.

... so the order these get calls is completely dependent on probe
order...

> Since the Instruction Specific Syndrome value for SError aborts is
> implementation specific the registerred handlers must implement
> their own parsing of the syndrome.

... and drivers have to be intimately familiar with the CPU, in order to
be able to parse its IMPLEMENTATION DEFINED ESR_ELx.ISS value.

Even then, there's no guarantee there's anything useful there, since it
is IMPLEMENTATION DEFINED and could simply be RES0 or UNKNOWN in all
cases.

I do not think it is a good idea to allow arbitrary drivers to hook
this fault in this manner.

> + .align 6
> +el0_error:
> + kernel_entry 0
> +el0_error_naked:
> + mrs x25, esr_el1 // read the syndrome register
> + lsr x24, x25, #ESR_ELx_EC_SHIFT // exception class
> + cmp x24, #ESR_ELx_EC_SERROR // SError exception in EL0
> + b.ne el0_error_inv
> +el0_serr:
> + mrs x26, far_el1
> + // enable interrupts before calling the main handler
> + enable_dbg_and_irq

... why?

We don't do this for inv_entry today.

> + ct_user_exit
> + bic x0, x26, #(0xff << 56)
> + mov x1, x25
> + mov x2, sp
> + bl do_serr_abort
> + b ret_to_user
> +el0_error_inv:
> + enable_dbg
> + mov x0, sp
> + mov x1, #BAD_ERROR
> + mov x2, x25
> + b bad_mode
> +ENDPROC(el0_error)

Clearly you expect these to be delivered at arbitrary times during
execution. What if a KVM guest is executing at the time the SError is
delivered?

To be quite frank, I don't believe that we can reliably and safely
handle this misfeature in the kernel, and this infrastructure only
provides the illusion that we can.

I do not think it makes sense to do this.

Thanks,
Mark.