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

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



Hi Ian,

On 31/03/2015 12:07, Ian Campbell wrote:
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
  xen/arch/arm/traps.c          |   32 ++++++++++++++++++++++++++++++++
  xen/include/asm-arm/cpregs.h  |    4 ++++
  xen/include/asm-arm/sysregs.h |    1 +
  3 files changed, 37 insertions(+)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 21bef01..7c37cec 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1790,6 +1790,17 @@ static void do_cp14_32(struct cpu_user_regs *regs, const 
union hsr hsr)

      switch ( hsr.bits & HSR_CP32_REGS_MASK )
      {
+    /*
+     * MDCR_EL2.TDRA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     *
+     * Unhandled:
+     *    DBGDRAR
+     *    DBGDSAR
+     */
+

Why did you put the comment here? For AArch32, only DBGDRAR and DBGSAR are trapped with this bit.

I think this should be moved above the label default.

      case HSR_CPREG32(DBGDIDR):
          /*
           * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
@@ -1840,6 +1851,8 @@ static void do_cp14_32(struct cpu_user_regs *regs, const 
union hsr hsr)
       *
       * ARMv7 (DDI 0406C.b): B1.14.16
       * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     * And all other unknown registers.
       */
      default:
          gdprintk(XENLOG_ERR,
@@ -1870,6 +1883,17 @@ static void do_cp14_64(struct cpu_user_regs *regs, const 
union hsr hsr)
       *
       * ARMv7 (DDI 0406C.b): B1.14.16
       * ARMv8 (DDI 0487A.d): D1-1507 Table D1-54
+     *
+     * MDCR_EL2.TDRA
+     *
+     * ARMv7 (DDI 0406C.b): B1.14.15
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     *
+     * Unhandled:
+     *    DBGDRAR64
+     *    DBGDSAR64

This is confusing. The real name of the register is DBGDRAR. I would say "DBGDRAR 64-bit".

Furthermore, this is the only registers not handled on AArch32 for this bit. This is rather strange to list them while you didn't do it for the trace registers.

+     *
+     * And all other unknown registers.

For consistency, I would have add this part of the comment in patch #10 (where the comment has been added).

Anyway, the patch is already written so I'm fine with it.
       */
      gdprintk(XENLOG_ERR,
               "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
@@ -1936,6 +1960,14 @@ static void do_sysreg(struct cpu_user_regs *regs,
             *x = v->arch.actlr;
          break;

+    /*
+     * MDCR_EL2.TDRA
+     *
+     * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57
+     */
+    case HSR_SYSREG_MDRAR_EL1:
+        return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 1);

This change should be in a separate patch or mention in the commit message.

Regards,

--
Julien Grall

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


 


Rackspace

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