On 6 October 2014 11:02, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 30 September 2014 22:49, Greg Bellows <greg.bell...@linaro.org> wrote:
> > From: Fabian Aggeler <aggel...@ethz.ch>
> >
> > Adds a dedicated function for IRQ and FIQ exceptions to determine
> > target_el and mode (Aarch32) according to tables in ARM ARMv8 and
> > ARM ARM v7.
> >
> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> >
> > ----------
> > v4 -> v5
> > - Simplify target EL function including removal of mode which was unused
> > - Merged with patch that plugs in the use of the function
> >
> > v3 -> v4
> > - Fixed arm_phys_excp_target_el() 0/0/0 case to return excp_mode when
> EL<2
> > rather than ABORT.
> > ---
> > target-arm/cpu.h | 2 +
> > target-arm/helper.c | 103
> ++++++++++++++++++++++++++++++++++++++++++----------
> > 2 files changed, 85 insertions(+), 20 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 30f57fd..601f8fe 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -809,6 +809,8 @@ static inline bool arm_el_is_aa64(CPUARMState *env,
> int el)
> >
> > void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
> > unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx);
> > +inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
> > + uint32_t cur_el, bool secure);
>
> This is only used in helper.c which is also the place where it
> is defined, so why are we making it a global function with
> a prototype here rather than having it be 'static'?
>
>
Removed from v6 and made static local to helper.c.
> >
> > /* Interface between CPU and Interrupt controller. */
> > void armv7m_nvic_set_pending(void *opaque, int irq);
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 7f3f049..a10f459 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -3706,6 +3706,12 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env,
> uint32_t mode)
> > return 0;
> > }
> >
> > +inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
> > + uint32_t cur_el, bool secure)
> > +{
> > + return 1;
> > +}
> > +
>
> This version is never used, so I think it can be deleted?
>
>
Yep, removed from v6.
It is actually called from
> > unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
> > {
> > return 1;
> > @@ -3767,6 +3773,80 @@ void switch_mode(CPUARMState *env, int mode)
> > }
> >
> > /*
> > + * Determine the target EL for physical exceptions
>
> What's a "physical exception" ?
>
I am guessing that this was taken from the ARM spec which refers to
"physical exceptions" as the resulting actual exception type that virtual
exceptions correspond to.
> > + */
> > +inline uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
> > + uint32_t cur_el, bool secure)
> > +{
> > + CPUARMState *env = cs->env_ptr;
> > + uint32_t target_el = 1;
> > +
> > + /* There is no SCR or HCR routing unless the respective EL3 and EL2
> > + * extensions are supported. This initial setting affects whether
> any
> > + * other conditions matter.
> > + */
> > + bool scr_routing = arm_feature(env, ARM_FEATURE_EL3); /* IRQ, FIQ,
> EA */
> > + bool hcr_routing = arm_feature(env, ARM_FEATURE_EL2); /* IMO, FMO,
> AMO */
> > +
> > + /* Fast-path if EL2 and EL3 are not enabled */
> > + if (!scr_routing && !hcr_routing) {
> > + return target_el;
> > + }
> > +
> > + switch (excp_idx) {
> > + case EXCP_IRQ:
> > + scr_routing &= ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
> > + hcr_routing &= ((env->cp15.hcr_el2 & HCR_IMO) == HCR_IMO);
> > + break;
> > + case EXCP_FIQ:
> > + scr_routing &= ((env->cp15.scr_el3 & SCR_FIQ) == SCR_FIQ);
> > + hcr_routing &= ((env->cp15.hcr_el2 & HCR_FMO) == HCR_FMO);
> > + }
> > +
> > + /* If SCR routing is enabled we always go to EL3 regardless of EL3
> > + * execution state
> > + */
> > + if (scr_routing) {
> > + /* IRQ|FIQ|EA == 1 */
> > + return 3;
> > + }
> > +
> > + /* If HCR.TGE is set all exceptions that would be routed to EL1 are
> > + * routed to EL2 (in non-secure world).
> > + */
> > + hcr_routing &= (env->cp15.hcr_el2 & HCR_TGE) == HCR_TGE;
> > +
> > + /* Determine target EL according to ARM ARMv8 tables G1-15 and
> G1-16 */
> > + if (arm_el_is_aa64(env, 3)) {
> > + /* EL3 in Aarch64 */
> > + if (!secure) {
> > + /* If non-secure, we may route to EL2 depending on other
> state.
> > + * If we are coming from the secure world then we always
> route to
> > + * EL1.
> > + */
> > + if (hcr_routing ||
> > + (cur_el == 2 && !(env->cp15.scr_el3 & SCR_RW))) {
> > + /* If HCR.FMO/IMO is set or we already in EL2 and it is
> not
> > + * configured to be AArch64 then route to EL2.
> > + */
> > + target_el = 2;
> > + }
> > + }
> > + } else {
> > + /* EL3 in Aarch32 */
> > + if (secure) {
> > + /* If coming from secure always route to EL3 */
> > + target_el = 3;
> > + } else if (hcr_routing || cur_el == 2) {
> > + /* If HCR.FMO/IMO is set or we are already EL2 then route
> to EL2 */
> > + target_el = 2;
> > + }
> > + }
> > +
> > + return target_el;
> > +}
> > +
> > +/*
> > * Determine the target EL for a given exception type.
> > */
> > unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
> > @@ -3774,14 +3854,8 @@ unsigned int arm_excp_target_el(CPUState *cs,
> unsigned int excp_idx)
> > ARMCPU *cpu = ARM_CPU(cs);
> > CPUARMState *env = &cpu->env;
> > unsigned int cur_el = arm_current_el(env);
> > - unsigned int target_el;
> > - /* FIXME: Use actual secure state. */
> > - bool secure = false;
> > -
> > - if (!env->aarch64) {
> > - /* TODO: Add EL2 and 3 exception handling for AArch32. */
> > - return 1;
> > - }
> > + unsigned int target_el = 1;
> > + bool secure = arm_is_secure(env);
> >
> > switch (excp_idx) {
> > case EXCP_HVC:
> > @@ -3793,19 +3867,8 @@ unsigned int arm_excp_target_el(CPUState *cs,
> unsigned int excp_idx)
> > break;
> > case EXCP_FIQ:
> > case EXCP_IRQ:
> > - {
> > - const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO :
> HCR_IMO;
> > - const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ :
> SCR_IRQ;
> > -
> > - target_el = 1;
> > - if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
> > - target_el = 2;
> > - }
> > - if (env->cp15.scr_el3 & scr_mask) {
> > - target_el = 3;
> > - }
> > + target_el = arm_phys_excp_target_el(cs, excp_idx, cur_el,
> secure);
> > break;
> > - }
> > case EXCP_VIRQ:
> > case EXCP_VFIQ:
> > target_el = 1;
> > --
> > 1.8.3.2
> >
>
>
> thanks
> -- PMM
>
