All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] xen/arm: Clean-up traps.c
@ 2017-09-12 10:36 Julien Grall
  2017-09-12 10:36 ` [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically Julien Grall
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini, volodymyr_babchuk

Hi all,

xen/arch/arm/traps.c is beginning to get very big. This series is moving out
the co-processor and sysreg emulation in separate files. This will avoid to
grow traps.c when adding more registers emulation (coming soon).

A branch with this series has been pushed:

https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git
branch cleanup-traps-v2

Cheers,

Cc: volodymyr_babchuk@epam.com

Julien Grall (7):
  xen/arm: traps: Re-order the includes alphabetically
  xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h
  xen/arm: traps: Export a bunch of helpers to handle emulation
  xen/arm: Move sysreg emulation outside of traps.c
  xen/arm: Move co-processor emulation outside of traps.c
  xen/arm: Move sysregs.h in arm64 sub-directory
  xen/arm: Limit the scope of cpregs.h

 xen/arch/arm/Makefile                      |   1 +
 xen/arch/arm/arm64/Makefile                |   1 +
 xen/arch/arm/arm64/vsysreg.c               | 229 ++++++++++
 xen/arch/arm/domain.c                      |   2 +-
 xen/arch/arm/smp.c                         |   1 -
 xen/arch/arm/traps.c                       | 702 ++---------------------------
 xen/arch/arm/vcpreg.c                      | 452 +++++++++++++++++++
 xen/arch/arm/vgic-v3.c                     |   1 +
 xen/arch/arm/vtimer.c                      |   2 +
 xen/include/asm-arm/arm32/processor.h      |   2 +
 xen/include/asm-arm/arm32/traps.h          |  13 +
 xen/include/asm-arm/arm64/processor.h      |   2 +
 xen/include/asm-arm/{ => arm64}/sysregs.h  |  10 +-
 xen/include/asm-arm/arm64/traps.h          |  18 +
 xen/include/asm-arm/percpu.h               |   1 -
 xen/include/asm-arm/processor.h            |   2 -
 xen/include/asm-arm/traps.h                |  44 ++
 xen/{arch/arm => include/asm-arm}/vtimer.h |   0
 18 files changed, 811 insertions(+), 672 deletions(-)
 create mode 100644 xen/arch/arm/arm64/vsysreg.c
 create mode 100644 xen/arch/arm/vcpreg.c
 create mode 100644 xen/include/asm-arm/arm32/traps.h
 rename xen/include/asm-arm/{ => arm64}/sysregs.h (98%)
 create mode 100644 xen/include/asm-arm/arm64/traps.h
 create mode 100644 xen/include/asm-arm/traps.h
 rename xen/{arch/arm => include/asm-arm}/vtimer.h (100%)

-- 
2.11.0


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

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

* [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 20:59   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h Julien Grall
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>

---
    Changes in v2:
        - Fix alphabetical order
        - Add Bhupinder's acked-by
---
 xen/arch/arm/traps.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 7f6ec15b5e..6b3dfd9bcf 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -16,41 +16,43 @@
  * GNU General Public License for more details.
  */
 
+#include <xen/domain_page.h>
+#include <xen/errno.h>
+#include <xen/hypercall.h>
 #include <xen/init.h>
-#include <xen/string.h>
-#include <xen/version.h>
-#include <xen/smp.h>
-#include <xen/symbols.h>
+#include <xen/iocap.h>
 #include <xen/irq.h>
 #include <xen/lib.h>
 #include <xen/livepatch.h>
+#include <xen/mem_access.h>
 #include <xen/mm.h>
-#include <xen/errno.h>
-#include <xen/hypercall.h>
-#include <xen/softirq.h>
-#include <xen/domain_page.h>
 #include <xen/perfc.h>
+#include <xen/smp.h>
+#include <xen/softirq.h>
+#include <xen/string.h>
+#include <xen/symbols.h>
+#include <xen/version.h>
 #include <xen/virtual_region.h>
-#include <xen/mem_access.h>
-#include <xen/iocap.h>
+
 #include <public/sched.h>
 #include <public/xen.h>
-#include <asm/debugger.h>
-#include <asm/event.h>
-#include <asm/regs.h>
+
+#include <asm/acpi.h>
 #include <asm/cpregs.h>
-#include <asm/psci.h>
-#include <asm/mmio.h>
+#include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
+#include <asm/debugger.h>
+#include <asm/event.h>
 #include <asm/flushtlb.h>
+#include <asm/gic.h>
+#include <asm/mmio.h>
 #include <asm/monitor.h>
+#include <asm/psci.h>
+#include <asm/regs.h>
+#include <asm/vgic.h>
 
 #include "decode.h"
 #include "vtimer.h"
-#include <asm/gic.h>
-#include <asm/vgic.h>
-#include <asm/cpuerrata.h>
-#include <asm/acpi.h>
 
 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
-- 
2.11.0


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

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

* [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
  2017-09-12 10:36 ` [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:02   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation Julien Grall
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

It will be necessary to include vtimer.h from subdirectory making the
inclusion a bit awkward.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain.c                      | 2 +-
 xen/arch/arm/traps.c                       | 2 +-
 xen/{arch/arm => include/asm-arm}/vtimer.h | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename xen/{arch/arm => include/asm-arm}/vtimer.h (100%)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 6512f01463..784ae392cf 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -33,8 +33,8 @@
 #include <asm/regs.h>
 #include <asm/vfp.h>
 #include <asm/vgic.h>
+#include <asm/vtimer.h>
 
-#include "vtimer.h"
 #include "vuart.h"
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6b3dfd9bcf..6f32f700e5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -50,9 +50,9 @@
 #include <asm/psci.h>
 #include <asm/regs.h>
 #include <asm/vgic.h>
+#include <asm/vtimer.h>
 
 #include "decode.h"
-#include "vtimer.h"
 
 /* The base of the stack must always be double-word aligned, which means
  * that both the kernel half of struct cpu_user_regs (which is pushed in
diff --git a/xen/arch/arm/vtimer.h b/xen/include/asm-arm/vtimer.h
similarity index 100%
rename from xen/arch/arm/vtimer.h
rename to xen/include/asm-arm/vtimer.h
-- 
2.11.0


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

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

* [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
  2017-09-12 10:36 ` [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically Julien Grall
  2017-09-12 10:36 ` [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:26   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c Julien Grall
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini, volodymyr_babchuk

A follow-up patch will move some parts of traps.c in separate files.
The will require to use helpers that are currently statically defined.
Export the following helpers:
    - inject_undef64_exception
    - inject_undef_exception
    - check_conditional_instr
    - advance_pc
    - handle_raz_wi
    - handle_wo_wi
    - handle_ro_raz

Note that asm-arm/arm32/traps.h is empty but it is to keep parity with
the arm64 counterpart.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---

Cc: volodymyr_babchuk@epam.com

    Changes in v2:
        - Fixup guards
        - Add newline for clarity
---
 xen/arch/arm/traps.c              | 43 +++++++++++++++++++--------------------
 xen/include/asm-arm/arm32/traps.h | 13 ++++++++++++
 xen/include/asm-arm/arm64/traps.h | 15 ++++++++++++++
 xen/include/asm-arm/traps.h       | 36 ++++++++++++++++++++++++++++++++
 4 files changed, 85 insertions(+), 22 deletions(-)
 create mode 100644 xen/include/asm-arm/arm32/traps.h
 create mode 100644 xen/include/asm-arm/arm64/traps.h
 create mode 100644 xen/include/asm-arm/traps.h

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6f32f700e5..1c334a7b99 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -49,6 +49,7 @@
 #include <asm/monitor.h>
 #include <asm/psci.h>
 #include <asm/regs.h>
+#include <asm/traps.h>
 #include <asm/vgic.h>
 #include <asm/vtimer.h>
 
@@ -547,7 +548,7 @@ static vaddr_t exception_handler64(struct cpu_user_regs *regs, vaddr_t offset)
 }
 
 /* Inject an undefined exception into a 64 bit guest */
-static void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
+void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
 {
     vaddr_t handler;
     const union hsr esr = {
@@ -620,8 +621,7 @@ static void inject_iabt64_exception(struct cpu_user_regs *regs,
 
 #endif
 
-static void inject_undef_exception(struct cpu_user_regs *regs,
-                                   const union hsr hsr)
+void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr)
 {
         if ( is_32bit_domain(current->domain) )
             inject_undef32_exception(regs);
@@ -1714,8 +1714,7 @@ static const unsigned short cc_map[16] = {
         0                       /* NV                     */
 };
 
-static int check_conditional_instr(struct cpu_user_regs *regs,
-                                   const union hsr hsr)
+int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr)
 {
     unsigned long cpsr, cpsr_cond;
     int cond;
@@ -1777,7 +1776,7 @@ static int check_conditional_instr(struct cpu_user_regs *regs,
     return 1;
 }
 
-static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
+void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 {
     unsigned long itbits, cond, cpsr = regs->cpsr;
 
@@ -1818,11 +1817,11 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
 }
 
 /* Read as zero and write ignore */
-static void handle_raz_wi(struct cpu_user_regs *regs,
-                          int regidx,
-                          bool read,
-                          const union hsr hsr,
-                          int min_el)
+void handle_raz_wi(struct cpu_user_regs *regs,
+                   int regidx,
+                   bool read,
+                   const union hsr hsr,
+                   int min_el)
 {
     ASSERT((min_el == 0) || (min_el == 1));
 
@@ -1836,12 +1835,12 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
     advance_pc(regs, hsr);
 }
 
-/* Write only as write ignore */
-static void handle_wo_wi(struct cpu_user_regs *regs,
-                         int regidx,
-                         bool read,
-                         const union hsr hsr,
-                         int min_el)
+/* write only as write ignore */
+void handle_wo_wi(struct cpu_user_regs *regs,
+                  int regidx,
+                  bool read,
+                  const union hsr hsr,
+                  int min_el)
 {
     ASSERT((min_el == 0) || (min_el == 1));
 
@@ -1856,11 +1855,11 @@ static void handle_wo_wi(struct cpu_user_regs *regs,
 }
 
 /* Read only as read as zero */
-static void handle_ro_raz(struct cpu_user_regs *regs,
-                          int regidx,
-                          bool read,
-                          const union hsr hsr,
-                          int min_el)
+void handle_ro_raz(struct cpu_user_regs *regs,
+                   int regidx,
+                   bool read,
+                   const union hsr hsr,
+                   int min_el)
 {
     ASSERT((min_el == 0) || (min_el == 1));
 
diff --git a/xen/include/asm-arm/arm32/traps.h b/xen/include/asm-arm/arm32/traps.h
new file mode 100644
index 0000000000..e3c4a8b473
--- /dev/null
+++ b/xen/include/asm-arm/arm32/traps.h
@@ -0,0 +1,13 @@
+#ifndef __ASM_ARM32_TRAPS__
+#define __ASM_ARM32_TRAPS__
+
+#endif /* __ASM_ARM32_TRAPS__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
new file mode 100644
index 0000000000..e5e5a4a036
--- /dev/null
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -0,0 +1,15 @@
+#ifndef __ASM_ARM64_TRAPS__
+#define __ASM_ARM64_TRAPS__
+
+void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
+
+#endif /* __ASM_ARM64_TRAPS__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
new file mode 100644
index 0000000000..6d99d228e8
--- /dev/null
+++ b/xen/include/asm-arm/traps.h
@@ -0,0 +1,36 @@
+#ifndef __ASM_ARM_TRAPS__
+#define __ASM_ARM_TRAPS__
+
+#include <asm/processor.h>
+
+#if defined(CONFIG_ARM_32)
+# include <asm/arm32/traps.h>
+#elif defined(CONFIG_ARM_64)
+# include <asm/arm64/traps.h>
+#endif
+
+int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
+
+void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
+
+void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr);
+
+void handle_raz_wi(struct cpu_user_regs *regs, int regidx, bool read,
+                   const union hsr hsr, int min_el);
+
+void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
+                  const union hsr hsr, int min_el);
+
+void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
+                   const union hsr hsr, int min_el);
+
+#endif /* __ASM_ARM_TRAPS__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
-- 
2.11.0


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

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

* [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
                   ` (2 preceding siblings ...)
  2017-09-12 10:36 ` [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:40   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 5/7] xen/arm: Move co-processor " Julien Grall
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

The sysreg emulation is 64-bit specific and surrounded by #ifdef. Move
them in a separate file arm/arm64/vsysreg.c to shrink down a bit traps.c

No functional change.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/Makefile       |   1 +
 xen/arch/arm/arm64/vsysreg.c      | 229 ++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c              | 198 --------------------------------
 xen/include/asm-arm/arm64/traps.h |   3 +
 4 files changed, 233 insertions(+), 198 deletions(-)
 create mode 100644 xen/arch/arm/arm64/vsysreg.c

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 149b6b3901..718fe44455 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
+obj-y += vsysreg.o
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
new file mode 100644
index 0000000000..c57ac12503
--- /dev/null
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -0,0 +1,229 @@
+/*
+ * xen/arch/arm/arm64/sysreg.c
+ *
+ * Emulate system registers trapped.
+ *
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/sched.h>
+
+#include <asm/current.h>
+#include <asm/regs.h>
+#include <asm/traps.h>
+#include <asm/vtimer.h>
+
+void do_sysreg(struct cpu_user_regs *regs,
+               const union hsr hsr)
+{
+    int regidx = 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 )
+            set_user_reg(regs, regidx, 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, regidx, 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, regidx, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_OSDLR_EL1:
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+
+    /*
+     * 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:
+        return handle_raz_wi(regs, regidx, 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, regidx, hsr.sysreg.read, hsr, 0);
+    HSR_SYSREG_DBG_CASES(DBGBVR):
+    HSR_SYSREG_DBG_CASES(DBGBCR):
+    HSR_SYSREG_DBG_CASES(DBGWVR):
+    HSR_SYSREG_DBG_CASES(DBGWCR):
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+
+    /*
+     * 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:
+        /*
+         * Accessible from EL1 only, but if EL0 trap happens handle as
+         * undef.
+         */
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_PMUSERENR_EL0:
+        /* RO at EL0. RAZ/WI at EL1 */
+        if ( psr_mode_is_user(regs) )
+            return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
+        else
+            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+    case HSR_SYSREG_PMCR_EL0:
+    case HSR_SYSREG_PMCNTENSET_EL0:
+    case HSR_SYSREG_PMCNTENCLR_EL0:
+    case HSR_SYSREG_PMOVSCLR_EL0:
+    case HSR_SYSREG_PMSWINC_EL0:
+    case HSR_SYSREG_PMSELR_EL0:
+    case HSR_SYSREG_PMCEID0_EL0:
+    case HSR_SYSREG_PMCEID1_EL0:
+    case HSR_SYSREG_PMCCNTR_EL0:
+    case HSR_SYSREG_PMXEVTYPER_EL0:
+    case HSR_SYSREG_PMXEVCNTR_EL0:
+    case HSR_SYSREG_PMOVSSET_EL0:
+        /*
+         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
+         * emulate that register as 0 above.
+         */
+        return handle_raz_wi(regs, regidx, 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;
+
+    /*
+     * HCR_EL2.FMO or HCR_EL2.IMO
+     *
+     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
+     */
+    case HSR_SYSREG_ICC_SGI1R_EL1:
+    case HSR_SYSREG_ICC_ASGI1R_EL1:
+    case HSR_SYSREG_ICC_SGI0R_EL1:
+
+        if ( !vgic_emulate(regs, hsr) )
+            return inject_undef64_exception(regs, hsr.len);
+        break;
+
+    /*
+     *  ICC_SRE_EL2.Enable = 0
+     *
+     *  GIC Architecture Specification (IHI 0069C): Section 8.1.9
+     */
+    case HSR_SYSREG_ICC_SRE_EL1:
+        /*
+         * Trapped when the guest is using GICv2 whilst the platform
+         * interrupt controller is GICv3. In this case, the register
+         * should be emulate as RAZ/WI to tell the guest to use the GIC
+         * memory mapped interface (i.e GICv2 compatibility).
+         */
+        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
+
+    /*
+     * HCR_EL2.TIDCP
+     *
+     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
+     *
+     *  - Reserved control space for IMPLEMENTATION DEFINED functionality.
+     *
+     * CPTR_EL2.TTA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     *  - All implemented trace registers.
+     *
+     * And all other unknown registers.
+     */
+    default:
+        {
+            const struct hsr_sysreg sysreg = hsr.sysreg;
+
+            gdprintk(XENLOG_ERR,
+                     "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",
+                     sysreg.read ? "mrs" : "msr",
+                     sysreg.op0, sysreg.op1,
+                     sysreg.crn, sysreg.crm,
+                     sysreg.op2,
+                     sysreg.read ? "=>" : "<=",
+                     sysreg.reg, regs->pc);
+            gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
+                     hsr.bits & HSR_SYSREG_REGS_MASK);
+            inject_undef_exception(regs, hsr);
+            return;
+        }
+    }
+
+    regs->pc += 4;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1c334a7b99..f00aa48892 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2295,204 +2295,6 @@ static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
     inject_undef_exception(regs, hsr);
 }
 
-#ifdef CONFIG_ARM_64
-static void do_sysreg(struct cpu_user_regs *regs,
-                      const union hsr hsr)
-{
-    int regidx = 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 )
-            set_user_reg(regs, regidx, 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, regidx, 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, regidx, hsr.sysreg.read, hsr, 1);
-    case HSR_SYSREG_OSDLR_EL1:
-        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
-
-    /*
-     * 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:
-        return handle_raz_wi(regs, regidx, 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, regidx, hsr.sysreg.read, hsr, 0);
-    HSR_SYSREG_DBG_CASES(DBGBVR):
-    HSR_SYSREG_DBG_CASES(DBGBCR):
-    HSR_SYSREG_DBG_CASES(DBGWVR):
-    HSR_SYSREG_DBG_CASES(DBGWCR):
-        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
-
-    /*
-     * 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:
-        /*
-         * Accessible from EL1 only, but if EL0 trap happens handle as
-         * undef.
-         */
-        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
-    case HSR_SYSREG_PMUSERENR_EL0:
-        /* RO at EL0. RAZ/WI at EL1 */
-        if ( psr_mode_is_user(regs) )
-            return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
-        else
-            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
-    case HSR_SYSREG_PMCR_EL0:
-    case HSR_SYSREG_PMCNTENSET_EL0:
-    case HSR_SYSREG_PMCNTENCLR_EL0:
-    case HSR_SYSREG_PMOVSCLR_EL0:
-    case HSR_SYSREG_PMSWINC_EL0:
-    case HSR_SYSREG_PMSELR_EL0:
-    case HSR_SYSREG_PMCEID0_EL0:
-    case HSR_SYSREG_PMCEID1_EL0:
-    case HSR_SYSREG_PMCCNTR_EL0:
-    case HSR_SYSREG_PMXEVTYPER_EL0:
-    case HSR_SYSREG_PMXEVCNTR_EL0:
-    case HSR_SYSREG_PMOVSSET_EL0:
-        /*
-         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
-         * emulate that register as 0 above.
-         */
-        return handle_raz_wi(regs, regidx, 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;
-
-    /*
-     * HCR_EL2.FMO or HCR_EL2.IMO
-     *
-     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
-     */
-    case HSR_SYSREG_ICC_SGI1R_EL1:
-    case HSR_SYSREG_ICC_ASGI1R_EL1:
-    case HSR_SYSREG_ICC_SGI0R_EL1:
-
-        if ( !vgic_emulate(regs, hsr) )
-            return inject_undef64_exception(regs, hsr.len);
-        break;
-
-    /*
-     *  ICC_SRE_EL2.Enable = 0
-     *
-     *  GIC Architecture Specification (IHI 0069C): Section 8.1.9
-     */
-    case HSR_SYSREG_ICC_SRE_EL1:
-        /*
-         * Trapped when the guest is using GICv2 whilst the platform
-         * interrupt controller is GICv3. In this case, the register
-         * should be emulate as RAZ/WI to tell the guest to use the GIC
-         * memory mapped interface (i.e GICv2 compatibility).
-         */
-        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
-
-    /*
-     * HCR_EL2.TIDCP
-     *
-     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
-     *
-     *  - Reserved control space for IMPLEMENTATION DEFINED functionality.
-     *
-     * CPTR_EL2.TTA
-     *
-     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
-     *
-     *  - All implemented trace registers.
-     *
-     * And all other unknown registers.
-     */
-    default:
-        {
-            const struct hsr_sysreg sysreg = hsr.sysreg;
-
-            gdprintk(XENLOG_ERR,
-                     "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",
-                     sysreg.read ? "mrs" : "msr",
-                     sysreg.op0, sysreg.op1,
-                     sysreg.crn, sysreg.crm,
-                     sysreg.op2,
-                     sysreg.read ? "=>" : "<=",
-                     sysreg.reg, regs->pc);
-            gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
-                     hsr.bits & HSR_SYSREG_REGS_MASK);
-            inject_undef_exception(regs, hsr);
-            return;
-        }
-    }
-
-    regs->pc += 4;
-}
-#endif
-
 void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 {
     register_t ttbcr = READ_SYSREG(TCR_EL1);
diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
index e5e5a4a036..2379b578cb 100644
--- a/xen/include/asm-arm/arm64/traps.h
+++ b/xen/include/asm-arm/arm64/traps.h
@@ -3,6 +3,9 @@
 
 void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
 
+void do_sysreg(struct cpu_user_regs *regs,
+               const union hsr hsr);
+
 #endif /* __ASM_ARM64_TRAPS__ */
 /*
  * Local variables:
-- 
2.11.0


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

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

* [PATCH v2 5/7] xen/arm: Move co-processor emulation outside of traps.c
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
                   ` (3 preceding siblings ...)
  2017-09-12 10:36 ` [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:43   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory Julien Grall
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

The co-processor emulation is quite big and pretty much standalone. Move
it in a separate file to shrink down the size of traps.c.

At the same time remove unused cpregs.h.

No functional change.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Makefile       |   1 +
 xen/arch/arm/traps.c        | 421 -----------------------------------------
 xen/arch/arm/vcpreg.c       | 451 ++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/traps.h |   8 +
 4 files changed, 460 insertions(+), 421 deletions(-)
 create mode 100644 xen/arch/arm/vcpreg.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 282d2c2949..17bff98033 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -45,6 +45,7 @@ obj-y += smpboot.o
 obj-y += sysctl.o
 obj-y += time.o
 obj-y += traps.o
+obj-y += vcpreg.o
 obj-y += vgic.o
 obj-y += vgic-v2.o
 obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f00aa48892..5e6bc3173f 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -38,7 +38,6 @@
 #include <public/xen.h>
 
 #include <asm/acpi.h>
-#include <asm/cpregs.h>
 #include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
 #include <asm/debugger.h>
@@ -1875,426 +1874,6 @@ void handle_ro_raz(struct cpu_user_regs *regs,
     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;
-    int regidx = cp32.reg;
-    struct vcpu *v = current;
-
-    if ( !check_conditional_instr(regs, hsr) )
-    {
-        advance_pc(regs, hsr);
-        return;
-    }
-
-    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) )
-            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);
-        if ( cp32.read )
-            set_user_reg(regs, regidx, 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.
-     *
-     * 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(PMUSERENR):
-        /* RO at EL0. RAZ/WI at EL1 */
-        if ( psr_mode_is_user(regs) )
-            return handle_ro_raz(regs, regidx, cp32.read, hsr, 0);
-        else
-            return handle_raz_wi(regs, regidx, 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. */
-        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
-    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(PMXEVTYPER):
-    case HSR_CPREG32(PMXEVCNTR):
-    case HSR_CPREG32(PMOVSSET):
-        /*
-         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
-         * emulate that register as 0 above.
-         */
-        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
-
-    /*
-     * HCR_EL2.TIDCP
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.3
-     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
-     *
-     *  - CRn==c9, opc1=={0-7}, CRm=={c0-c2, c5-c8}, opc2=={0-7}
-     *    (Cache and TCM lockdown registers)
-     *  - CRn==c10, opc1=={0-7}, CRm=={c0, c1, c4, c8}, opc2=={0-7}
-     *    (VMSA CP15 c10 registers)
-     *  - CRn==c11, opc1=={0-7}, CRm=={c0-c8, c15}, opc2=={0-7}
-     *    (VMSA CP15 c11 registers)
-     *
-     * CPTR_EL2.T{0..9,12..13}
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.12
-     * ARMv8 (DDI 0487A.d): N/A
-     *
-     *  - All accesses to coprocessors 0..9 and 12..13
-     *
-     * HSTR_EL2.T15
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.14
-     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
-     *
-     *  - All accesses to cp15, c15 registers.
-     *
-     * And all other unknown registers.
-     */
-    default:
-        gdprintk(XENLOG_ERR,
-                 "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
-                 cp32.read ? "mrc" : "mcr",
-                 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);
-        inject_undef_exception(regs, hsr);
-        return;
-    }
-    advance_pc(regs, hsr);
-}
-
-static void do_cp15_64(struct cpu_user_regs *regs,
-                       const union hsr hsr)
-{
-    if ( !check_conditional_instr(regs, hsr) )
-    {
-        advance_pc(regs, hsr);
-        return;
-    }
-
-    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);
-        break;
-
-    /*
-     * HCR_EL2.FMO or HCR_EL2.IMO
-     *
-     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
-     */
-    case HSR_CPREG64(ICC_SGI1R):
-    case HSR_CPREG64(ICC_ASGI1R):
-    case HSR_CPREG64(ICC_SGI0R):
-        if ( !vgic_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
-     *
-     *  - All accesses to coprocessors 0..9 and 12..13
-     *
-     * HSTR_EL2.T15
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.14
-     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
-     *
-     *  - All accesses to cp15, c15 registers.
-     *
-     * And all other unknown registers.
-     */
-    default:
-        {
-            const struct hsr_cp64 cp64 = hsr.cp64;
-
-            gdprintk(XENLOG_ERR,
-                     "%s p15, %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 CP15 access %#x\n",
-                     hsr.bits & HSR_CP64_REGS_MASK);
-            inject_undef_exception(regs, hsr);
-            return;
-        }
-    }
-    advance_pc(regs, hsr);
-}
-
-static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
-{
-    const struct hsr_cp32 cp32 = hsr.cp32;
-    int regidx = cp32.reg;
-    struct domain *d = current->domain;
-
-    if ( !check_conditional_instr(regs, hsr) )
-    {
-        advance_pc(regs, hsr);
-        return;
-    }
-
-    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, regidx, cp32.read, hsr, 1);
-    case HSR_CPREG32(DBGOSDLR):
-        return handle_raz_wi(regs, regidx, 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):
-    {
-        uint32_t val;
-
-        /*
-         * 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);
-
-        /* Implement the minimum requirements:
-         *  - Number of watchpoints: 1
-         *  - Number of breakpoints: 2
-         *  - Version: ARMv7 v7.1
-         *  - Variant and Revision bits match MDIR
-         */
-        val = (1 << 24) | (5 << 16);
-        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
-        set_user_reg(regs, regidx, val);
-
-        break;
-    }
-
-    case HSR_CPREG32(DBGDSCRINT):
-        /*
-         * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
-         * is set to 0, which we emulated below.
-         */
-        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
-
-    case HSR_CPREG32(DBGDSCREXT):
-        /*
-         * 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, regidx, cp32.read, hsr, 1);
-
-    case HSR_CPREG32(DBGVCR):
-    case HSR_CPREG32(DBGBVR0):
-    case HSR_CPREG32(DBGBCR0):
-    case HSR_CPREG32(DBGWVR0):
-    case HSR_CPREG32(DBGWCR0):
-    case HSR_CPREG32(DBGBVR1):
-    case HSR_CPREG32(DBGBCR1):
-        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
-
-    /*
-     * CPTR_EL2.TTA
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.16
-     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
-     *
-     *  - All implemented trace registers.
-     *
-     * 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:
-        gdprintk(XENLOG_ERR,
-                 "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
-                  cp32.read ? "mrc" : "mcr",
-                  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);
-        inject_undef_exception(regs, hsr);
-        return;
-    }
-
-    advance_pc(regs, hsr);
-}
-
-static void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
-{
-    const struct hsr_cp64 cp64 = hsr.cp64;
-
-    if ( !check_conditional_instr(regs, hsr) )
-    {
-        advance_pc(regs, hsr);
-        return;
-    }
-
-    /*
-     * CPTR_EL2.TTA
-     *
-     * ARMv7 (DDI 0406C.b): B1.14.16
-     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
-     *
-     *  - All implemented trace registers.
-     *
-     * 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",
-             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;
-    }
-
-    /*
-     * 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",
-             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);
-}
-
-static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
-{
-    const struct hsr_cp cp = hsr.cp;
-
-    if ( !check_conditional_instr(regs, hsr) )
-    {
-        advance_pc(regs, hsr);
-        return;
-    }
-
-    ASSERT(!cp.tas); /* We don't trap SIMD instruction */
-    gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc);
-    inject_undef_exception(regs, hsr);
-}
-
 void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 {
     register_t ttbcr = READ_SYSREG(TCR_EL1);
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
new file mode 100644
index 0000000000..f3b08403fb
--- /dev/null
+++ b/xen/arch/arm/vcpreg.c
@@ -0,0 +1,451 @@
+/*
+ * xen/arch/arm/arm64/vcpreg.c
+ *
+ * Emulate co-processor registers trapped.
+ *
+ * Copyright (c) 2011 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <xen/sched.h>
+
+#include <asm/current.h>
+#include <asm/regs.h>
+#include <asm/traps.h>
+#include <asm/vtimer.h>
+
+void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    const struct hsr_cp32 cp32 = hsr.cp32;
+    int regidx = cp32.reg;
+    struct vcpu *v = current;
+
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    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) )
+            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);
+        if ( cp32.read )
+            set_user_reg(regs, regidx, 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.
+     *
+     * 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(PMUSERENR):
+        /* RO at EL0. RAZ/WI at EL1 */
+        if ( psr_mode_is_user(regs) )
+            return handle_ro_raz(regs, regidx, cp32.read, hsr, 0);
+        else
+            return handle_raz_wi(regs, regidx, 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. */
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
+    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(PMXEVTYPER):
+    case HSR_CPREG32(PMXEVCNTR):
+    case HSR_CPREG32(PMOVSSET):
+        /*
+         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
+         * emulate that register as 0 above.
+         */
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
+
+    /*
+     * HCR_EL2.TIDCP
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.3
+     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
+     *
+     *  - CRn==c9, opc1=={0-7}, CRm=={c0-c2, c5-c8}, opc2=={0-7}
+     *    (Cache and TCM lockdown registers)
+     *  - CRn==c10, opc1=={0-7}, CRm=={c0, c1, c4, c8}, opc2=={0-7}
+     *    (VMSA CP15 c10 registers)
+     *  - CRn==c11, opc1=={0-7}, CRm=={c0-c8, c15}, opc2=={0-7}
+     *    (VMSA CP15 c11 registers)
+     *
+     * CPTR_EL2.T{0..9,12..13}
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.12
+     * ARMv8 (DDI 0487A.d): N/A
+     *
+     *  - All accesses to coprocessors 0..9 and 12..13
+     *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
+     *  - All accesses to cp15, c15 registers.
+     *
+     * And all other unknown registers.
+     */
+    default:
+        gdprintk(XENLOG_ERR,
+                 "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
+                 cp32.read ? "mrc" : "mcr",
+                 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);
+        inject_undef_exception(regs, hsr);
+        return;
+    }
+    advance_pc(regs, hsr);
+}
+
+void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    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);
+        break;
+
+    /*
+     * HCR_EL2.FMO or HCR_EL2.IMO
+     *
+     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
+     */
+    case HSR_CPREG64(ICC_SGI1R):
+    case HSR_CPREG64(ICC_ASGI1R):
+    case HSR_CPREG64(ICC_SGI0R):
+        if ( !vgic_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
+     *
+     *  - All accesses to coprocessors 0..9 and 12..13
+     *
+     * HSTR_EL2.T15
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.14
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
+     *
+     *  - All accesses to cp15, c15 registers.
+     *
+     * And all other unknown registers.
+     */
+    default:
+        {
+            const struct hsr_cp64 cp64 = hsr.cp64;
+
+            gdprintk(XENLOG_ERR,
+                     "%s p15, %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 CP15 access %#x\n",
+                     hsr.bits & HSR_CP64_REGS_MASK);
+            inject_undef_exception(regs, hsr);
+            return;
+        }
+    }
+    advance_pc(regs, hsr);
+}
+
+void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    const struct hsr_cp32 cp32 = hsr.cp32;
+    int regidx = cp32.reg;
+    struct domain *d = current->domain;
+
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    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, regidx, cp32.read, hsr, 1);
+    case HSR_CPREG32(DBGOSDLR):
+        return handle_raz_wi(regs, regidx, 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):
+    {
+        uint32_t val;
+
+        /*
+         * 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);
+
+        /* Implement the minimum requirements:
+         *  - Number of watchpoints: 1
+         *  - Number of breakpoints: 2
+         *  - Version: ARMv7 v7.1
+         *  - Variant and Revision bits match MDIR
+         */
+        val = (1 << 24) | (5 << 16);
+        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
+        set_user_reg(regs, regidx, val);
+
+        break;
+    }
+
+    case HSR_CPREG32(DBGDSCRINT):
+        /*
+         * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
+         * is set to 0, which we emulated below.
+         */
+        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
+
+    case HSR_CPREG32(DBGDSCREXT):
+        /*
+         * 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, regidx, cp32.read, hsr, 1);
+
+    case HSR_CPREG32(DBGVCR):
+    case HSR_CPREG32(DBGBVR0):
+    case HSR_CPREG32(DBGBCR0):
+    case HSR_CPREG32(DBGWVR0):
+    case HSR_CPREG32(DBGWCR0):
+    case HSR_CPREG32(DBGBVR1):
+    case HSR_CPREG32(DBGBCR1):
+        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
+
+    /*
+     * CPTR_EL2.TTA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.16
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     *  - All implemented trace registers.
+     *
+     * 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:
+        gdprintk(XENLOG_ERR,
+                 "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
+                  cp32.read ? "mrc" : "mcr",
+                  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);
+        inject_undef_exception(regs, hsr);
+        return;
+    }
+
+    advance_pc(regs, hsr);
+}
+
+void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    const struct hsr_cp64 cp64 = hsr.cp64;
+
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    /*
+     * CPTR_EL2.TTA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.16
+     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     *  - All implemented trace registers.
+     *
+     * 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",
+             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);
+}
+
+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;
+    }
+
+    /*
+     * 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",
+             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);
+}
+
+void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
+{
+    const struct hsr_cp cp = hsr.cp;
+
+    if ( !check_conditional_instr(regs, hsr) )
+    {
+        advance_pc(regs, hsr);
+        return;
+    }
+
+    ASSERT(!cp.tas); /* We don't trap SIMD instruction */
+    gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc);
+    inject_undef_exception(regs, hsr);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
index 6d99d228e8..53d386d8e5 100644
--- a/xen/include/asm-arm/traps.h
+++ b/xen/include/asm-arm/traps.h
@@ -24,6 +24,14 @@ void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
 void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
                    const union hsr hsr, int min_el);
 
+/* Co-processor registers emulation (see arch/arm/vcpreg.c). */
+void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
+void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
+
 #endif /* __ASM_ARM_TRAPS__ */
 /*
  * Local variables:
-- 
2.11.0


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

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

* [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
                   ` (4 preceding siblings ...)
  2017-09-12 10:36 ` [PATCH v2 5/7] xen/arm: Move co-processor " Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:49   ` Stefano Stabellini
  2017-09-12 10:36 ` [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h Julien Grall
  2017-09-12 21:57 ` [PATCH v2 0/7] xen/arm: Clean-up traps.c Stefano Stabellini
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

sysregs.h contains only code protected by #ifdef CONFIG_ARM_64. Move it
in arm64 sub-directory to reflect that and remove the #ifdef.

At the same time, fixup the guards.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/arm64/processor.h     |  2 ++
 xen/include/asm-arm/{ => arm64}/sysregs.h | 10 +++-------
 xen/include/asm-arm/processor.h           |  1 -
 3 files changed, 5 insertions(+), 8 deletions(-)
 rename xen/include/asm-arm/{ => arm64}/sysregs.h (98%)

diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
index 24f836b023..c18ab7203d 100644
--- a/xen/include/asm-arm/arm64/processor.h
+++ b/xen/include/asm-arm/arm64/processor.h
@@ -3,6 +3,8 @@
 
 #include <xen/stringify.h>
 
+#include <asm/arm64/sysregs.h>
+
 #ifndef __ASSEMBLY__
 
 /* Anonymous union includes both 32- and 64-bit names (e.g., r0/x0). */
diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
similarity index 98%
rename from xen/include/asm-arm/sysregs.h
rename to xen/include/asm-arm/arm64/sysregs.h
index 887368e248..084d2a1e5d 100644
--- a/xen/include/asm-arm/sysregs.h
+++ b/xen/include/asm-arm/arm64/sysregs.h
@@ -1,7 +1,5 @@
-#ifndef __ASM_ARM_SYSREGS_H
-#define __ASM_ARM_SYSREGS_H
-
-#ifdef CONFIG_ARM_64
+#ifndef __ASM_ARM_ARM64_SYSREGS_H
+#define __ASM_ARM_ARM64_SYSREGS_H
 
 #include <xen/stringify.h>
 
@@ -168,9 +166,7 @@
 #define ICH_AP1R2_EL2             __AP1Rx_EL2(2)
 #define ICH_AP1R3_EL2             __AP1Rx_EL2(3)
 
-#endif
-
-#endif
+#endif /* _ASM_ARM_ARM64_SYSREGS_H */
 
 /*
  * Local variables:
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 9f7a42f86b..d791c12c9c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -2,7 +2,6 @@
 #define __ASM_ARM_PROCESSOR_H
 
 #include <asm/cpregs.h>
-#include <asm/sysregs.h>
 #ifndef __ASSEMBLY__
 #include <xen/types.h>
 #endif
-- 
2.11.0


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

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

* [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
                   ` (5 preceding siblings ...)
  2017-09-12 10:36 ` [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory Julien Grall
@ 2017-09-12 10:36 ` Julien Grall
  2017-09-12 21:53   ` Stefano Stabellini
  2017-09-12 21:57 ` [PATCH v2 0/7] xen/arm: Clean-up traps.c Stefano Stabellini
  7 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-12 10:36 UTC (permalink / raw)
  To: xen-devel; +Cc: bhupinder.thakur, Julien Grall, sstabellini

Currently, cpregs.h is included in pretty much every files even for
arm64. However, the only use for arm64 is when emulating co-processors.

For arm32, all users of processor.h expect cpregs.h to be included in
order to access co-processors. So move the inclusion in
asm-arm/arm32/processor.h.

cpregs.h will also be directly included in the co-processors emulation
to accommodate arm64.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Update commit message
---
 xen/arch/arm/smp.c                    | 1 -
 xen/arch/arm/vcpreg.c                 | 1 +
 xen/arch/arm/vgic-v3.c                | 1 +
 xen/arch/arm/vtimer.c                 | 2 ++
 xen/include/asm-arm/arm32/processor.h | 2 ++
 xen/include/asm-arm/percpu.h          | 1 -
 xen/include/asm-arm/processor.h       | 1 -
 7 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
index e7df0874d6..554f4992e6 100644
--- a/xen/arch/arm/smp.c
+++ b/xen/arch/arm/smp.c
@@ -1,6 +1,5 @@
 #include <asm/system.h>
 #include <asm/smp.h>
-#include <asm/cpregs.h>
 #include <asm/page.h>
 #include <asm/gic.h>
 #include <asm/flushtlb.h>
diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
index f3b08403fb..e363183ba8 100644
--- a/xen/arch/arm/vcpreg.c
+++ b/xen/arch/arm/vcpreg.c
@@ -18,6 +18,7 @@
 
 #include <xen/sched.h>
 
+#include <asm/cpregs.h>
 #include <asm/current.h>
 #include <asm/regs.h>
 #include <asm/traps.h>
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index cbeac28b28..a0cf993d13 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -26,6 +26,7 @@
 #include <xen/softirq.h>
 #include <xen/sizes.h>
 
+#include <asm/cpregs.h>
 #include <asm/current.h>
 #include <asm/gic_v3_defs.h>
 #include <asm/gic_v3_its.h>
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 9c7e8f441c..0460962f08 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -22,6 +22,7 @@
 #include <xen/sched.h>
 #include <xen/timer.h>
 
+#include <asm/cpregs.h>
 #include <asm/div64.h>
 #include <asm/gic.h>
 #include <asm/irq.h>
@@ -29,6 +30,7 @@
 #include <asm/time.h>
 #include <asm/vgic.h>
 #include <asm/vreg.h>
+#include <asm/regs.h>
 
 /*
  * Check if regs is allowed access, user_gate is tail end of a
diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
index 68cc82147e..fb330812af 100644
--- a/xen/include/asm-arm/arm32/processor.h
+++ b/xen/include/asm-arm/arm32/processor.h
@@ -1,6 +1,8 @@
 #ifndef __ASM_ARM_ARM32_PROCESSOR_H
 #define __ASM_ARM_ARM32_PROCESSOR_H
 
+#include <asm/cpregs.h>
+
 #define ACTLR_CAXX_SMP      (1<<6)
 
 #ifndef __ASSEMBLY__
diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
index 7968532462..cdf64e0f77 100644
--- a/xen/include/asm-arm/percpu.h
+++ b/xen/include/asm-arm/percpu.h
@@ -4,7 +4,6 @@
 #ifndef __ASSEMBLY__
 
 #include <xen/types.h>
-#include <asm/cpregs.h>
 #if defined(CONFIG_ARM_32)
 # include <asm/arm32/processor.h>
 #elif defined(CONFIG_ARM_64)
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index d791c12c9c..cd45e5f48f 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -1,7 +1,6 @@
 #ifndef __ASM_ARM_PROCESSOR_H
 #define __ASM_ARM_PROCESSOR_H
 
-#include <asm/cpregs.h>
 #ifndef __ASSEMBLY__
 #include <xen/types.h>
 #endif
-- 
2.11.0


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

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

* Re: [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically
  2017-09-12 10:36 ` [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically Julien Grall
@ 2017-09-12 20:59   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 20:59 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Acked-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>     Changes in v2:
>         - Fix alphabetical order
>         - Add Bhupinder's acked-by
> ---
>  xen/arch/arm/traps.c | 40 +++++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 7f6ec15b5e..6b3dfd9bcf 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -16,41 +16,43 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <xen/domain_page.h>
> +#include <xen/errno.h>
> +#include <xen/hypercall.h>
>  #include <xen/init.h>
> -#include <xen/string.h>
> -#include <xen/version.h>
> -#include <xen/smp.h>
> -#include <xen/symbols.h>
> +#include <xen/iocap.h>
>  #include <xen/irq.h>
>  #include <xen/lib.h>
>  #include <xen/livepatch.h>
> +#include <xen/mem_access.h>
>  #include <xen/mm.h>
> -#include <xen/errno.h>
> -#include <xen/hypercall.h>
> -#include <xen/softirq.h>
> -#include <xen/domain_page.h>
>  #include <xen/perfc.h>
> +#include <xen/smp.h>
> +#include <xen/softirq.h>
> +#include <xen/string.h>
> +#include <xen/symbols.h>
> +#include <xen/version.h>
>  #include <xen/virtual_region.h>
> -#include <xen/mem_access.h>
> -#include <xen/iocap.h>
> +
>  #include <public/sched.h>
>  #include <public/xen.h>
> -#include <asm/debugger.h>
> -#include <asm/event.h>
> -#include <asm/regs.h>
> +
> +#include <asm/acpi.h>
>  #include <asm/cpregs.h>
> -#include <asm/psci.h>
> -#include <asm/mmio.h>
> +#include <asm/cpuerrata.h>
>  #include <asm/cpufeature.h>
> +#include <asm/debugger.h>
> +#include <asm/event.h>
>  #include <asm/flushtlb.h>
> +#include <asm/gic.h>
> +#include <asm/mmio.h>
>  #include <asm/monitor.h>
> +#include <asm/psci.h>
> +#include <asm/regs.h>
> +#include <asm/vgic.h>
>  
>  #include "decode.h"
>  #include "vtimer.h"
> -#include <asm/gic.h>
> -#include <asm/vgic.h>
> -#include <asm/cpuerrata.h>
> -#include <asm/acpi.h>
>  
>  /* The base of the stack must always be double-word aligned, which means
>   * that both the kernel half of struct cpu_user_regs (which is pushed in
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h
  2017-09-12 10:36 ` [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h Julien Grall
@ 2017-09-12 21:02   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:02 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> It will be necessary to include vtimer.h from subdirectory making the
> inclusion a bit awkward.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/domain.c                      | 2 +-
>  xen/arch/arm/traps.c                       | 2 +-
>  xen/{arch/arm => include/asm-arm}/vtimer.h | 0
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename xen/{arch/arm => include/asm-arm}/vtimer.h (100%)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 6512f01463..784ae392cf 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -33,8 +33,8 @@
>  #include <asm/regs.h>
>  #include <asm/vfp.h>
>  #include <asm/vgic.h>
> +#include <asm/vtimer.h>
>  
> -#include "vtimer.h"
>  #include "vuart.h"
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 6b3dfd9bcf..6f32f700e5 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -50,9 +50,9 @@
>  #include <asm/psci.h>
>  #include <asm/regs.h>
>  #include <asm/vgic.h>
> +#include <asm/vtimer.h>
>  
>  #include "decode.h"
> -#include "vtimer.h"
>  
>  /* The base of the stack must always be double-word aligned, which means
>   * that both the kernel half of struct cpu_user_regs (which is pushed in
> diff --git a/xen/arch/arm/vtimer.h b/xen/include/asm-arm/vtimer.h
> similarity index 100%
> rename from xen/arch/arm/vtimer.h
> rename to xen/include/asm-arm/vtimer.h
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation
  2017-09-12 10:36 ` [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation Julien Grall
@ 2017-09-12 21:26   ` Stefano Stabellini
  2017-09-13  8:52     ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:26 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, volodymyr_babchuk, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> A follow-up patch will move some parts of traps.c in separate files.
> The will require to use helpers that are currently statically defined.
> Export the following helpers:
>     - inject_undef64_exception
>     - inject_undef_exception
>     - check_conditional_instr
>     - advance_pc
>     - handle_raz_wi
>     - handle_wo_wi
>     - handle_ro_raz
> 
> Note that asm-arm/arm32/traps.h is empty but it is to keep parity with
> the arm64 counterpart.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> 
> Cc: volodymyr_babchuk@epam.com
> 
>     Changes in v2:
>         - Fixup guards
>         - Add newline for clarity
> ---
>  xen/arch/arm/traps.c              | 43 +++++++++++++++++++--------------------
>  xen/include/asm-arm/arm32/traps.h | 13 ++++++++++++
>  xen/include/asm-arm/arm64/traps.h | 15 ++++++++++++++
>  xen/include/asm-arm/traps.h       | 36 ++++++++++++++++++++++++++++++++
>  4 files changed, 85 insertions(+), 22 deletions(-)
>  create mode 100644 xen/include/asm-arm/arm32/traps.h
>  create mode 100644 xen/include/asm-arm/arm64/traps.h
>  create mode 100644 xen/include/asm-arm/traps.h
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 6f32f700e5..1c334a7b99 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -49,6 +49,7 @@
>  #include <asm/monitor.h>
>  #include <asm/psci.h>
>  #include <asm/regs.h>
> +#include <asm/traps.h>
>  #include <asm/vgic.h>
>  #include <asm/vtimer.h>
>  
> @@ -547,7 +548,7 @@ static vaddr_t exception_handler64(struct cpu_user_regs *regs, vaddr_t offset)
>  }
>  
>  /* Inject an undefined exception into a 64 bit guest */
> -static void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
> +void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len)
>  {
>      vaddr_t handler;
>      const union hsr esr = {
> @@ -620,8 +621,7 @@ static void inject_iabt64_exception(struct cpu_user_regs *regs,
>  
>  #endif
>  
> -static void inject_undef_exception(struct cpu_user_regs *regs,
> -                                   const union hsr hsr)
> +void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>          if ( is_32bit_domain(current->domain) )
>              inject_undef32_exception(regs);
> @@ -1714,8 +1714,7 @@ static const unsigned short cc_map[16] = {
>          0                       /* NV                     */
>  };
>  
> -static int check_conditional_instr(struct cpu_user_regs *regs,
> -                                   const union hsr hsr)
> +int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>      unsigned long cpsr, cpsr_cond;
>      int cond;
> @@ -1777,7 +1776,7 @@ static int check_conditional_instr(struct cpu_user_regs *regs,
>      return 1;
>  }
>  
> -static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
> +void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>      unsigned long itbits, cond, cpsr = regs->cpsr;
>  
> @@ -1818,11 +1817,11 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
>  }
>  
>  /* Read as zero and write ignore */
> -static void handle_raz_wi(struct cpu_user_regs *regs,
> -                          int regidx,
> -                          bool read,
> -                          const union hsr hsr,
> -                          int min_el)
> +void handle_raz_wi(struct cpu_user_regs *regs,
> +                   int regidx,
> +                   bool read,
> +                   const union hsr hsr,
> +                   int min_el)
>  {
>      ASSERT((min_el == 0) || (min_el == 1));
>  
> @@ -1836,12 +1835,12 @@ static void handle_raz_wi(struct cpu_user_regs *regs,
>      advance_pc(regs, hsr);
>  }
>  
> -/* Write only as write ignore */
> -static void handle_wo_wi(struct cpu_user_regs *regs,
> -                         int regidx,
> -                         bool read,
> -                         const union hsr hsr,
> -                         int min_el)
> +/* write only as write ignore */
> +void handle_wo_wi(struct cpu_user_regs *regs,
> +                  int regidx,
> +                  bool read,
> +                  const union hsr hsr,
> +                  int min_el)
>  {
>      ASSERT((min_el == 0) || (min_el == 1));
>  
> @@ -1856,11 +1855,11 @@ static void handle_wo_wi(struct cpu_user_regs *regs,
>  }
>  
>  /* Read only as read as zero */
> -static void handle_ro_raz(struct cpu_user_regs *regs,
> -                          int regidx,
> -                          bool read,
> -                          const union hsr hsr,
> -                          int min_el)
> +void handle_ro_raz(struct cpu_user_regs *regs,
> +                   int regidx,
> +                   bool read,
> +                   const union hsr hsr,
> +                   int min_el)
>  {
>      ASSERT((min_el == 0) || (min_el == 1));
>  
> diff --git a/xen/include/asm-arm/arm32/traps.h b/xen/include/asm-arm/arm32/traps.h
> new file mode 100644
> index 0000000000..e3c4a8b473
> --- /dev/null
> +++ b/xen/include/asm-arm/arm32/traps.h
> @@ -0,0 +1,13 @@
> +#ifndef __ASM_ARM32_TRAPS__
> +#define __ASM_ARM32_TRAPS__
> +
> +#endif /* __ASM_ARM32_TRAPS__ */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
> new file mode 100644
> index 0000000000..e5e5a4a036
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/traps.h
> @@ -0,0 +1,15 @@
> +#ifndef __ASM_ARM64_TRAPS__
> +#define __ASM_ARM64_TRAPS__
> +
> +void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
> +
> +#endif /* __ASM_ARM64_TRAPS__ */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> +
> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
> new file mode 100644
> index 0000000000..6d99d228e8
> --- /dev/null
> +++ b/xen/include/asm-arm/traps.h
> @@ -0,0 +1,36 @@
> +#ifndef __ASM_ARM_TRAPS__
> +#define __ASM_ARM_TRAPS__
> +
> +#include <asm/processor.h>
> +
> +#if defined(CONFIG_ARM_32)
> +# include <asm/arm32/traps.h>
> +#elif defined(CONFIG_ARM_64)
> +# include <asm/arm64/traps.h>
> +#endif
> +
> +int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
> +
> +void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
> +
> +void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr);
> +
> +void handle_raz_wi(struct cpu_user_regs *regs, int regidx, bool read,
> +                   const union hsr hsr, int min_el);
> +
> +void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
> +                  const union hsr hsr, int min_el);
> +
> +void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
> +                   const union hsr hsr, int min_el);
>

Could you please copy or move the simple one-line comments we have in
traps.c here as well to describe these functions? Such as "Read only as
read as zero" and "Read as zero and write ignore".

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

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

* Re: [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c
  2017-09-12 10:36 ` [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c Julien Grall
@ 2017-09-12 21:40   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:40 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> The sysreg emulation is 64-bit specific and surrounded by #ifdef. Move
> them in a separate file arm/arm64/vsysreg.c to shrink down a bit traps.c
> 
> No functional change.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/arm64/Makefile       |   1 +
>  xen/arch/arm/arm64/vsysreg.c      | 229 ++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c              | 198 --------------------------------
>  xen/include/asm-arm/arm64/traps.h |   3 +
>  4 files changed, 233 insertions(+), 198 deletions(-)
>  create mode 100644 xen/arch/arm/arm64/vsysreg.c
> 
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 149b6b3901..718fe44455 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_LIVEPATCH) += livepatch.o
>  obj-y += smpboot.o
>  obj-y += traps.o
>  obj-y += vfp.o
> +obj-y += vsysreg.o
> diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
> new file mode 100644
> index 0000000000..c57ac12503
> --- /dev/null
> +++ b/xen/arch/arm/arm64/vsysreg.c
> @@ -0,0 +1,229 @@
> +/*
> + * xen/arch/arm/arm64/sysreg.c
> + *
> + * Emulate system registers trapped.
> + *
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/sched.h>
> +
> +#include <asm/current.h>
> +#include <asm/regs.h>
> +#include <asm/traps.h>
> +#include <asm/vtimer.h>
> +
> +void do_sysreg(struct cpu_user_regs *regs,
> +               const union hsr hsr)
> +{
> +    int regidx = 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 )
> +            set_user_reg(regs, regidx, 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, regidx, 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, regidx, hsr.sysreg.read, hsr, 1);
> +    case HSR_SYSREG_OSDLR_EL1:
> +        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +
> +    /*
> +     * 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:
> +        return handle_raz_wi(regs, regidx, 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, regidx, hsr.sysreg.read, hsr, 0);
> +    HSR_SYSREG_DBG_CASES(DBGBVR):
> +    HSR_SYSREG_DBG_CASES(DBGBCR):
> +    HSR_SYSREG_DBG_CASES(DBGWVR):
> +    HSR_SYSREG_DBG_CASES(DBGWCR):
> +        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +
> +    /*
> +     * 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:
> +        /*
> +         * Accessible from EL1 only, but if EL0 trap happens handle as
> +         * undef.
> +         */
> +        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +    case HSR_SYSREG_PMUSERENR_EL0:
> +        /* RO at EL0. RAZ/WI at EL1 */
> +        if ( psr_mode_is_user(regs) )
> +            return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
> +        else
> +            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +    case HSR_SYSREG_PMCR_EL0:
> +    case HSR_SYSREG_PMCNTENSET_EL0:
> +    case HSR_SYSREG_PMCNTENCLR_EL0:
> +    case HSR_SYSREG_PMOVSCLR_EL0:
> +    case HSR_SYSREG_PMSWINC_EL0:
> +    case HSR_SYSREG_PMSELR_EL0:
> +    case HSR_SYSREG_PMCEID0_EL0:
> +    case HSR_SYSREG_PMCEID1_EL0:
> +    case HSR_SYSREG_PMCCNTR_EL0:
> +    case HSR_SYSREG_PMXEVTYPER_EL0:
> +    case HSR_SYSREG_PMXEVCNTR_EL0:
> +    case HSR_SYSREG_PMOVSSET_EL0:
> +        /*
> +         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
> +         * emulate that register as 0 above.
> +         */
> +        return handle_raz_wi(regs, regidx, 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;
> +
> +    /*
> +     * HCR_EL2.FMO or HCR_EL2.IMO
> +     *
> +     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
> +     */
> +    case HSR_SYSREG_ICC_SGI1R_EL1:
> +    case HSR_SYSREG_ICC_ASGI1R_EL1:
> +    case HSR_SYSREG_ICC_SGI0R_EL1:
> +
> +        if ( !vgic_emulate(regs, hsr) )
> +            return inject_undef64_exception(regs, hsr.len);
> +        break;
> +
> +    /*
> +     *  ICC_SRE_EL2.Enable = 0
> +     *
> +     *  GIC Architecture Specification (IHI 0069C): Section 8.1.9
> +     */
> +    case HSR_SYSREG_ICC_SRE_EL1:
> +        /*
> +         * Trapped when the guest is using GICv2 whilst the platform
> +         * interrupt controller is GICv3. In this case, the register
> +         * should be emulate as RAZ/WI to tell the guest to use the GIC
> +         * memory mapped interface (i.e GICv2 compatibility).
> +         */
> +        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> +
> +    /*
> +     * HCR_EL2.TIDCP
> +     *
> +     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
> +     *
> +     *  - Reserved control space for IMPLEMENTATION DEFINED functionality.
> +     *
> +     * CPTR_EL2.TTA
> +     *
> +     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> +     *
> +     *  - All implemented trace registers.
> +     *
> +     * And all other unknown registers.
> +     */
> +    default:
> +        {
> +            const struct hsr_sysreg sysreg = hsr.sysreg;
> +
> +            gdprintk(XENLOG_ERR,
> +                     "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",
> +                     sysreg.read ? "mrs" : "msr",
> +                     sysreg.op0, sysreg.op1,
> +                     sysreg.crn, sysreg.crm,
> +                     sysreg.op2,
> +                     sysreg.read ? "=>" : "<=",
> +                     sysreg.reg, regs->pc);
> +            gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
> +                     hsr.bits & HSR_SYSREG_REGS_MASK);
> +            inject_undef_exception(regs, hsr);
> +            return;
> +        }
> +    }
> +
> +    regs->pc += 4;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 1c334a7b99..f00aa48892 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2295,204 +2295,6 @@ static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
>      inject_undef_exception(regs, hsr);
>  }
>  
> -#ifdef CONFIG_ARM_64
> -static void do_sysreg(struct cpu_user_regs *regs,
> -                      const union hsr hsr)
> -{
> -    int regidx = 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 )
> -            set_user_reg(regs, regidx, 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, regidx, 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, regidx, hsr.sysreg.read, hsr, 1);
> -    case HSR_SYSREG_OSDLR_EL1:
> -        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> -
> -    /*
> -     * 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:
> -        return handle_raz_wi(regs, regidx, 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, regidx, hsr.sysreg.read, hsr, 0);
> -    HSR_SYSREG_DBG_CASES(DBGBVR):
> -    HSR_SYSREG_DBG_CASES(DBGBCR):
> -    HSR_SYSREG_DBG_CASES(DBGWVR):
> -    HSR_SYSREG_DBG_CASES(DBGWCR):
> -        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> -
> -    /*
> -     * 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:
> -        /*
> -         * Accessible from EL1 only, but if EL0 trap happens handle as
> -         * undef.
> -         */
> -        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> -    case HSR_SYSREG_PMUSERENR_EL0:
> -        /* RO at EL0. RAZ/WI at EL1 */
> -        if ( psr_mode_is_user(regs) )
> -            return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
> -        else
> -            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> -    case HSR_SYSREG_PMCR_EL0:
> -    case HSR_SYSREG_PMCNTENSET_EL0:
> -    case HSR_SYSREG_PMCNTENCLR_EL0:
> -    case HSR_SYSREG_PMOVSCLR_EL0:
> -    case HSR_SYSREG_PMSWINC_EL0:
> -    case HSR_SYSREG_PMSELR_EL0:
> -    case HSR_SYSREG_PMCEID0_EL0:
> -    case HSR_SYSREG_PMCEID1_EL0:
> -    case HSR_SYSREG_PMCCNTR_EL0:
> -    case HSR_SYSREG_PMXEVTYPER_EL0:
> -    case HSR_SYSREG_PMXEVCNTR_EL0:
> -    case HSR_SYSREG_PMOVSSET_EL0:
> -        /*
> -         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
> -         * emulate that register as 0 above.
> -         */
> -        return handle_raz_wi(regs, regidx, 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;
> -
> -    /*
> -     * HCR_EL2.FMO or HCR_EL2.IMO
> -     *
> -     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
> -     */
> -    case HSR_SYSREG_ICC_SGI1R_EL1:
> -    case HSR_SYSREG_ICC_ASGI1R_EL1:
> -    case HSR_SYSREG_ICC_SGI0R_EL1:
> -
> -        if ( !vgic_emulate(regs, hsr) )
> -            return inject_undef64_exception(regs, hsr.len);
> -        break;
> -
> -    /*
> -     *  ICC_SRE_EL2.Enable = 0
> -     *
> -     *  GIC Architecture Specification (IHI 0069C): Section 8.1.9
> -     */
> -    case HSR_SYSREG_ICC_SRE_EL1:
> -        /*
> -         * Trapped when the guest is using GICv2 whilst the platform
> -         * interrupt controller is GICv3. In this case, the register
> -         * should be emulate as RAZ/WI to tell the guest to use the GIC
> -         * memory mapped interface (i.e GICv2 compatibility).
> -         */
> -        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
> -
> -    /*
> -     * HCR_EL2.TIDCP
> -     *
> -     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
> -     *
> -     *  - Reserved control space for IMPLEMENTATION DEFINED functionality.
> -     *
> -     * CPTR_EL2.TTA
> -     *
> -     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> -     *
> -     *  - All implemented trace registers.
> -     *
> -     * And all other unknown registers.
> -     */
> -    default:
> -        {
> -            const struct hsr_sysreg sysreg = hsr.sysreg;
> -
> -            gdprintk(XENLOG_ERR,
> -                     "%s %d, %d, c%d, c%d, %d %s x%d @ 0x%"PRIregister"\n",
> -                     sysreg.read ? "mrs" : "msr",
> -                     sysreg.op0, sysreg.op1,
> -                     sysreg.crn, sysreg.crm,
> -                     sysreg.op2,
> -                     sysreg.read ? "=>" : "<=",
> -                     sysreg.reg, regs->pc);
> -            gdprintk(XENLOG_ERR, "unhandled 64-bit sysreg access %#x\n",
> -                     hsr.bits & HSR_SYSREG_REGS_MASK);
> -            inject_undef_exception(regs, hsr);
> -            return;
> -        }
> -    }
> -
> -    regs->pc += 4;
> -}
> -#endif
> -
>  void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>  {
>      register_t ttbcr = READ_SYSREG(TCR_EL1);
> diff --git a/xen/include/asm-arm/arm64/traps.h b/xen/include/asm-arm/arm64/traps.h
> index e5e5a4a036..2379b578cb 100644
> --- a/xen/include/asm-arm/arm64/traps.h
> +++ b/xen/include/asm-arm/arm64/traps.h
> @@ -3,6 +3,9 @@
>  
>  void inject_undef64_exception(struct cpu_user_regs *regs, int instr_len);
>  
> +void do_sysreg(struct cpu_user_regs *regs,
> +               const union hsr hsr);
> +
>  #endif /* __ASM_ARM64_TRAPS__ */
>  /*
>   * Local variables:
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 5/7] xen/arm: Move co-processor emulation outside of traps.c
  2017-09-12 10:36 ` [PATCH v2 5/7] xen/arm: Move co-processor " Julien Grall
@ 2017-09-12 21:43   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:43 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> The co-processor emulation is quite big and pretty much standalone. Move
> it in a separate file to shrink down the size of traps.c.
> 
> At the same time remove unused cpregs.h.
> 
> No functional change.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/Makefile       |   1 +
>  xen/arch/arm/traps.c        | 421 -----------------------------------------
>  xen/arch/arm/vcpreg.c       | 451 ++++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/traps.h |   8 +
>  4 files changed, 460 insertions(+), 421 deletions(-)
>  create mode 100644 xen/arch/arm/vcpreg.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 282d2c2949..17bff98033 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -45,6 +45,7 @@ obj-y += smpboot.o
>  obj-y += sysctl.o
>  obj-y += time.o
>  obj-y += traps.o
> +obj-y += vcpreg.o
>  obj-y += vgic.o
>  obj-y += vgic-v2.o
>  obj-$(CONFIG_HAS_GICV3) += vgic-v3.o
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f00aa48892..5e6bc3173f 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -38,7 +38,6 @@
>  #include <public/xen.h>
>  
>  #include <asm/acpi.h>
> -#include <asm/cpregs.h>
>  #include <asm/cpuerrata.h>
>  #include <asm/cpufeature.h>
>  #include <asm/debugger.h>
> @@ -1875,426 +1874,6 @@ void handle_ro_raz(struct cpu_user_regs *regs,
>      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;
> -    int regidx = cp32.reg;
> -    struct vcpu *v = current;
> -
> -    if ( !check_conditional_instr(regs, hsr) )
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> -    }
> -
> -    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) )
> -            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);
> -        if ( cp32.read )
> -            set_user_reg(regs, regidx, 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.
> -     *
> -     * 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(PMUSERENR):
> -        /* RO at EL0. RAZ/WI at EL1 */
> -        if ( psr_mode_is_user(regs) )
> -            return handle_ro_raz(regs, regidx, cp32.read, hsr, 0);
> -        else
> -            return handle_raz_wi(regs, regidx, 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. */
> -        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> -    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(PMXEVTYPER):
> -    case HSR_CPREG32(PMXEVCNTR):
> -    case HSR_CPREG32(PMOVSSET):
> -        /*
> -         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
> -         * emulate that register as 0 above.
> -         */
> -        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> -
> -    /*
> -     * HCR_EL2.TIDCP
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.3
> -     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
> -     *
> -     *  - CRn==c9, opc1=={0-7}, CRm=={c0-c2, c5-c8}, opc2=={0-7}
> -     *    (Cache and TCM lockdown registers)
> -     *  - CRn==c10, opc1=={0-7}, CRm=={c0, c1, c4, c8}, opc2=={0-7}
> -     *    (VMSA CP15 c10 registers)
> -     *  - CRn==c11, opc1=={0-7}, CRm=={c0-c8, c15}, opc2=={0-7}
> -     *    (VMSA CP15 c11 registers)
> -     *
> -     * CPTR_EL2.T{0..9,12..13}
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.12
> -     * ARMv8 (DDI 0487A.d): N/A
> -     *
> -     *  - All accesses to coprocessors 0..9 and 12..13
> -     *
> -     * HSTR_EL2.T15
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.14
> -     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
> -     *
> -     *  - All accesses to cp15, c15 registers.
> -     *
> -     * And all other unknown registers.
> -     */
> -    default:
> -        gdprintk(XENLOG_ERR,
> -                 "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> -                 cp32.read ? "mrc" : "mcr",
> -                 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);
> -        inject_undef_exception(regs, hsr);
> -        return;
> -    }
> -    advance_pc(regs, hsr);
> -}
> -
> -static void do_cp15_64(struct cpu_user_regs *regs,
> -                       const union hsr hsr)
> -{
> -    if ( !check_conditional_instr(regs, hsr) )
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> -    }
> -
> -    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);
> -        break;
> -
> -    /*
> -     * HCR_EL2.FMO or HCR_EL2.IMO
> -     *
> -     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
> -     */
> -    case HSR_CPREG64(ICC_SGI1R):
> -    case HSR_CPREG64(ICC_ASGI1R):
> -    case HSR_CPREG64(ICC_SGI0R):
> -        if ( !vgic_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
> -     *
> -     *  - All accesses to coprocessors 0..9 and 12..13
> -     *
> -     * HSTR_EL2.T15
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.14
> -     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
> -     *
> -     *  - All accesses to cp15, c15 registers.
> -     *
> -     * And all other unknown registers.
> -     */
> -    default:
> -        {
> -            const struct hsr_cp64 cp64 = hsr.cp64;
> -
> -            gdprintk(XENLOG_ERR,
> -                     "%s p15, %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 CP15 access %#x\n",
> -                     hsr.bits & HSR_CP64_REGS_MASK);
> -            inject_undef_exception(regs, hsr);
> -            return;
> -        }
> -    }
> -    advance_pc(regs, hsr);
> -}
> -
> -static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
> -{
> -    const struct hsr_cp32 cp32 = hsr.cp32;
> -    int regidx = cp32.reg;
> -    struct domain *d = current->domain;
> -
> -    if ( !check_conditional_instr(regs, hsr) )
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> -    }
> -
> -    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, regidx, cp32.read, hsr, 1);
> -    case HSR_CPREG32(DBGOSDLR):
> -        return handle_raz_wi(regs, regidx, 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):
> -    {
> -        uint32_t val;
> -
> -        /*
> -         * 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);
> -
> -        /* Implement the minimum requirements:
> -         *  - Number of watchpoints: 1
> -         *  - Number of breakpoints: 2
> -         *  - Version: ARMv7 v7.1
> -         *  - Variant and Revision bits match MDIR
> -         */
> -        val = (1 << 24) | (5 << 16);
> -        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
> -        set_user_reg(regs, regidx, val);
> -
> -        break;
> -    }
> -
> -    case HSR_CPREG32(DBGDSCRINT):
> -        /*
> -         * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
> -         * is set to 0, which we emulated below.
> -         */
> -        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
> -
> -    case HSR_CPREG32(DBGDSCREXT):
> -        /*
> -         * 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, regidx, cp32.read, hsr, 1);
> -
> -    case HSR_CPREG32(DBGVCR):
> -    case HSR_CPREG32(DBGBVR0):
> -    case HSR_CPREG32(DBGBCR0):
> -    case HSR_CPREG32(DBGWVR0):
> -    case HSR_CPREG32(DBGWCR0):
> -    case HSR_CPREG32(DBGBVR1):
> -    case HSR_CPREG32(DBGBCR1):
> -        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> -
> -    /*
> -     * CPTR_EL2.TTA
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.16
> -     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> -     *
> -     *  - All implemented trace registers.
> -     *
> -     * 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:
> -        gdprintk(XENLOG_ERR,
> -                 "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> -                  cp32.read ? "mrc" : "mcr",
> -                  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);
> -        inject_undef_exception(regs, hsr);
> -        return;
> -    }
> -
> -    advance_pc(regs, hsr);
> -}
> -
> -static void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
> -{
> -    const struct hsr_cp64 cp64 = hsr.cp64;
> -
> -    if ( !check_conditional_instr(regs, hsr) )
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> -    }
> -
> -    /*
> -     * CPTR_EL2.TTA
> -     *
> -     * ARMv7 (DDI 0406C.b): B1.14.16
> -     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> -     *
> -     *  - All implemented trace registers.
> -     *
> -     * 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",
> -             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;
> -    }
> -
> -    /*
> -     * 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",
> -             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);
> -}
> -
> -static void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
> -{
> -    const struct hsr_cp cp = hsr.cp;
> -
> -    if ( !check_conditional_instr(regs, hsr) )
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> -    }
> -
> -    ASSERT(!cp.tas); /* We don't trap SIMD instruction */
> -    gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc);
> -    inject_undef_exception(regs, hsr);
> -}
> -
>  void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
>  {
>      register_t ttbcr = READ_SYSREG(TCR_EL1);
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> new file mode 100644
> index 0000000000..f3b08403fb
> --- /dev/null
> +++ b/xen/arch/arm/vcpreg.c
> @@ -0,0 +1,451 @@
> +/*
> + * xen/arch/arm/arm64/vcpreg.c
> + *
> + * Emulate co-processor registers trapped.
> + *
> + * Copyright (c) 2011 Citrix Systems.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/sched.h>
> +
> +#include <asm/current.h>
> +#include <asm/regs.h>
> +#include <asm/traps.h>
> +#include <asm/vtimer.h>
> +
> +void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    const struct hsr_cp32 cp32 = hsr.cp32;
> +    int regidx = cp32.reg;
> +    struct vcpu *v = current;
> +
> +    if ( !check_conditional_instr(regs, hsr) )
> +    {
> +        advance_pc(regs, hsr);
> +        return;
> +    }
> +
> +    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) )
> +            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);
> +        if ( cp32.read )
> +            set_user_reg(regs, regidx, 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.
> +     *
> +     * 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(PMUSERENR):
> +        /* RO at EL0. RAZ/WI at EL1 */
> +        if ( psr_mode_is_user(regs) )
> +            return handle_ro_raz(regs, regidx, cp32.read, hsr, 0);
> +        else
> +            return handle_raz_wi(regs, regidx, 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. */
> +        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> +    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(PMXEVTYPER):
> +    case HSR_CPREG32(PMXEVCNTR):
> +    case HSR_CPREG32(PMOVSSET):
> +        /*
> +         * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We
> +         * emulate that register as 0 above.
> +         */
> +        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> +
> +    /*
> +     * HCR_EL2.TIDCP
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.3
> +     * ARMv8 (DDI 0487A.d): D1-1501 Table D1-43
> +     *
> +     *  - CRn==c9, opc1=={0-7}, CRm=={c0-c2, c5-c8}, opc2=={0-7}
> +     *    (Cache and TCM lockdown registers)
> +     *  - CRn==c10, opc1=={0-7}, CRm=={c0, c1, c4, c8}, opc2=={0-7}
> +     *    (VMSA CP15 c10 registers)
> +     *  - CRn==c11, opc1=={0-7}, CRm=={c0-c8, c15}, opc2=={0-7}
> +     *    (VMSA CP15 c11 registers)
> +     *
> +     * CPTR_EL2.T{0..9,12..13}
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.12
> +     * ARMv8 (DDI 0487A.d): N/A
> +     *
> +     *  - All accesses to coprocessors 0..9 and 12..13
> +     *
> +     * HSTR_EL2.T15
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.14
> +     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
> +     *
> +     *  - All accesses to cp15, c15 registers.
> +     *
> +     * And all other unknown registers.
> +     */
> +    default:
> +        gdprintk(XENLOG_ERR,
> +                 "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> +                 cp32.read ? "mrc" : "mcr",
> +                 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);
> +        inject_undef_exception(regs, hsr);
> +        return;
> +    }
> +    advance_pc(regs, hsr);
> +}
> +
> +void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    if ( !check_conditional_instr(regs, hsr) )
> +    {
> +        advance_pc(regs, hsr);
> +        return;
> +    }
> +
> +    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);
> +        break;
> +
> +    /*
> +     * HCR_EL2.FMO or HCR_EL2.IMO
> +     *
> +     * GIC Architecture Specification (IHI 0069C): Section 4.6.3
> +     */
> +    case HSR_CPREG64(ICC_SGI1R):
> +    case HSR_CPREG64(ICC_ASGI1R):
> +    case HSR_CPREG64(ICC_SGI0R):
> +        if ( !vgic_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
> +     *
> +     *  - All accesses to coprocessors 0..9 and 12..13
> +     *
> +     * HSTR_EL2.T15
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.14
> +     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-55
> +     *
> +     *  - All accesses to cp15, c15 registers.
> +     *
> +     * And all other unknown registers.
> +     */
> +    default:
> +        {
> +            const struct hsr_cp64 cp64 = hsr.cp64;
> +
> +            gdprintk(XENLOG_ERR,
> +                     "%s p15, %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 CP15 access %#x\n",
> +                     hsr.bits & HSR_CP64_REGS_MASK);
> +            inject_undef_exception(regs, hsr);
> +            return;
> +        }
> +    }
> +    advance_pc(regs, hsr);
> +}
> +
> +void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    const struct hsr_cp32 cp32 = hsr.cp32;
> +    int regidx = cp32.reg;
> +    struct domain *d = current->domain;
> +
> +    if ( !check_conditional_instr(regs, hsr) )
> +    {
> +        advance_pc(regs, hsr);
> +        return;
> +    }
> +
> +    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, regidx, cp32.read, hsr, 1);
> +    case HSR_CPREG32(DBGOSDLR):
> +        return handle_raz_wi(regs, regidx, 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):
> +    {
> +        uint32_t val;
> +
> +        /*
> +         * 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);
> +
> +        /* Implement the minimum requirements:
> +         *  - Number of watchpoints: 1
> +         *  - Number of breakpoints: 2
> +         *  - Version: ARMv7 v7.1
> +         *  - Variant and Revision bits match MDIR
> +         */
> +        val = (1 << 24) | (5 << 16);
> +        val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
> +        set_user_reg(regs, regidx, val);
> +
> +        break;
> +    }
> +
> +    case HSR_CPREG32(DBGDSCRINT):
> +        /*
> +         * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
> +         * is set to 0, which we emulated below.
> +         */
> +        return handle_ro_raz(regs, regidx, cp32.read, hsr, 1);
> +
> +    case HSR_CPREG32(DBGDSCREXT):
> +        /*
> +         * 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, regidx, cp32.read, hsr, 1);
> +
> +    case HSR_CPREG32(DBGVCR):
> +    case HSR_CPREG32(DBGBVR0):
> +    case HSR_CPREG32(DBGBCR0):
> +    case HSR_CPREG32(DBGWVR0):
> +    case HSR_CPREG32(DBGWCR0):
> +    case HSR_CPREG32(DBGBVR1):
> +    case HSR_CPREG32(DBGBCR1):
> +        return handle_raz_wi(regs, regidx, cp32.read, hsr, 1);
> +
> +    /*
> +     * CPTR_EL2.TTA
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.16
> +     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> +     *
> +     *  - All implemented trace registers.
> +     *
> +     * 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:
> +        gdprintk(XENLOG_ERR,
> +                 "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
> +                  cp32.read ? "mrc" : "mcr",
> +                  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);
> +        inject_undef_exception(regs, hsr);
> +        return;
> +    }
> +
> +    advance_pc(regs, hsr);
> +}
> +
> +void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    const struct hsr_cp64 cp64 = hsr.cp64;
> +
> +    if ( !check_conditional_instr(regs, hsr) )
> +    {
> +        advance_pc(regs, hsr);
> +        return;
> +    }
> +
> +    /*
> +     * CPTR_EL2.TTA
> +     *
> +     * ARMv7 (DDI 0406C.b): B1.14.16
> +     * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
> +     *
> +     *  - All implemented trace registers.
> +     *
> +     * 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",
> +             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);
> +}
> +
> +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;
> +    }
> +
> +    /*
> +     * 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",
> +             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);
> +}
> +
> +void do_cp(struct cpu_user_regs *regs, const union hsr hsr)
> +{
> +    const struct hsr_cp cp = hsr.cp;
> +
> +    if ( !check_conditional_instr(regs, hsr) )
> +    {
> +        advance_pc(regs, hsr);
> +        return;
> +    }
> +
> +    ASSERT(!cp.tas); /* We don't trap SIMD instruction */
> +    gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc);
> +    inject_undef_exception(regs, hsr);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/traps.h b/xen/include/asm-arm/traps.h
> index 6d99d228e8..53d386d8e5 100644
> --- a/xen/include/asm-arm/traps.h
> +++ b/xen/include/asm-arm/traps.h
> @@ -24,6 +24,14 @@ void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
>  void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
>                     const union hsr hsr, int min_el);
>  
> +/* Co-processor registers emulation (see arch/arm/vcpreg.c). */
> +void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp14_64(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr);
> +void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
> +
>  #endif /* __ASM_ARM_TRAPS__ */
>  /*
>   * Local variables:
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory
  2017-09-12 10:36 ` [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory Julien Grall
@ 2017-09-12 21:49   ` Stefano Stabellini
  0 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:49 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> sysregs.h contains only code protected by #ifdef CONFIG_ARM_64. Move it
> in arm64 sub-directory to reflect that and remove the #ifdef.
> 
> At the same time, fixup the guards.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/include/asm-arm/arm64/processor.h     |  2 ++
>  xen/include/asm-arm/{ => arm64}/sysregs.h | 10 +++-------
>  xen/include/asm-arm/processor.h           |  1 -
>  3 files changed, 5 insertions(+), 8 deletions(-)
>  rename xen/include/asm-arm/{ => arm64}/sysregs.h (98%)
> 
> diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
> index 24f836b023..c18ab7203d 100644
> --- a/xen/include/asm-arm/arm64/processor.h
> +++ b/xen/include/asm-arm/arm64/processor.h
> @@ -3,6 +3,8 @@
>  
>  #include <xen/stringify.h>
>  
> +#include <asm/arm64/sysregs.h>
> +
>  #ifndef __ASSEMBLY__
>  
>  /* Anonymous union includes both 32- and 64-bit names (e.g., r0/x0). */
> diff --git a/xen/include/asm-arm/sysregs.h b/xen/include/asm-arm/arm64/sysregs.h
> similarity index 98%
> rename from xen/include/asm-arm/sysregs.h
> rename to xen/include/asm-arm/arm64/sysregs.h
> index 887368e248..084d2a1e5d 100644
> --- a/xen/include/asm-arm/sysregs.h
> +++ b/xen/include/asm-arm/arm64/sysregs.h
> @@ -1,7 +1,5 @@
> -#ifndef __ASM_ARM_SYSREGS_H
> -#define __ASM_ARM_SYSREGS_H
> -
> -#ifdef CONFIG_ARM_64
> +#ifndef __ASM_ARM_ARM64_SYSREGS_H
> +#define __ASM_ARM_ARM64_SYSREGS_H
>  
>  #include <xen/stringify.h>
>  
> @@ -168,9 +166,7 @@
>  #define ICH_AP1R2_EL2             __AP1Rx_EL2(2)
>  #define ICH_AP1R3_EL2             __AP1Rx_EL2(3)
>  
> -#endif
> -
> -#endif
> +#endif /* _ASM_ARM_ARM64_SYSREGS_H */
>  
>  /*
>   * Local variables:
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 9f7a42f86b..d791c12c9c 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -2,7 +2,6 @@
>  #define __ASM_ARM_PROCESSOR_H
>  
>  #include <asm/cpregs.h>
> -#include <asm/sysregs.h>
>  #ifndef __ASSEMBLY__
>  #include <xen/types.h>
>  #endif
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h
  2017-09-12 10:36 ` [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h Julien Grall
@ 2017-09-12 21:53   ` Stefano Stabellini
  2017-09-13  9:11     ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:53 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> Currently, cpregs.h is included in pretty much every files even for
> arm64. However, the only use for arm64 is when emulating co-processors.
> 
> For arm32, all users of processor.h expect cpregs.h to be included in
> order to access co-processors. So move the inclusion in
> asm-arm/arm32/processor.h.
> 
> cpregs.h will also be directly included in the co-processors emulation
> to accommodate arm64.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

I can see that the patch works and does what you describe, but what is
the benefit? OK, we remove #include <asm/cpregs.h> from
asm-arm/processor.h, but then we have to add it to vcpreg.c, vgic-v3.c,
vtimer.c, and arm32/processor.h. Is there a long term benefit? What
prompted you into writing this patch?


> ---
>     Changes in v2:
>         - Update commit message
> ---
>  xen/arch/arm/smp.c                    | 1 -
>  xen/arch/arm/vcpreg.c                 | 1 +
>  xen/arch/arm/vgic-v3.c                | 1 +
>  xen/arch/arm/vtimer.c                 | 2 ++
>  xen/include/asm-arm/arm32/processor.h | 2 ++
>  xen/include/asm-arm/percpu.h          | 1 -
>  xen/include/asm-arm/processor.h       | 1 -
>  7 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
> index e7df0874d6..554f4992e6 100644
> --- a/xen/arch/arm/smp.c
> +++ b/xen/arch/arm/smp.c
> @@ -1,6 +1,5 @@
>  #include <asm/system.h>
>  #include <asm/smp.h>
> -#include <asm/cpregs.h>
>  #include <asm/page.h>
>  #include <asm/gic.h>
>  #include <asm/flushtlb.h>
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index f3b08403fb..e363183ba8 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -18,6 +18,7 @@
>  
>  #include <xen/sched.h>
>  
> +#include <asm/cpregs.h>
>  #include <asm/current.h>
>  #include <asm/regs.h>
>  #include <asm/traps.h>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index cbeac28b28..a0cf993d13 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -26,6 +26,7 @@
>  #include <xen/softirq.h>
>  #include <xen/sizes.h>
>  
> +#include <asm/cpregs.h>
>  #include <asm/current.h>
>  #include <asm/gic_v3_defs.h>
>  #include <asm/gic_v3_its.h>
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 9c7e8f441c..0460962f08 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -22,6 +22,7 @@
>  #include <xen/sched.h>
>  #include <xen/timer.h>
>  
> +#include <asm/cpregs.h>
>  #include <asm/div64.h>
>  #include <asm/gic.h>
>  #include <asm/irq.h>
> @@ -29,6 +30,7 @@
>  #include <asm/time.h>
>  #include <asm/vgic.h>
>  #include <asm/vreg.h>
> +#include <asm/regs.h>
>  
>  /*
>   * Check if regs is allowed access, user_gate is tail end of a
> diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
> index 68cc82147e..fb330812af 100644
> --- a/xen/include/asm-arm/arm32/processor.h
> +++ b/xen/include/asm-arm/arm32/processor.h
> @@ -1,6 +1,8 @@
>  #ifndef __ASM_ARM_ARM32_PROCESSOR_H
>  #define __ASM_ARM_ARM32_PROCESSOR_H
>  
> +#include <asm/cpregs.h>
> +
>  #define ACTLR_CAXX_SMP      (1<<6)
>  
>  #ifndef __ASSEMBLY__
> diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
> index 7968532462..cdf64e0f77 100644
> --- a/xen/include/asm-arm/percpu.h
> +++ b/xen/include/asm-arm/percpu.h
> @@ -4,7 +4,6 @@
>  #ifndef __ASSEMBLY__
>  
>  #include <xen/types.h>
> -#include <asm/cpregs.h>
>  #if defined(CONFIG_ARM_32)
>  # include <asm/arm32/processor.h>
>  #elif defined(CONFIG_ARM_64)
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index d791c12c9c..cd45e5f48f 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -1,7 +1,6 @@
>  #ifndef __ASM_ARM_PROCESSOR_H
>  #define __ASM_ARM_PROCESSOR_H
>  
> -#include <asm/cpregs.h>
>  #ifndef __ASSEMBLY__
>  #include <xen/types.h>
>  #endif
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 0/7] xen/arm: Clean-up traps.c
  2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
                   ` (6 preceding siblings ...)
  2017-09-12 10:36 ` [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h Julien Grall
@ 2017-09-12 21:57 ` Stefano Stabellini
  7 siblings, 0 replies; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-12 21:57 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, sstabellini, volodymyr_babchuk, xen-devel

On Tue, 12 Sep 2017, Julien Grall wrote:
> Hi all,
> 
> xen/arch/arm/traps.c is beginning to get very big. This series is moving out
> the co-processor and sysreg emulation in separate files. This will avoid to
> grow traps.c when adding more registers emulation (coming soon).
> 
> A branch with this series has been pushed:
> 
> https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git
> branch cleanup-traps-v2

I committed the first two patches. Only a couple of very minor comments
on the rest.


> Cheers,
> 
> Cc: volodymyr_babchuk@epam.com
> 
> Julien Grall (7):
>   xen/arm: traps: Re-order the includes alphabetically
>   xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h
>   xen/arm: traps: Export a bunch of helpers to handle emulation
>   xen/arm: Move sysreg emulation outside of traps.c
>   xen/arm: Move co-processor emulation outside of traps.c
>   xen/arm: Move sysregs.h in arm64 sub-directory
>   xen/arm: Limit the scope of cpregs.h
> 
>  xen/arch/arm/Makefile                      |   1 +
>  xen/arch/arm/arm64/Makefile                |   1 +
>  xen/arch/arm/arm64/vsysreg.c               | 229 ++++++++++
>  xen/arch/arm/domain.c                      |   2 +-
>  xen/arch/arm/smp.c                         |   1 -
>  xen/arch/arm/traps.c                       | 702 ++---------------------------
>  xen/arch/arm/vcpreg.c                      | 452 +++++++++++++++++++
>  xen/arch/arm/vgic-v3.c                     |   1 +
>  xen/arch/arm/vtimer.c                      |   2 +
>  xen/include/asm-arm/arm32/processor.h      |   2 +
>  xen/include/asm-arm/arm32/traps.h          |  13 +
>  xen/include/asm-arm/arm64/processor.h      |   2 +
>  xen/include/asm-arm/{ => arm64}/sysregs.h  |  10 +-
>  xen/include/asm-arm/arm64/traps.h          |  18 +
>  xen/include/asm-arm/percpu.h               |   1 -
>  xen/include/asm-arm/processor.h            |   2 -
>  xen/include/asm-arm/traps.h                |  44 ++
>  xen/{arch/arm => include/asm-arm}/vtimer.h |   0
>  18 files changed, 811 insertions(+), 672 deletions(-)
>  create mode 100644 xen/arch/arm/arm64/vsysreg.c
>  create mode 100644 xen/arch/arm/vcpreg.c
>  create mode 100644 xen/include/asm-arm/arm32/traps.h
>  rename xen/include/asm-arm/{ => arm64}/sysregs.h (98%)
>  create mode 100644 xen/include/asm-arm/arm64/traps.h
>  create mode 100644 xen/include/asm-arm/traps.h
>  rename xen/{arch/arm => include/asm-arm}/vtimer.h (100%)
> 
> -- 
> 2.11.0
> 

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

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

* Re: [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation
  2017-09-12 21:26   ` Stefano Stabellini
@ 2017-09-13  8:52     ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2017-09-13  8:52 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: bhupinder.thakur, volodymyr_babchuk, xen-devel

Hi,

On 09/12/2017 10:26 PM, Stefano Stabellini wrote:
>> +int check_conditional_instr(struct cpu_user_regs *regs, const union hsr hsr);
>> +
>> +void advance_pc(struct cpu_user_regs *regs, const union hsr hsr);
>> +
>> +void inject_undef_exception(struct cpu_user_regs *regs, const union hsr hsr);
>> +
>> +void handle_raz_wi(struct cpu_user_regs *regs, int regidx, bool read,
>> +                   const union hsr hsr, int min_el);
>> +
>> +void handle_wo_wi(struct cpu_user_regs *regs, int regidx, bool read,
>> +                  const union hsr hsr, int min_el);
>> +
>> +void handle_ro_raz(struct cpu_user_regs *regs, int regidx, bool read,
>> +                   const union hsr hsr, int min_el);
>>
> 
> Could you please copy or move the simple one-line comments we have in
> traps.c here as well to describe these functions? Such as "Read only as
> read as zero" and "Read as zero and write ignore".

I will do that.

Cheers,

-- 
Julien Grall

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

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

* Re: [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h
  2017-09-12 21:53   ` Stefano Stabellini
@ 2017-09-13  9:11     ` Julien Grall
  2017-09-13 21:13       ` Stefano Stabellini
  0 siblings, 1 reply; 20+ messages in thread
From: Julien Grall @ 2017-09-13  9:11 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: bhupinder.thakur, xen-devel

Hi Stefano,

On 09/12/2017 10:53 PM, Stefano Stabellini wrote:
> On Tue, 12 Sep 2017, Julien Grall wrote:
>> Currently, cpregs.h is included in pretty much every files even for
>> arm64. However, the only use for arm64 is when emulating co-processors.
>>
>> For arm32, all users of processor.h expect cpregs.h to be included in
>> order to access co-processors. So move the inclusion in
>> asm-arm/arm32/processor.h.
>>
>> cpregs.h will also be directly included in the co-processors emulation
>> to accommodate arm64.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> I can see that the patch works and does what you describe, but what is
> the benefit? OK, we remove #include <asm/cpregs.h> from
> asm-arm/processor.h, but then we have to add it to vcpreg.c, vgic-v3.c,
> vtimer.c, and arm32/processor.h. Is there a long term benefit? What
> prompted you into writing this patch?

asm/processor.h is included indirectly by every single file of the 
source code. It seems that we use it as a placeholder for anything 
rather than properly splitting in different headers. For instance it 
contains all the definitions of the registers, the traps vector, SMC 
call, traps prototype... For most of those stuff only a limited of files 
really care about it. So we are polluting the name space of each file 
for no real benefits.

On AArch64, cpregs.h is only useful when emulating co-processor 
gregisters. So there are no point to include it in a main header that 
will be pulled by 100s files just for the benetifs of 4 files.

This patch is only a first attempt of clean-up processor.h. Ideally we 
should have more patch to split it.

Cheers,

> 
> 
>> ---
>>      Changes in v2:
>>          - Update commit message
>> ---
>>   xen/arch/arm/smp.c                    | 1 -
>>   xen/arch/arm/vcpreg.c                 | 1 +
>>   xen/arch/arm/vgic-v3.c                | 1 +
>>   xen/arch/arm/vtimer.c                 | 2 ++
>>   xen/include/asm-arm/arm32/processor.h | 2 ++
>>   xen/include/asm-arm/percpu.h          | 1 -
>>   xen/include/asm-arm/processor.h       | 1 -
>>   7 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
>> index e7df0874d6..554f4992e6 100644
>> --- a/xen/arch/arm/smp.c
>> +++ b/xen/arch/arm/smp.c
>> @@ -1,6 +1,5 @@
>>   #include <asm/system.h>
>>   #include <asm/smp.h>
>> -#include <asm/cpregs.h>
>>   #include <asm/page.h>
>>   #include <asm/gic.h>
>>   #include <asm/flushtlb.h>
>> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
>> index f3b08403fb..e363183ba8 100644
>> --- a/xen/arch/arm/vcpreg.c
>> +++ b/xen/arch/arm/vcpreg.c
>> @@ -18,6 +18,7 @@
>>   
>>   #include <xen/sched.h>
>>   
>> +#include <asm/cpregs.h>
>>   #include <asm/current.h>
>>   #include <asm/regs.h>
>>   #include <asm/traps.h>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index cbeac28b28..a0cf993d13 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -26,6 +26,7 @@
>>   #include <xen/softirq.h>
>>   #include <xen/sizes.h>
>>   
>> +#include <asm/cpregs.h>
>>   #include <asm/current.h>
>>   #include <asm/gic_v3_defs.h>
>>   #include <asm/gic_v3_its.h>
>> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
>> index 9c7e8f441c..0460962f08 100644
>> --- a/xen/arch/arm/vtimer.c
>> +++ b/xen/arch/arm/vtimer.c
>> @@ -22,6 +22,7 @@
>>   #include <xen/sched.h>
>>   #include <xen/timer.h>
>>   
>> +#include <asm/cpregs.h>
>>   #include <asm/div64.h>
>>   #include <asm/gic.h>
>>   #include <asm/irq.h>
>> @@ -29,6 +30,7 @@
>>   #include <asm/time.h>
>>   #include <asm/vgic.h>
>>   #include <asm/vreg.h>
>> +#include <asm/regs.h>
>>   
>>   /*
>>    * Check if regs is allowed access, user_gate is tail end of a
>> diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
>> index 68cc82147e..fb330812af 100644
>> --- a/xen/include/asm-arm/arm32/processor.h
>> +++ b/xen/include/asm-arm/arm32/processor.h
>> @@ -1,6 +1,8 @@
>>   #ifndef __ASM_ARM_ARM32_PROCESSOR_H
>>   #define __ASM_ARM_ARM32_PROCESSOR_H
>>   
>> +#include <asm/cpregs.h>
>> +
>>   #define ACTLR_CAXX_SMP      (1<<6)
>>   
>>   #ifndef __ASSEMBLY__
>> diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
>> index 7968532462..cdf64e0f77 100644
>> --- a/xen/include/asm-arm/percpu.h
>> +++ b/xen/include/asm-arm/percpu.h
>> @@ -4,7 +4,6 @@
>>   #ifndef __ASSEMBLY__
>>   
>>   #include <xen/types.h>
>> -#include <asm/cpregs.h>
>>   #if defined(CONFIG_ARM_32)
>>   # include <asm/arm32/processor.h>
>>   #elif defined(CONFIG_ARM_64)
>> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
>> index d791c12c9c..cd45e5f48f 100644
>> --- a/xen/include/asm-arm/processor.h
>> +++ b/xen/include/asm-arm/processor.h
>> @@ -1,7 +1,6 @@
>>   #ifndef __ASM_ARM_PROCESSOR_H
>>   #define __ASM_ARM_PROCESSOR_H
>>   
>> -#include <asm/cpregs.h>
>>   #ifndef __ASSEMBLY__
>>   #include <xen/types.h>
>>   #endif
>> -- 
>> 2.11.0
>>

-- 
Julien Grall

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

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

* Re: [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h
  2017-09-13  9:11     ` Julien Grall
@ 2017-09-13 21:13       ` Stefano Stabellini
  2017-09-14 16:58         ` Julien Grall
  0 siblings, 1 reply; 20+ messages in thread
From: Stefano Stabellini @ 2017-09-13 21:13 UTC (permalink / raw)
  To: Julien Grall; +Cc: bhupinder.thakur, Stefano Stabellini, xen-devel

On Wed, 13 Sep 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 09/12/2017 10:53 PM, Stefano Stabellini wrote:
> > On Tue, 12 Sep 2017, Julien Grall wrote:
> > > Currently, cpregs.h is included in pretty much every files even for
> > > arm64. However, the only use for arm64 is when emulating co-processors.
> > > 
> > > For arm32, all users of processor.h expect cpregs.h to be included in
> > > order to access co-processors. So move the inclusion in
> > > asm-arm/arm32/processor.h.
> > > 
> > > cpregs.h will also be directly included in the co-processors emulation
> > > to accommodate arm64.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > 
> > I can see that the patch works and does what you describe, but what is
> > the benefit? OK, we remove #include <asm/cpregs.h> from
> > asm-arm/processor.h, but then we have to add it to vcpreg.c, vgic-v3.c,
> > vtimer.c, and arm32/processor.h. Is there a long term benefit? What
> > prompted you into writing this patch?
> 
> asm/processor.h is included indirectly by every single file of the source
> code. It seems that we use it as a placeholder for anything rather than
> properly splitting in different headers. For instance it contains all the
> definitions of the registers, the traps vector, SMC call, traps prototype...
> For most of those stuff only a limited of files really care about it. So we
> are polluting the name space of each file for no real benefits.
> 
> On AArch64, cpregs.h is only useful when emulating co-processor gregisters. So
> there are no point to include it in a main header that will be pulled by 100s
> files just for the benetifs of 4 files.
> 
> This patch is only a first attempt of clean-up processor.h. Ideally we should
> have more patch to split it.

All right.

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Maybe add a couple of lines to the commit description saying that by
removing the inclusion of cpregs.h from asm/processor.h we drastically
reduce the exposure of cpregs.h to any source files on ARM64.


> > > ---
> > >      Changes in v2:
> > >          - Update commit message
> > > ---
> > >   xen/arch/arm/smp.c                    | 1 -
> > >   xen/arch/arm/vcpreg.c                 | 1 +
> > >   xen/arch/arm/vgic-v3.c                | 1 +
> > >   xen/arch/arm/vtimer.c                 | 2 ++
> > >   xen/include/asm-arm/arm32/processor.h | 2 ++
> > >   xen/include/asm-arm/percpu.h          | 1 -
> > >   xen/include/asm-arm/processor.h       | 1 -
> > >   7 files changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
> > > index e7df0874d6..554f4992e6 100644
> > > --- a/xen/arch/arm/smp.c
> > > +++ b/xen/arch/arm/smp.c
> > > @@ -1,6 +1,5 @@
> > >   #include <asm/system.h>
> > >   #include <asm/smp.h>
> > > -#include <asm/cpregs.h>
> > >   #include <asm/page.h>
> > >   #include <asm/gic.h>
> > >   #include <asm/flushtlb.h>
> > > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> > > index f3b08403fb..e363183ba8 100644
> > > --- a/xen/arch/arm/vcpreg.c
> > > +++ b/xen/arch/arm/vcpreg.c
> > > @@ -18,6 +18,7 @@
> > >     #include <xen/sched.h>
> > >   +#include <asm/cpregs.h>
> > >   #include <asm/current.h>
> > >   #include <asm/regs.h>
> > >   #include <asm/traps.h>
> > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> > > index cbeac28b28..a0cf993d13 100644
> > > --- a/xen/arch/arm/vgic-v3.c
> > > +++ b/xen/arch/arm/vgic-v3.c
> > > @@ -26,6 +26,7 @@
> > >   #include <xen/softirq.h>
> > >   #include <xen/sizes.h>
> > >   +#include <asm/cpregs.h>
> > >   #include <asm/current.h>
> > >   #include <asm/gic_v3_defs.h>
> > >   #include <asm/gic_v3_its.h>
> > > diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> > > index 9c7e8f441c..0460962f08 100644
> > > --- a/xen/arch/arm/vtimer.c
> > > +++ b/xen/arch/arm/vtimer.c
> > > @@ -22,6 +22,7 @@
> > >   #include <xen/sched.h>
> > >   #include <xen/timer.h>
> > >   +#include <asm/cpregs.h>
> > >   #include <asm/div64.h>
> > >   #include <asm/gic.h>
> > >   #include <asm/irq.h>
> > > @@ -29,6 +30,7 @@
> > >   #include <asm/time.h>
> > >   #include <asm/vgic.h>
> > >   #include <asm/vreg.h>
> > > +#include <asm/regs.h>
> > >     /*
> > >    * Check if regs is allowed access, user_gate is tail end of a
> > > diff --git a/xen/include/asm-arm/arm32/processor.h
> > > b/xen/include/asm-arm/arm32/processor.h
> > > index 68cc82147e..fb330812af 100644
> > > --- a/xen/include/asm-arm/arm32/processor.h
> > > +++ b/xen/include/asm-arm/arm32/processor.h
> > > @@ -1,6 +1,8 @@
> > >   #ifndef __ASM_ARM_ARM32_PROCESSOR_H
> > >   #define __ASM_ARM_ARM32_PROCESSOR_H
> > >   +#include <asm/cpregs.h>
> > > +
> > >   #define ACTLR_CAXX_SMP      (1<<6)
> > >     #ifndef __ASSEMBLY__
> > > diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h
> > > index 7968532462..cdf64e0f77 100644
> > > --- a/xen/include/asm-arm/percpu.h
> > > +++ b/xen/include/asm-arm/percpu.h
> > > @@ -4,7 +4,6 @@
> > >   #ifndef __ASSEMBLY__
> > >     #include <xen/types.h>
> > > -#include <asm/cpregs.h>
> > >   #if defined(CONFIG_ARM_32)
> > >   # include <asm/arm32/processor.h>
> > >   #elif defined(CONFIG_ARM_64)
> > > diff --git a/xen/include/asm-arm/processor.h
> > > b/xen/include/asm-arm/processor.h
> > > index d791c12c9c..cd45e5f48f 100644
> > > --- a/xen/include/asm-arm/processor.h
> > > +++ b/xen/include/asm-arm/processor.h
> > > @@ -1,7 +1,6 @@
> > >   #ifndef __ASM_ARM_PROCESSOR_H
> > >   #define __ASM_ARM_PROCESSOR_H
> > >   -#include <asm/cpregs.h>
> > >   #ifndef __ASSEMBLY__
> > >   #include <xen/types.h>
> > >   #endif
> > > -- 
> > > 2.11.0
> > > 
> 
> -- 
> Julien Grall
> 

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

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

* Re: [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h
  2017-09-13 21:13       ` Stefano Stabellini
@ 2017-09-14 16:58         ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2017-09-14 16:58 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: bhupinder.thakur, xen-devel

Hi Stefano,

On 13/09/17 22:13, Stefano Stabellini wrote:
> On Wed, 13 Sep 2017, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 09/12/2017 10:53 PM, Stefano Stabellini wrote:
>>> On Tue, 12 Sep 2017, Julien Grall wrote:
>>>> Currently, cpregs.h is included in pretty much every files even for
>>>> arm64. However, the only use for arm64 is when emulating co-processors.
>>>>
>>>> For arm32, all users of processor.h expect cpregs.h to be included in
>>>> order to access co-processors. So move the inclusion in
>>>> asm-arm/arm32/processor.h.
>>>>
>>>> cpregs.h will also be directly included in the co-processors emulation
>>>> to accommodate arm64.
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>>
>>> I can see that the patch works and does what you describe, but what is
>>> the benefit? OK, we remove #include <asm/cpregs.h> from
>>> asm-arm/processor.h, but then we have to add it to vcpreg.c, vgic-v3.c,
>>> vtimer.c, and arm32/processor.h. Is there a long term benefit? What
>>> prompted you into writing this patch?
>>
>> asm/processor.h is included indirectly by every single file of the source
>> code. It seems that we use it as a placeholder for anything rather than
>> properly splitting in different headers. For instance it contains all the
>> definitions of the registers, the traps vector, SMC call, traps prototype...
>> For most of those stuff only a limited of files really care about it. So we
>> are polluting the name space of each file for no real benefits.
>>
>> On AArch64, cpregs.h is only useful when emulating co-processor gregisters. So
>> there are no point to include it in a main header that will be pulled by 100s
>> files just for the benetifs of 4 files.
>>
>> This patch is only a first attempt of clean-up processor.h. Ideally we should
>> have more patch to split it.
> 
> All right.
> 
> Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Maybe add a couple of lines to the commit description saying that by
> removing the inclusion of cpregs.h from asm/processor.h we drastically
> reduce the exposure of cpregs.h to any source files on ARM64.

I will do that.

Cheers,

-- 
Julien Grall

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

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

end of thread, other threads:[~2017-09-14 16:58 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-12 10:36 [PATCH v2 0/7] xen/arm: Clean-up traps.c Julien Grall
2017-09-12 10:36 ` [PATCH v2 1/7] xen/arm: traps: Re-order the includes alphabetically Julien Grall
2017-09-12 20:59   ` Stefano Stabellini
2017-09-12 10:36 ` [PATCH v2 2/7] xen/arm: Move arch/arm/vtimer.h to include/asm-arm/vtimer.h Julien Grall
2017-09-12 21:02   ` Stefano Stabellini
2017-09-12 10:36 ` [PATCH v2 3/7] xen/arm: traps: Export a bunch of helpers to handle emulation Julien Grall
2017-09-12 21:26   ` Stefano Stabellini
2017-09-13  8:52     ` Julien Grall
2017-09-12 10:36 ` [PATCH v2 4/7] xen/arm: Move sysreg emulation outside of traps.c Julien Grall
2017-09-12 21:40   ` Stefano Stabellini
2017-09-12 10:36 ` [PATCH v2 5/7] xen/arm: Move co-processor " Julien Grall
2017-09-12 21:43   ` Stefano Stabellini
2017-09-12 10:36 ` [PATCH v2 6/7] xen/arm: Move sysregs.h in arm64 sub-directory Julien Grall
2017-09-12 21:49   ` Stefano Stabellini
2017-09-12 10:36 ` [PATCH v2 7/7] xen/arm: Limit the scope of cpregs.h Julien Grall
2017-09-12 21:53   ` Stefano Stabellini
2017-09-13  9:11     ` Julien Grall
2017-09-13 21:13       ` Stefano Stabellini
2017-09-14 16:58         ` Julien Grall
2017-09-12 21:57 ` [PATCH v2 0/7] xen/arm: Clean-up traps.c Stefano Stabellini

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.