ARM-software / tf-issues

Issue tracking for the ARM Trusted Firmware project
37 stars 16 forks source link

Asynchronous exceptions not taken in EL3 on Cortex A53 r0p2 and above #368

Closed ljerry closed 8 years ago

ljerry commented 8 years ago

Hi,

When EL3 triggers an Abort Exception, the EL3 abort handler is not called and ISR_EL1 reports that an Abort is pending. SCR_EL3[EA] allows to trap Aborts from lower levels but secure EL3 interrupts are not meant to be dropped to lower EL.

A similar problem occurs with FIQ (SGI in our case): When EL3 triggers an SGI, the core that wait-for-interrupt in EL3 is not woken up and ISR_EL1 reports that an FIQ is pending. Again SCR_EL3 thanks to its FIQ bit allows to trap FIQ from lower levels and to exit from WFI state.

After investigations, on r0p2 revision, despite what is stated in ARM DDI0500D TRM Cortex A53 r0p2 SCR_EL3 description in Table 4-77, asynchronous exceptions generated by the code running in EL3 are not taken in EL3 when SCR_EL3[EA] is zero. DDI0487A.b Architecture Reference Manual does not reflect the actual behaviour of r0p2.

On the other hand DDI0487A.e Architecture Reference Manual D7.2.80 SCR_EL3 description depicts the observed behaviour.

With best regards Jerry

danh-arm commented 8 years ago

Hi Jerry

I just had a look at version DDI0487A.e of the ARM ARM. I don't see how it describes the behaviour you observed; when executing in EL3, aborts/interrupts should never be taken to a lower exception level.

It sounds like you are reporting a defect on r0p2 of the Cortex-A53? If so, this is the wrong place to report it; it should be sent to ARM support.

Please confirm my understanding and if you intend to report to ARM support. Altenatively, I can forward this for you instead.

Regards

Dan.

danh-arm commented 8 years ago

Sorry, I thnk I've misunderstood you. On re-reading, I think you're saying that while executing at an EL lower than EL3, you are not getting aborts/interrupts to EL3. This is because SCR.EL3.EA is zero at reset and this is the documented behaviour according to DDI0487A.e of the ARM ARM.

So is your issue with the Cortex A53 TRM (and earlier versions of the ARM ARM)? If so, that still should be reported to ARM support.

ljerry commented 8 years ago

Yes it can be viewed as a documentation bug as well. TRM Cortex-A53 r0p2 refers to DDI0487A ARM ARM without mentioning the DDI0487A revision. This leads to inconsistency between them about Asynchronous Exceptions handling: TRM Cortex-A53 r0p2 Table 4-77 states

EA External Abort and SError interrupt Routing. This bit controls which mode takes external aborts. The possible values are: 0 External Aborts and SError Interrupts while executing at exception levels other than EL3 are not taken in EL3. This is the reset value. 1 External Aborts and SError Interrupts while executing at all exception levels are taken in EL3.

implies that these exceptions occurring in EL3 will be taken in EL3 if bit is clear which is not the case since r0p2. Indeed it behaves like this on a r0p0 (and maybe on r0p1).

On the other hand, DDI0487A at least since revision "e" ARM states:

EA, bit [3] External Abort and SError Interrupt Routing. 0 When executing at Exception levels below EL3, External Aborts and SError Interrupts are not taken to EL3. In addition, when executing at EL3: • SError Interrupts are not taken. • External Aborts are taken to EL3. 1 When executing at any Exception level, External Aborts and SError Interrupts are taken to EL3.

implies that SError occurring in EL3 will NOT be taken in EL3 if bit is clear. Here is the inconsistency.

Revision "a" and "b" are consistent with TRM Cortex-A53 r0p2.

The software bug is that if an asynchronous SError occurs, ATF handler won't be triggered: only ISR_EL1.A will be raised. Therefore, when we get into ELx x<3 in BL33 (resp. BL32) and unmask the interrupt, we get in BL33 (resp. BL32) exception handler.

Typically SError occurs when a write access is performed at an address no mapped by the system (a Data Abort will be reported by MMU if this address is not mapped by MMU). As this illegal access goes through the write buffer, the subsequent error is asynchronous.

With best regards Jerry

danh-arm commented 8 years ago

OK, I think this is long way of saying "we should set the SCR.EA bit in TF". Am I right?

Regarding the TRM documentation, are you doing to report this or shall I?

Regards

Dan.

danh-arm commented 8 years ago

Actually, we need to do more than that. The EL3 Abort/SError handlers will need to detect if the abort/interrupt came from a lower EL and pass control back to the lower EL.

danh-arm commented 8 years ago

The more I read this thread, the more confused I get. The intial description suggests that an external abort while executing in EL3 is not taken to EL3, despite what DDI0487A.e of the ARM ARM says. So are you suggesting there is bug in the r0p2 Cortex-A53 (as well as bugs in the TRM and TF)?

athoelke commented 8 years ago

I would suggest the the A53 TRM is incorrect, and that the DDI0487A.e (or later) and A53 CPU implementation are correct.

The handling of all physical interrupts (IRQ,FIQ, SError) is consistent with regards to routing and the SCR_EL3 control bits. If SCR_EL3 not set to trap a physical interrupt, then that interrupt does not target EL3 - so such an interrupt will not cause an asynchronous exception at EL3 (they are implicitly masked), and is not required to wake up from WFI. (source: sections D1.14.1, D1.14.2 and D1.17.2 in DDI0487A.g)

Please can you describe what behaviour you want to achieve for your system's memory access errors and we can then consider if that can be achieved with the current Trusted Firmware, or if some enhancement might be required.

ljerry commented 8 years ago

Hi Andrew,

Neither I think that A53 TRM r0p2 is incorrect too nor I think also that ATF behaviour wrt asynchronous exceptions is not desirable.

On our platform, a patch is applied to cope with this issue. I will share it with you if possible.

Mind that without this patch, when BL31 made a faulty write access on the platform, nothing is reported but BL33 crashed just after. This is just unacceptable.

athoelke commented 8 years ago

I can see that you would like faulty accesses in BL31 to result in an exception at EL3 so that you can report the fault and stop the program. This requires that both the SCR_EL3.EA bit is set (to make SError target EL3) and PSTATE.A clear (to prevent masking at EL3) whilst running BL31 code.

The side effects of this also need to be considered. In particular, what should the value of SCR_EL3.EA be when running code in normal world or secure world (i.e. not EL3)?

ljerry commented 8 years ago

This is the purpose of the patch I wrote: handle SCR_EL3.EA along with PSTATE.A switching back and forth EL3.

Indeed, L2 cache or DMA -initiated problems can come in both 0 or 1 cases depending on the root initiator i.e. in EL3 or EL1.

ljerry commented 8 years ago

Let me precise that the defect depicted here is linked to Asynchronous exceptions (Abort and FIQ and likely IRQ) generated by A53 with its master role on the bus. SEI input is not connected on the platform I use to test so I can only guess A53 behaviour regarding SEI, SEI not directly linked to the present report.

danh-arm commented 8 years ago

It seems to me that:

ljerry commented 8 years ago

Knowing that present behavior of the same TF is different when running on r0pn n<2 or r0pn n>=2, the integrator should at least be able to make a choice. Once EA occurs when there is a bug, the only way to avoid this situation is to avoid errors... MMU can't help if something goes wrong with an underlying peripheral.

athoelke commented 8 years ago

If the behaviour is different in different revisions of the A53 CPU, is this related to an fix for an Errata that is present in the earlier versions of the CPU?

ARMv8-A specifies that asynchronous external aborts are now categorised as System Error Interrupts and are signalled through that exception vector. Only synchronous external aborts are signalled via the synchronous exception vector.

athoelke commented 8 years ago

To be very precise - the A53 TRM is not inconsistent with the behaviour of the CPU, nor with the ARMv8A Reference Manual (revision g). Table 4-77 in the r0p2 TRM states that

"External Aborts and SError Interrupts while executing at exception levels other than EL3 are not taken in EL3. This is the reset value."

when SCR_EL3.EA=0.

This makes no statement regarding the expected or actual behaviour of the CPU if an External Abort or SError Interrupt occurs while executing in EL3: specifically it does not say that such an abort or interrupt will be taken in EL3 if they occur while executing in EL3.

The ARMv8-A Reference Manual tables regarding the routing of Asynchronous exceptions make the expected behaviour clear (and matches the behaviour of the A53 CPU). All revisions of this document are consistent. For example: revision DDI0487.b in table D1-10 SError interrupts remain pending if executing at EL3 and SCR_EL3.EA=0.

I am still failing to identify the error that you are describing. The documentation of the EA bit with regards to what happens when executing in EL3 could be clearer, but it is not in error.

Can you describe how the r0p1 revision of the A53 behaves differently to r0p2?

ljerry commented 8 years ago

I disagree, the statement is a logic implication not(q)->not(p) which is equivalent to p->q i.e. the "contraposed" formula would be "External Aborts and SError Interrupts are taken in EL3 while executing at EL3" with SCR_EL3.EA=0.

This ambiguity may be the origin of opposite interpretations and implementations on r0p1 and r0p2.

Asynchronous exception r0p1 running in EL3 SCR_EL3.EA=0 is taken in EL3. Asynchronous exception r0p2 running in EL3 SCR_EL3.EA=0 is not taken in EL3.

Is it clear enough or maybe you should simply try on your side?

athoelke commented 8 years ago

Asynchronous exception r0p1 running in EL3 SCR_EL3.EA=0 is taken in EL3. Asynchronous exception r0p2 running in EL3 SCR_EL3.EA=0 is not taken in EL3.

So that looks like A53 r0p1 has an errata relating to the SCR_EL3.EA bit. The ARMv8-A engineering documentation and reference manuals have always stated that asynchronous exceptions not targeting EL3 (corresponding SCR_EL3 bit is zero) are never taken in EL3 - this is clear from the routing tables.

I agree that the SCR_EL3 description leaves the situation ambiguous which might explain the initial implementation.

However, if the desired effect is to always take an EL3 exception on External Aborts (of any type) that are detected whilst executing in EL3 then there is no need to distinguish revisions of the CPU. Assume all CPUs behave as the ARMv8-A Reference Manual describe and set the SCR_EL3.EA bit on entry to EL3 and clear on exit. This will still have the desired effect on early A53 CPUs as well as other revisions and other CPUs. As Dan said, this still has unpredictable behaviour if SCR_EL3.EA is zero when running at other exception levels - and thus would not be acceptable as the default for ARM Trusted Firmware, though may be useful to debug defects that are causing illegal memory accesses from EL3.

ljerry commented 8 years ago

Nevertheless DDI0487A.b Architecture Reference Manual D7.2.79 regarding SCR_EL3 has got the same sentence than DDI0500A Cortex A53 r0p0 TRM and DDI0500D Cortex A53 r0p2 TRM:

0 External Aborts and SError Interrupts while executing at exception levels other than EL3 are not taken in EL3.

ARM ARM b publication was available along with r0p0 TRM.

I agree that SCR_EL3.[EA,FIQ,IRQ] cannot be tied to 1s running outside EL3 since it prevents the current EL to handle the exception itself. The proposed patch https://github.com/ARM-software/arm-trusted-firmware/pull/543 dynamically sets and clears those bits getting in and out EL3 according to CPU revision. Static/dynamic implementation is a matter of trade-off: static configuration will be faster but less portable than dynamic handling.

danh-arm commented 8 years ago

Jerry - I'm starting to see the value in the fundamental reasons for your patch. But the current patch checks CPU revision in generic and code and doesn't actually check that the code is running on A53. Potentially we could add extra hooks into the CPU libraries where the bits need changing but as Andrew says, if we add this behaviour, we could do it for all revisions.

I think the main issues to resolve are:

  1. Can the generic implementation guarantee that SCR_EL3.EA is zero when running at other exception levels? If platform code can easily subvert this behaviour, even accidentally, then this makes for fragile code.
  2. Can the generic implementation guarantee there are no significant performance (or other non-functional) issues?
  3. If the answer to either of the above is no, then all the changes will need to be made configurable (e.g. wrapped in a build flag).
ljerry commented 8 years ago

Yes this patch is A53-oriented and should be reworked to be generic for A5x. Don't know actual A57 behavior regarding this. If it can be done in CPU libraries/ops, it would be better.

  1. A bug can corrupt it. Platform code may corrupt SCR_EL3 as well. Tests have shown that it is not the case for the time being. On the other hand, if there is a unique exit point from bl31 (el3_exit?), SCR_EL3 and ISR_EL1 expected values can be checked there. It has not been done to avoid to burden code.
  2. No measurement was performed but, reading code, it is a matter of cycles or dozen of cycles (no loop, no external memory access)
  3. I'm not fond of compilation switches especially when original code is quite switch-free and therefore clean and mostly readable :) But if you are willing to do so or if it can be achieved at source file granularity level and not at line-of-code granularity level...
athoelke commented 8 years ago

All ARMv8-A CPUs should have the same behaviour as the later A53 revisions. So there is limited benefit (and some cost) to make the EA bit changing conditional on the CPU type or revision. But I do not think the changes should be unconditional.

There are perhaps three reasonable system wide approaches to External Aborts:

  1. Handle all of them at EL3 - such errors are either indicative of a fatal programming error in an operating system/firmware or indicate a recoverable or fatal memory fault. If it is recoverable, firmware at EL3 could do so and restart; if it is fatal then it is best to log the fault and restart, For such a system, SCR_EL3.EA should initialised to one and never modified.
  2. Handle all of them at EL1/EL2. This allows an operating system to diagnose, repair or restart from a locally caused external fault; but depends on the secure firmware causing no fatal errors. SCR_EL3.EA would be left at zero.
  3. Handle all of them locally to the EL that caused them. This allows software to report, repair, restart as appropriate, and is perhaps the most useful during development for diagnoses of programming errors that generate external aborts. SCR_EL3.EA must be one when in EL3 and zero otherwise.

The trouble with (2) and (3) is that asynchronous external aborts (which appear as System Error Interrupts) may not arrive in the execution regime that caused the fault - resulting in a failure to handle the fault in a reliable manner. The challenge with (1) is that it requires additional software infrastructure in Trusted Firmware to do anything other than log the fault and halt the system. That might be a good thing, but we don't have this upstream today.

The code today implements option (2), which is least useful during development. Your patch suggests that Trusted Firmware unconditionally implements option (3), which is equally problematic in a production device. The best long term approach would be (1) with a framework for handling External Aborts in Trusted Firmware - but this is not trivial - although a simple build option to set EA to one all the time might be useful for development.

ljerry commented 8 years ago

I guess that you mean "Your patch suggests that Trusted Firmware should unconditionally implement option (3)" (knowing that this option is realized by A53 r0p0 and by r0p1 hardwares).

Please note again that, in ARM ARM e revision, under the banner "External Abort", asynchronous external aborts are reported as "SError Interrupts" and synchronous external aborts as "External Aborts" which adds confusion to confusion. Moreover the latest are told to be always taken in EL3 in D7.2.80 in contradiction with D1.14.1

Option (1) experiment makes our OS team complain about the fact that they don't get the usual OS dump along with their own bug. Therefore at least both reports should be dumped (TF and OS ones).

Option (3) is also the best way to get closer to root cause of a possible SError Interrupt (was called "imprecise aborts") triggered by the CPU itself. A pending write error will be seen as soon as a write-buffer flush will be performed. Such a dsb has to be performed before smc and eret.

Sadly, if SError Interrupt is triggered by another master on the bus (such as DMA) none of these three options will help troubleshooting because it can be considered at a higher level of asynchronism.

Present option (2) implementation should not stay as is in a next future: if SCR_EL3.EA is always zero then TF should at least check that ISR_EL1.A does not toggle during EL3 execution before dropping to EL2 or EL1 especially at boot time when EL2 or EL1 have never been entered yet and therefore cannot be physically the root cause of a possible SError.

athoelke commented 8 years ago

I realise that the description of SCR_EL3.EA=0 in the ARMv8-A ARM has undergone several updates in the last year.

Despite the confusion with the documentation, the correct architectural behaviour has always been described in section D1.14, and is now fully and correctly described in the SCR_EL3 definition.

The generic Trusted Firmware software must be based on the [latest] architectural specification for ARMv8-A, with any alterations for CPU errata handled as special cases (where necessary).

The error in the A53 r0p0 and r0p1 CPUs is not a justification for changing Trusted Firmware behaviour so that it matches those CPUs. However, wanting a build of the firmware that provides better diagnostics for external aborts in BL31 is a reasonable justification for setting EA to one during EL3 execution or checking ISR_EL1.A before ERET from EL3.

ljerry commented 8 years ago

Current patch can be easily modified to remove core revision condition.

danh-arm commented 8 years ago

I agree that the current implementation should be fixed to ensure an EA caused by secure world code during cold boot is handled before ERET'ing to the normal world. However, I'm not comfortable with this switching of the bit at runtime due to the lack of clarity over which entity handles SErrors. It seems to me that whatever the setting of SCR_EL3.EA, an SError may be handled in a different execution regime than the one that caused it and it's not feasible to reliably pass this on to the execution regime that did cause it. The important thing is to have a fixed entity handle it.

For a synchronous EA, I think it's possible to ensure this is handled in the execution regime that caused it (if that's the desired behaviour) by always handling the EA at EL3 first, and if necessary ERET'ing to the EL that caused it.

In terms of framework for handling EAs in TF, I don't think much is needed, at least not initially. We have a patch in flight that adds a new plat_panic_handler assembly function, which is called when there is unrecoverable error (like an EA at EL3). The default implementation of this will spin, and on ARM platforms will eventually trigger a watchdog reset. That seems to be enough for now, at least for SErrors and synchronous EAs caused by EL3 code.

I propose the following patches:

  1. Non-configurably set SCR_EL3.EA to 1 in BL1 and BL31 during cold boot. Initially, set this back to 0 when ERET'ing to the normal world. This will catch the misconconfigued BL31 platform code case.
  2. Add a build option (e.g. HANDLE_SYNC_EA_LOCALLY) that controls what happens in BL31 after crash reporting due to a synchronous EA. If this option is enabled and the exception was not generated by EL3 code, then ERET back to where it was generated. Otherwise call plat_panic_handler. Initially this patch will have no effect, unless BL32 generates a sync EA during cold boot.
  3. Add another build option (e.g. HANDLE_EA_EL3_FIRST), which controls the runtime setting of SCR_EL3.EA. When enabled, SCR_EL3.EA remains set to 1; when disabled, it it is set to zero when ERET'ing to the normal world during cold boot.

Finally there's the question of default values for these build options. To get the desired behaviour in a debug scenerio (i.e. an OS generated synchronous EA causing a dump from both TF and the OS) both options will need setting. To preserve behavioural compatibility, both options will need disabling. I'd say we should enable HANDLE_EA_EL3_FIRST by default (though it changes the existing SError handling behaviour), and we should enable HANDLE_SYNC_EA_LOCALLY in debug builds, but disable it in release builds.

What do others think?

vwadekar commented 8 years ago

Dan, just want to understand. With #3, does EL3 handle synchronous EA if there is an abort in BL33?

On Mon, Mar 14, 2016, 7:23 AM danh-arm notifications@github.com wrote:

I agree that the current implementation should be fixed to ensure an EA caused by secure world code during cold boot is handled before ERET'ing to the normal world. However, I'm not comfortable with this switching of the bit at runtime due to the lack of clarity over which entity handles SErrors. It seems to me that whatever the setting of SCR_EL3.EA, an SError may be handled in a different execution regime than the one that caused it and it's not feasible to reliably pass this on to the execution regime that did cause it. The important thing is to have a fixed entity handle it.

For a synchronous EA, I think it's possible to ensure this is handled in the execution regime that caused it (if that's the desired behaviour) by always handling the EA at EL3 first, and if necessary ERET'ing to the EL that caused it.

In terms of framework for handling EAs in TF, I don't think much is needed, at least not initially. We have a patch in flight that adds a new plat_panic_handler assembly function, which is called when there is unrecoverable error (like an EA at EL3). The default implementation of this will spin, and on ARM platforms will eventually trigger a watchdog reset. That seems to be enough for now, at least for SErrors and synchronous EAs caused by EL3 code.

I propose the following patches:

  1. Non-configurably set SCR_EL3.EA to 1 in BL1 and BL31 during cold boot. Initially, set this back to 0 when ERET'ing to the normal world. This will catch the misconconfigued BL31 platform code case.
  2. Add a build option (e.g. HANDLE_SYNC_EA_LOCALLY) that controls what happens in BL31 after crash reporting due to a synchronous EA. If this option is enabled and the exception was not generated by EL3 code, then ERET back to where it was generated. Otherwise call plat_panic_handler. Initially this patch will have no effect, unless BL32 generates a sync EA during cold boot.
  3. Add another build option (e.g. HANDLE_EA_EL3_FIRST), which controls the runtime setting of SCR_EL3.EA. When enabled, SCR_EL3.EA remains set to 1; when disabled, it it is set to zero when ERET'ing to the normal world during cold boot.

Finally there's the question of default values for these build options. To get the desired behaviour in a debug scenerio (i.e. an OS generated synchronous EA causing a dump from both TF and the OS) both options will need setting. To preserve behavioural compatibility, both options will need disabling. I'd say we should enable HANDLE_EA_EL3_FIRST by default (though it changes the existing SError handling behaviour), and we should enable HANDLE_SYNC_EA_LOCALLY in debug builds, but disable it in release builds.

What do others think?

— Reply to this email directly or view it on GitHub https://github.com/ARM-software/tf-issues/issues/368#issuecomment-196329027 .

danh-arm commented 8 years ago

Hi Varun. Yes, with SCR_EL3.EA set to 1, all synchronous EAs are taken at EL3, including those raised in BL33. Do you rely on synchronous EAs being taken at BL33's EL in a production environment (or just a debugging environment)?

Andrew just pointed out to me that implementing patch 2 above might be more difficult/hacky to implement than I suggested, if we want to support all possible EL / register width combinations.

vwadekar commented 8 years ago

Hi Dan. Is there a guideline on which layer should handle the synchronous EA? We have not decided on a way to handle them yet.

On Mon, 14 Mar 2016 at 09:04 danh-arm notifications@github.com wrote:

Hi Varun. Yes, with SCR_EL3.EA set to 1, all synchronous EAs are taken at EL3, including those raised in BL33. Do you rely on synchronous EAs being taken at BL33's EL in a production environment (or just a debugging environment)?

Andrew just pointed out to me that implementing patch 2 above might be more difficult/hacky to implement than I suggested, if we want to support all possible EL / register width combinations.

— Reply to this email directly or view it on GitHub https://github.com/ARM-software/tf-issues/issues/368#issuecomment-196386021 .

ljerry commented 8 years ago

Pushed new patch to remove revision condition to set and clear SCR_EL3.EA bit.

Dan, your proposal goes far beyond the asynchronous abort handling. Couldn't it be done in later. I'm going to push on tf_issue_368_bis what is implemented here to match the need of platform registers dump which remind your plat_panic_handler (without EL drop). I thought I would have push it later but these dumps came with asynchronous abort debugging too.

danh-arm commented 8 years ago

Hi Jerry.

As I said in my previous comment, I'm not keen on the EA bit switching in your current PR. Although it "improves the chances" of SErrors being handled by the EL that caused it, it doesn't guarantee it. Certainly not all platforms will want this and so we won't allow the current non-configurable implementation.

I accept my patch 2 above would take some time to get right, although I believe it would be more robust since the code would be more localised.

This whole thread has highlighted that it's probably not very helpful for both synchronous and asynchronous EA handling to be controlled by the EA bit. Perhaps a longer term solution might be to provide a software API that allows lower ELs to register for synchronous EA (and other) events, instead of directly handling them in the lower EL exception vectors. If that support were in place then the EA bit could be permanently set to 1.

In the short term, I think we could skip patch 2 and just implement 1 and 3. Without patch 2, HANDLE_EA_EL3_FIRST would have to be disabled by default (to avoid unhelpful synchronous EA handling behaviour). I think patches 1 and 3 together would be simpler than your current implementation and would improve debugging of the most common EA cases.

Regards

Dan.

ljerry commented 8 years ago

Hi Dan,

A new patch set for synchronous and asynchronous aborts handling in EL3 for boot only has been pushed. Please note that I haven't tested it yet in all use-cases I used to run for this complex issue.

It is a first step to an acceptable compromise before a complete and configurable asynchronous exception handler implementation.

Indeed, solving the concern about EA bit switching has a counterpart with potential silent toggling of ISR_EL1.A while servicing an EL3 runtime SMC exceptions. I imagine that such a detection mechanism could be implemented before ERET to lower level. It could lead to implement in TF some kind of Linux-like "oops" report (as crash_panic does) that does not block further execution of lower level i.e. OS. Therefore OS will be ought to cope too with this asynchronous exception. It may be the aim of another ticket if you agree.

With best regards Jerry

danh-arm commented 8 years ago

Hi Jerry

I agree that current your patch(es) should only handle the cold boot case. We can continue review of those patches in the PR itself.

I've raised a separate ticket to address the handling of synchronous EAs when SCR_EL3.EA is set to 1: #374.

Regards

Dan.