All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 01/19] xen: arm: constify union hsr and struct hsr_* where possible.
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 02/19] xen: arm: Fix handling of ICC_{SGI1R, SGI0R, ASGI1R}_EL1 Ian Campbell
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/traps.c |   41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index aaa9d93..69b9513 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 02/19] xen: arm: Fix handling of ICC_{SGI1R, SGI0R, ASGI1R}_EL1
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 01/19] xen: arm: constify union hsr and struct hsr_* where possible Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 03/19] xen: arm: call inject_undef_exception directly Ian Campbell
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
v2: Remove vestigial second commit message
---
 xen/arch/arm/traps.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 69b9513..99ceaea 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 03/19] xen: arm: call inject_undef_exception directly
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 01/19] xen: arm: constify union hsr and struct hsr_* where possible Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 02/19] xen: arm: Fix handling of ICC_{SGI1R, SGI0R, ASGI1R}_EL1 Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper Ian Campbell
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Reducing the amount of goto maze considerably.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/traps.c |   56 +++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 99ceaea..7270116 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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;
         }
     }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 00/19] xen: arm: cleanup traps.c
@ 2015-04-17 14:01 Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 01/19] xen: arm: constify union hsr and struct hsr_* where possible Ian Campbell
                   ` (19 more replies)
  0 siblings, 20 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: Julien Grall, Tim Deegan, Stefano Stabellini

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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (2 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 03/19] xen: arm: call inject_undef_exception directly Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:09   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers Ian Campbell
                   ` (15 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
v2: Added reference to B1.2.1
---
 xen/arch/arm/traps.c |   51 +++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7270116..8b1846a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (3 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:16   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers Ian Campbell
                   ` (14 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
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(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8b1846a..b54aef6 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (4 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:23   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC} Ian Campbell
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Removes a load of boiler plate.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
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(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index b54aef6..7110c66 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC}
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (5 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:25   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap Ian Campbell
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
v2: s/HSR_EL2/HCR_EL2/
---
 xen/arch/arm/traps.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7110c66..7b79990 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (6 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC} Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:29   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 09/19] xen: arm: Annotate registers trapped by HCR_EL1.TIDCP Ian Campbell
                   ` (11 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
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(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7b79990..522701b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/sysregs.h
index 2284fd7..d75e154 100644
--- a/xen/include/asm-arm/sysregs.h
+++ b/xen/include/asm-arm/sysregs.h
@@ -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)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 09/19] xen: arm: Annotate registers trapped by HCR_EL1.TIDCP
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (7 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA Ian Campbell
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
v2: Typo in subject
---
 xen/arch/arm/traps.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 522701b..d908738 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (8 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 09/19] xen: arm: Annotate registers trapped by HCR_EL1.TIDCP Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:42   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx Ian Campbell
                   ` (9 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
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(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d908738..afa8a95 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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);
diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h
index 46015f5..69fabe7 100644
--- a/xen/include/asm-arm/perfc_defn.h
+++ b/xen/include/asm-arm/perfc_defn.h
@@ -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")
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (9 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:44   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15 Ian Campbell
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
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(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index afa8a95..a2bae51 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (10 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:48   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA Ian Campbell
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: s/Tx/T15/
---
 xen/arch/arm/traps.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a2bae51..86b5655 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (11 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15 Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:50   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA Ian Campbell
                   ` (6 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
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(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 86b5655..17ddcd0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
index afe9148..9db8cfd 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -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 */
 
 
diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/sysregs.h
index d75e154..55457fd 100644
--- a/xen/include/asm-arm/sysregs.h
+++ b/xen/include/asm-arm/sysregs.h
@@ -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)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (12 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:53   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA Ian Campbell
                   ` (5 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
---
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(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 17ddcd0..7606bff 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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:
diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
index 9db8cfd..e5cb00c 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -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 */
diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/sysregs.h
index 55457fd..570f43e 100644
--- a/xen/include/asm-arm/sysregs.h
+++ b/xen/include/asm-arm/sysregs.h
@@ -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)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (13 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-25 17:55   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR Ian Campbell
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/traps.c |   60 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7606bff..97cde45 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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 */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (14 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-26 13:46   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 17/19] xen: arm: Remove CNTPCT_EL0 trap handling Ian Campbell
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/traps.c |   39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 97cde45..d4505b5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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) )
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 17/19] xen: arm: Remove CNTPCT_EL0 trap handling.
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (15 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-17 14:01 ` [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0 Ian Campbell
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/traps.c  |    1 -
 xen/arch/arm/vtimer.c |   30 ------------------------------
 2 files changed, 31 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index d4505b5..c869b96 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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);
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index be65c9f..685bfea 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -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;
     }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (16 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 17/19] xen: arm: Remove CNTPCT_EL0 trap handling Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-04-26 13:48   ` Julien Grall
  2015-04-17 14:01 ` [PATCH v2 19/19] xen: arm: Annotate source of ICC SGI register trapping Ian Campbell
  2015-05-08 10:37 ` [PATCH v2 00/19] xen: arm: cleanup traps.c Julien Grall
  19 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/traps.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c869b96..ad6ff05 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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) )
         {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* [PATCH v2 19/19] xen: arm: Annotate source of ICC SGI register trapping
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (17 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0 Ian Campbell
@ 2015-04-17 14:01 ` Ian Campbell
  2015-05-08 10:37 ` [PATCH v2 00/19] xen: arm: cleanup traps.c Julien Grall
  19 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-04-17 14:01 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, tim, Ian Campbell, stefano.stabellini

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 <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>
---
 xen/arch/arm/traps.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index ad6ff05..6fe9b7a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -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) )
         {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper
  2015-04-17 14:01 ` [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper Ian Campbell
@ 2015-04-25 17:09   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:09 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
  2015-04-17 14:01 ` [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers Ian Campbell
@ 2015-04-25 17:16   ` Julien Grall
  2015-05-08 10:16     ` Ian Campbell
  0 siblings, 1 reply; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:16 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> 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(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 8b1846a..b54aef6 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
>       advance_pc(regs, hsr);
>   }
>
> +/* Write only + write ignore */

[..]

> +/* 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,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers
  2015-04-17 14:01 ` [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers Ian Campbell
@ 2015-04-25 17:23   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:23 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> Removes a load of boiler plate.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC}
  2015-04-17 14:01 ` [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC} Ian Campbell
@ 2015-04-25 17:25   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:25 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

Subject: s/HSR_EL2/HCR_EL2/

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>

Other than the typo in the subject:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap
  2015-04-17 14:01 ` [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap Ian Campbell
@ 2015-04-25 17:29   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:29 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA
  2015-04-17 14:01 ` [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA Ian Campbell
@ 2015-04-25 17:42   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:42 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx
  2015-04-17 14:01 ` [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx Ian Campbell
@ 2015-04-25 17:44   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:44 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15
  2015-04-17 14:01 ` [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15 Ian Campbell
@ 2015-04-25 17:48   ` Julien Grall
  2015-05-08 10:17     ` Ian Campbell
  0 siblings, 1 reply; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:48 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> v2: s/Tx/T15/
> ---
>   xen/arch/arm/traps.c |   10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a2bae51..86b5655 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -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

Should not this Tx switch to T15 too?

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA
  2015-04-17 14:01 ` [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA Ian Campbell
@ 2015-04-25 17:50   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:50 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA
  2015-04-17 14:01 ` [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA Ian Campbell
@ 2015-04-25 17:53   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:53 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> 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 <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA
  2015-04-17 14:01 ` [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA Ian Campbell
@ 2015-04-25 17:55   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-25 17:55 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 19:01, Ian Campbell wrote:
> Gather the affected handlers in a single place per trap type.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR
  2015-04-17 14:01 ` [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR Ian Campbell
@ 2015-04-26 13:46   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-26 13:46 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

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,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0
  2015-04-17 14:01 ` [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0 Ian Campbell
@ 2015-04-26 13:48   ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-04-26 13:48 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: julien.grall, tim, stefano.stabellini

Hi Ian,

On 17/04/2015 16:01, Ian Campbell wrote:
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
  2015-04-25 17:16   ` Julien Grall
@ 2015-05-08 10:16     ` Ian Campbell
  2015-05-08 10:33       ` Julien Grall
  0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-05-08 10:16 UTC (permalink / raw)
  To: Julien Grall; +Cc: julien.grall, tim, stefano.stabellini, xen-devel

On Sat, 2015-04-25 at 22:16 +0500, Julien Grall wrote:
> Hi Ian,
> 
> On 17/04/2015 19:01, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > 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(-)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index 8b1846a..b54aef6 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
> >       advance_pc(regs, hsr);
> >   }
> >
> > +/* Write only + write ignore */
> 
> [..]
> 
> > +/* 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.

> 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
<1429266891.25195.260.camel@citrix.com>

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

Is that clear enough do you think?

Ian.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15
  2015-04-25 17:48   ` Julien Grall
@ 2015-05-08 10:17     ` Ian Campbell
  2015-05-08 10:35       ` Julien Grall
  0 siblings, 1 reply; 39+ messages in thread
From: Ian Campbell @ 2015-05-08 10:17 UTC (permalink / raw)
  To: Julien Grall; +Cc: julien.grall, tim, stefano.stabellini, xen-devel

On Sat, 2015-04-25 at 22:48 +0500, Julien Grall wrote:
> Hi Ian,
> 
> On 17/04/2015 19:01, Ian Campbell wrote:
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > v2: s/Tx/T15/
> > ---
> >   xen/arch/arm/traps.c |   10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index a2bae51..86b5655 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -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
> 
> 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!

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers
  2015-05-08 10:16     ` Ian Campbell
@ 2015-05-08 10:33       ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-05-08 10:33 UTC (permalink / raw)
  To: Ian Campbell, Julien Grall
  Cc: xen-devel, julien.grall, tim, stefano.stabellini

Hi Ian,

On 08/05/15 11:16, Ian Campbell wrote:
> On Sat, 2015-04-25 at 22:16 +0500, Julien Grall wrote:
>> Hi Ian,
>>
>> On 17/04/2015 19:01, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>> 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(-)
>>>
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index 8b1846a..b54aef6 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -1587,6 +1587,34 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
>>>       advance_pc(regs, hsr);
>>>   }
>>>
>>> +/* Write only + write ignore */
>>
>> [..]
>>
>>> +/* 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.
> 
>> 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
> <1429266891.25195.260.camel@citrix.com>
> 
> 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 <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15
  2015-05-08 10:17     ` Ian Campbell
@ 2015-05-08 10:35       ` Julien Grall
  0 siblings, 0 replies; 39+ messages in thread
From: Julien Grall @ 2015-05-08 10:35 UTC (permalink / raw)
  To: Ian Campbell, Julien Grall
  Cc: xen-devel, julien.grall, tim, stefano.stabellini

Hi Ian,

On 08/05/15 11:17, Ian Campbell wrote:
> On Sat, 2015-04-25 at 22:48 +0500, Julien Grall wrote:
>> Hi Ian,
>>
>> On 17/04/2015 19:01, Ian Campbell wrote:
>>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>>> ---
>>> v2: s/Tx/T15/
>>> ---
>>>   xen/arch/arm/traps.c |   10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index a2bae51..86b5655 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -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
>>
>> 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!

With that change:

Reviewed-by: Julien Grall <julien.grall@citrix.com>

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 00/19] xen: arm: cleanup traps.c
  2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
                   ` (18 preceding siblings ...)
  2015-04-17 14:01 ` [PATCH v2 19/19] xen: arm: Annotate source of ICC SGI register trapping Ian Campbell
@ 2015-05-08 10:37 ` Julien Grall
  2015-05-08 10:57   ` Ian Campbell
  19 siblings, 1 reply; 39+ messages in thread
From: Julien Grall @ 2015-05-08 10:37 UTC (permalink / raw)
  To: Ian Campbell, xen-devel; +Cc: Julien Grall, Tim Deegan, Stefano Stabellini

Hi Ian,

On 17/04/15 15:01, Ian Campbell wrote:
> (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

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,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v2 00/19] xen: arm: cleanup traps.c
  2015-05-08 10:37 ` [PATCH v2 00/19] xen: arm: cleanup traps.c Julien Grall
@ 2015-05-08 10:57   ` Ian Campbell
  0 siblings, 0 replies; 39+ messages in thread
From: Ian Campbell @ 2015-05-08 10:57 UTC (permalink / raw)
  To: Julien Grall; +Cc: Stefano Stabellini, Tim Deegan, xen-devel

On Fri, 2015-05-08 at 11:37 +0100, Julien Grall wrote:
> 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.

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2015-05-08 10:57 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-17 14:01 [PATCH v2 00/19] xen: arm: cleanup traps.c Ian Campbell
2015-04-17 14:01 ` [PATCH v2 01/19] xen: arm: constify union hsr and struct hsr_* where possible Ian Campbell
2015-04-17 14:01 ` [PATCH v2 02/19] xen: arm: Fix handling of ICC_{SGI1R, SGI0R, ASGI1R}_EL1 Ian Campbell
2015-04-17 14:01 ` [PATCH v2 03/19] xen: arm: call inject_undef_exception directly Ian Campbell
2015-04-17 14:01 ` [PATCH v2 04/19] xen: arm: provide and use a handle_raz_wi helper Ian Campbell
2015-04-25 17:09   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 05/19] xen: arm: Add and use r/o+raz and w/o+wi helpers Ian Campbell
2015-04-25 17:16   ` Julien Grall
2015-05-08 10:16     ` Ian Campbell
2015-05-08 10:33       ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 06/19] xen: arm: add minimum exception level argument to trap handler helpers Ian Campbell
2015-04-25 17:23   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 07/19] xen: arm: Annotate trap handler for HSR_EL2.{TWI, TWE, TSC} Ian Campbell
2015-04-25 17:25   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 08/19] xen: arm: implement handling of ACTLR_EL1 trap Ian Campbell
2015-04-25 17:29   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 09/19] xen: arm: Annotate registers trapped by HCR_EL1.TIDCP Ian Campbell
2015-04-17 14:01 ` [PATCH v2 10/19] xen: arm: implement handling of registers trapped by CPTR_EL2.TTA Ian Campbell
2015-04-25 17:42   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 11/19] xen: arm: Annotate handlers for CPTR_EL2.Tx Ian Campbell
2015-04-25 17:44   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 12/19] xen: arm: Annotate the handlers for HSTR_EL2.T15 Ian Campbell
2015-04-25 17:48   ` Julien Grall
2015-05-08 10:17     ` Ian Campbell
2015-05-08 10:35       ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 13/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDRA Ian Campbell
2015-04-25 17:50   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 14/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDOSA Ian Campbell
2015-04-25 17:53   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA Ian Campbell
2015-04-25 17:55   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 16/19] xen: arm: Annotate registers trapped by MDCR_EL2.TPM and TPMCR Ian Campbell
2015-04-26 13:46   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 17/19] xen: arm: Remove CNTPCT_EL0 trap handling Ian Campbell
2015-04-17 14:01 ` [PATCH v2 18/19] xen: arm: Annotate registers trapped when CNTHCTL_EL2.EL1PCEN == 0 Ian Campbell
2015-04-26 13:48   ` Julien Grall
2015-04-17 14:01 ` [PATCH v2 19/19] xen: arm: Annotate source of ICC SGI register trapping Ian Campbell
2015-05-08 10:37 ` [PATCH v2 00/19] xen: arm: cleanup traps.c Julien Grall
2015-05-08 10:57   ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.