[Qemu-devel] [PATCH 0/5] target/arm: Implement HCR_EL2.TGE ("trap general exceptions")

[Qemu-devel] [PATCH 0/5] target/arm: Implement HCR_EL2.TGE ("trap general exceptions")

[Peter Maydell]

In the Arm architecture the HCR_EL2.TGE bit allows software to
configure the CPU so that all exceptions that would target EL1
are instead delivered to EL2. This patchset adds support for
this to QEMU.

The required behaviour is described in the ARMv8 Arm ARM documentation
of the HCR_EL2.TGE bit (in section D10.2.45 of DDI0487C.a):
 * all exceptions that would normally go to NS EL1 must go to NS EL2
 * SCTLR_EL1.M is treated as zero when NS
 * virtual interrupts are disabled
 * exception returns to NS EL1 are treated as illegal exception returns
   (and effectively the CPU cannot execute at NS EL1)
 * HCR_EL2.{FMO,IMO,AMO} are treated as one
 * MDCR_EL2.{TDRA,TDOSA,TDA,TDE} are treated as one

Parts of this we have already implemented:
 * the logic to route asynchronous exceptions (ie interrupts) in
   arm_phys_excp_target_el() already correctly handles the TGE==1 case
 * exception return code in HELPER(exception_return) for AArch64 and
   bad_mode_switch() for AArch32 already catch the exception returns
   which are illegal if TGE==1

This patchset provides the remaining parts, which turn out to
be fairly straightforward. (I was worried that this was going to
require complicated changes to all the sysreg access check code...)

(Needs a fresh git master to apply, since there's a textual
dependency on commit 042374c92e83.)

thanks
-- PMM

Peter Maydell (5):
  target/arm: Mask virtual interrupts if HCR_EL2.TGE is set
  target/arm: Honour HCR_EL2.TGE and MDCR_EL2.TDE in debug register
    access checks
  target/arm: Honour HCR_EL2.TGE when raising synchronous exceptions
  target/arm: Provide accessor functions for HCR_EL2.{IMO,FMO,AMO}
  target/arm: Treat SCTLR_EL1.M as if it were zero when HCR_EL2.TGE is
    set

 target/arm/cpu.h          | 62 ++++++++++++++++++++++++++++++++++++---
 hw/intc/arm_gicv3_cpuif.c | 19 ++++++------
 target/arm/helper.c       | 32 ++++++++++++++------
 target/arm/op_helper.c    | 14 +++++++++
 4 files changed, 105 insertions(+), 22 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/5] target/arm: Mask virtual interrupts if HCR_EL2.TGE is set

[Peter Maydell]

If the "trap general exceptions" bit HCR_EL2.TGE is set, we
must mask all virtual interrupts (as per DDI0487C.a D1.14.3).
Implement this in arm_excp_unmasked().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e310ffc29d2..efb2a8d3f3d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2261,13 +2261,15 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
         break;
 
     case EXCP_VFIQ:
-        if (secure || !(env->cp15.hcr_el2 & HCR_FMO)) {
+        if (secure || !(env->cp15.hcr_el2 & HCR_FMO)
+            || (env->cp15.hcr_el2 & HCR_TGE)) {
             /* VFIQs are only taken when hypervized and non-secure.  */
             return false;
         }
         return !(env->daif & PSTATE_F);
     case EXCP_VIRQ:
-        if (secure || !(env->cp15.hcr_el2 & HCR_IMO)) {
+        if (secure || !(env->cp15.hcr_el2 & HCR_IMO)
+            || (env->cp15.hcr_el2 & HCR_TGE)) {
             /* VIRQs are only taken when hypervized and non-secure.  */
             return false;
         }
-- 
2.17.1

[Qemu-devel] [PATCH 2/5] target/arm: Honour HCR_EL2.TGE and MDCR_EL2.TDE in debug register access checks

[Peter Maydell]

Some debug registers can be trapped via MDCR_EL2 bits TDRA, TDOSA,
and TDA, which we implement in the functions access_tdra(),
access_tdosa() and access_tda(). If MDCR_EL2.TDE or HCR_EL2.TGE
are 1, the TDRA, TDOSA and TDA bits should behave as if they were 1.
Implement this by having the access functions check MDCR_EL2.TDE
and HCR_EL2.TGE.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 22d812240af..e2930e99388 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -444,9 +444,11 @@ static CPAccessResult access_tdosa(CPUARMState *env, const ARMCPRegInfo *ri,
                                    bool isread)
 {
     int el = arm_current_el(env);
+    bool mdcr_el2_tdosa = (env->cp15.mdcr_el2 & MDCR_TDOSA) ||
+        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+        (env->cp15.hcr_el2 & HCR_TGE);
 
-    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDOSA)
-        && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tdosa && !arm_is_secure_below_el3(env)) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDOSA)) {
@@ -462,9 +464,11 @@ static CPAccessResult access_tdra(CPUARMState *env, const ARMCPRegInfo *ri,
                                   bool isread)
 {
     int el = arm_current_el(env);
+    bool mdcr_el2_tdra = (env->cp15.mdcr_el2 & MDCR_TDRA) ||
+        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+        (env->cp15.hcr_el2 & HCR_TGE);
 
-    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDRA)
-        && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tdra && !arm_is_secure_below_el3(env)) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
@@ -480,9 +484,11 @@ static CPAccessResult access_tda(CPUARMState *env, const ARMCPRegInfo *ri,
                                   bool isread)
 {
     int el = arm_current_el(env);
+    bool mdcr_el2_tda = (env->cp15.mdcr_el2 & MDCR_TDA) ||
+        (env->cp15.mdcr_el2 & MDCR_TDE) ||
+        (env->cp15.hcr_el2 & HCR_TGE);
 
-    if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDA)
-        && !arm_is_secure_below_el3(env)) {
+    if (el < 2 && mdcr_el2_tda && !arm_is_secure_below_el3(env)) {
         return CP_ACCESS_TRAP_EL2;
     }
     if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
-- 
2.17.1

[Qemu-devel] [PATCH 3/5] target/arm: Honour HCR_EL2.TGE when raising synchronous exceptions

[Peter Maydell]

Whene we raise a synchronous exception, if HCR_EL2.TGE is set then
exceptions targeting NS EL1 must be redirected to EL2.  Implement
this in raise_exception() -- all synchronous exceptions go through
this function.

(Asynchronous exceptions go via arm_cpu_exec_interrupt(), which
already honours HCR_EL2.TGE when it determines the target EL
in arm_phys_excp_target_el().)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/op_helper.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index f728f25e4bf..d550978b5b9 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -33,6 +33,20 @@ static void raise_exception(CPUARMState *env, uint32_t excp,
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
 
+    if ((env->cp15.hcr_el2 & HCR_TGE) &&
+        target_el == 1 && !arm_is_secure(env)) {
+        /*
+         * Redirect NS EL1 exceptions to NS EL2. These are reported with
+         * their original syndrome register value, with the exception of
+         * SIMD/FP access traps, which are reported as uncategorized
+         * (see DDI0478C.a D1.10.4)
+         */
+        target_el = 2;
+        if (syndrome >> ARM_EL_EC_SHIFT == EC_ADVSIMDFPACCESSTRAP) {
+            syndrome = syn_uncategorized();
+        }
+    }
+
     assert(!excp_is_internal(excp));
     cs->exception_index = excp;
     env->exception.syndrome = syndrome;
-- 
2.17.1

[Qemu-devel] [PATCH 4/5] target/arm: Provide accessor functions for HCR_EL2.{IMO, FMO, AMO}

[Peter Maydell]

The IMO, FMO and AMO bits in HCR_EL2 are defined to "behave as
1 for all purposes other than direct reads" if HCR_EL2.TGE
is set and HCR_EL2.E2H is 0, and to "behave as 0 for all
purposes other than direct reads" if HCR_EL2.TGE is set
and HRC_EL2.E2H is 1.

To avoid having to check E2H and TGE everywhere where we test IMO and
FMO, provide accessors arm_hcr_el2_imo(), arm_hcr_el2_fmo()and
arm_hcr_el2_amo().  We don't implement ARMv8.1-VHE yet, so the E2H
case will never be true, but we include the logic to save effort when
we eventually do get to that.

(Note that in several of these callsites the change doesn't
actually make a difference as either the callsite is handling
TGE specially anyway, or the CPU can't get into that situation
with TGE set; we change everywhere for consistency.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h          | 64 +++++++++++++++++++++++++++++++++++----
 hw/intc/arm_gicv3_cpuif.c | 19 ++++++------
 target/arm/helper.c       |  6 ++--
 3 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index efb2a8d3f3d..4289c33ef4c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1229,6 +1229,12 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 #define HCR_RW        (1ULL << 31)
 #define HCR_CD        (1ULL << 32)
 #define HCR_ID        (1ULL << 33)
+#define HCR_E2H       (1ULL << 34)
+/*
+ * When we actually implement ARMv8.1-VHE we should add HCR_E2H to
+ * HCR_MASK and then clear it again if the feature bit is not set in
+ * hcr_write().
+ */
 #define HCR_MASK      ((1ULL << 34) - 1)
 
 #define SCR_NS                (1U << 0)
@@ -2234,6 +2240,54 @@ bool write_cpustate_to_list(ARMCPU *cpu);
 #  define TARGET_VIRT_ADDR_SPACE_BITS 32
 #endif
 
+/**
+ * arm_hcr_el2_imo(): Return the effective value of HCR_EL2.IMO.
+ * Depending on the values of HCR_EL2.E2H and TGE, this may be
+ * "behaves as 1 for all purposes other than direct read/write" or
+ * "behaves as 0 for all purposes other than direct read/write"
+ */
+static inline bool arm_hcr_el2_imo(CPUARMState *env)
+{
+    switch (env->cp15.hcr_el2 & (HCR_TGE | HCR_E2H)) {
+    case HCR_TGE:
+        return true;
+    case HCR_TGE | HCR_E2H:
+        return false;
+    default:
+        return env->cp15.hcr_el2 & HCR_IMO;
+    }
+}
+
+/**
+ * arm_hcr_el2_fmo(): Return the effective value of HCR_EL2.FMO.
+ */
+static inline bool arm_hcr_el2_fmo(CPUARMState *env)
+{
+    switch (env->cp15.hcr_el2 & (HCR_TGE | HCR_E2H)) {
+    case HCR_TGE:
+        return true;
+    case HCR_TGE | HCR_E2H:
+        return false;
+    default:
+        return env->cp15.hcr_el2 & HCR_FMO;
+    }
+}
+
+/**
+ * arm_hcr_el2_amo(): Return the effective value of HCR_EL2.AMO.
+ */
+static inline bool arm_hcr_el2_amo(CPUARMState *env)
+{
+    switch (env->cp15.hcr_el2 & (HCR_TGE | HCR_E2H)) {
+    case HCR_TGE:
+        return true;
+    case HCR_TGE | HCR_E2H:
+        return false;
+    default:
+        return env->cp15.hcr_el2 & HCR_AMO;
+    }
+}
+
 static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
                                      unsigned int target_el)
 {
@@ -2261,15 +2315,13 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
         break;
 
     case EXCP_VFIQ:
-        if (secure || !(env->cp15.hcr_el2 & HCR_FMO)
-            || (env->cp15.hcr_el2 & HCR_TGE)) {
+        if (secure || !arm_hcr_el2_fmo(env) || (env->cp15.hcr_el2 & HCR_TGE)) {
             /* VFIQs are only taken when hypervized and non-secure.  */
             return false;
         }
         return !(env->daif & PSTATE_F);
     case EXCP_VIRQ:
-        if (secure || !(env->cp15.hcr_el2 & HCR_IMO)
-            || (env->cp15.hcr_el2 & HCR_TGE)) {
+        if (secure || !arm_hcr_el2_imo(env) || (env->cp15.hcr_el2 & HCR_TGE)) {
             /* VIRQs are only taken when hypervized and non-secure.  */
             return false;
         }
@@ -2308,7 +2360,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
                  * to the CPSR.F setting otherwise we further assess the state
                  * below.
                  */
-                hcr = (env->cp15.hcr_el2 & HCR_FMO);
+                hcr = arm_hcr_el2_fmo(env);
                 scr = (env->cp15.scr_el3 & SCR_FIQ);
 
                 /* When EL3 is 32-bit, the SCR.FW bit controls whether the
@@ -2325,7 +2377,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx,
                  * when setting the target EL, so it does not have a further
                  * affect here.
                  */
-                hcr = (env->cp15.hcr_el2 & HCR_IMO);
+                hcr = arm_hcr_el2_imo(env);
                 scr = false;
                 break;
             default:
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index 2a60568d82c..068a8e8e9b9 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -85,7 +85,10 @@ static bool icv_access(CPUARMState *env, int hcr_flags)
      *  * access if NS EL1 and either IMO or FMO == 1:
      *    CTLR, DIR, PMR, RPR
      */
-    return (env->cp15.hcr_el2 & hcr_flags) && arm_current_el(env) == 1
+    bool flagmatch = ((hcr_flags & HCR_IMO) && arm_hcr_el2_imo(env)) ||
+        ((hcr_flags & HCR_FMO) && arm_hcr_el2_fmo(env));
+
+    return flagmatch && arm_current_el(env) == 1
         && !arm_is_secure_below_el3(env);
 }
 
@@ -1549,8 +1552,8 @@ static void icc_dir_write(CPUARMState *env, const ARMCPRegInfo *ri,
     /* No need to include !IsSecure in route_*_to_el2 as it's only
      * tested in cases where we know !IsSecure is true.
      */
-    route_fiq_to_el2 = env->cp15.hcr_el2 & HCR_FMO;
-    route_irq_to_el2 = env->cp15.hcr_el2 & HCR_IMO;
+    route_fiq_to_el2 = arm_hcr_el2_fmo(env);
+    route_irq_to_el2 = arm_hcr_el2_imo(env);
 
     switch (arm_current_el(env)) {
     case 3:
@@ -1893,7 +1896,7 @@ static CPAccessResult gicv3_irqfiq_access(CPUARMState *env,
         switch (el) {
         case 1:
             if (arm_is_secure_below_el3(env) ||
-                ((env->cp15.hcr_el2 & (HCR_IMO | HCR_FMO)) == 0)) {
+                (arm_hcr_el2_imo(env) == 0 && arm_hcr_el2_fmo(env) == 0)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
@@ -1933,7 +1936,7 @@ static CPAccessResult gicv3_dir_access(CPUARMState *env,
 static CPAccessResult gicv3_sgi_access(CPUARMState *env,
                                        const ARMCPRegInfo *ri, bool isread)
 {
-    if ((env->cp15.hcr_el2 & (HCR_IMO | HCR_FMO)) &&
+    if ((arm_hcr_el2_imo(env) || arm_hcr_el2_fmo(env)) &&
         arm_current_el(env) == 1 && !arm_is_secure_below_el3(env)) {
         /* Takes priority over a possible EL3 trap */
         return CP_ACCESS_TRAP_EL2;
@@ -1958,8 +1961,7 @@ static CPAccessResult gicv3_fiq_access(CPUARMState *env,
     if (env->cp15.scr_el3 & SCR_FIQ) {
         switch (el) {
         case 1:
-            if (arm_is_secure_below_el3(env) ||
-                ((env->cp15.hcr_el2 & HCR_FMO) == 0)) {
+            if (arm_is_secure_below_el3(env) || !arm_hcr_el2_fmo(env)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
@@ -1998,8 +2000,7 @@ static CPAccessResult gicv3_irq_access(CPUARMState *env,
     if (env->cp15.scr_el3 & SCR_IRQ) {
         switch (el) {
         case 1:
-            if (arm_is_secure_below_el3(env) ||
-                ((env->cp15.hcr_el2 & HCR_IMO) == 0)) {
+            if (arm_is_secure_below_el3(env) || !arm_hcr_el2_imo(env)) {
                 r = CP_ACCESS_TRAP_EL3;
             }
             break;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e2930e99388..699189ebd7b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6342,15 +6342,15 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
     switch (excp_idx) {
     case EXCP_IRQ:
         scr = ((env->cp15.scr_el3 & SCR_IRQ) == SCR_IRQ);
-        hcr = ((env->cp15.hcr_el2 & HCR_IMO) == HCR_IMO);
+        hcr = arm_hcr_el2_imo(env);
         break;
     case EXCP_FIQ:
         scr = ((env->cp15.scr_el3 & SCR_FIQ) == SCR_FIQ);
-        hcr = ((env->cp15.hcr_el2 & HCR_FMO) == HCR_FMO);
+        hcr = arm_hcr_el2_fmo(env);
         break;
     default:
         scr = ((env->cp15.scr_el3 & SCR_EA) == SCR_EA);
-        hcr = ((env->cp15.hcr_el2 & HCR_AMO) == HCR_AMO);
+        hcr = arm_hcr_el2_amo(env);
         break;
     };
 
-- 
2.17.1

[Qemu-devel] [PATCH 5/5] target/arm: Treat SCTLR_EL1.M as if it were zero when HCR_EL2.TGE is set

[Peter Maydell]

One of the required effects of setting HCR_EL2.TGE is that when
SCR_EL3.NS is 1 then SCTLR_EL1.M must behave as if it is zero for
all purposes except direct reads. That is, it effectively disables
the MMU for the NS EL0/EL1 translation regime.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 699189ebd7b..efd258fdb59 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8402,6 +8402,14 @@ static inline bool regime_translation_disabled(CPUARMState *env,
     if (mmu_idx == ARMMMUIdx_S2NS) {
         return (env->cp15.hcr_el2 & HCR_VM) == 0;
     }
+
+    if (env->cp15.hcr_el2 & HCR_TGE) {
+        /* TGE means that NS EL0/1 act as if SCTLR_EL1.M is zero */
+        if (!regime_is_secure(env, mmu_idx) && regime_el(env, mmu_idx) == 1) {
+            return true;
+        }
+    }
+
     return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0;
 }
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/5] target/arm: Mask virtual interrupts if HCR_EL2.TGE is set

[Richard Henderson]

On 07/24/2018 04:59 AM, Peter Maydell wrote:
> If the "trap general exceptions" bit HCR_EL2.TGE is set, we
> must mask all virtual interrupts (as per DDI0487C.a D1.14.3).
> Implement this in arm_excp_unmasked().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 5/5] target/arm: Treat SCTLR_EL1.M as if it were zero when HCR_EL2.TGE is set

[Richard Henderson]

On 07/24/2018 04:59 AM, Peter Maydell wrote:
> One of the required effects of setting HCR_EL2.TGE is that when
> SCR_EL3.NS is 1 then SCTLR_EL1.M must behave as if it is zero for
> all purposes except direct reads. That is, it effectively disables
> the MMU for the NS EL0/EL1 translation regime.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 8 ++++++++
>  1 file changed, 8 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 2/5] target/arm: Honour HCR_EL2.TGE and MDCR_EL2.TDE in debug register access checks

[Richard Henderson]

On 07/24/2018 04:59 AM, Peter Maydell wrote:
> Some debug registers can be trapped via MDCR_EL2 bits TDRA, TDOSA,
> and TDA, which we implement in the functions access_tdra(),
> access_tdosa() and access_tda(). If MDCR_EL2.TDE or HCR_EL2.TGE
> are 1, the TDRA, TDOSA and TDA bits should behave as if they were 1.
> Implement this by having the access functions check MDCR_EL2.TDE
> and HCR_EL2.TGE.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 3/5] target/arm: Honour HCR_EL2.TGE when raising synchronous exceptions

[Richard Henderson]

On 07/24/2018 04:59 AM, Peter Maydell wrote:
> Whene we raise a synchronous exception, if HCR_EL2.TGE is set then
> exceptions targeting NS EL1 must be redirected to EL2.  Implement
> this in raise_exception() -- all synchronous exceptions go through
> this function.
> 
> (Asynchronous exceptions go via arm_cpu_exec_interrupt(), which
> already honours HCR_EL2.TGE when it determines the target EL
> in arm_phys_excp_target_el().)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/op_helper.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH 4/5] target/arm: Provide accessor functions for HCR_EL2.{IMO, FMO, AMO}

[Richard Henderson]

On 07/24/2018 04:59 AM, Peter Maydell wrote:
> The IMO, FMO and AMO bits in HCR_EL2 are defined to "behave as
> 1 for all purposes other than direct reads" if HCR_EL2.TGE
> is set and HCR_EL2.E2H is 0, and to "behave as 0 for all
> purposes other than direct reads" if HCR_EL2.TGE is set
> and HRC_EL2.E2H is 1.
> 
> To avoid having to check E2H and TGE everywhere where we test IMO and
> FMO, provide accessors arm_hcr_el2_imo(), arm_hcr_el2_fmo()and
> arm_hcr_el2_amo().  We don't implement ARMv8.1-VHE yet, so the E2H
> case will never be true, but we include the logic to save effort when
> we eventually do get to that.
> 
> (Note that in several of these callsites the change doesn't
> actually make a difference as either the callsite is handling
> TGE specially anyway, or the CPU can't get into that situation
> with TGE set; we change everywhere for consistency.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.h          | 64 +++++++++++++++++++++++++++++++++++----
>  hw/intc/arm_gicv3_cpuif.c | 19 ++++++------
>  target/arm/helper.c       |  6 ++--
>  3 files changed, 71 insertions(+), 18 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~