Yes, apparently I did, thanks for catching this.
I'll address the comments in v5.
Regards,
Greg
On 1 July 2014 03:22, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote:
> On Mon, Jun 30, 2014 at 06:09:09PM -0500, greg.bell...@linaro.org wrote:
> > From: Fabian Aggeler <aggel...@ethz.ch>
> >
> > This patch extends arm_excp_unmasked() according to ARM ARMv7 and
> > ARM ARMv8 (all EL running in Aarch32) and adds comments.
> >
> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> > ---
> > target-arm/cpu.h | 77
> ++++++++++++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 61 insertions(+), 16 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 44e0943..fbd7cad 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -597,6 +597,8 @@ static inline void xpsr_write(CPUARMState *env,
> uint32_t val, uint32_t mask)
> > #define SCR_IRQ (1U << 1)
> > #define SCR_FIQ (1U << 2)
> > #define SCR_EA (1U << 3)
> > +#define SCR_FW (1U << 4)
> > +#define SCR_AW (1U << 5)
> > #define SCR_SMD (1U << 7)
> > #define SCR_HCE (1U << 8)
> > #define SCR_SIF (1U << 9)
> > @@ -1199,30 +1201,73 @@ static inline bool arm_excp_unmasked(CPUState
> *cs, unsigned int excp_idx)
> > {
> > CPUARMState *env = cs->env_ptr;
> > unsigned int cur_el = arm_current_pl(env);
> > - unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> > - /* FIXME: Use actual secure state. */
> > - bool secure = false;
> > - /* Interrupts can only be hypervised and routed to
> > - * EL2 if we are in NS EL0/1.
> > - */
> > - bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> > + bool secure = arm_is_secure(env);
> > +
> > bool irq_unmasked = ((IS_M(env) && env->regs[15] < 0xfffffff0)
> > || !(env->daif & PSTATE_I));
> >
> > - /* Don't take exceptions if they target a lower EL. */
> > - if (cur_el > target_el) {
> > - return false;
> > - }
> > -
>
> Hi,
>
> I think you might have missed a few of comments on the previous review.
> If we remove the check for cur_el > target_el, we need to replace it
> with something similar for VIRQ/VFIQ. VIRQ and VFIQ target EL1 and
> cannot be taken while in EL2 or 3.
>
> Cheers,
> Edgar
>
>
> > + /* ARM ARMv7 B1.8.6 Asynchronous exception masking (table
> B1-12/B1-13)
> > + * ARM ARMv8 G1.11.3 Asynchronous exception masking controls
> > + * (table G1-18/G1-19) */
> > switch (excp_idx) {
> > case EXCP_FIQ:
> > - if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_FMO)) {
> > - return true;
> > + if (!secure) {
> > + if (arm_feature(env, ARM_FEATURE_EL2) &&
> > + (env->cp15.hcr_el2 & HCR_FMO)) {
> > + /* CPSR.F/PSTATE.F ignored if
> > + * - exception is taken from Non-secure state
> > + * - HCR.FMO == 1
> > + * - either: - not in Hyp mode
> > + * - SCR.FIQ routes exception to monitor
> mode
> > + */
> > + if (cur_el < 2) {
> > + return true;
> > + } else if (arm_feature(env, ARM_FEATURE_EL3) &&
> > + (env->cp15.scr_el3 & SCR_FIQ)) {
> > + return true;
> > + }
> > + }
> > + /* In ARMv7 only applies if both Security Extensions (EL3)
> and
> > + * Hypervirtualization Extensions (EL2) implemented, while
> > + * for ARMv8 it applies also if only EL3 implemented.
> > + */
> > + if (arm_feature(env, ARM_FEATURE_EL3) &&
> > + (arm_feature(env, ARM_FEATURE_EL2) ||
> > + arm_feature(env, ARM_FEATURE_V8))) {
> > + /* CPSR.F/PSTATE.F ignored if
> > + * - exception is taken from Non-secure state
> > + * - SCR.FIQ routes exception to monitor mode
> > + * - SCR.FW bit is set to 0
> > + * - HCR.FMO == 0 (if EL2 implemented)
> > + */
> > + if ((env->cp15.scr_el3 & SCR_FIQ) &&
> > + !(env->cp15.scr_el3 & SCR_FW)) {
> > + if (!arm_feature(env, ARM_FEATURE_EL2)) {
> > + return true;
> > + } else if (!(env->cp15.hcr_el2 & HCR_FMO)) {
> > + return true;
> > + }
> > + }
> > + }
> > }
> > return !(env->daif & PSTATE_F);
> > case EXCP_IRQ:
> > - if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
> > - return true;
> > + if (!secure) {
> > + if (arm_feature(env, ARM_FEATURE_EL2) &&
> > + (env->cp15.hcr_el2 & HCR_IMO)) {
> > + /* CPSR.I/PSTATE.I ignored if
> > + * - exception is taken from Non-secure state
> > + * - HCR.IMO == 1
> > + * - either: - not in Hyp mode
> > + * - SCR.IRQ routes exception to monitor
> mode
> > + */
> > + if (cur_el < 2) {
> > + return true;
> > + } else if (arm_feature(env, ARM_FEATURE_EL3) &&
> > + (env->cp15.scr_el3 & SCR_IRQ)) {
> > + return true;
> > + }
> > + }
> > }
> > return irq_unmasked;
> > case EXCP_VFIQ:
> > --
> > 1.8.3.2
> >
>
