xen: arm: cleanup traps.c

PATCH In review Ian Campbell 2 participants 39 messages Apr 17, 2015 – May 8, 2015

Share & tools

Ian Campbell

author

2015-04-17 14:01 UTC #0 lore raw

v2 cover xen: arm: cleanup traps.c

While working on reenabling 32-bit user space on arm63 I concluded that the trap handling in traps.c had grown into a twisty confusing mess. Lets try and sort that out.

This series contains two halves (after a couple of preparatory cleanups).

First clean up the goto maze which we've found ourselves in, by providing a selection of handle_* helpers e.g. for raz/ro etc and by calling those and the existing inject_* helpers directly instead of trying to have only one call to each of the latter by using goto. The handle_* helpers can also deal with the minimum allowable exception level, which simplifies things further.

To keep things simpler I've used "return handle_..." when the caller and callee both return void, since that avoids the need for 3 more lines (2 braces and the return), I think this improves clarity.

Second go through init_traps and for each bit there consolidate the handling for each type of trap (e.g. do_cp15_32, do_cp15_64, do_sysreg etc) such that all the registers whose traps are associated with that bit are kept together beneath a comment which documents why those bits are trapped, references the appropriate section of the ARMv7 and ARMv8 ARM (the v8 one in particular has a series of very useful tables per bit) and notes which registers are not explicitly handled (and therefore take the default case).

For traps which have no explicit handling (i.e. those which trap implementation defined registers) and which always hit the default case add the comment above that instead.

Do the same for the GICv3 ICC traps and timer traps.

There is probably scope for doing more, i.e. refactoring related functionality into subsystem helpers (like we do for vtimer) and even moving into separate files, but I think this is a good start.

This is a lot of patches, sorry, because I wanted to mostly go through the trap bits one at a time per patch to keep each one manageable, although I did end up compressing some of the more obvious ones.

Since last time I've addressed all (I hope!) of Julien's comments.

(R)eviewed

R xen: arm: constify union hsr and struct hsr_* where possible.
R xen: arm: Fix handling of ICC_{SGI1R,SGI0R,ASGI1R}_EL1
R xen: arm: call inject_undef_exception directly
M xen: arm: provide and use a handle_raz_wi helper
M xen: arm: Add and use r/o+raz and w/o+wi helpers
M xen: arm: add minimum exception level argument to trap handler helpers
M xen: arm: Annotate trap handler for HSR_EL2.{TWI,TWE,TSC}
M xen: arm: implement handling of ACTLR_EL1 trap
RM xen: arm: Annotate registers trapped by HCR_EL1.TIDCP
M xen: arm: implement handling of registers trapped by CPTR_EL2.TTA
M xen: arm: Annotate handlers for CPTR_EL2.Tx
M xen: arm: Annotate the handlers for HSTR_EL2.T15
M xen: arm: Annotate registers trapped by MDCR_EL2.TDRA
M xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA
xen: arm: Annotate registers trapped by MDCR_EL2.TDA
xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR
R xen: arm: Remove CNTPCT_EL0 trap handling.

	xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0

R xen: arm: Annotate source of ICC SGI register trapping

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Campbell

author

2015-04-17 14:01 UTC #1 lore raw

PATCH v2 1/19 xen: arm: constify union hsr and struct hsr_* where possible.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
 xen/arch/arm/traps.c |   41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

xen/arch/arm/traps.c +21 ~~-20~~

index aaa9d93..69b9513 100644
@@ -447,7 +447,7 @@ static vaddr_t exception_handler64(struct cpu_user_regs *regs, vaddr_t offset)
 static void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
 {
     vaddr_t handler;
-    union hsr esr = {
+    const union hsr esr = {
         .iss = 0,
         .len = instr_len,
         .ec = HSR_EC_UNKNOWN,

@@ -1141,7 +1141,7 @@ int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc)
 }
 
 #ifdef CONFIG_ARM_64
-static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr)
+static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
 {
     /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive
      * software breakpoint exception for EL1 and EL0 here.

@@ -1488,7 +1488,8 @@ static const unsigned short cc_map[16] = {
         0                       /* NV                     */
 };
 
-static int check_conditional_instr(struct cpu_user_regs *regs, union hsr hsr)
+static int check_conditional_instr(struct cpu_user_regs *regs,
+                                   const union hsr hsr)
 {
     unsigned long cpsr, cpsr_cond;
     int cond;

@@ -1533,7 +1534,7 @@ static int check_conditional_instr(struct cpu_user_regs *regs, union hsr hsr)
     return 1;
 }
 
-static void advance_pc(struct cpu_user_regs *regs, union hsr hsr)
+static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 {
     unsigned long itbits, cond, cpsr = regs->cpsr;

@@ -1574,9 +1575,9 @@ static void advance_pc(struct cpu_user_regs *regs, union hsr hsr)
 }
 
 static void do_cp15_32(struct cpu_user_regs *regs,
-                       union hsr hsr)
+                       const union hsr hsr)
 {
-    struct hsr_cp32 cp32 = hsr.cp32;
+    const struct hsr_cp32 cp32 = hsr.cp32;
     uint32_t *r = (uint32_t*)select_user_reg(regs, cp32.reg);
     struct vcpu *v = current;

@@ -1659,7 +1660,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 }
 
 static void do_cp15_64(struct cpu_user_regs *regs,
-                       union hsr hsr)
+                       const union hsr hsr)
 {
     if ( !check_conditional_instr(regs, hsr) )
     {

@@ -1676,7 +1677,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
         break;
     default:
         {
-            struct hsr_cp64 cp64 = hsr.cp64;
+            const struct hsr_cp64 cp64 = hsr.cp64;
 
             gdprintk(XENLOG_ERR,
                      "%s p15, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",

@@ -1692,9 +1693,9 @@ static void do_cp15_64(struct cpu_user_regs *regs,
     advance_pc(regs, hsr);
 }
 
-static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr)
+static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 {
-    struct hsr_cp32 cp32 = hsr.cp32;
+    const struct hsr_cp32 cp32 = hsr.cp32;
     uint32_t *r = (uint32_t *)select_user_reg(regs, cp32.reg);
     struct domain *d = current->domain;

@@ -1784,9 +1785,9 @@ static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr)
     advance_pc(regs, hsr);
 }
 
-static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr)
+static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
 {
-    struct hsr_cp64 cp64 = hsr.cp64;
+    const struct hsr_cp64 cp64 = hsr.cp64;
 
     if ( !check_conditional_instr(regs, hsr) )
     {

@@ -1804,9 +1805,9 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr)
     inject_undef_exception(regs, hsr.len);
 }
 
-static void do_cp(struct cpu_user_regs *regs, union hsr hsr)
+static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
 {
-    struct hsr_cp cp = hsr.cp;
+    const struct hsr_cp cp = hsr.cp;
 
     if ( !check_conditional_instr(regs, hsr) )
     {

@@ -1821,7 +1822,7 @@ static void do_cp(struct cpu_user_regs *regs, union hsr hsr)
 
 #ifdef CONFIG_ARM_64
 static void do_sysreg(struct cpu_user_regs *regs,
-                      union hsr hsr)
+                      const union hsr hsr)
 {
     register_t *x = select_user_reg(regs, hsr.sysreg.reg);

@@ -1918,7 +1919,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
         inject_undef64_exception(regs, hsr.len);
     default:
         {
-            struct hsr_sysreg sysreg = hsr.sysreg;
+            const struct hsr_sysreg sysreg = hsr.sysreg;
 
             gdprintk(XENLOG_ERR,
                      "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",

@@ -1997,16 +1998,16 @@ done:
 }
 
 static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
-                                      union hsr hsr)
+                                      const union hsr hsr)
 {
     register_t addr = READ_SYSREG(FAR_EL2);
     inject_iabt_exception(regs, addr, hsr.len);
 }
 
 static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
-                                     union hsr hsr)
+                                     const union hsr hsr)
 {
-    struct hsr_dabt dabt = hsr.dabt;
+    const struct hsr_dabt dabt = hsr.dabt;
     int rc;
     mmio_info_t info;

@@ -2066,7 +2067,7 @@ static void enter_hypervisor_head(struct cpu_user_regs *regs)
 
 asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 {
-    union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
+    const union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
 
     enter_hypervisor_head(regs);

Signature · 1 line

1.7.10.4

Ian Campbell

author

2015-04-17 14:01 UTC #2 lore raw

PATCH v2 2/19 xen: arm: Fix handling of ICC_{SGI1R, SGI0R, ASGI1R}_EL1

In reply to Ian Campbell

Having injected an undefined instruction we don't want to also advance pc. So return.

The ICC_{SGI0R,ASGI1R}_EL1 case was previously missing a break, so would have fallen through to the default case and injected a second undef, corrupting SPSR_EL1 and ELR_EL1 for the guest.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
v2: Remove vestigial second commit message

---
 xen/arch/arm/traps.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

xen/arch/arm/traps.c +2 -2

index 69b9513..99ceaea 100644
@@ -1908,7 +1908,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
         {
             dprintk(XENLOG_WARNING,
                     "failed emulation of sysreg ICC_SGI1R_EL1 access\n");
-            inject_undef64_exception(regs, hsr.len);
+            return inject_undef64_exception(regs, hsr.len);
         }
         break;
     case HSR_SYSREG_ICC_SGI0R_EL1:

@@ -1916,7 +1916,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
         /* TBD: Implement to support secure grp0/1 SGI forwarding */
         dprintk(XENLOG_WARNING,
                 "Emulation of sysreg ICC_SGI0R_EL1/ASGI1R_EL1 not supported\n");
-        inject_undef64_exception(regs, hsr.len);
+        return inject_undef64_exception(regs, hsr.len);
     default:
         {
             const struct hsr_sysreg sysreg = hsr.sysreg;

Signature · 1 line

1.7.10.4

Ian Campbell

author

2015-04-17 14:01 UTC #3 lore raw

PATCH v2 3/19 xen: arm: call inject_undef_exception directly

In reply to Ian Campbell

Reducing the amount of goto maze considerably.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
 xen/arch/arm/traps.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

xen/arch/arm/traps.c +26 ~~-30~~

index 99ceaea..7270116 100644
@@ -518,13 +518,13 @@ static void inject_iabt64_exception(struct cpu_user_regs *regs,
 #endif
 
 static void inject_undef_exception(struct cpu_user_regs *regs,
-                                   int instr_len)
+                                   const union hsr hsr)
 {
         if ( is_32bit_domain(current->domain) )
             inject_undef32_exception(regs);
 #ifdef CONFIG_ARM_64
         else
-            inject_undef64_exception(regs, instr_len);
+            inject_undef64_exception(regs, hsr.len);
 #endif
 }

@@ -1592,11 +1592,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
     case HSR_CPREG32(CNTP_CTL):
     case HSR_CPREG32(CNTP_TVAL):
         if ( !vtimer_emulate(regs, hsr) )
-            goto undef_cp15_32;
+            return inject_undef_exception(regs, hsr);
         break;
     case HSR_CPREG32(ACTLR):
         if ( psr_mode_is_user(regs) )
-            goto undef_cp15_32;
+            return inject_undef_exception(regs, hsr);
         if ( cp32.read )
            *r = v->arch.actlr;
         break;

@@ -1612,14 +1612,14 @@ static void do_cp15_32(struct cpu_user_regs *regs,
     case HSR_CPREG32(PMUSERENR):
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) && !hsr.cp32.read )
-            goto undef_cp15_32;
+            return inject_undef_exception(regs, hsr);
         goto cp15_32_raz_wi;
 
     case HSR_CPREG32(PMINTENSET):
     case HSR_CPREG32(PMINTENCLR):
         /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */
         if ( psr_mode_is_user(regs) )
-            goto undef_cp15_32;
+            return inject_undef_exception(regs, hsr);
         goto cp15_32_raz_wi;
     case HSR_CPREG32(PMCR):
     case HSR_CPREG32(PMCNTENSET):

@@ -1638,7 +1638,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
          * emulate that register as 0 above.
          */
         if ( psr_mode_is_user(regs) )
-            goto undef_cp15_32;
+            return inject_undef_exception(regs, hsr);
  cp15_32_raz_wi:
         if ( cp32.read )
             *r = 0;

@@ -1652,8 +1652,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
                  cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
         gdprintk(XENLOG_ERR, "unhandled 32-bit CP15 access %#x\n",
                  hsr.bits & HSR_CP32_REGS_MASK);
- undef_cp15_32:
-        inject_undef_exception(regs, hsr.len);
+        inject_undef_exception(regs, hsr);
         return;
     }
     advance_pc(regs, hsr);

@@ -1673,7 +1672,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
     case HSR_CPREG64(CNTPCT):
     case HSR_CPREG64(CNTP_CVAL):
         if ( !vtimer_emulate(regs, hsr) )
-            goto undef_cp15_64;
+            return inject_undef_exception(regs, hsr);
         break;
     default:
         {

@@ -1685,8 +1684,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
                      cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
             gdprintk(XENLOG_ERR, "unhandled 64-bit CP15 access %#x\n",
                      hsr.bits & HSR_CP64_REGS_MASK);
- undef_cp15_64:
-            inject_undef_exception(regs, hsr.len);
+            inject_undef_exception(regs, hsr);
             return;
         }
     }

@@ -1713,7 +1711,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
          * is set to 0, which we emulated below.
          */
         if ( !cp32.read )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
 
         /* Implement the minimum requirements:
          *  - Number of watchpoints: 1

@@ -1731,14 +1729,14 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
          * is set to 0, which we emulated below.
          */
         if ( !cp32.read )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
 
         *r = 0;
         break;
 
     case HSR_CPREG32(DBGDSCREXT):
         if ( usr_mode(regs) )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
 
         /* Implement debug status and control register as RAZ/WI.
          * The OS won't use Hardware debug if MDBGen not set

@@ -1756,7 +1754,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGBCR1):
     case HSR_CPREG32(DBGOSDLR):
         if ( usr_mode(regs) )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
         /* RAZ/WI */
         if ( cp32.read )
             *r = 0;

@@ -1764,10 +1762,10 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 
     case HSR_CPREG32(DBGOSLAR):
         if ( usr_mode(regs) )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
         /* WO */
         if ( cp32.read )
-            goto undef_cp14_32;
+            return inject_undef_exception(regs, hsr);
         /* else: ignore */
         break;
     default:

@@ -1777,8 +1775,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
                   cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
         gdprintk(XENLOG_ERR, "unhandled 32-bit cp14 access %#x\n",
                  hsr.bits & HSR_CP32_REGS_MASK);
- undef_cp14_32:
-        inject_undef_exception(regs, hsr.len);
+        inject_undef_exception(regs, hsr);
         return;
     }

@@ -1802,7 +1799,7 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
     gdprintk(XENLOG_ERR, "unhandled 64-bit CP14 access %#x\n",
              hsr.bits & HSR_CP64_REGS_MASK);
 
-    inject_undef_exception(regs, hsr.len);
+    inject_undef_exception(regs, hsr);
 }
 
 static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)

@@ -1817,7 +1814,7 @@ static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
 
     ASSERT(!cp.tas); /* We don't trap SIMD instruction */
     gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc);
-    inject_undef_exception(regs, hsr.len);
+    inject_undef_exception(regs, hsr);
 }
 
 #ifdef CONFIG_ARM_64

@@ -1847,7 +1844,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * undef.
          */
         if ( psr_mode_is_user(regs) )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
         goto sysreg_raz_wi;
 
     case HSR_SYSREG_MDCCSR_EL0:

@@ -1856,7 +1853,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * register as RAZ/WI above. So RO at both EL0 and EL1.
          */
         if ( !hsr.sysreg.read )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
 
         *x = 0;
         break;

@@ -1865,7 +1862,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
     case HSR_SYSREG_PMUSERENR_EL0:
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) && !hsr.sysreg.read )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
         goto sysreg_raz_wi;
     case HSR_SYSREG_PMCR_EL0:
     case HSR_SYSREG_PMCNTENSET_EL0:

@@ -1884,7 +1881,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * emulate that register as 0 above.
          */
         if ( psr_mode_is_user(regs) )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
  sysreg_raz_wi:
         if ( hsr.sysreg.read )
             *x = 0;

@@ -1894,14 +1891,14 @@ static void do_sysreg(struct cpu_user_regs *regs,
     /* Write only, Write ignore registers: */
     case HSR_SYSREG_OSLAR_EL1:
         if ( hsr.sysreg.read )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
         /* else: write ignored */
         break;
     case HSR_SYSREG_CNTP_CTL_EL0:
     case HSR_SYSREG_CNTP_TVAL_EL0:
     case HSR_SYSREG_CNTP_CVAL_EL0:
         if ( !vtimer_emulate(regs, hsr) )
-            goto undef_sysreg;
+            return inject_undef_exception(regs, hsr);
         break;
     case HSR_SYSREG_ICC_SGI1R_EL1:
         if ( !vgic_emulate(regs, hsr) )

@@ -1931,8 +1928,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
                      sysreg.reg, regs->pc);
             gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
                      hsr.bits & HSR_SYSREG_REGS_MASK);
- undef_sysreg:
-            inject_undef_exception(regs, hsr.sysreg.len);
+            inject_undef_exception(regs, hsr);
             return;
         }
     }

Signature · 1 line

1.7.10.4

Ian Campbell

author

2015-04-17 14:01 UTC #4 lore raw

PATCH v2 4/19 xen: arm: provide and use a handle_raz_wi helper

In reply to Ian Campbell

Reduces the use of goto in the trap handlers to none.

Some explicitly 32-bit types become register_t here, but that's OK, on 32-bit they are 32-bit already and on 64-bit it is fine/harmless to set the larger register, a 32-bit guest won't see the top half in any case.

Per section B1.2.1 (ARMv8 DDI0487 A.d) writes to wN registers are zero extended, so there is no risk of leaking the top half here.

Unlike the previous code the advancing of PC is handled within the helper, rather than after the end of the switch as before. So return as the handler is called.

Signed-off-by Ian Campbell <[email protected]>

---
v2: Added reference to B1.2.1

---
 xen/arch/arm/traps.c |   51 +++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

xen/arch/arm/traps.c +26 ~~-25~~

index 7270116..8b1846a 100644
@@ -1574,11 +1574,24 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
     regs->pc += hsr.len ? 4 : 2;
 }
 
+/* Read as zero + write ignore */
+static void handle_raz_wi(struct cpu_user_regs *regs,
+                          register_t *reg,
+                          bool_t read,
+                          const union hsr hsr)
+{
+    if ( read )
+        *reg = 0;
+    /* else: write ignored */
+
+    advance_pc(regs, hsr);
+}
+
 static void do_cp15_32(struct cpu_user_regs *regs,
                        const union hsr hsr)
 {
     const struct hsr_cp32 cp32 = hsr.cp32;
-    uint32_t *r = (uint32_t*)select_user_reg(regs, cp32.reg);
+    register_t *r = select_user_reg(regs, cp32.reg);
     struct vcpu *v = current;
 
     if ( !check_conditional_instr(regs, hsr) )

@@ -1613,14 +1626,14 @@ static void do_cp15_32(struct cpu_user_regs *regs,
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) && !hsr.cp32.read )
             return inject_undef_exception(regs, hsr);
-        goto cp15_32_raz_wi;
+        return handle_raz_wi(regs, r, cp32.read, hsr);
 
     case HSR_CPREG32(PMINTENSET):
     case HSR_CPREG32(PMINTENCLR):
         /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
-        goto cp15_32_raz_wi;
+        return handle_raz_wi(regs, r, cp32.read, hsr);
     case HSR_CPREG32(PMCR):
     case HSR_CPREG32(PMCNTENSET):
     case HSR_CPREG32(PMCNTENCLR):

@@ -1639,11 +1652,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
          */
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
- cp15_32_raz_wi:
-        if ( cp32.read )
-            *r = 0;
-        /* else: write ignored */
-        break;
+        return handle_raz_wi(regs, r, cp32.read, hsr);
 
     default:
         gdprintk(XENLOG_ERR,

@@ -1694,7 +1703,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
 static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 {
     const struct hsr_cp32 cp32 = hsr.cp32;
-    uint32_t *r = (uint32_t *)select_user_reg(regs, cp32.reg);
+    register_t *r = select_user_reg(regs, cp32.reg);
     struct domain *d = current->domain;
 
     if ( !check_conditional_instr(regs, hsr) )

@@ -1738,12 +1747,11 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
         if ( usr_mode(regs) )
             return inject_undef_exception(regs, hsr);
 
-        /* Implement debug status and control register as RAZ/WI.
-         * The OS won't use Hardware debug if MDBGen not set
+        /*
+         * Implement debug status and control register as RAZ/WI.
+         * The OS won't use Hardware debug if MDBGen not set.
          */
-        if ( cp32.read )
-           *r = 0;
-        break;
+        return handle_raz_wi(regs, r, cp32.read, hsr);
 
     case HSR_CPREG32(DBGVCR):
     case HSR_CPREG32(DBGBVR0):

@@ -1755,10 +1763,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGOSDLR):
         if ( usr_mode(regs) )
             return inject_undef_exception(regs, hsr);
-        /* RAZ/WI */
-        if ( cp32.read )
-            *r = 0;
-        break;
+        return handle_raz_wi(regs, r, cp32.read, hsr);
 
     case HSR_CPREG32(DBGOSLAR):
         if ( usr_mode(regs) )

@@ -1845,7 +1850,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          */
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
-        goto sysreg_raz_wi;
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
 
     case HSR_SYSREG_MDCCSR_EL0:
         /*

@@ -1863,7 +1868,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) && !hsr.sysreg.read )
             return inject_undef_exception(regs, hsr);
-        goto sysreg_raz_wi;
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
     case HSR_SYSREG_PMCR_EL0:
     case HSR_SYSREG_PMCNTENSET_EL0:
     case HSR_SYSREG_PMCNTENCLR_EL0:

@@ -1882,11 +1887,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          */
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);
- sysreg_raz_wi:
-        if ( hsr.sysreg.read )
-            *x = 0;
-        /* else: write ignored */
-        break;
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
 
     /* Write only, Write ignore registers: */
     case HSR_SYSREG_OSLAR_EL1:

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 4/19

2015-04-25 17:09 UTC #5 lore raw

Re: [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper

Hi Ian,

Quoted 7 lines · Ian Campbell

Reduces the use of goto in the trap handlers to none.
Some explicitly 32-bit types become register_t here, but that's OK, on 32-bit they are 32-bit already and on 64-bit it is fine/harmless to set the larger register, a 32-bit guest won't see the top half in any case.

Quoted 8 lines

Per section B1.2.1 (ARMv8 DDI0487 A.d) writes to wN registers are zero extended, so there is no risk of leaking the top half here.
Unlike the previous code the advancing of PC is handled within the helper, rather than after the end of the switch as before. So return as the handler is called.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #6 lore raw

PATCH v2 5/19 xen: arm: Add and use r/o+raz and w/o+wi helpers

In reply to Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: Move last paramter of a handle_ro_raz call to next patch where it
belongs.

---
 xen/arch/arm/traps.c |   52 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

xen/arch/arm/traps.c +33 ~~-19~~

index 8b1846a..b54aef6 100644
@@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
     advance_pc(regs, hsr);
 }
 
+/* Write only + write ignore */
+static void handle_wo_wi(struct cpu_user_regs *regs,
+                         register_t *reg,
+                         bool_t read,
+                         const union hsr hsr)
+{
+    if ( read )
+        return inject_undef_exception(regs, hsr);
+    /* else: ignore */
+
+    advance_pc(regs, hsr);
+}
+
+/* Read only + read as zero */
+static void handle_ro_raz(struct cpu_user_regs *regs,
+                          register_t *reg,
+                          bool_t read,
+                          const union hsr hsr)
+{
+    if ( !read )
+        return inject_undef_exception(regs, hsr);
+    /* else: raz */
+
+    *reg = 0;
+
+    advance_pc(regs, hsr);
+}
+
 static void do_cp15_32(struct cpu_user_regs *regs,
                        const union hsr hsr)
 {

@@ -1737,11 +1765,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
          * is set to 0, which we emulated below.
          */
-        if ( !cp32.read )
-            return inject_undef_exception(regs, hsr);
-
-        *r = 0;
-        break;
+        return handle_ro_raz(regs, r, cp32.read, hsr);
 
     case HSR_CPREG32(DBGDSCREXT):
         if ( usr_mode(regs) )

@@ -1768,11 +1792,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGOSLAR):
         if ( usr_mode(regs) )
             return inject_undef_exception(regs, hsr);
-        /* WO */
-        if ( cp32.read )
-            return inject_undef_exception(regs, hsr);
-        /* else: ignore */
-        break;
+        return handle_wo_wi(regs, r, cp32.read, hsr);
     default:
         gdprintk(XENLOG_ERR,
                  "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",

@@ -1857,11 +1877,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that
          * register as RAZ/WI above. So RO at both EL0 and EL1.
          */
-        if ( !hsr.sysreg.read )
-            return inject_undef_exception(regs, hsr);
-
-        *x = 0;
-        break;
+        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr);
 
     /* - Perf monitors */
     case HSR_SYSREG_PMUSERENR_EL0:

@@ -1891,10 +1907,8 @@ static void do_sysreg(struct cpu_user_regs *regs,
 
     /* Write only, Write ignore registers: */
     case HSR_SYSREG_OSLAR_EL1:
-        if ( hsr.sysreg.read )
-            return inject_undef_exception(regs, hsr);
-        /* else: write ignored */
-        break;
+        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr);
+
     case HSR_SYSREG_CNTP_CTL_EL0:
     case HSR_SYSREG_CNTP_TVAL_EL0:
     case HSR_SYSREG_CNTP_CVAL_EL0:

Signature · 1 line

1.7.10.4

Julien Grall

2015-04-25 17:16 UTC #7 lore raw

Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers

Hi Ian,

Quoted 17 lines · Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: Move last paramter of a handle_ro_raz call to next patch where it
belongs.

---
  xen/arch/arm/traps.c |   52 ++++++++++++++++++++++++++++++++------------------
  1 file changed, 33 insertions(+), 19 deletions(-)

xen/arch/arm/traps.c +1 -0

index 8b1846a..b54aef6 100644
@@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
      advance_pc(regs, hsr);
  }
 
+/* Write only + write ignore */

[..]

Quoted 1 line

+/* Read only + read as zero */

I'm not sure if we finished the discussion on those comment on v1 before you sent the v2.

The "+" is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ).

I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0.

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-05-08 10:16 UTC #8 lore raw

Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers

Quoted 19 lines · Julien Grall

Hi Ian,
Quoted 16 lines · Ian Campbell
Signed-off-by Ian Campbell <[email protected]>
---
v2: Move last paramter of a handle_ro_raz call to next patch where it
belongs.
---
  xen/arch/arm/traps.c |   52 ++++++++++++++++++++++++++++++++------------------
  1 file changed, 33 insertions(+), 19 deletions(-)
xen/arch/arm/traps.c +0 -0
index 8b1846a..b54aef6 100644
@@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
      advance_pc(regs, hsr);
  }
 

Quoted 8 lines

Quoted 1 line
xen/arch/arm/traps.c +1 -0
+/* Write only + write ignore */
[..]
Quoted 1 line
+/* Read only + read as zero */
I'm not sure if we finished the discussion on those comment on v1 before you sent the v2.

I think we hadn't.

Quoted 7 lines

The "+" is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ).
I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0.

For v3 I've made the change I proposed in
<[email protected]>

Specifically "Write only as write ignore" and "Read only as read as zero" (essentially s/+/as/)

Is that clear enough do you think?

Ian.

Julien Grall

Reviewed-by

2015-05-08 10:33 UTC #9 lore raw

Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers

Hi Ian,

Quoted 39 lines · Ian Campbell

Quoted 19 lines · Julien Grall
Hi Ian,
Quoted 16 lines · Ian Campbell
Signed-off-by Ian Campbell <[email protected]>
---
v2: Move last paramter of a handle_ro_raz call to next patch where it
belongs.
---
  xen/arch/arm/traps.c |   52 ++++++++++++++++++++++++++++++++------------------
  1 file changed, 33 insertions(+), 19 deletions(-)
xen/arch/arm/traps.c +0 -0
index 8b1846a..b54aef6 100644
@@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
      advance_pc(regs, hsr);
  }
 
Quoted 8 lines
Quoted 1 line
xen/arch/arm/traps.c +1 -0
+/* Write only + write ignore */
[..]
Quoted 1 line
+/* Read only + read as zero */
I'm not sure if we finished the discussion on those comment on v1 before you sent the v2.
I think we hadn't.
Quoted 7 lines
The "+" is very confusing for me because it indicates two parts: write only and write ignore (same for the read). Both part doesn't really fit together. Although this helper clearly choose to implement WO as WI (resp. RO as RAZ).
I think this should be clearer in order to avoid people think this can be used for RO but with a different value than 0.

Quoted 7 lines

For v3 I've made the change I proposed in
<[email protected]>
Specifically "Write only as write ignore" and "Read only as read as zero" (essentially s/+/as/)
Is that clear enough do you think?

Yes. Thanks.

With this changes:

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #10 lore raw

PATCH v2 6/19 xen: arm: add minimum exception level argument to trap handler helpers

In reply to Ian Campbell

Removes a load of boiler plate.

Signed-off-by Ian Campbell <[email protected]>

---
v2: Move last parameter of a call to handle_ro_raz here where it
belongs.
Added asserts for valid min_el values

---
 xen/arch/arm/traps.c |   73 +++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

xen/arch/arm/traps.c +39 ~~-34~~

index b54aef6..7110c66 100644
@@ -1578,8 +1578,14 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 static void handle_raz_wi(struct cpu_user_regs *regs,
                           register_t *reg,
                           bool_t read,
-                          const union hsr hsr)
+                          const union hsr hsr,
+                          int min_el)
 {
+    ASSERT((min_el == 0) || (min_el == 1));
+
+    if ( min_el > 0 && psr_mode_is_user(regs) )
+        return inject_undef_exception(regs, hsr);
+
     if ( read )
         *reg = 0;
     /* else: write ignored */

@@ -1591,8 +1597,14 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
 static void handle_wo_wi(struct cpu_user_regs *regs,
                          register_t *reg,
                          bool_t read,
-                         const union hsr hsr)
+                         const union hsr hsr,
+                         int min_el)
 {
+    ASSERT((min_el == 0) || (min_el == 1));
+
+    if ( min_el > 0 && psr_mode_is_user(regs) )
+        return inject_undef_exception(regs, hsr);
+
     if ( read )
         return inject_undef_exception(regs, hsr);
     /* else: ignore */

@@ -1604,8 +1616,14 @@ static void handle_wo_wi(struct cpu_user_regs *regs,
 static void handle_ro_raz(struct cpu_user_regs *regs,
                           register_t *reg,
                           bool_t read,
-                          const union hsr hsr)
+                          const union hsr hsr,
+                          int min_el)
 {
+    ASSERT((min_el == 0) || (min_el == 1));
+
+    if ( min_el > 0 && psr_mode_is_user(regs) )
+        return inject_undef_exception(regs, hsr);
+
     if ( !read )
         return inject_undef_exception(regs, hsr);
     /* else: raz */

@@ -1652,16 +1670,15 @@ static void do_cp15_32(struct cpu_user_regs *regs,
      */
     case HSR_CPREG32(PMUSERENR):
         /* RO at EL0. RAZ/WI at EL1 */
-        if ( psr_mode_is_user(regs) && !hsr.cp32.read )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, r, cp32.read, hsr);
+        if ( psr_mode_is_user(regs) )
+            return handle_ro_raz(regs, r, cp32.read, hsr, 0);
+        else
+            return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
     case HSR_CPREG32(PMINTENSET):
     case HSR_CPREG32(PMINTENCLR):
         /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */
-        if ( psr_mode_is_user(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, r, cp32.read, hsr);
+        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
     case HSR_CPREG32(PMCR):
     case HSR_CPREG32(PMCNTENSET):
     case HSR_CPREG32(PMCNTENCLR):

@@ -1678,9 +1695,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
          * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
          * emulate that register as 0 above.
          */
-        if ( psr_mode_is_user(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, r, cp32.read, hsr);
+        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
     default:
         gdprintk(XENLOG_ERR,

@@ -1765,17 +1780,14 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
          * is set to 0, which we emulated below.
          */
-        return handle_ro_raz(regs, r, cp32.read, hsr);
+        return handle_ro_raz(regs, r, cp32.read, hsr, 1);
 
     case HSR_CPREG32(DBGDSCREXT):
-        if ( usr_mode(regs) )
-            return inject_undef_exception(regs, hsr);
-
         /*
          * Implement debug status and control register as RAZ/WI.
          * The OS won't use Hardware debug if MDBGen not set.
          */
-        return handle_raz_wi(regs, r, cp32.read, hsr);
+        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
     case HSR_CPREG32(DBGVCR):
     case HSR_CPREG32(DBGBVR0):

@@ -1785,14 +1797,10 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGBVR1):
     case HSR_CPREG32(DBGBCR1):
     case HSR_CPREG32(DBGOSDLR):
-        if ( usr_mode(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, r, cp32.read, hsr);
+        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
     case HSR_CPREG32(DBGOSLAR):
-        if ( usr_mode(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_wo_wi(regs, r, cp32.read, hsr);
+        return handle_wo_wi(regs, r, cp32.read, hsr, 1);
     default:
         gdprintk(XENLOG_ERR,
                  "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",

@@ -1868,23 +1876,22 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * Accessible from EL1 only, but if EL0 trap happens handle as
          * undef.
          */
-        if ( psr_mode_is_user(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
     case HSR_SYSREG_MDCCSR_EL0:
         /*
          * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that
          * register as RAZ/WI above. So RO at both EL0 and EL1.
          */
-        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr);
+        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
 
     /* - Perf monitors */
     case HSR_SYSREG_PMUSERENR_EL0:
         /* RO at EL0. RAZ/WI at EL1 */
-        if ( psr_mode_is_user(regs) && !hsr.sysreg.read )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
+        if ( psr_mode_is_user(regs) )
+            return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
+        else
+            return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
     case HSR_SYSREG_PMCR_EL0:
     case HSR_SYSREG_PMCNTENSET_EL0:
     case HSR_SYSREG_PMCNTENCLR_EL0:

@@ -1901,13 +1908,11 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
          * emulate that register as 0 above.
          */
-        if ( psr_mode_is_user(regs) )
-            return inject_undef_exception(regs, hsr);
-        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr);
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
     /* Write only, Write ignore registers: */
     case HSR_SYSREG_OSLAR_EL1:
-        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr);
+        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
     case HSR_SYSREG_CNTP_CTL_EL0:
     case HSR_SYSREG_CNTP_TVAL_EL0:

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 6/19

2015-04-25 17:23 UTC #11 lore raw

Re: [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers

Hi Ian,

Quoted 3 lines · Ian Campbell

Removes a load of boiler plate.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #12 lore raw

PATCH v2 7/19 xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC}

In reply to Ian Campbell

Reference the bit which enables the trap and the section/page which describes what that bit enables.

These ones are pretty trivial, included for completeness.

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/HSR_EL2/HCR_EL2/

---
 xen/arch/arm/traps.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

xen/arch/arm/traps.c +17 -0

index 7110c66..7b79990 100644
@@ -2089,6 +2089,12 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
 
     switch (hsr.ec) {
     case HSR_EC_WFI_WFE:
+        /*
+         * HCR_EL2.TWI, HCR_EL2.TWE
+         *
+         * ARMv7 (DDI 0406C.b): B1.14.9
+         * ARMv8 (DDI 0487A.d): D1-1505 Table D1-51
+         */
         if ( !check_conditional_instr(regs, hsr) )
         {
             advance_pc(regs, hsr);

@@ -2131,6 +2137,12 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
         do_cp(regs, hsr);
         break;
     case HSR_EC_SMC32:
+        /*
+         * HCR_EL2.TSC
+         *
+         * ARMv7 (DDI 0406C.b): B1.14.8
+         * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
+         */
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_smc32);
         inject_undef32_exception(regs);

@@ -2159,6 +2171,11 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
         do_trap_hypercall(regs, &regs->x16, hsr.iss);
         break;
     case HSR_EC_SMC64:
+        /*
+         * HCR_EL2.TSC
+         *
+         * ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
+         */
         GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_smc64);
         inject_undef64_exception(regs, hsr.len);

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 7/19

2015-04-25 17:25 UTC #13 lore raw

Re: [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC}

Hi Ian,

Subject: s/HSR_EL2/HCR_EL2/

Quoted 6 lines · Ian Campbell

Reference the bit which enables the trap and the section/page which describes what that bit enables.
These ones are pretty trivial, included for completeness.
Signed-off-by Ian Campbell <[email protected]>

Other than the typo in the subject:

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #14 lore raw

PATCH v2 8/19 xen: arm: implement handling of ACTLR_EL1 trap

In reply to Ian Campbell

While annotating ACTLR I noticed that we don't appear to handle the 64-bit version of this trap. Do so and annotate everything.

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/TASC/TACR/ and s/HSR/HCR/

---
 xen/arch/arm/traps.c          |   20 ++++++++++++++++++++
 xen/include/asm-arm/sysregs.h |    1 +
 2 files changed, 21 insertions(+)

xen/arch/arm/traps.c +20 -0

index 7b79990..522701b 100644
@@ -1653,6 +1653,13 @@ static void do_cp15_32(struct cpu_user_regs *regs,
         if ( !vtimer_emulate(regs, hsr) )
             return inject_undef_exception(regs, hsr);
         break;
+
+    /*
+     * HCR_EL2.TACR / HCR.TAC
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.6
+     * ARMv8 (DDI 0487A.d): G6.2.1
+     */
     case HSR_CPREG32(ACTLR):
         if ( psr_mode_is_user(regs) )
             return inject_undef_exception(regs, hsr);

@@ -1855,9 +1862,22 @@ static void do_sysreg(struct cpu_user_regs *regs,
                       const union hsr hsr)
 {
     register_t *x = select_user_reg(regs, hsr.sysreg.reg);
+    struct vcpu *v = current;
 
     switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
     {
+    /*
+     * HCR_EL2.TACR
+     *
+     * ARMv8 (DDI 0487A.d): D7.2.1
+     */
+    case HSR_SYSREG_ACTLR_EL1:
+        if ( psr_mode_is_user(regs) )
+            return inject_undef_exception(regs, hsr);
+        if ( hsr.sysreg.read )
+           *x = v->arch.actlr;
+        break;
+
     /* RAZ/WI registers: */
     /*  - Debug */
     case HSR_SYSREG_MDSCR_EL1:

xen/include/asm-arm/sysregs.h +1 -0

index 2284fd7..d75e154 100644
@@ -72,6 +72,7 @@
                                   case HSR_SYSREG_##REG##n_EL1(15)
 
 #define HSR_SYSREG_SCTLR_EL1      HSR_SYSREG(3,0,c1, c0,0)
+#define HSR_SYSREG_ACTLR_EL1      HSR_SYSREG(3,0,c1, c0,1)
 #define HSR_SYSREG_TTBR0_EL1      HSR_SYSREG(3,0,c2, c0,0)
 #define HSR_SYSREG_TTBR1_EL1      HSR_SYSREG(3,0,c2, c0,1)
 #define HSR_SYSREG_TCR_EL1        HSR_SYSREG(3,0,c2, c0,2)

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 8/19

2015-04-25 17:29 UTC #15 lore raw

Re: [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap

Hi Ian,

Quoted 4 lines · Ian Campbell

While annotating ACTLR I noticed that we don't appear to handle the 64-bit version of this trap. Do so and annotate everything.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #16 lore raw

PATCH v2 9/19 xen: arm: Annotate registers trapped by HCR_EL1.TIDCP

In reply to Ian Campbell

This traps variety of implementation defined registers, so add a note to the default case of the respective handler.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
v2: Typo in subject

---
 xen/arch/arm/traps.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

xen/arch/arm/traps.c +16 -0

index 522701b..d908738 100644
@@ -1704,6 +1704,14 @@ static void do_cp15_32(struct cpu_user_regs *regs,
          */
         return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
+    /*
+     * HCR_EL2.TIDCP
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.3
+     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
+     *
+     * And all other unknown registers.
+     */
     default:
         gdprintk(XENLOG_ERR,
                  "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",

@@ -1954,6 +1962,14 @@ static void do_sysreg(struct cpu_user_regs *regs,
         dprintk(XENLOG_WARNING,
                 "Emulation of sysreg ICC_SGI0R_EL1/ASGI1R_EL1 not supported\n");
         return inject_undef64_exception(regs, hsr.len);
+
+    /*
+     * HCR_EL2.TIDCP
+     *
+     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
+     *
+     * And all other unknown registers.
+     */
     default:
         {
             const struct hsr_sysreg sysreg = hsr.sysreg;

Signature · 1 line

1.7.10.4

Ian Campbell

author

2015-04-17 14:01 UTC #17 lore raw

PATCH v2 10/19 xen: arm: implement handling of registers trapped by CPTR_EL2.TTA

In reply to Ian Campbell

Add explicit handler for 64-bit CP14 accesses, with more relevant debug message (as per other handlers) and to provide a place for a comment.

Signed-off-by Ian Campbell <[email protected]>

---
v2: Changed title from "xen: arm: Annotate registers trapped by
CPTR_EL2.TTA"
Add "And all other unknown registers" to new annotation.

---
 xen/arch/arm/traps.c             |   45 +++++++++++++++++++++++++++++++++++++-
 xen/include/asm-arm/perfc_defn.h |    1 +
 2 files changed, 45 insertions(+), 1 deletion(-)

xen/arch/arm/traps.c +44 -1

index d908738..afa8a95 100644
@@ -1816,6 +1816,15 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 
     case HSR_CPREG32(DBGOSLAR):
         return handle_wo_wi(regs, r, cp32.read, hsr, 1);
+
+    /*
+     * CPTR_EL2.TTA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.16
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     * And all other unknown registers.
+     */
     default:
         gdprintk(XENLOG_ERR,
                  "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",

@@ -1830,7 +1839,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     advance_pc(regs, hsr);
 }
 
-static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
+static void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
 {
     const struct hsr_cp64 cp64 = hsr.cp64;

@@ -1840,12 +1849,37 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
         return;
     }
 
+    /*
+     * CPTR_EL2.TTA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.16
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     */
     gdprintk(XENLOG_ERR,
              "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
              cp64.read ? "mrrc" : "mcrr",
              cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
     gdprintk(XENLOG_ERR, "unhandled 64-bit CP14 access %#x\n",
              hsr.bits & HSR_CP64_REGS_MASK);
+    inject_undef_exception(regs, hsr);
+}
+
+static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    struct hsr_cp64 cp64 = hsr.cp64;
+
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    gdprintk(XENLOG_ERR,
+             "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
+             cp64.read ? "mrrc" : "mcrr",
+             cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
+    gdprintk(XENLOG_ERR, "unhandled 64-bit CP14 DBG access %#x\n",
+             hsr.bits & HSR_CP64_REGS_MASK);
 
     inject_undef_exception(regs, hsr);
 }

@@ -1968,6 +2002,10 @@ static void do_sysreg(struct cpu_user_regs *regs,
      *
      * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
      *
+     * CPTR_EL2.TTA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
      * And all other unknown registers.
      */
     default:

@@ -2162,6 +2200,11 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
         perfc_incr(trap_cp14_32);
         do_cp14_32(regs, hsr);
         break;
+    case HSR_EC_CP14_64:
+        GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
+        perfc_incr(trap_cp14_64);
+        do_cp14_64(regs, hsr);
+        break;
     case HSR_EC_CP14_DBG:
         GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_cp14_dbg);

xen/include/asm-arm/perfc_defn.h +1 -0

index 46015f5..69fabe7 100644
@@ -9,6 +9,7 @@ PERFCOUNTER(trap_wfe,      "trap: wfe")
 PERFCOUNTER(trap_cp15_32,  "trap: cp15 32-bit access")
 PERFCOUNTER(trap_cp15_64,  "trap: cp15 64-bit access")
 PERFCOUNTER(trap_cp14_32,  "trap: cp14 32-bit access")
+PERFCOUNTER(trap_cp14_64,  "trap: cp14 64-bit access")
 PERFCOUNTER(trap_cp14_dbg, "trap: cp14 dbg access")
 PERFCOUNTER(trap_cp,       "trap: cp access")
 PERFCOUNTER(trap_smc32,    "trap: 32-bit smc")

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 10/19

2015-04-25 17:42 UTC #18 lore raw

Re: [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA

Hi Ian,

Quoted 5 lines · Ian Campbell

Add explicit handler for 64-bit CP14 accesses, with more relevant debug message (as per other handlers) and to provide a place for a comment.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #19 lore raw

PATCH v2 11/19 xen: arm: Annotate handlers for CPTR_EL2.Tx

In reply to Ian Campbell

Also expand on the comment when writing CPTR_EL2 to mention that most of the bits we are setting are RES1 on arm64 anyway.

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/PCTR/CPTR/
Expand the comment when writing to CPTR_EL2

---
 xen/arch/arm/traps.c |   23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

xen/arch/arm/traps.c +21 -2

index afa8a95..a2bae51 100644
@@ -110,8 +110,13 @@ void __cpuinit init_traps(void)
     /* Trap CP15 c15 used for implementation defined registers */
     WRITE_SYSREG(HSTR_T(15), HSTR_EL2);
 
-    /* Trap all coprocessor registers (0-13) except cp10 and cp11 for VFP
-     * /!\ All processors except cp10 and cp11 cannot be used in Xen
+    /* Trap all coprocessor registers (0-13) except cp10 and
+     * cp11 for VFP.
+     *
+     * /!\ All coprocessors except cp10 and cp11 cannot be used in Xen.
+     *
+     * On ARM64 the TCPx bits which we set here (0..9,12,13) are all
+     * RES1, i.e. they would trap whether we did this write or not.
      */
     WRITE_SYSREG((HCPTR_CP_MASK & ~(HCPTR_CP(10) | HCPTR_CP(11))) | HCPTR_TTA,
                  CPTR_EL2);

@@ -1710,6 +1715,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
      * ARMv7 (DDI 0406C.b): B1.14.3
      * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
      *
+     * CPTR_EL2.T{0..9,12..13}
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.12
+     * ARMv8 (DDI 0487A.d): N/A
+     *
      * And all other unknown registers.
      */
     default:

@@ -1741,6 +1751,15 @@ static void do_cp15_64(struct cpu_user_regs *regs,
         if ( !vtimer_emulate(regs, hsr) )
             return inject_undef_exception(regs, hsr);
         break;
+
+    /*
+     * CPTR_EL2.T{0..9,12..13}
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.12
+     * ARMv8 (DDI 0487A.d): N/A
+     *
+     * And all other unknown registers.
+     */
     default:
         {
             const struct hsr_cp64 cp64 = hsr.cp64;

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 11/19

2015-04-25 17:44 UTC #20 lore raw

Re: [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx

Hi Ian,

Quoted 4 lines · Ian Campbell

Also expand on the comment when writing CPTR_EL2 to mention that most of the bits we are setting are RES1 on arm64 anyway.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #21 lore raw

PATCH v2 12/19 xen: arm: Annotate the handlers for HSTR_EL2.T15

In reply to Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/Tx/T15/

---
 xen/arch/arm/traps.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

xen/arch/arm/traps.c +10 -0

index a2bae51..86b5655 100644
@@ -1720,6 +1720,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
      * ARMv7 (DDI 0406C.b): B1.14.12
      * ARMv8 (DDI 0487A.d): N/A
      *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
      * And all other unknown registers.
      */
     default:

@@ -1758,6 +1763,11 @@ static void do_cp15_64(struct cpu_user_regs *regs,
      * ARMv7 (DDI 0406C.b): B1.14.12
      * ARMv8 (DDI 0487A.d): N/A
      *
+     * HSTR_EL2.Tx
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
      * And all other unknown registers.
      */
     default:

Signature · 1 line

1.7.10.4

Julien Grall

2015-04-25 17:48 UTC #22 lore raw

Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15

Hi Ian,

Quoted 24 lines · Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/Tx/T15/

---
  xen/arch/arm/traps.c |   10 ++++++++++
  1 file changed, 10 insertions(+)

xen/arch/arm/traps.c +5 -0

index a2bae51..86b5655 100644
@@ -1720,6 +1720,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
       * And all other unknown registers.
       */
      default:

xen/arch/arm/traps.c static void do_cp15_64(struct cpu_user_regs *regs, · line 1766

Quoted 4 lines

xen/arch/arm/traps.c +1 -0

@@ -1758,6 +1763,11 @@ static void do_cp15_64(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.Tx

Should not this Tx switch to T15 too?

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-05-08 10:17 UTC #23 lore raw

Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15

Quoted 25 lines · Julien Grall

Hi Ian,

Quoted 22 lines · Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/Tx/T15/

---
  xen/arch/arm/traps.c |   10 ++++++++++
  1 file changed, 10 insertions(+)

xen/arch/arm/traps.c +5 -0

index a2bae51..86b5655 100644
@@ -1720,6 +1720,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
       * And all other unknown registers.
       */

Quoted 8 lines

xen/arch/arm/traps.c static void do_cp15_64(struct cpu_user_regs *regs, · line 1766
Quoted 6 lines
xen/arch/arm/traps.c +1 -0
      default:
@@ -1758,6 +1763,11 @@ static void do_cp15_64(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.Tx
Should not this Tx switch to T15 too?

It should and apparently I've already changed it in my tree for v3, not sure when I did that!

Julien Grall

Reviewed-by

2015-05-08 10:35 UTC #24 lore raw

Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15

Hi Ian,

Quoted 35 lines · Ian Campbell

Quoted 25 lines · Julien Grall

Hi Ian,

Quoted 22 lines · Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
v2: s/Tx/T15/

---
  xen/arch/arm/traps.c |   10 ++++++++++
  1 file changed, 10 insertions(+)

xen/arch/arm/traps.c +5 -0

index a2bae51..86b5655 100644
@@ -1720,6 +1720,11 @@ static void do_cp15_32(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
       * And all other unknown registers.
       */

Quoted 8 lines

xen/arch/arm/traps.c static void do_cp15_64(struct cpu_user_regs *regs, · line 1766
Quoted 6 lines
xen/arch/arm/traps.c +1 -0
      default:
@@ -1758,6 +1763,11 @@ static void do_cp15_64(struct cpu_user_regs *regs,
       * ARMv7 (DDI 0406C.b): B1.14.12
       * ARMv8 (DDI 0487A.d): N/A
       *
+     * HSTR_EL2.Tx
Should not this Tx switch to T15 too?

Quoted 2 lines

It should and apparently I've already changed it in my tree for v3, not sure when I did that!

With that change:

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #25 lore raw

PATCH v2 13/19 xen: arm: Annotate registers trapped by MDCR_EL2.TDRA

In reply to Ian Campbell

DBGDRAR and DBGDSAR are actually two cp or sys registers each, one 32-bit and one 64-bit. The cpregs #define is suffixed "64" and annotations are added to both handlers.

MDRAR_EL1 (arm64 version of DBGDRAR) wasn't handled, so add that here.

Signed-off-by Ian Campbell <[email protected]>

---
v2: Move comment next to default label where it belongs.
Clarify DBGDRAR vs DBGDRAR64

---
 xen/arch/arm/traps.c          |   28 ++++++++++++++++++++++++++++
 xen/include/asm-arm/cpregs.h  |    4 ++++
 xen/include/asm-arm/sysregs.h |    1 +
 3 files changed, 33 insertions(+)

xen/arch/arm/traps.c +28 -0

index 86b5655..17ddcd0 100644
@@ -1852,6 +1852,15 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
      * ARMv7 (DDI 0406C.b): B1.14.16
      * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
      *
+     * MDCR_EL2.TDRA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     *
+     * Unhandled:
+     *    DBGDRAR (32-bit accesses)
+     *    DBGDSAR (32-bit accesses)
+     *
      * And all other unknown registers.
      */
     default:

@@ -1883,6 +1892,17 @@ static void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
      *
      * ARMv7 (DDI 0406C.b): B1.14.16
      * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     * MDCR_EL2.TDRA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     *
+     * Unhandled:
+     *    DBGDRAR (64-bit accesses)
+     *    DBGDSAR (64-bit accesses)
+     *
+     * And all other unknown registers.
      */
     gdprintk(XENLOG_ERR,
              "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",

@@ -1949,6 +1969,14 @@ static void do_sysreg(struct cpu_user_regs *regs,
            *x = v->arch.actlr;
         break;
 
+    /*
+     * MDCR_EL2.TDRA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     */
+    case HSR_SYSREG_MDRAR_EL1:
+        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 1);
+
     /* RAZ/WI registers: */
     /*  - Debug */
     case HSR_SYSREG_MDSCR_EL1:

xen/include/asm-arm/cpregs.h +4 -0

index afe9148..9db8cfd 100644
@@ -89,10 +89,14 @@
 #define TEECR           p14,6,c0,c0,0   /* ThumbEE Configuration Register */
 
 /* CP14 CR1: */
+#define DBGDRAR64       p14,0,c1        /* Debug ROM Address Register (64-bit access) */
+#define DBGDRAR         p14,0,c1,c0,0   /* Debug ROM Address Register (32-bit access) */
 #define TEEHBR          p14,6,c1,c0,0   /* ThumbEE Handler Base Register */
 #define JOSCR           p14,7,c1,c0,0   /* Jazelle OS Control Register */
 
 /* CP14 CR2: */
+#define DBGDSAR64       p14,0,c2        /* Debug Self Address Offset Register (64-bit access) */
+#define DBGDSAR         p14,0,c2,c0,0   /* Debug Self Address Offset Register (32-bit access) */
 #define JMCR            p14,7,c2,c0,0   /* Jazelle Main Configuration Register */

xen/include/asm-arm/sysregs.h +1 -0

index d75e154..55457fd 100644
@@ -45,6 +45,7 @@
 #define HSR_SYSREG_DCCISW         HSR_SYSREG(1,0,c7,c14,2)
 
 #define HSR_SYSREG_MDSCR_EL1      HSR_SYSREG(2,0,c0,c2,2)
+#define HSR_SYSREG_MDRAR_EL1      HSR_SYSREG(2,0,c1,c0,0)
 #define HSR_SYSREG_OSLAR_EL1      HSR_SYSREG(2,0,c1,c0,4)
 #define HSR_SYSREG_OSDLR_EL1      HSR_SYSREG(2,0,c1,c3,4)
 #define HSR_SYSREG_MDCCSR_EL0     HSR_SYSREG(2,3,c0,c1,0)

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 13/19

2015-04-25 17:50 UTC #26 lore raw

Re: [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA

Hi Ian,

Quoted 7 lines · Ian Campbell

DBGDRAR and DBGDSAR are actually two cp or sys registers each, one 32-bit and one 64-bit. The cpregs #define is suffixed "64" and annotations are added to both handlers.
MDRAR_EL1 (arm64 version of DBGDRAR) wasn't handled, so add that here.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #27 lore raw

PATCH v2 14/19 xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA

In reply to Ian Campbell

Gather the affected handlers in a single place per trap type.

Add some HSR_SYSREG and AArch32 defines for those registers (because I'd already typed them in when I realised I didn't need them).

Signed-off-by Ian Campbell <[email protected]>

---
v2: Move comment block in cp14_dbg handler from incorrect place in
next patch
Drop stray comment

---
 xen/arch/arm/traps.c          |   51 +++++++++++++++++++++++++++++++++--------
 xen/include/asm-arm/cpregs.h  |    2 ++
 xen/include/asm-arm/sysregs.h |    2 ++
 3 files changed, 45 insertions(+), 10 deletions(-)

xen/arch/arm/traps.c +41 ~~-10~~

index 17ddcd0..7606bff 100644
@@ -1801,6 +1801,21 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
 
     switch ( hsr.bits & HSR_CP32_REGS_MASK )
     {
+    /*
+     * MDCR_EL2.TDOSA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
+     *
+     * Unhandled:
+     *    DBGOSLSR
+     *    DBGPRCR
+     */
+    case HSR_CPREG32(DBGOSLAR):
+        return handle_wo_wi(regs, r, cp32.read, hsr, 1);
+    case HSR_CPREG32(DBGOSDLR):
+        return handle_raz_wi(regs, r, cp32.read, hsr, 1);
+
     case HSR_CPREG32(DBGDIDR):
         /*
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis

@@ -1840,12 +1855,8 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGWCR0):
     case HSR_CPREG32(DBGBVR1):
     case HSR_CPREG32(DBGBCR1):
-    case HSR_CPREG32(DBGOSDLR):
         return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
-    case HSR_CPREG32(DBGOSLAR):
-        return handle_wo_wi(regs, r, cp32.read, hsr, 1);
-
     /*
      * CPTR_EL2.TTA
      *

@@ -1923,6 +1934,18 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
         return;
     }
 
+    /*
+     * MDCR_EL2.TDOSA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
+     *
+     * Unhandled:
+     *    DBGDTRTXint
+     *    DBGDTRRXint
+     *
+     * And all other unknown registers.
+     */
     gdprintk(XENLOG_ERR,
              "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
              cp64.read ? "mrrc" : "mcrr",

@@ -1977,6 +2000,20 @@ static void do_sysreg(struct cpu_user_regs *regs,
     case HSR_SYSREG_MDRAR_EL1:
         return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 1);
 
+    /*
+     * MDCR_EL2.TDOSA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58
+     *
+     * Unhandled:
+     *    OSLSR_EL1
+     *    DBGPRCR_EL1
+     */
+    case HSR_SYSREG_OSLAR_EL1:
+        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_OSDLR_EL1:
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+
     /* RAZ/WI registers: */
     /*  - Debug */
     case HSR_SYSREG_MDSCR_EL1:

@@ -1986,8 +2023,6 @@ static void do_sysreg(struct cpu_user_regs *regs,
     /*  - Watchpoints */
     HSR_SYSREG_DBG_CASES(DBGWVR):
     HSR_SYSREG_DBG_CASES(DBGWCR):
-    /*  - Double Lock Register */
-    case HSR_SYSREG_OSDLR_EL1:
     /*  - Perf monitors */
     case HSR_SYSREG_PMINTENSET_EL1:
     case HSR_SYSREG_PMINTENCLR_EL1:

@@ -2029,10 +2064,6 @@ static void do_sysreg(struct cpu_user_regs *regs,
          */
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
-    /* Write only, Write ignore registers: */
-    case HSR_SYSREG_OSLAR_EL1:
-        return handle_wo_wi(regs, x, hsr.sysreg.read, hsr, 1);
-
     case HSR_SYSREG_CNTP_CTL_EL0:
     case HSR_SYSREG_CNTP_TVAL_EL0:
     case HSR_SYSREG_CNTP_CVAL_EL0:

xen/include/asm-arm/cpregs.h +2 -0

index 9db8cfd..e5cb00c 100644
@@ -83,7 +83,9 @@
 #define DBGBVR1         p14,0,c0,c1,4   /* Breakpoint Value 1 */
 #define DBGBCR1         p14,0,c0,c1,5   /* Breakpoint Control 1 */
 #define DBGOSLAR        p14,0,c1,c0,4   /* OS Lock Access */
+#define DBGOSLSR        p14,0,c1,c1,4   /* OS Lock Status Register */
 #define DBGOSDLR        p14,0,c1,c3,4   /* OS Double Lock */
+#define DBGPRCR         p14,0,c1,c4,4   /* Debug Power Control Register */
 
 /* CP14 CR0: */
 #define TEECR           p14,6,c0,c0,0   /* ThumbEE Configuration Register */

xen/include/asm-arm/sysregs.h +2 -0

index 55457fd..570f43e 100644
@@ -47,7 +47,9 @@
 #define HSR_SYSREG_MDSCR_EL1      HSR_SYSREG(2,0,c0,c2,2)
 #define HSR_SYSREG_MDRAR_EL1      HSR_SYSREG(2,0,c1,c0,0)
 #define HSR_SYSREG_OSLAR_EL1      HSR_SYSREG(2,0,c1,c0,4)
+#define HSR_SYSREG_OSLSR_EL1      HSR_SYSREG(2,0,c1,c1,4)
 #define HSR_SYSREG_OSDLR_EL1      HSR_SYSREG(2,0,c1,c3,4)
+#define HSR_SYSREG_DBGPRCR_EL1    HSR_SYSREG(2,0,c1,c4,4)
 #define HSR_SYSREG_MDCCSR_EL0     HSR_SYSREG(2,3,c0,c1,0)
 
 #define HSR_SYSREG_DBGBVRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,4)

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 14/19

2015-04-25 17:53 UTC #28 lore raw

Re: [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA

Hi Ian,

Quoted 6 lines · Ian Campbell

Gather the affected handlers in a single place per trap type.
Add some HSR_SYSREG and AArch32 defines for those registers (because I'd already typed them in when I realised I didn't need them).
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #29 lore raw

PATCH v2 15/19 xen: arm: Annotate registers trapped by MDCR_EL2.TDA

In reply to Ian Campbell

Gather the affected handlers in a single place per trap type.

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/traps.c |   60 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 11 deletions(-)

xen/arch/arm/traps.c +49 ~~-11~~

index 7606bff..97cde45 100644
@@ -1816,6 +1816,28 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
     case HSR_CPREG32(DBGOSDLR):
         return handle_raz_wi(regs, r, cp32.read, hsr, 1);
 
+    /*
+     * MDCR_EL2.TDA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1510 Table D1-59
+     *
+     * Unhandled:
+     *    DBGDCCINT
+     *    DBGDTRRXint
+     *    DBGDTRTXint
+     *    DBGWFAR
+     *    DBGDTRTXext
+     *    DBGDTRRXext,
+     *    DBGBXVR<n>
+     *    DBGCLAIMSET
+     *    DBGCLAIMCLR
+     *    DBGAUTHSTATUS
+     *    DBGDEVID
+     *    DBGDEVID1
+     *    DBGDEVID2
+     *    DBGOSECCR
+     */
     case HSR_CPREG32(DBGDIDR):
         /*
          * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis

@@ -2014,15 +2036,38 @@ static void do_sysreg(struct cpu_user_regs *regs,
     case HSR_SYSREG_OSDLR_EL1:
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
-    /* RAZ/WI registers: */
-    /*  - Debug */
+    /*
+     * MDCR_EL2.TDA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1510 Table D1-59
+     *
+     * Unhandled:
+     *    MDCCINT_EL1
+     *    DBGDTR_EL0
+     *    DBGDTRRX_EL0
+     *    DBGDTRTX_EL0
+     *    OSDTRRX_EL1
+     *    OSDTRTX_EL1
+     *    OSECCR_EL1
+     *    DBGCLAIMSET_EL1
+     *    DBGCLAIMCLR_EL1
+     *    DBGAUTHSTATUS_EL1
+     */
     case HSR_SYSREG_MDSCR_EL1:
-    /*  - Breakpoints */
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_MDCCSR_EL0:
+        /*
+         * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that
+         * register as RAZ/WI above. So RO at both EL0 and EL1.
+         */
+        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
     HSR_SYSREG_DBG_CASES(DBGBVR):
     HSR_SYSREG_DBG_CASES(DBGBCR):
-    /*  - Watchpoints */
     HSR_SYSREG_DBG_CASES(DBGWVR):
     HSR_SYSREG_DBG_CASES(DBGWCR):
+        return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
+
+    /* RAZ/WI registers: */
     /*  - Perf monitors */
     case HSR_SYSREG_PMINTENSET_EL1:
     case HSR_SYSREG_PMINTENCLR_EL1:

@@ -2032,13 +2077,6 @@ static void do_sysreg(struct cpu_user_regs *regs,
          */
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
-    case HSR_SYSREG_MDCCSR_EL0:
-        /*
-         * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that
-         * register as RAZ/WI above. So RO at both EL0 and EL1.
-         */
-        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0);
-
     /* - Perf monitors */
     case HSR_SYSREG_PMUSERENR_EL0:
         /* RO at EL0. RAZ/WI at EL1 */

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 15/19

2015-04-25 17:55 UTC #30 lore raw

Re: [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA

Hi Ian,

Quoted 3 lines · Ian Campbell

Gather the affected handlers in a single place per trap type.
Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #31 lore raw

PATCH v2 16/19 xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR

In reply to Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/traps.c |   39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

xen/arch/arm/traps.c +34 -5

index 97cde45..d4505b5 100644
@@ -1672,6 +1672,24 @@ static void do_cp15_32(struct cpu_user_regs *regs,
            *r = v->arch.actlr;
         break;
 
+    /*
+     * MDCR_EL2.TPM
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.17
+     * ARMv8 (DDI 0487A.d): D1-1511 Table D1-61
+     *
+     * Unhandled:
+     *    PMEVCNTR<n>
+     *    PMEVTYPER<n>
+     *    PMCCFILTR
+     *
+     * MDCR_EL2.TPMCR
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.17
+     * ARMv8 (DDI 0487A.d): D1-1511 Table D1-62
+     *
+     * NB: Both MDCR_EL2.TPM and MDCR_EL2.TPMCR cause trapping of PMCR.
+     */
     /* We could trap ID_DFR0 and tell the guest we don't support
      * performance monitoring, but Linux doesn't check the ID_DFR0.
      * Therefore it will read PMCR.

@@ -1686,7 +1704,6 @@ static void do_cp15_32(struct cpu_user_regs *regs,
             return handle_ro_raz(regs, r, cp32.read, hsr, 0);
         else
             return handle_raz_wi(regs, r, cp32.read, hsr, 1);
-
     case HSR_CPREG32(PMINTENSET):
     case HSR_CPREG32(PMINTENCLR):
         /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */

@@ -2067,8 +2084,22 @@ static void do_sysreg(struct cpu_user_regs *regs,
     HSR_SYSREG_DBG_CASES(DBGWCR):
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
-    /* RAZ/WI registers: */
-    /*  - Perf monitors */
+    /*
+     * MDCR_EL2.TPM
+     *
+     * ARMv8 (DDI 0487A.d): D1-1511 Table D1-61
+     *
+     * Unhandled:
+     *    PMEVCNTR<n>_EL0
+     *    PMEVTYPER<n>_EL0
+     *    PMCCFILTR_EL0
+     * MDCR_EL2.TPMCR
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.17
+     * ARMv8 (DDI 0487A.d): D1-1511 Table D1-62
+     *
+     * NB: Both MDCR_EL2.TPM and MDCR_EL2.TPMCR cause trapping of PMCR.
+     */
     case HSR_SYSREG_PMINTENSET_EL1:
     case HSR_SYSREG_PMINTENCLR_EL1:
         /*

@@ -2076,8 +2107,6 @@ static void do_sysreg(struct cpu_user_regs *regs,
          * undef.
          */
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
-
-    /* - Perf monitors */
     case HSR_SYSREG_PMUSERENR_EL0:
         /* RO at EL0. RAZ/WI at EL1 */
         if ( psr_mode_is_user(regs) )

Signature · 1 line

1.7.10.4

Julien Grall

2015-04-26 13:46 UTC #32 lore raw

Re: [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR

Hi Ian,

NIT on the subject: I would write MDCR_EL2.{TPM,TPMCR} to show that TPMCR is a field of MDCR_EL2 and not a separate registers.

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #33 lore raw

PATCH v2 17/19 xen: arm: Remove CNTPCT_EL0 trap handling.

In reply to Ian Campbell

We set CNTHCTL_EL2.EL1PCTEN and therefore according to ARMv8 (DDI 0487A.d) D1-1510 Table D1-60 we are not trapping this.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
 xen/arch/arm/traps.c  |    1 -
 xen/arch/arm/vtimer.c |   30 ------------------------------
 2 files changed, 31 deletions(-)

xen/arch/arm/traps.c +0 -1

index d4505b5..c869b96 100644
@@ -1768,7 +1768,6 @@ static void do_cp15_64(struct cpu_user_regs *regs,
 
     switch ( hsr.bits & HSR_CP64_REGS_MASK )
     {
-    case HSR_CPREG64(CNTPCT):
     case HSR_CPREG64(CNTP_CVAL):
         if ( !vtimer_emulate(regs, hsr) )
             return inject_undef_exception(regs, hsr);

xen/arch/arm/vtimer.c +0 ~~-30~~

index be65c9f..685bfea 100644
@@ -243,28 +243,6 @@ static int vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r, int read)
     }
     return 1;
 }
-static int vtimer_cntpct(struct cpu_user_regs *regs, uint64_t *r, int read)
-{
-    struct vcpu *v = current;
-    uint64_t ticks;
-    s_time_t now;
-
-    if ( read )
-    {
-        if ( !ACCESS_ALLOWED(regs, EL0PCTEN) )
-            return 0;
-        now = NOW() - v->domain->arch.phys_timer_base.offset;
-        ticks = ns_to_ticks(now);
-        *r = ticks;
-        return 1;
-    }
-    else
-    {
-        gprintk(XENLOG_DEBUG, "WRITE to R/O CNTPCT\n");
-        return 0;
-    }
-}
-
 
 static int vtimer_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr)
 {

@@ -303,11 +281,6 @@ static int vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
 
     switch ( hsr.bits & HSR_CP64_REGS_MASK )
     {
-    case HSR_CPREG64(CNTPCT):
-        if ( !vtimer_cntpct(regs, &x, cp64.read) )
-            return 0;
-        break;
-
     case HSR_CPREG64(CNTP_CVAL):
         if ( !vtimer_cntp_cval(regs, &x, cp64.read) )
             return 0;

@@ -356,9 +329,6 @@ static int vtimer_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
     case HSR_SYSREG_CNTP_CVAL_EL0:
         return vtimer_cntp_cval(regs, x, sysreg.read);
 
-    case HSR_SYSREG_CNTPCT_EL0:
-        return vtimer_cntpct(regs, x, sysreg.read);
-
     default:
         return 0;
     }

Signature · 1 line

1.7.10.4

Ian Campbell

author

2015-04-17 14:01 UTC #34 lore raw

PATCH v2 18/19 xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0

In reply to Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

---
 xen/arch/arm/traps.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

xen/arch/arm/traps.c +18 -0

index c869b96..ad6ff05 100644
@@ -1653,6 +1653,12 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 
     switch ( hsr.bits & HSR_CP32_REGS_MASK )
     {
+    /*
+     * !CNTHCTL_EL2.EL1PCEN / !CNTHCTL.PL1PCEN
+     *
+     * ARMv7 (DDI 0406C.b): B4.1.22
+     * ARMv8 (DDI 0487A.d): D1-1510 Table D1-60
+     */
     case HSR_CPREG32(CNTP_CTL):
     case HSR_CPREG32(CNTP_TVAL):
         if ( !vtimer_emulate(regs, hsr) )

@@ -1768,6 +1774,12 @@ static void do_cp15_64(struct cpu_user_regs *regs,
 
     switch ( hsr.bits & HSR_CP64_REGS_MASK )
     {
+    /*
+     * !CNTHCTL_EL2.EL1PCEN / !CNTHCTL.PL1PCEN
+     *
+     * ARMv7 (DDI 0406C.b): B4.1.22
+     * ARMv8 (DDI 0487A.d): D1-1510 Table D1-60
+     */
     case HSR_CPREG64(CNTP_CVAL):
         if ( !vtimer_emulate(regs, hsr) )
             return inject_undef_exception(regs, hsr);

@@ -2130,12 +2142,18 @@ static void do_sysreg(struct cpu_user_regs *regs,
          */
         return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1);
 
+    /*
+     * !CNTHCTL_EL2.EL1PCEN
+     *
+     * ARMv8 (DDI 0487A.d): D1-1510 Table D1-60
+     */
     case HSR_SYSREG_CNTP_CTL_EL0:
     case HSR_SYSREG_CNTP_TVAL_EL0:
     case HSR_SYSREG_CNTP_CVAL_EL0:
         if ( !vtimer_emulate(regs, hsr) )
             return inject_undef_exception(regs, hsr);
         break;
+
     case HSR_SYSREG_ICC_SGI1R_EL1:
         if ( !vgic_emulate(regs, hsr) )
         {

Signature · 1 line

1.7.10.4

Julien Grall

Reviewed-by on v2 18/19

2015-04-26 13:48 UTC #35 lore raw

Re: [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0

Hi Ian,

Quoted 1 line · Ian Campbell

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-04-17 14:01 UTC #36 lore raw

PATCH v2 19/19 xen: arm: Annotate source of ICC SGI register trapping

In reply to Ian Campbell

I was unable to find an ARMv8 ARM reference to this, so refer to the GIC Architecture Specification instead.

ARMv8 ARM does cover other ways of trapping these accesses via ICH_HCR_EL2 but we don't use those and they trap additional registers as well.

Signed-off-by Ian Campbell <[email protected]>

Reviewed-by Julien Grall <[email protected]>

---
 xen/arch/arm/traps.c |    6 ++++++
 1 file changed, 6 insertions(+)

xen/arch/arm/traps.c +6 -0

index ad6ff05..6fe9b7a 100644
@@ -2154,6 +2154,12 @@ static void do_sysreg(struct cpu_user_regs *regs,
             return inject_undef_exception(regs, hsr);
         break;
 
+    /*
+     * HCR_EL2.FMO or HCR_EL2.IMO
+     *
+     * ARMv8: GIC Architecture Specification (PRD03-GENC-010745 24.0)
+     *        Section 4.6.8.
+     */
     case HSR_SYSREG_ICC_SGI1R_EL1:
         if ( !vgic_emulate(regs, hsr) )
         {

Signature · 1 line

1.7.10.4

Julien Grall

2015-05-08 10:37 UTC #37 lore raw

In reply to Ian Campbell

Hi Ian,

Quoted 19 lines · Ian Campbell

(R)eviewed
R xen: arm: constify union hsr and struct hsr_* where possible.
R xen: arm: Fix handling of ICC_{SGI1R,SGI0R,ASGI1R}_EL1
R xen: arm: call inject_undef_exception directly
M xen: arm: provide and use a handle_raz_wi helper
M xen: arm: Add and use r/o+raz and w/o+wi helpers
M xen: arm: add minimum exception level argument to trap handler helpers
M xen: arm: Annotate trap handler for HSR_EL2.{TWI,TWE,TSC}
M xen: arm: implement handling of ACTLR_EL1 trap
RM xen: arm: Annotate registers trapped by HCR_EL1.TIDCP
M xen: arm: implement handling of registers trapped by CPTR_EL2.TTA
M xen: arm: Annotate handlers for CPTR_EL2.Tx
M xen: arm: Annotate the handlers for HSTR_EL2.T15
M xen: arm: Annotate registers trapped by MDCR_EL2.TDRA
M xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA
xen: arm: Annotate registers trapped by MDCR_EL2.TDA
xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR
R xen: arm: Remove CNTPCT_EL0 trap handling.

Quoted 2 lines

	xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0
R xen: arm: Annotate source of ICC SGI register trapping

I added my Reviewed-by to all the patch except #16.

Although it was only a NIT in the subject. If you don't want to bother resending the whole series on the ML for few NITs, you can add my Reviewed-by to this patch too.

Regards,

Signature · 1 line

Julien Grall

Ian Campbell

author

2015-05-08 10:57 UTC #38 lore raw

Quoted 3 lines · Julien Grall

Although it was only a NIT in the subject. If you don't want to bother resending the whole series on the ML for few NITs, you can add my Reviewed-by to this patch too.

Thanks, I've taken you up on that offer rather than spamming the list and applied what would have been v3 with your R-by, thanks.

Ian.