opened
Leo-Yan
username_0
When some interrupts are configured as group 1 in GICv2, these
interrupts trigger FIQ signal; this results in the Linux kernel panic
by reporting log: "Bad mode in FIQ handler detected on CPU0, code
0x00000000 -- Unknown/Uncategorized".  Unfortunately from kernel side it
has no permission to read the GIC register for group 1 interrupts so we
have no chance to get to know which interrupt is configured as secure
interrupt and cause the kernel panic.

For upper reason, we can enable exception handling framework in ARM-TF;
after enable the EHF then the FIQ is to be routed into EL3 level for
exception handling and EL3 has permission to read interrupt number so
can easily locate which interrupt causes issue. For enabling EHF, except
this patch we also need pass below parameters for ARM-TF building:

  "EL3_EXCEPTION_HANDLING=1 GICV2_G0_FOR_EL3=1"

If we need integrate service into ARM-TF with specific interrupt routing
model, we can simply remove building options "EL3_EXCEPTION_HANDLING=1
GICV2_G0_FOR_EL3=1" so can avoid conflict issue.
Hikey960: Enable exception handling framework
issue
created
soby-mathew
username_1
Hi @username_0,
I think you meant Group0 interrupts as they are the secure interrupts whereas Group1 in Non Secure Interrupts.
Only secure world can configure Interrupts as Group0 on GICv2. So if there are some interrupts misconfigured as Group0, it should be fixed in the Secure Software.  Also, there is no handler registered for the EHF priorities registered and this will cause panic at runtime. I didn't understand the intention of this patch.
comment
created
Leo-Yan
username_0
Hi @username_1 @jeenu-arm,

I have not clearly explain this patch's purpose, so let me elaborate for this:

For example, if the system has enabled interrupt routing model, the non secure OS handle non secure interrupt and secure OS (e.g. after we have enabled OP-TEE) handle the secure interrupt, then the secure and non-secure interrupts can be handle well.

But if the system has not enabled OP-TEE, and one interrupt is wrongly configured with secure interrupt in group 0; for this case the FIQ will not be routed to EL3 or S-EL1. So the kernel side directly reports the panic and dump the backtrace. We have this kind bug on Hikey960:   https://bugs.96boards.org/show_bug.cgi?id=614; from the kernel panic log, we can know there have FIQ happens in non-secure world, but the Linux kernel (in NS-EL1) we cannot know which interrupt number trigger this panic, this is because the Linux kernel has no permission to read back the secure interrupt number and secure registers in GIC.

So for the case if we has not enabled interrupt routing model for secure interrupt, if we want to quickly root cause which interrupt introduce FIQ panic, I think we can enable EHF for the debugging. We can trigger the panic in ARM-TF and ARM-TF can help dump more clearly information for this.

On Hikey960 we can enable EHF firstly when we have not enabled OP-TEE, after the platform has enabled OP-TEE then we can disable EHF by removing building options "EL3_EXCEPTION_HANDLING=1 GICV2_G0_FOR_EL3=1".

So essentially the purpose to enable EHF is for debug FIQ panic, we can easily locate the wrongly configured interrupt.
comment
created
soby-mathew
username_1
Ok, I see. But this is not the intended purpose of EHF and EHF needs a priority handler installed at every priority of interrupt expected.

Ideally in the platform layer the interrupt should only be enabled in GIC only if Optee is enabled. Since you are trying to catch misconfigured Secure interrupts, I would suggest to install a dummy S-EL1 interrupt handler which prints the Secure Interrupt information and panics. This dummy handler can be installed if there is no SPD (i.e. SPD_none). Would that work for you?

You can do this from the platform code.
```
#ifdef SPD_NONE
 			flags = 0;
			set_interrupt_rm_flag(flags, NON_SECURE);
			rc = register_interrupt_type_handler(INTR_TYPE_S_EL1,
						hikey_debug_fiq_handler,
						flags);
#endif
```

This will catch all the FIQ triggered in NS world irrespective of priority and also means you dont have to compile ARM TF with different options if OP-TEE is not enabled.
comment
created
Leo-Yan
username_0
Thanks for guidance, @username_1. I will try your suggestion and will let you know if it works on Hikey960. If so I will drop this PR and send new one. Thanks!
comment
created
davidcunado-arm
username_2
@username_0 
Given your last comment, can we close this PR?
comment
created
davidcunado-arm
username_2
@username_0 
I'm going to close this PR - please re-open if needed.
comment
created
Leo-Yan
username_0
@username_2 sorry for late response. It's good to close this PR and I have committed one new patch for this.
comment