diff mbox

[Xen-devel,2/4] xen/arm: Implement a dummy Performance Monitor for ARM32

Message ID 1398379556-1132-3-git-send-email-julien.grall@linaro.org
State Accepted, archived
Headers show

Commit Message

Julien Grall April 24, 2014, 10:45 p.m. UTC
XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and Performance
Monitor registers") disable Performance Monitor.

When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, regardless the
ID_DFR0 (which tell if Perfomance Monitors Extension is implemented) the
kernel will try to access to PMCR.

Therefore we tell the guest we have 0 counters. Unfortunately we must always
support PMCCNTR (the cycle counter): we just RAZ/WI for all PM register,
which doesn't crash the kernel at least.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
    This commit is candidate for backporting on Xen 4.4. Some distribution
    (such as Linaro Ubuntu image) enable Perf Events (CONFIG_PERF_EVENTS=y).
    Linux will unconditionally access to theses registers and therefore will
    crash.
---
 xen/arch/arm/traps.c         |   28 ++++++++++++++++++++++++++++
 xen/include/asm-arm/cpregs.h |   17 ++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

Comments

Ian Campbell May 2, 2014, 11:01 a.m. UTC | #1
On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
> XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and Performance
> Monitor registers") disable Performance Monitor.
> 
> When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, regardless the
> ID_DFR0 (which tell if Perfomance Monitors Extension is implemented) the
> kernel will try to access to PMCR.
> 
> Therefore we tell the guest we have 0 counters. Unfortunately we must always
> support PMCCNTR (the cycle counter): we just RAZ/WI for all PM register,
> which doesn't crash the kernel at least.

How often does this trap occur in practice? Once at start of day? Only
if you run perf? Or on every guest context switch? (obviously the last
one would be bad...)
Julien Grall May 2, 2014, 12:41 p.m. UTC | #2
On 05/02/2014 12:01 PM, Ian Campbell wrote:
> On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
>> XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and Performance
>> Monitor registers") disable Performance Monitor.
>>
>> When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, regardless the
>> ID_DFR0 (which tell if Perfomance Monitors Extension is implemented) the
>> kernel will try to access to PMCR.
>>
>> Therefore we tell the guest we have 0 counters. Unfortunately we must always
>> support PMCCNTR (the cycle counter): we just RAZ/WI for all PM register,
>> which doesn't crash the kernel at least.
> 
> How often does this trap occur in practice? Once at start of day? Only
> if you run perf? Or on every guest context switch? (obviously the last
> one would be bad...)

There is few calls to the perf registers during the boot (when Linux is
compiled with CONFIG_PERF_EVENTS=y).

I didn't see any usage during guest context switch. I haven't try perf.

Regards,
Ian Campbell June 13, 2014, 12:02 p.m. UTC | #3
On Fri, 2014-05-02 at 13:41 +0100, Julien Grall wrote:
> On 05/02/2014 12:01 PM, Ian Campbell wrote:
> > On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
> >> XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and Performance
> >> Monitor registers") disable Performance Monitor.
> >>
> >> When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, regardless the
> >> ID_DFR0 (which tell if Perfomance Monitors Extension is implemented) the
> >> kernel will try to access to PMCR.
> >>
> >> Therefore we tell the guest we have 0 counters. Unfortunately we must always
> >> support PMCCNTR (the cycle counter): we just RAZ/WI for all PM register,
> >> which doesn't crash the kernel at least.
> > 
> > How often does this trap occur in practice? Once at start of day? Only
> > if you run perf? Or on every guest context switch? (obviously the last
> > one would be bad...)
> 
> There is few calls to the perf registers during the boot (when Linux is
> compiled with CONFIG_PERF_EVENTS=y).
> 
> I didn't see any usage during guest context switch. I haven't try perf.

Great. Acked-by: Ian Campbell <ian.campbell@citrix.com>
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 700665c..319bbe9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1387,6 +1387,34 @@  static void do_cp15_32(struct cpu_user_regs *regs,
         if ( cp32.read )
            *r = v->arch.actlr;
         break;
+
+    /* 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.
+     *
+     * We tell the guest we have 0 counters. Unfortunately we must
+     * always support PMCCNTR (the cyle counter): we just RAZ/WI for all
+     * PM register, which doesn't crash the kernel at least
+     */
+    case HSR_CPREG32(PMCR):
+    case HSR_CPREG32(PMCNTENSET):
+    case HSR_CPREG32(PMCNTENCLR):
+    case HSR_CPREG32(PMOVSR):
+    case HSR_CPREG32(PMSWINC):
+    case HSR_CPREG32(PMSELR):
+    case HSR_CPREG32(PMCEID0):
+    case HSR_CPREG32(PMCEID1):
+    case HSR_CPREG32(PMCCNTR):
+    case HSR_CPREG32(PMXEVCNTR):
+    case HSR_CPREG32(PMXEVCNR):
+    case HSR_CPREG32(PMUSERENR):
+    case HSR_CPREG32(PMINTENSET):
+    case HSR_CPREG32(PMINTENCLR):
+    case HSR_CPREG32(PMOVSSET):
+        if ( cp32.read )
+            *r = 0;
+        break;
+
     default:
 #ifndef NDEBUG
         gdprintk(XENLOG_ERR,
diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
index bf8133e..f44e3b5 100644
--- a/xen/include/asm-arm/cpregs.h
+++ b/xen/include/asm-arm/cpregs.h
@@ -198,7 +198,22 @@ 
 #define TLBIMVAH        p15,4,c8,c7,1   /* Invalidate Unified Hyp. TLB by MVA */
 #define TLBIALLNSNH     p15,4,c8,c7,4   /* Invalidate Entire Non-Secure Non-Hyp. Unified TLB */
 
-/* CP15 CR9: */
+/* CP15 CR9: Performance monitors */
+#define PMCR            p15,0,c9,c12,0  /* Perf. Mon. Control Register */
+#define PMCNTENSET      p15,0,c9,c12,1  /* Perf. Mon. Count Enable Set register */
+#define PMCNTENCLR      p15,0,c9,c12,2  /* Perf. Mon. Count Enable Clear register */
+#define PMOVSR          p15,0,c9,c12,3  /* Perf. Mon. Overflow Flag Status Register */
+#define PMSWINC         p15,0,c9,c12,4  /* Perf. Mon. Software Increment register */
+#define PMSELR          p15,0,c9,c12,5  /* Perf. Mon. Event Counter Selection Register */
+#define PMCEID0         p15,0,c9,c12,6  /* Perf. Mon. Common Event Identification register 0 */
+#define PMCEID1         p15,0,c9,c12,7  /* Perf. Mon. Common Event Identification register 1 */
+#define PMCCNTR         p15,0,c9,c13,0  /* Perf. Mon. Cycle Count Register */
+#define PMXEVCNTR       p15,0,c9,c13,1  /* Perf. Mon. Event Type Select Register */
+#define PMXEVCNR        p15,0,c9,c13,2  /* Perf. Mon. Event Count Register */
+#define PMUSERENR       p15,0,c9,c14,0  /* Perf. Mon. User Enable Register */
+#define PMINTENSET      p15,0,c9,c14,1  /* Perf. Mon. Interrupt Enable Set Register */
+#define PMINTENCLR      p15,0,c9,c14,2  /* Perf. Mon. Interrupt Enable Clear Register */
+#define PMOVSSET        p15,0,c9,c14,3  /* Perf. Mon. Overflow Flag Status Set register */
 
 /* CP15 CR10: */
 #define MAIR0           p15,0,c10,c2,0  /* Memory Attribute Indirection Register 0 AKA PRRR */