Hi Xiongfeng Wang,
On 20/04/2019 10:06, Xiongfeng Wang wrote:
> Functions called in '_sdei_handler' are needed to be marked as
> 'nokprobe'.
+ "Because these functions are called in NMI context and neither the
arch-code's debug
infrastructure nor kprobes core supports this."
> ---
> When I kprobe 'sdei_smccc_smc',
Heh, when debugging this I put printk()s on the other side.
> the cpu hungs. I am not sure if it is
> becase when SDEI events interrupt EL0, '__sdei_asm_entry_trampoline'
> didn't restore vbar_el1 with kernel vectors.
Yes, we don't expect exceptions from the NMI handler, so VBAR_EL1 is left as
whatever its
current state is. Today it could be the EL0 trampoline, the kernel vectors
proper, or KVMs
vectors. This may change in the future, all SDEI does with VBAR_EL1 is emulate
the
architecture: read it in order to fake up an interrupt on exit.
> Or is it becasue brk
> exception corrupt elr_el1 and trust firmware didn't preserve it for us?
Yes, this too! "5.2.1.1 Client responsibilities" of [0]
| The handler may modify the accessible System registers, but these must be
restored
| before the handler completes.
We don't do this, because its extra work, and we don't ever anticipate it being
necessary.
> drivers/firmware/arm_sdei.c | 3 +++
> 1 file changed, 3 insertions(+)
Nit: Because this patch only touches this file, the subject prefix should
really be
"firmware: arm_sdei:". Running 'git log --online' over a file should give you a
hint at
what the expected style is. This lets people spot the patches they need to look
at.
> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index e6376f9..9cd70d1 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -165,6 +165,7 @@ static int invoke_sdei_fn(unsigned long function_id,
> unsigned long arg0,
>
> return err;
> }
> +NOKPROBE_SYMBOL(invoke_sdei_fn);
Ah! These are called via sdei_api_event_context(). Oops.
> static struct sdei_event *sdei_event_find(u32 event_num)
> {
> @@ -879,6 +880,7 @@ static void sdei_smccc_smc(unsigned long function_id,
> {
> arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, arg4, 0, 0, res);
> }
> +NOKPROBE_SYMBOL(sdei_smccc_smc);
>
> static void sdei_smccc_hvc(unsigned long function_id,
> unsigned long arg0, unsigned long arg1,
> @@ -887,6 +889,7 @@ static void sdei_smccc_hvc(unsigned long function_id,
> {
> arm_smccc_hvc(function_id, arg0, arg1, arg2, arg3, arg4, 0, 0, res);
> }
> +NOKPROBE_SYMBOL(sdei_smccc_hvc);
>
> int sdei_register_ghes(struct ghes *ghes, sdei_event_callback *normal_cb,
> sdei_event_callback *critical_cb)
Thanks for making this more robust!
Reviewed-by: James Morse <james.mo...@arm.com>
Thanks,
James
[0]
http://infocenter.arm.com/help/topic/com.arm.doc.den0054a/ARM_DEN0054A_Software_Delegated_Exception_Interface.pdf
