[Qemu-devel] [PATCH v4 0/6] arm: Steps towards EL2 support round 3

[Qemu-devel] [PATCH v4 0/6] arm: Steps towards EL2 support round 3

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Hi,

This is what is left of round 3 of our series towards support for EL2 for
AArch64.

Comments welcome!

Best regards,
Edgar

v3 -> v4:
* Add comment clarifing the unsigned/signed timer hit arithmetics
* Replace GIC magic constants with macros
* Slightly expanded commit messages

v2 -> v3:
* Use CP_ACCESS_TRAP_EL2 instead of setting target_el

v1 -> v2:
* Drop PAR_EL1
* Add AArch32 mappings of MAIR_EL2
* Add AArch32 mappings of TCR_EL2
* Add AArch32 mappings of SCTLR_EL2
* Add AArch32 mappings of TTBR0_EL2
* Add AArch32 mappings of TPIDR_EL2
* Add AArch32 mappings of CNTHCTL_EL2
* Add AArch32 mappings of CNTVOFF_EL2
* Tag CNTVOFF_EL2 and CNTVOFF as ARM_CP_IO
* Rename TLIBALLE2 -> TLBI_ALLE2
* Break down TLB_LOCKDOWN to v7 level

Edgar E. Iglesias (6):
  target-arm: Add CNTVOFF_EL2
  target-arm: Add CNTHCTL_EL2
  target-arm: Pass timeridx as argument to various timer functions
  target-arm: Add the Hypervisor timer
  hw/arm/virt: Replace magic IRQ constants with macros
  hw/arm/virt: Connect the Hypervisor timer

 hw/arm/virt.c        |  13 ++-
 target-arm/cpu-qom.h |   1 +
 target-arm/cpu.c     |   2 +
 target-arm/cpu.h     |   5 +-
 target-arm/helper.c  | 248 ++++++++++++++++++++++++++++++++++++++++++++-------
 5 files changed, 235 insertions(+), 34 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v4 1/6] target-arm: Add CNTVOFF_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Adds support for the virtual timer offset controlled by EL2.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 21b5b8e..1a66aa4 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -355,6 +355,7 @@ typedef struct CPUARMState {
         };
         uint64_t c14_cntfrq; /* Counter Frequency register */
         uint64_t c14_cntkctl; /* Timer Control register */
+        uint64_t cntvoff_el2; /* Counter Virtual Offset register */
         ARMGenericTimer c14_timer[NUM_GTIMERS];
         uint32_t c15_cpar; /* XScale Coprocessor Access Register */
         uint32_t c15_ticonfig; /* TI925T configuration byte.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3da0c05..7901da1 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1208,9 +1208,20 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
         /* Timer enabled: calculate and set current ISTATUS, irq, and
          * reset timer to when ISTATUS next has to change
          */
+        uint64_t offset = timeridx == GTIMER_VIRT ?
+                                      cpu->env.cp15.cntvoff_el2 : 0;
         uint64_t count = gt_get_countervalue(&cpu->env);
-        /* Note that this must be unsigned 64 bit arithmetic: */
-        int istatus = count >= gt->cval;
+        /* The ARM spec says that count, offset and gt->cval are all
+         * unsigned 64bit values.
+         * The event trig is described as:
+         * (Counter[63:0] - Offset[63:0])[63:0] - CompareValue[63:0]) >= 0
+         *
+         * We do the subtractions as unsigned values to avoid under/overflowing
+         * signed integers (undefined behaviour in C).
+         * To be able to do the compare >= 0 we cast the result into a
+         * signed int64_t.
+         */
+        int istatus = (int64_t) (count - offset - gt->cval) >= 0;
         uint64_t nexttick;
 
         gt->ctl = deposit32(gt->ctl, 2, 1, istatus);
@@ -1221,7 +1232,7 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
             nexttick = UINT64_MAX;
         } else {
             /* Next transition is when we hit cval */
-            nexttick = gt->cval;
+            nexttick = gt->cval + offset;
         }
         /* Note that the desired next expiry time might be beyond the
          * signed-64-bit range of a QEMUTimer -- in this case we just
@@ -1253,6 +1264,11 @@ static uint64_t gt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
     return gt_get_countervalue(env);
 }
 
+static uint64_t gt_virt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return gt_get_countervalue(env) - env->cp15.cntvoff_el2;
+}
+
 static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
                           uint64_t value)
 {
@@ -1265,17 +1281,19 @@ static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
 static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     int timeridx = ri->crm & 1;
+    uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
 
     return (uint32_t)(env->cp15.c14_timer[timeridx].cval -
-                      gt_get_countervalue(env));
+                      gt_get_countervalue(env) - offset);
 }
 
 static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
                           uint64_t value)
 {
     int timeridx = ri->crm & 1;
+    uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
 
-    env->cp15.c14_timer[timeridx].cval = gt_get_countervalue(env) +
+    env->cp15.c14_timer[timeridx].cval = gt_get_countervalue(env) - offset +
                                          sextract64(value, 0, 32);
     gt_recalc_timer(arm_env_get_cpu(env), timeridx);
 }
@@ -1300,6 +1318,15 @@ static void gt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
     }
 }
 
+static void gt_cntvoff_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    ARMCPU *cpu = arm_env_get_cpu(env);
+
+    raw_write(env, ri, value);
+    gt_recalc_timer(cpu, GTIMER_VIRT);
+}
+
 void arm_gt_ptimer_cb(void *opaque)
 {
     ARMCPU *cpu = opaque;
@@ -1409,13 +1436,13 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
     { .name = "CNTVCT", .cp = 15, .crm = 14, .opc1 = 1,
       .access = PL0_R, .type = ARM_CP_64BIT | ARM_CP_NO_RAW | ARM_CP_IO,
       .accessfn = gt_vct_access,
-      .readfn = gt_cnt_read, .resetfn = arm_cp_reset_ignore,
+      .readfn = gt_virt_cnt_read, .resetfn = arm_cp_reset_ignore,
     },
     { .name = "CNTVCT_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 2,
       .access = PL0_R, .type = ARM_CP_NO_RAW | ARM_CP_IO,
       .accessfn = gt_vct_access,
-      .readfn = gt_cnt_read, .resetfn = gt_cnt_reset,
+      .readfn = gt_virt_cnt_read, .resetfn = gt_cnt_reset,
     },
     /* Comparison value, indicating when the timer goes off */
     { .name = "CNTP_CVAL", .cp = 15, .crm = 14, .opc1 = 2,
@@ -2539,6 +2566,12 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
     { .name = "HTTBR", .cp = 15, .opc1 = 4, .crm = 2,
       .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
       .resetvalue = 0 },
+    { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
+      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "CNTVOFF", .cp = 15, .opc1 = 4, .crm = 14,
+      .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
+      .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -2651,6 +2684,17 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 1,
       .type = ARM_CP_NO_RAW, .access = PL2_W,
       .writefn = tlbi_aa64_vaa_write },
+#ifndef CONFIG_USER_ONLY
+    { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
+      .access = PL2_RW, .type = ARM_CP_IO, .resetvalue = 0,
+      .writefn = gt_cntvoff_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.cntvoff_el2) },
+    { .name = "CNTVOFF", .cp = 15, .opc1 = 4, .crm = 14,
+      .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_ALIAS | ARM_CP_IO,
+      .writefn = gt_cntvoff_write,
+      .fieldoffset = offsetof(CPUARMState, cp15.cntvoff_el2) },
+#endif
     REGINFO_SENTINEL
 };
 
-- 
1.9.1

[Qemu-devel] [PATCH v4 2/6] target-arm: Add CNTHCTL_EL2

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Adds control for trapping selected timer and counter accesses to EL2.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 30 ++++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1a66aa4..f39c32b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -355,6 +355,7 @@ typedef struct CPUARMState {
         };
         uint64_t c14_cntfrq; /* Counter Frequency register */
         uint64_t c14_cntkctl; /* Timer Control register */
+        uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
         uint64_t cntvoff_el2; /* Counter Virtual Offset register */
         ARMGenericTimer c14_timer[NUM_GTIMERS];
         uint32_t c15_cpar; /* XScale Coprocessor Access Register */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7901da1..1795e5f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1153,8 +1153,17 @@ static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri)
 
 static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
 {
+    unsigned int cur_el = arm_current_el(env);
+    bool secure = arm_is_secure(env);
+
+    if (arm_feature(env, ARM_FEATURE_EL2) &&
+        timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
+        !extract32(env->cp15.cnthctl_el2, 0, 1)) {
+        return CP_ACCESS_TRAP_EL2;
+    }
+
     /* CNT[PV]CT: not visible from PL0 if ELO[PV]CTEN is zero */
-    if (arm_current_el(env) == 0 &&
+    if (cur_el == 0 &&
         !extract32(env->cp15.c14_cntkctl, timeridx, 1)) {
         return CP_ACCESS_TRAP;
     }
@@ -1163,10 +1172,20 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
 
 static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx)
 {
+    unsigned int cur_el = arm_current_el(env);
+    bool secure = arm_is_secure(env);
+
+    if (arm_feature(env, ARM_FEATURE_EL2)) {
+        if (timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
+            !extract32(env->cp15.cnthctl_el2, 1, 1)) {
+            return CP_ACCESS_TRAP_EL2;
+        }
+    }
+
     /* CNT[PV]_CVAL, CNT[PV]_CTL, CNT[PV]_TVAL: not visible from PL0 if
      * EL0[PV]TEN is zero.
      */
-    if (arm_current_el(env) == 0 &&
+    if (cur_el == 0 &&
         !extract32(env->cp15.c14_cntkctl, 9 - timeridx, 1)) {
         return CP_ACCESS_TRAP;
     }
@@ -2566,6 +2585,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
     { .name = "HTTBR", .cp = 15, .opc1 = 4, .crm = 2,
       .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
       .resetvalue = 0 },
+    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
@@ -2685,6 +2707,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .type = ARM_CP_NO_RAW, .access = PL2_W,
       .writefn = tlbi_aa64_vaa_write },
 #ifndef CONFIG_USER_ONLY
+    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
+      .access = PL2_RW, .resetvalue = 3,
+      .fieldoffset = offsetof(CPUARMState, cp15.cnthctl_el2) },
     { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
       .access = PL2_RW, .type = ARM_CP_IO, .resetvalue = 0,
-- 
1.9.1

[Qemu-devel] [PATCH v4 3/6] target-arm: Pass timeridx as argument to various timer functions

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Prepare for adding the Hypervisor timer, no functional change.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/helper.c | 98 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 76 insertions(+), 22 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1795e5f..410e814 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1270,10 +1270,9 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
     }
 }
 
-static void gt_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+static void gt_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri, int timeridx)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
-    int timeridx = ri->opc1 & 1;
 
     timer_del(cpu->gt_timer[timeridx]);
 }
@@ -1289,17 +1288,16 @@ static uint64_t gt_virt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
 }
 
 static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          int timeridx,
                           uint64_t value)
 {
-    int timeridx = ri->opc1 & 1;
-
     env->cp15.c14_timer[timeridx].cval = value;
     gt_recalc_timer(arm_env_get_cpu(env), timeridx);
 }
 
-static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
+static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri,
+                             int timeridx)
 {
-    int timeridx = ri->crm & 1;
     uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
 
     return (uint32_t)(env->cp15.c14_timer[timeridx].cval -
@@ -1307,9 +1305,9 @@ static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
 }
 
 static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                          int timeridx,
                           uint64_t value)
 {
-    int timeridx = ri->crm & 1;
     uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
 
     env->cp15.c14_timer[timeridx].cval = gt_get_countervalue(env) - offset +
@@ -1318,10 +1316,10 @@ static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
 }
 
 static void gt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                         int timeridx,
                          uint64_t value)
 {
     ARMCPU *cpu = arm_env_get_cpu(env);
-    int timeridx = ri->crm & 1;
     uint32_t oldval = env->cp15.c14_timer[timeridx].ctl;
 
     env->cp15.c14_timer[timeridx].ctl = deposit64(oldval, 0, 2, value);
@@ -1337,6 +1335,62 @@ static void gt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
     }
 }
 
+static void gt_phys_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    gt_cnt_reset(env, ri, GTIMER_PHYS);
+}
+
+static void gt_phys_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                               uint64_t value)
+{
+    gt_cval_write(env, ri, GTIMER_PHYS, value);
+}
+
+static uint64_t gt_phys_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return gt_tval_read(env, ri, GTIMER_PHYS);
+}
+
+static void gt_phys_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                               uint64_t value)
+{
+    gt_tval_write(env, ri, GTIMER_PHYS, value);
+}
+
+static void gt_phys_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    gt_ctl_write(env, ri, GTIMER_PHYS, value);
+}
+
+static void gt_virt_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    gt_cnt_reset(env, ri, GTIMER_VIRT);
+}
+
+static void gt_virt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                               uint64_t value)
+{
+    gt_cval_write(env, ri, GTIMER_VIRT, value);
+}
+
+static uint64_t gt_virt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return gt_tval_read(env, ri, GTIMER_VIRT);
+}
+
+static void gt_virt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                               uint64_t value)
+{
+    gt_tval_write(env, ri, GTIMER_VIRT, value);
+}
+
+static void gt_virt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    gt_ctl_write(env, ri, GTIMER_VIRT, value);
+}
+
 static void gt_cntvoff_write(CPUARMState *env, const ARMCPRegInfo *ri,
                               uint64_t value)
 {
@@ -1391,7 +1445,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .fieldoffset = offsetoflow32(CPUARMState,
                                    cp15.c14_timer[GTIMER_PHYS].ctl),
       .resetfn = arm_cp_reset_ignore,
-      .writefn = gt_ctl_write, .raw_writefn = raw_write,
+      .writefn = gt_phys_ctl_write, .raw_writefn = raw_write,
     },
     { .name = "CNTP_CTL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 2, .opc2 = 1,
@@ -1399,7 +1453,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .accessfn = gt_ptimer_access,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_PHYS].ctl),
       .resetvalue = 0,
-      .writefn = gt_ctl_write, .raw_writefn = raw_write,
+      .writefn = gt_phys_ctl_write, .raw_writefn = raw_write,
     },
     { .name = "CNTV_CTL", .cp = 15, .crn = 14, .crm = 3, .opc1 = 0, .opc2 = 1,
       .type = ARM_CP_IO | ARM_CP_ALIAS, .access = PL1_RW | PL0_R,
@@ -1407,7 +1461,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .fieldoffset = offsetoflow32(CPUARMState,
                                    cp15.c14_timer[GTIMER_VIRT].ctl),
       .resetfn = arm_cp_reset_ignore,
-      .writefn = gt_ctl_write, .raw_writefn = raw_write,
+      .writefn = gt_virt_ctl_write, .raw_writefn = raw_write,
     },
     { .name = "CNTV_CTL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 3, .opc2 = 1,
@@ -1415,30 +1469,30 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .accessfn = gt_vtimer_access,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_VIRT].ctl),
       .resetvalue = 0,
-      .writefn = gt_ctl_write, .raw_writefn = raw_write,
+      .writefn = gt_virt_ctl_write, .raw_writefn = raw_write,
     },
     /* TimerValue views: a 32 bit downcounting view of the underlying state */
     { .name = "CNTP_TVAL", .cp = 15, .crn = 14, .crm = 2, .opc1 = 0, .opc2 = 0,
       .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL1_RW | PL0_R,
       .accessfn = gt_ptimer_access,
-      .readfn = gt_tval_read, .writefn = gt_tval_write,
+      .readfn = gt_phys_tval_read, .writefn = gt_phys_tval_write,
     },
     { .name = "CNTP_TVAL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 2, .opc2 = 0,
       .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL1_RW | PL0_R,
       .accessfn = gt_ptimer_access,
-      .readfn = gt_tval_read, .writefn = gt_tval_write,
+      .readfn = gt_phys_tval_read, .writefn = gt_phys_tval_write,
     },
     { .name = "CNTV_TVAL", .cp = 15, .crn = 14, .crm = 3, .opc1 = 0, .opc2 = 0,
       .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL1_RW | PL0_R,
       .accessfn = gt_vtimer_access,
-      .readfn = gt_tval_read, .writefn = gt_tval_write,
+      .readfn = gt_virt_tval_read, .writefn = gt_virt_tval_write,
     },
     { .name = "CNTV_TVAL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 3, .opc2 = 0,
       .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL1_RW | PL0_R,
       .accessfn = gt_vtimer_access,
-      .readfn = gt_tval_read, .writefn = gt_tval_write,
+      .readfn = gt_virt_tval_read, .writefn = gt_virt_tval_write,
     },
     /* The counter itself */
     { .name = "CNTPCT", .cp = 15, .crm = 14, .opc1 = 0,
@@ -1450,7 +1504,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 1,
       .access = PL0_R, .type = ARM_CP_NO_RAW | ARM_CP_IO,
       .accessfn = gt_pct_access,
-      .readfn = gt_cnt_read, .resetfn = gt_cnt_reset,
+      .readfn = gt_cnt_read, .resetfn = gt_phys_cnt_reset,
     },
     { .name = "CNTVCT", .cp = 15, .crm = 14, .opc1 = 1,
       .access = PL0_R, .type = ARM_CP_64BIT | ARM_CP_NO_RAW | ARM_CP_IO,
@@ -1461,7 +1515,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 2,
       .access = PL0_R, .type = ARM_CP_NO_RAW | ARM_CP_IO,
       .accessfn = gt_vct_access,
-      .readfn = gt_virt_cnt_read, .resetfn = gt_cnt_reset,
+      .readfn = gt_virt_cnt_read, .resetfn = gt_virt_cnt_reset,
     },
     /* Comparison value, indicating when the timer goes off */
     { .name = "CNTP_CVAL", .cp = 15, .crm = 14, .opc1 = 2,
@@ -1469,7 +1523,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .type = ARM_CP_64BIT | ARM_CP_IO | ARM_CP_ALIAS,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_PHYS].cval),
       .accessfn = gt_ptimer_access, .resetfn = arm_cp_reset_ignore,
-      .writefn = gt_cval_write, .raw_writefn = raw_write,
+      .writefn = gt_phys_cval_write, .raw_writefn = raw_write,
     },
     { .name = "CNTP_CVAL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 2, .opc2 = 2,
@@ -1477,14 +1531,14 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .type = ARM_CP_IO,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_PHYS].cval),
       .resetvalue = 0, .accessfn = gt_ptimer_access,
-      .writefn = gt_cval_write, .raw_writefn = raw_write,
+      .writefn = gt_phys_cval_write, .raw_writefn = raw_write,
     },
     { .name = "CNTV_CVAL", .cp = 15, .crm = 14, .opc1 = 3,
       .access = PL1_RW | PL0_R,
       .type = ARM_CP_64BIT | ARM_CP_IO | ARM_CP_ALIAS,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_VIRT].cval),
       .accessfn = gt_vtimer_access, .resetfn = arm_cp_reset_ignore,
-      .writefn = gt_cval_write, .raw_writefn = raw_write,
+      .writefn = gt_virt_cval_write, .raw_writefn = raw_write,
     },
     { .name = "CNTV_CVAL_EL0", .state = ARM_CP_STATE_AA64,
       .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 3, .opc2 = 2,
@@ -1492,7 +1546,7 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
       .type = ARM_CP_IO,
       .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_VIRT].cval),
       .resetvalue = 0, .accessfn = gt_vtimer_access,
-      .writefn = gt_cval_write, .raw_writefn = raw_write,
+      .writefn = gt_virt_cval_write, .raw_writefn = raw_write,
     },
     REGINFO_SENTINEL
 };
-- 
1.9.1

[Qemu-devel] [PATCH v4 4/6] target-arm: Add the Hypervisor timer

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 target-arm/cpu-qom.h |  1 +
 target-arm/cpu.c     |  2 ++
 target-arm/cpu.h     |  3 ++-
 target-arm/helper.c  | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index ed5a644..3aaa7b6 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -214,6 +214,7 @@ int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
+void arm_gt_htimer_cb(void *opaque);
 
 #ifdef TARGET_AARCH64
 int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 4a888ab..b631482 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -409,6 +409,8 @@ static void arm_cpu_initfn(Object *obj)
                                                 arm_gt_ptimer_cb, cpu);
     cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
                                                 arm_gt_vtimer_cb, cpu);
+    cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+                                                arm_gt_htimer_cb, cpu);
     qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
                        ARRAY_SIZE(cpu->gt_timer_outputs));
 #endif
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index f39c32b..dfa9d77 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -113,7 +113,8 @@ typedef struct ARMGenericTimer {
 
 #define GTIMER_PHYS 0
 #define GTIMER_VIRT 1
-#define NUM_GTIMERS 2
+#define GTIMER_HYP  2
+#define NUM_GTIMERS 3
 
 typedef struct {
     uint64_t raw_tcr;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 410e814..aafb5d4 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1400,6 +1400,34 @@ static void gt_cntvoff_write(CPUARMState *env, const ARMCPRegInfo *ri,
     gt_recalc_timer(cpu, GTIMER_VIRT);
 }
 
+static void gt_hyp_cnt_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    gt_cnt_reset(env, ri, GTIMER_HYP);
+}
+
+static void gt_hyp_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    gt_cval_write(env, ri, GTIMER_HYP, value);
+}
+
+static uint64_t gt_hyp_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    return gt_tval_read(env, ri, GTIMER_HYP);
+}
+
+static void gt_hyp_tval_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    gt_tval_write(env, ri, GTIMER_HYP, value);
+}
+
+static void gt_hyp_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                              uint64_t value)
+{
+    gt_ctl_write(env, ri, GTIMER_HYP, value);
+}
+
 void arm_gt_ptimer_cb(void *opaque)
 {
     ARMCPU *cpu = opaque;
@@ -1414,6 +1442,13 @@ void arm_gt_vtimer_cb(void *opaque)
     gt_recalc_timer(cpu, GTIMER_VIRT);
 }
 
+void arm_gt_htimer_cb(void *opaque)
+{
+    ARMCPU *cpu = opaque;
+
+    gt_recalc_timer(cpu, GTIMER_HYP);
+}
+
 static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
     /* Note that CNTFRQ is purely reads-as-written for the benefit
      * of software; writing it doesn't actually change the timer frequency.
@@ -2648,6 +2683,18 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
     { .name = "CNTVOFF", .cp = 15, .opc1 = 4, .crm = 14,
       .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
       .resetvalue = 0 },
+    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
+    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
+    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
     REGINFO_SENTINEL
 };
 
@@ -2774,6 +2821,23 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_ALIAS | ARM_CP_IO,
       .writefn = gt_cntvoff_write,
       .fieldoffset = offsetof(CPUARMState, cp15.cntvoff_el2) },
+    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
+      .type = ARM_CP_IO,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
+      .access = PL2_RW,
+      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].ctl),
+      .resetvalue = 0,
+      .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
+    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
+      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].cval),
+      .type = ARM_CP_IO, .access = PL2_RW,
+      .writefn = gt_hyp_cval_write, .raw_writefn = raw_write },
+    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
+      .type = ARM_CP_IO, .access = PL2_RW,
+      .resetfn = gt_hyp_cnt_reset,
+      .readfn = gt_hyp_tval_read, .writefn = gt_hyp_tval_write },
 #endif
     REGINFO_SENTINEL
 };
-- 
1.9.1

[Qemu-devel] [PATCH v4 5/6] hw/arm/virt: Replace magic IRQ constants with macros

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Replace magic constants with macros from
hw/arm/virt.h and hw/intc/arm_gic_common.h.

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/virt.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 0a75cc8..4fa2265 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -47,6 +47,7 @@
 #include "hw/arm/virt-acpi-build.h"
 #include "hw/arm/sysbus-fdt.h"
 #include "hw/platform-bus.h"
+#include "hw/intc/arm_gic_common.h"
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 256
@@ -395,15 +396,17 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
      */
     for (i = 0; i < smp_cpus; i++) {
         DeviceState *cpudev = DEVICE(qemu_get_cpu(i));
-        int ppibase = NUM_IRQS + i * 32;
+        int ppibase = NUM_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS;
         /* physical timer; we wire it up to the non-secure timer's ID,
          * since a real A15 always has TrustZone but QEMU doesn't.
          */
         qdev_connect_gpio_out(cpudev, 0,
-                              qdev_get_gpio_in(gicdev, ppibase + 30));
+                              qdev_get_gpio_in(gicdev,
+                                             ppibase + ARCH_TIMER_NS_EL1_IRQ));
         /* virtual timer */
         qdev_connect_gpio_out(cpudev, 1,
-                              qdev_get_gpio_in(gicdev, ppibase + 27));
+                              qdev_get_gpio_in(gicdev,
+                                               ppibase + ARCH_TIMER_VIRT_IRQ));
 
         sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
         sysbus_connect_irq(gicbusdev, i + smp_cpus,
-- 
1.9.1

[Qemu-devel] [PATCH v4 6/6] hw/arm/virt: Connect the Hypervisor timer

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
---
 hw/arm/virt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 4fa2265..70678f7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -407,6 +407,10 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
         qdev_connect_gpio_out(cpudev, 1,
                               qdev_get_gpio_in(gicdev,
                                                ppibase + ARCH_TIMER_VIRT_IRQ));
+        /* Hypervisor timer.  */
+        qdev_connect_gpio_out(cpudev, 2,
+                              qdev_get_gpio_in(gicdev,
+                                             ppibase + ARCH_TIMER_NS_EL2_IRQ));
 
         sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
         sysbus_connect_irq(gicbusdev, i + smp_cpus,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v4 1/6] target-arm: Add CNTVOFF_EL2

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Adds support for the virtual timer offset controlled by EL2.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---

> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1208,9 +1208,20 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
>          /* Timer enabled: calculate and set current ISTATUS, irq, and
>           * reset timer to when ISTATUS next has to change
>           */
> +        uint64_t offset = timeridx == GTIMER_VIRT ?
> +                                      cpu->env.cp15.cntvoff_el2 : 0;
>          uint64_t count = gt_get_countervalue(&cpu->env);
> -        /* Note that this must be unsigned 64 bit arithmetic: */
> -        int istatus = count >= gt->cval;
> +        /* The ARM spec says that count, offset and gt->cval are all
> +         * unsigned 64bit values.
> +         * The event trig is described as:
> +         * (Counter[63:0] - Offset[63:0])[63:0] - CompareValue[63:0]) >= 0
> +         *
> +         * We do the subtractions as unsigned values to avoid under/overflowing
> +         * signed integers (undefined behaviour in C).
> +         * To be able to do the compare >= 0 we cast the result into a
> +         * signed int64_t.
> +         */
> +        int istatus = (int64_t) (count - offset - gt->cval) >= 0;

This is wrong. Consider the case where:
 count is 0x1000,0000,0000,0002 (it's a really large unsigned number)
 offset is zero
 cval is 1

The ARM ARM required calculation gives you
  0x1000,0000,0000,0002 - 1 >= 0
ie 0x1000,0000,0000,0001 >= 0
which is true. (Note that ARM ARM pseudocode works with infinite
precision integers, not 2s-complement.)

With your code:
  (count - offset - gt->cval) is 0x1000,0000,0000,0001
  Cast to an int64_t this is negative (top bit is set)
  Comparison against 0 is done as a signed value, and returns false.

This is exactly the tricky case which is why we must do this as unsigned
arithmetic.

What you want is
    int istatus = count - offset >= gt->cval;

which comes out to
    0x1000,0000,0000,0002 >= 1
which is true.

(That's the code we had before, but just "use 'count - offset' rather than
'count'".)

> @@ -1265,17 +1281,19 @@ static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
>  static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      int timeridx = ri->crm & 1;
> +    uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
>
>      return (uint32_t)(env->cp15.c14_timer[timeridx].cval -
> -                      gt_get_countervalue(env));
> +                      gt_get_countervalue(env) - offset);

The docs say that the timerval read view is
   (comparevalue - (counter - offset))
not (comparevalue - counter - offset)...

>  }

Looks OK otherwise.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 2/6] target-arm: Add CNTHCTL_EL2

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Adds control for trapping selected timer and counter accesses to EL2.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  target-arm/cpu.h    |  1 +
>  target-arm/helper.c | 30 ++++++++++++++++++++++++++++--
>  2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1a66aa4..f39c32b 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -355,6 +355,7 @@ typedef struct CPUARMState {
>          };
>          uint64_t c14_cntfrq; /* Counter Frequency register */
>          uint64_t c14_cntkctl; /* Timer Control register */
> +        uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
>          uint64_t cntvoff_el2; /* Counter Virtual Offset register */
>          ARMGenericTimer c14_timer[NUM_GTIMERS];
>          uint32_t c15_cpar; /* XScale Coprocessor Access Register */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7901da1..1795e5f 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1153,8 +1153,17 @@ static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri)
>
>  static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
>  {
> +    unsigned int cur_el = arm_current_el(env);
> +    bool secure = arm_is_secure(env);
> +
> +    if (arm_feature(env, ARM_FEATURE_EL2) &&
> +        timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
> +        !extract32(env->cp15.cnthctl_el2, 0, 1)) {
> +        return CP_ACCESS_TRAP_EL2;
> +    }

The CNTKCTL controls take precedence over the CNTHCTL ones, so
this check needs to go below the existing one.

> +
>      /* CNT[PV]CT: not visible from PL0 if ELO[PV]CTEN is zero */
> -    if (arm_current_el(env) == 0 &&
> +    if (cur_el == 0 &&
>          !extract32(env->cp15.c14_cntkctl, timeridx, 1)) {
>          return CP_ACCESS_TRAP;
>      }
> @@ -1163,10 +1172,20 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
>
>  static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx)
>  {
> +    unsigned int cur_el = arm_current_el(env);
> +    bool secure = arm_is_secure(env);
> +
> +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> +        if (timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
> +            !extract32(env->cp15.cnthctl_el2, 1, 1)) {
> +            return CP_ACCESS_TRAP_EL2;
> +        }
> +    }

Wrong order again.

> +
>      /* CNT[PV]_CVAL, CNT[PV]_CTL, CNT[PV]_TVAL: not visible from PL0 if
>       * EL0[PV]TEN is zero.
>       */
> -    if (arm_current_el(env) == 0 &&
> +    if (cur_el == 0 &&
>          !extract32(env->cp15.c14_cntkctl, 9 - timeridx, 1)) {
>          return CP_ACCESS_TRAP;
>      }
> @@ -2566,6 +2585,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>      { .name = "HTTBR", .cp = 15, .opc1 = 4, .crm = 2,
>        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
>        .resetvalue = 0 },
> +    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>      { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> @@ -2685,6 +2707,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .type = ARM_CP_NO_RAW, .access = PL2_W,
>        .writefn = tlbi_aa64_vaa_write },
>  #ifndef CONFIG_USER_ONLY
> +    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
> +      .access = PL2_RW, .resetvalue = 3,

Why 3? The ARM ARM says the resetvalue is IMPDEF and might
be UNKNOWN.

> +      .fieldoffset = offsetof(CPUARMState, cp15.cnthctl_el2) },
>      { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
>        .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
>        .access = PL2_RW, .type = ARM_CP_IO, .resetvalue = 0,
> --
> 1.9.1
>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 3/6] target-arm: Pass timeridx as argument to various timer functions

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Prepare for adding the Hypervisor timer, no functional change.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Kind of sad to get a pile of wrapper functions, but I guess there's
no choice.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/6] target-arm: Add the Hypervisor timer

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---

>  static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
>      /* Note that CNTFRQ is purely reads-as-written for the benefit
>       * of software; writing it doesn't actually change the timer frequency.
> @@ -2648,6 +2683,18 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>      { .name = "CNTVOFF", .cp = 15, .opc1 = 4, .crm = 14,
>        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
>        .resetvalue = 0 },
> +    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
> +    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
> +    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },

raz/wi should be implemented as ARM_CP_CONST...

Consider ordering these three defs CVAL, TVAL, CTL, so they're
in the natural order by opc2? (Ditto below.)

Can we have the AArch32 bindings too, please?

>      REGINFO_SENTINEL
>  };
>
> @@ -2774,6 +2821,23 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_ALIAS | ARM_CP_IO,
>        .writefn = gt_cntvoff_write,
>        .fieldoffset = offsetof(CPUARMState, cp15.cntvoff_el2) },
> +    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
> +      .type = ARM_CP_IO,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
> +      .access = PL2_RW,
> +      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].ctl),
> +      .resetvalue = 0,
> +      .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
> +    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
> +      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].cval),
> +      .type = ARM_CP_IO, .access = PL2_RW,
> +      .writefn = gt_hyp_cval_write, .raw_writefn = raw_write },
> +    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
> +      .type = ARM_CP_IO, .access = PL2_RW,
> +      .resetfn = gt_hyp_cnt_reset,
> +      .readfn = gt_hyp_tval_read, .writefn = gt_hyp_tval_write },
>  #endif
>      REGINFO_SENTINEL
>  };

-- PMM

Re: [Qemu-devel] [PATCH v4 5/6] hw/arm/virt: Replace magic IRQ constants with macros

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Replace magic constants with macros from
> hw/arm/virt.h and hw/intc/arm_gic_common.h.
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 6/6] hw/arm/virt: Connect the Hypervisor timer

[Peter Maydell]

On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 1/6] target-arm: Add CNTVOFF_EL2

[Edgar E. Iglesias]

On Fri, Jun 12, 2015 at 05:44:24PM +0100, Peter Maydell wrote:
> On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Adds support for the virtual timer offset controlled by EL2.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> 
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -1208,9 +1208,20 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
> >          /* Timer enabled: calculate and set current ISTATUS, irq, and
> >           * reset timer to when ISTATUS next has to change
> >           */
> > +        uint64_t offset = timeridx == GTIMER_VIRT ?
> > +                                      cpu->env.cp15.cntvoff_el2 : 0;
> >          uint64_t count = gt_get_countervalue(&cpu->env);
> > -        /* Note that this must be unsigned 64 bit arithmetic: */
> > -        int istatus = count >= gt->cval;
> > +        /* The ARM spec says that count, offset and gt->cval are all
> > +         * unsigned 64bit values.
> > +         * The event trig is described as:
> > +         * (Counter[63:0] - Offset[63:0])[63:0] - CompareValue[63:0]) >= 0
> > +         *
> > +         * We do the subtractions as unsigned values to avoid under/overflowing
> > +         * signed integers (undefined behaviour in C).
> > +         * To be able to do the compare >= 0 we cast the result into a
> > +         * signed int64_t.
> > +         */
> > +        int istatus = (int64_t) (count - offset - gt->cval) >= 0;
> 
> This is wrong. Consider the case where:
>  count is 0x1000,0000,0000,0002 (it's a really large unsigned number)
>  offset is zero
>  cval is 1
> 
> The ARM ARM required calculation gives you
>   0x1000,0000,0000,0002 - 1 >= 0
> ie 0x1000,0000,0000,0001 >= 0
> which is true. (Note that ARM ARM pseudocode works with infinite
> precision integers, not 2s-complement.)
> 
> With your code:
>   (count - offset - gt->cval) is 0x1000,0000,0000,0001
>   Cast to an int64_t this is negative (top bit is set)
>   Comparison against 0 is done as a signed value, and returns false.
> 
> This is exactly the tricky case which is why we must do this as unsigned
> arithmetic.
> 
> What you want is
>     int istatus = count - offset >= gt->cval;
> 
> which comes out to
>     0x1000,0000,0000,0002 >= 1
> which is true.
> 
> (That's the code we had before, but just "use 'count - offset' rather than
> 'count'".)

Thanks, I've changed it to what you suggest allthough I'm probably missing
something cause I'm still finding the spec confusing :S


> > @@ -1265,17 +1281,19 @@ static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri,
> >  static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri)
> >  {
> >      int timeridx = ri->crm & 1;
> > +    uint64_t offset = timeridx == GTIMER_VIRT ? env->cp15.cntvoff_el2 : 0;
> >
> >      return (uint32_t)(env->cp15.c14_timer[timeridx].cval -
> > -                      gt_get_countervalue(env));
> > +                      gt_get_countervalue(env) - offset);
> 
> The docs say that the timerval read view is
>    (comparevalue - (counter - offset))
> not (comparevalue - counter - offset)...

Fixed for next version.

Cheers,
Edgar


> 
> >  }
> 
> Looks OK otherwise.
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v4 2/6] target-arm: Add CNTHCTL_EL2

[Edgar E. Iglesias]

On Fri, Jun 12, 2015 at 05:51:55PM +0100, Peter Maydell wrote:
> On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Adds control for trapping selected timer and counter accesses to EL2.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> >  target-arm/cpu.h    |  1 +
> >  target-arm/helper.c | 30 ++++++++++++++++++++++++++++--
> >  2 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> > index 1a66aa4..f39c32b 100644
> > --- a/target-arm/cpu.h
> > +++ b/target-arm/cpu.h
> > @@ -355,6 +355,7 @@ typedef struct CPUARMState {
> >          };
> >          uint64_t c14_cntfrq; /* Counter Frequency register */
> >          uint64_t c14_cntkctl; /* Timer Control register */
> > +        uint32_t cnthctl_el2; /* Counter/Timer Hyp Control register */
> >          uint64_t cntvoff_el2; /* Counter Virtual Offset register */
> >          ARMGenericTimer c14_timer[NUM_GTIMERS];
> >          uint32_t c15_cpar; /* XScale Coprocessor Access Register */
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 7901da1..1795e5f 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -1153,8 +1153,17 @@ static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo *ri)
> >
> >  static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
> >  {
> > +    unsigned int cur_el = arm_current_el(env);
> > +    bool secure = arm_is_secure(env);
> > +
> > +    if (arm_feature(env, ARM_FEATURE_EL2) &&
> > +        timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
> > +        !extract32(env->cp15.cnthctl_el2, 0, 1)) {
> > +        return CP_ACCESS_TRAP_EL2;
> > +    }
> 
> The CNTKCTL controls take precedence over the CNTHCTL ones, so
> this check needs to go below the existing one.
> 
> > +
> >      /* CNT[PV]CT: not visible from PL0 if ELO[PV]CTEN is zero */
> > -    if (arm_current_el(env) == 0 &&
> > +    if (cur_el == 0 &&
> >          !extract32(env->cp15.c14_cntkctl, timeridx, 1)) {
> >          return CP_ACCESS_TRAP;
> >      }
> > @@ -1163,10 +1172,20 @@ static CPAccessResult gt_counter_access(CPUARMState *env, int timeridx)
> >
> >  static CPAccessResult gt_timer_access(CPUARMState *env, int timeridx)
> >  {
> > +    unsigned int cur_el = arm_current_el(env);
> > +    bool secure = arm_is_secure(env);
> > +
> > +    if (arm_feature(env, ARM_FEATURE_EL2)) {
> > +        if (timeridx == GTIMER_PHYS && !secure && cur_el < 2 &&
> > +            !extract32(env->cp15.cnthctl_el2, 1, 1)) {
> > +            return CP_ACCESS_TRAP_EL2;
> > +        }
> > +    }
> 
> Wrong order again.

Fixed both.

> 
> > +
> >      /* CNT[PV]_CVAL, CNT[PV]_CTL, CNT[PV]_TVAL: not visible from PL0 if
> >       * EL0[PV]TEN is zero.
> >       */
> > -    if (arm_current_el(env) == 0 &&
> > +    if (cur_el == 0 &&
> >          !extract32(env->cp15.c14_cntkctl, 9 - timeridx, 1)) {
> >          return CP_ACCESS_TRAP;
> >      }
> > @@ -2566,6 +2585,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
> >      { .name = "HTTBR", .cp = 15, .opc1 = 4, .crm = 2,
> >        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
> >        .resetvalue = 0 },
> > +    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
> > +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> >      { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
> >        .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
> >        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> > @@ -2685,6 +2707,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
> >        .type = ARM_CP_NO_RAW, .access = PL2_W,
> >        .writefn = tlbi_aa64_vaa_write },
> >  #ifndef CONFIG_USER_ONLY
> > +    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
> > +      .access = PL2_RW, .resetvalue = 3,
> 
> Why 3? The ARM ARM says the resetvalue is IMPDEF and might
> be UNKNOWN.

Hi, I sohuld added a comment about this. The ARMv7 manual says that
bit 0 and 1 reset to 1. ARMv8 has these as IMPDEF so I figured
3 would be OK in both cases.

Does that sound OK?

Cheers,
Edgar




> 
> > +      .fieldoffset = offsetof(CPUARMState, cp15.cnthctl_el2) },
> >      { .name = "CNTVOFF_EL2", .state = ARM_CP_STATE_AA64,
> >        .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 0, .opc2 = 3,
> >        .access = PL2_RW, .type = ARM_CP_IO, .resetvalue = 0,
> > --
> > 1.9.1
> >
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH v4 4/6] target-arm: Add the Hypervisor timer

[Edgar E. Iglesias]

On Fri, Jun 12, 2015 at 06:00:15PM +0100, Peter Maydell wrote:
> On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> > ---
> 
> >  static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
> >      /* Note that CNTFRQ is purely reads-as-written for the benefit
> >       * of software; writing it doesn't actually change the timer frequency.
> > @@ -2648,6 +2683,18 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
> >      { .name = "CNTVOFF", .cp = 15, .opc1 = 4, .crm = 14,
> >        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_CONST,
> >        .resetvalue = 0 },
> > +    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
> > +      .access = PL2_RW,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
> > +    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
> > +      .access = PL2_RW,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
> > +    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
> > +      .access = PL2_RW,
> > +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
> 
> raz/wi should be implemented as ARM_CP_CONST...
> 
> Consider ordering these three defs CVAL, TVAL, CTL, so they're
> in the natural order by opc2? (Ditto below.)
> 
> Can we have the AArch32 bindings too, please?

I've added AArch32 binding, changed the reg order and used CP_CONST for next version.

Thanks,
Edgar



> 
> >      REGINFO_SENTINEL
> >  };
> >
> > @@ -2774,6 +2821,23 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
> >        .access = PL2_RW, .type = ARM_CP_64BIT | ARM_CP_ALIAS | ARM_CP_IO,
> >        .writefn = gt_cntvoff_write,
> >        .fieldoffset = offsetof(CPUARMState, cp15.cntvoff_el2) },
> > +    { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .type = ARM_CP_IO,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
> > +      .access = PL2_RW,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].ctl),
> > +      .resetvalue = 0,
> > +      .writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
> > +    { .name = "CNTHP_CVAL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 2,
> > +      .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_HYP].cval),
> > +      .type = ARM_CP_IO, .access = PL2_RW,
> > +      .writefn = gt_hyp_cval_write, .raw_writefn = raw_write },
> > +    { .name = "CNTHP_TVAL_EL2", .state = ARM_CP_STATE_AA64,
> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 0,
> > +      .type = ARM_CP_IO, .access = PL2_RW,
> > +      .resetfn = gt_hyp_cnt_reset,
> > +      .readfn = gt_hyp_tval_read, .writefn = gt_hyp_tval_write },
> >  #endif
> >      REGINFO_SENTINEL
> >  };
> 
> -- PMM

Re: [Qemu-devel] [PATCH v4 2/6] target-arm: Add CNTHCTL_EL2

[Peter Maydell]

On 15 June 2015 at 02:03, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Fri, Jun 12, 2015 at 05:51:55PM +0100, Peter Maydell wrote:
>> On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> > +    { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,
>> > +      .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,
>> > +      .access = PL2_RW, .resetvalue = 3,
>>
>> Why 3? The ARM ARM says the resetvalue is IMPDEF and might
>> be UNKNOWN.
>
> Hi, I sohuld added a comment about this. The ARMv7 manual says that
> bit 0 and 1 reset to 1. ARMv8 has these as IMPDEF so I figured
> 3 would be OK in both cases.
>
> Does that sound OK?

Yeah, with an explanatory comment that's fine.

-- PMM

Re: [Qemu-devel] [PATCH v4 1/6] target-arm: Add CNTVOFF_EL2

[Peter Maydell]

On 15 June 2015 at 01:52, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote:
> On Fri, Jun 12, 2015 at 05:44:24PM +0100, Peter Maydell wrote:
>> On 5 June 2015 at 11:33, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>> > +        int istatus = (int64_t) (count - offset - gt->cval) >= 0;
>>
>> This is wrong. Consider the case where:
>>  count is 0x1000,0000,0000,0002 (it's a really large unsigned number)
>>  offset is zero
>>  cval is 1
>>
>> The ARM ARM required calculation gives you
>>   0x1000,0000,0000,0002 - 1 >= 0
>> ie 0x1000,0000,0000,0001 >= 0
>> which is true. (Note that ARM ARM pseudocode works with infinite
>> precision integers, not 2s-complement.)
>>
>> With your code:
>>   (count - offset - gt->cval) is 0x1000,0000,0000,0001
>>   Cast to an int64_t this is negative (top bit is set)
>>   Comparison against 0 is done as a signed value, and returns false.
>>
>> This is exactly the tricky case which is why we must do this as unsigned
>> arithmetic.
>>
>> What you want is
>>     int istatus = count - offset >= gt->cval;
>>
>> which comes out to
>>     0x1000,0000,0000,0002 >= 1
>> which is true.
>>
>> (That's the code we had before, but just "use 'count - offset' rather than
>> 'count'".)
>
> Thanks, I've changed it to what you suggest allthough I'm probably missing
> something cause I'm still finding the spec confusing :S

If we had 128 bit integers we could do it your way, only casting
to int128_t rather than int64_t (which would be approximating the
pseudocode's infinite-precision signed integers with 128-bit ints,
which works because we know we don't have anything out of that
range). The tricky stuff with uint64_t is just because we don't
want to have to go to 128-bit arithmetic if we can avoid it.

As I say the thing it's easy to forget when reading ARM ARM
pseudocode is that the integer and real data types are true
mathematical integers and reals, not the limited-width uint32_t,
uint64_t, float and double types the CPU actually deals with.
There's always a conversion to/from bitstring somewhere along
the line.

(Appendix J9 has a description of the pseudocode data types and
syntax.)

-- PMM