[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 06/12] xen: arm: correctly handle vtimer traps from userspace



Previously 32-bit userspace on 32-bit kernel and 64-bit userspace on
64-bit kernel could access these registers irrespective of whether the
kernel had configured them to be allowed to. To fix this:

 - Userspace access to CNTP_CTL_EL0 and CNTP_TVAL_EL0 should be gated
   on CNTKCTL_EL1.EL0PTEN.
 - Userspace access to CNTPCT_EL0 should be gated on
   CNTKCTL_EL1.EL0PCTEN.

When we do not handle an access we now silently inject an undef even
in debug mode since the debugging messages are not helpful (we have
handled the access, by explicitly choosing not to).

Since HSR_EC_CP15_64 cannot be taken from a guest in AArch64 mode
except due to a hardware bug switch the associated check to a BUG_ON
(which will be switched to something more appropriate in a subsequent
patch)

Fix a coding style issue in HSR_CPREG64(CNTPCT) while touching similar
code.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
v2: Replaces "xen: arm: turn vtimer traps for cp32/64 and sysreg into #undef"
---
 xen/arch/arm/traps.c            |   28 +++++++++-------------------
 xen/arch/arm/vtimer.c           |   38 +++++++++++++++++++++++---------------
 xen/include/asm-arm/processor.h |    6 ++++++
 3 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 4eda561..c5ffcc5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1598,11 +1598,7 @@ 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) )
-        {
-            dprintk(XENLOG_ERR,
-                    "failed emulation of 32-bit vtimer CP register access\n");
-            domain_crash_synchronous();
-        }
+            goto undef_cp15_32;
         break;
     case HSR_CPREG32(ACTLR):
         if ( cp32.read )
@@ -1645,6 +1641,7 @@ static void do_cp15_32(struct cpu_user_regs *regs,
         gdprintk(XENLOG_ERR, "unhandled 32-bit CP15 access %#x\n",
                  hsr.bits & HSR_CP32_REGS_MASK);
 #endif
+ undef_cp15_32:
         inject_undef_exception(regs, hsr.len);
         return;
     }
@@ -1665,11 +1662,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) )
-        {
-            dprintk(XENLOG_ERR,
-                    "failed emulation of 64-bit vtimer CP register access\n");
-            domain_crash_synchronous();
-        }
+            goto undef_cp15_64;
         break;
     default:
         {
@@ -1683,6 +1676,7 @@ static void do_cp15_64(struct cpu_user_regs *regs,
             gdprintk(XENLOG_ERR, "unhandled 64-bit CP15 access %#x\n",
                      hsr.bits & HSR_CP64_REGS_MASK);
 #endif
+ undef_cp15_64:
             inject_undef_exception(regs, hsr.len);
             return;
         }
@@ -1851,11 +1845,7 @@ static void do_sysreg(struct cpu_user_regs *regs,
     case HSR_SYSREG_CNTP_TVAL_EL0:
     case HSR_SYSREG_CNTP_CVAL_EL0:
         if ( !vtimer_emulate(regs, hsr) )
-        {
-            dprintk(XENLOG_ERR,
-                    "failed emulation of 64-bit vtimer sysreg access\n");
-            domain_crash_synchronous();
-        }
+            goto undef_sysreg;
         break;
     case HSR_SYSREG_ICC_SGI1R_EL1:
         if ( !vgic_emulate(regs, hsr) )
@@ -1874,8 +1864,8 @@ static void do_sysreg(struct cpu_user_regs *regs,
     default:
  bad_sysreg:
         {
-            struct hsr_sysreg sysreg = hsr.sysreg;
 #ifndef NDEBUG
+            struct hsr_sysreg sysreg = hsr.sysreg;
 
             gdprintk(XENLOG_ERR,
                      "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",
@@ -1888,7 +1878,8 @@ static void do_sysreg(struct cpu_user_regs *regs,
             gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
                      hsr.bits & HSR_SYSREG_REGS_MASK);
 #endif
-            inject_undef_exception(regs, sysreg.len);
+ undef_sysreg:
+            inject_undef_exception(regs, hsr.len);
             return;
         }
     }
@@ -2064,8 +2055,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs 
*regs)
         do_cp15_32(regs, hsr);
         break;
     case HSR_EC_CP15_64:
-        if ( !is_32bit_domain(current->domain) )
-            goto bad_trap;
+        BUG_ON(!psr_mode_is_32bit(regs->cpsr));
         perfc_incr(trap_cp15_64);
         do_cp15_64(regs, hsr);
         break;
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 0c67f20..5aca65e 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -154,9 +154,14 @@ int virt_timer_restore(struct vcpu *v)
     return 0;
 }
 
-static void vtimer_cntp_ctl(struct cpu_user_regs *regs, uint32_t *r, int read)
+static int vtimer_cntp_ctl(struct cpu_user_regs *regs, uint32_t *r, int read)
 {
     struct vcpu *v = current;
+
+    if ( psr_mode_is_user(regs) &&
+         !(READ_SYSREG(CNTKCTL_EL1) & CNTKCTL_EL1_EL0PTEN) )
+        return 0;
+
     if ( read )
     {
         *r = v->arch.phys_timer.ctl;
@@ -176,13 +181,18 @@ static void vtimer_cntp_ctl(struct cpu_user_regs *regs, 
uint32_t *r, int read)
         else
             stop_timer(&v->arch.phys_timer.timer);
     }
+    return 1;
 }
 
-static void vtimer_cntp_tval(struct cpu_user_regs *regs, uint32_t *r, int read)
+static int vtimer_cntp_tval(struct cpu_user_regs *regs, uint32_t *r, int read)
 {
     struct vcpu *v = current;
     s_time_t now;
 
+    if ( psr_mode_is_user(regs) &&
+         !(READ_SYSREG(CNTKCTL_EL1) & CNTKCTL_EL1_EL0PTEN) )
+        return 0;
+
     now = NOW() - v->domain->arch.phys_timer_base.offset;
 
     if ( read )
@@ -200,6 +210,7 @@ static void vtimer_cntp_tval(struct cpu_user_regs *regs, 
uint32_t *r, int read)
                       v->domain->arch.phys_timer_base.offset);
         }
     }
+    return 1;
 }
 
 static int vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r, int read)
@@ -235,6 +246,9 @@ static int vtimer_cntpct(struct cpu_user_regs *regs, 
uint64_t *r, int read)
 
     if ( read )
     {
+        if ( psr_mode_is_user(regs) &&
+             !(READ_SYSREG(CNTKCTL_EL1) & CNTKCTL_EL1_EL0PCTEN) )
+            return 0;
         now = NOW() - v->domain->arch.phys_timer_base.offset;
         ticks = ns_to_ticks(now);
         *r = ticks;
@@ -261,12 +275,10 @@ static int vtimer_emulate_cp32(struct cpu_user_regs 
*regs, union hsr hsr)
     switch ( hsr.bits & HSR_CP32_REGS_MASK )
     {
     case HSR_CPREG32(CNTP_CTL):
-        vtimer_cntp_ctl(regs, r, cp32.read);
-        return 1;
+        return vtimer_cntp_ctl(regs, r, cp32.read);
 
     case HSR_CPREG32(CNTP_TVAL):
-        vtimer_cntp_tval(regs, r, cp32.read);
-        return 1;
+        return vtimer_cntp_tval(regs, r, cp32.read);
 
     default:
         return 0;
@@ -288,7 +300,7 @@ 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))
+        if ( !vtimer_cntpct(regs, &x, cp64.read) )
             return 0;
         break;
 
@@ -325,12 +337,14 @@ static int vtimer_emulate_sysreg(struct cpu_user_regs 
*regs, union hsr hsr)
     switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
     {
     case HSR_SYSREG_CNTP_CTL_EL0:
-        vtimer_cntp_ctl(regs, &r, sysreg.read);
+        if ( !vtimer_cntp_ctl(regs, &r, sysreg.read) )
+            return 0;
         if ( sysreg.read )
             *x = r;
         return 1;
     case HSR_SYSREG_CNTP_TVAL_EL0:
-        vtimer_cntp_tval(regs, &r, sysreg.read);
+        if ( !vtimer_cntp_tval(regs, &r, sysreg.read) )
+            return 0;
         if ( sysreg.read )
             *x = r;
         return 1;
@@ -353,17 +367,11 @@ int vtimer_emulate(struct cpu_user_regs *regs, union hsr 
hsr)
 
     switch (hsr.ec) {
     case HSR_EC_CP15_32:
-        if ( !is_32bit_domain(current->domain) )
-            return 0;
         return vtimer_emulate_cp32(regs, hsr);
     case HSR_EC_CP15_64:
-        if ( !is_32bit_domain(current->domain) )
-            return 0;
         return vtimer_emulate_cp64(regs, hsr);
 #ifdef CONFIG_ARM_64
     case HSR_EC_SYSREG:
-        if ( is_32bit_domain(current->domain) )
-            return 0;
         return vtimer_emulate_sysreg(regs, hsr);
 #endif
     default:
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 570d2a1..5ccf61f 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -558,6 +558,12 @@ union hsr {
 #define CNTHCTL_EL2_EL1PCTEN (1u<<0) /* Kernel/user access to physical counter 
*/
 #define CNTHCTL_EL2_EL1PCEN  (1u<<1) /* Kernel/user access to CNTP timer regs 
*/
 
+/* Time counter kernel control register */
+#define CNTKCTL_EL1_EL0PCTEN (1u<<0) /* Expose phys counters to EL0 */
+#define CNTKCTL_EL1_EL0VCTEN (1u<<1) /* Expose virt counters to EL0 */
+#define CNTKCTL_EL1_EL0VTEN  (1u<<8) /* Expose virt timer registers to EL0 */
+#define CNTKCTL_EL1_EL0PTEN  (1u<<9) /* Expose phys timer registers to EL0 */
+
 /* Timer control registers */
 #define CNTx_CTL_ENABLE   (1u<<0)  /* Enable timer */
 #define CNTx_CTL_MASK     (1u<<1)  /* Mask IRQ */
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.