Re: [Xen-devel] [PATCH 3/4] xen/arm: Implement a dummy debug monitor for ARM32

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
> XSA-93 (commit 0b18220 "xen/arm: Don't let guess access to Debug and 
> Performance
> Monitors registers") disable Debug Registers access.
> 
> When CONFIG_PERF_EVENTS is enabled in the Linux Kernel, it will try to
> initialize the debug monitors. If an error occured Linux won't use this
> feature.
> 
> The implementation made Xen expose a minimal set of registers which let think
> the guest (i.e.) thinks HW debug won't work.

Why only for arm32?

I think arm64 makes more use than arm32 (unconditionally touches
MDSCR_EL1 on the ctx switch path).

I think we should be considering allow the guest to access these and
context switching them instead.

> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> ---
>     This commit is candidate for backporting on Xen 4.4. Distribution may
>     enable HW DEBUG (e.g. Linaro Ubuntu image), therefore Linux will try
>     to initialize it.
> ---
>  xen/arch/arm/traps.c         |   77 
> ++++++++++++++++++++++++++++++++++++++++--
>  xen/include/asm-arm/cpregs.h |   14 ++++++++
>  2 files changed, 89 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 319bbe9..1f61e6e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1468,7 +1468,76 @@ static void do_cp15_64(struct cpu_user_regs *regs,
>      advance_pc(regs, hsr);
>  }
>  
> -static void do_cp14(struct cpu_user_regs *regs, union hsr hsr)
> +static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr)
> +{
> +    struct hsr_cp32 cp32 = hsr.cp32;
> +    uint32_t *r = (uint32_t *)select_user_reg(regs, 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 )
> +    {
> +    case HSR_CPREG32(DBGDIDR):
> +
> +        /* Read-only register */
> +        if ( !cp32.read )
> +            goto bad_cp;
> +
> +        /* Implement the minimum requirements:
> +         *  - Number of watchpoints: 1
> +         *  - Number of breakpoints: 2
> +         *  - Version: ARMv7 v7.1
> +         *  - Variant and Revision bits match MDIR
> +         */
> +        *r = (1 << 24) | (5 << 16);
> +        *r |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf);
> +        break;
> +
> +    case HSR_CPREG32(DBGDSCRINT):
> +    case HSR_CPREG32(DBGDSCREXT):
> +        /* Implement debug status and control register as RAZ/WI.
> +         * The OS won't use Hardware debug if MDBGen not set
> +         */
> +        if ( cp32.read )
> +           *r = 0;
> +        break;
> +    case HSR_CPREG32(DBGVCR):
> +    case HSR_CPREG32(DBGOSLAR):
> +    case HSR_CPREG32(DBGBVR0):
> +    case HSR_CPREG32(DBGCR0):
> +    case HSR_CPREG32(DBGWVR0):
> +    case HSR_CPREG32(DBGWCR0):
> +    case HSR_CPREG32(DBGBVR1):
> +    case HSR_CPREG32(DBGCR1):
> +    case HSR_CPREG32(DBGOSDLR):
> +        /* RAZ/WI */
> +        if ( cp32.read )
> +            *r = 0;
> +        break;
> +
> +    default:
> +bad_cp:
> +#ifndef NDEBUG
> +        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);
> +#endif
> +        inject_undef32_exception(regs);
> +        return;
> +    }
> +
> +    advance_pc(regs, hsr);
> +}
> +
> +static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr)
>  {
>      if ( !check_conditional_instr(regs, hsr) )
>      {
> @@ -1720,10 +1789,14 @@ asmlinkage void do_trap_hypervisor(struct 
> cpu_user_regs *regs)
>          do_cp15_64(regs, hsr);
>          break;
>      case HSR_EC_CP14_32:
> +        if ( !is_32bit_domain(current->domain) )
> +            goto bad_trap;
> +        do_cp14_32(regs, hsr);
> +        break;
>      case HSR_EC_CP14_DBG:
>          if ( !is_32bit_domain(current->domain) )
>              goto bad_trap;
> -        do_cp14(regs, hsr);
> +        do_cp14_dbg(regs, hsr);
>          break;
>      case HSR_EC_CP:
>          if ( !is_32bit_domain(current->domain) )
> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
> index f44e3b5..306e506 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -71,6 +71,20 @@
>  
>  /* Coprocessor 14 */
>  
> +/* CP14 0: Debug Register interface */
> +#define DBGDIDR         p14,0,c0,c0,0   /* Debug ID Register */
> +#define DBGDSCRINT      p14,0,c0,c1,0   /* Debug Status and Control Internal 
> */
> +#define DBGDSCREXT      p14,0,c0,c2,2   /* Debug Status and Control External 
> */
> +#define DBGVCR          p14,0,c0,c7,0   /* Vector Catch */
> +#define DBGBVR0         p14,0,c0,c0,4   /* Breakpoint Value 0 */
> +#define DBGCR0          p14,0,c0,c0,5   /* Breakpoint Control 0 */
> +#define DBGWVR0         p14,0,c0,c0,6   /* Watchpoint Value 0 */
> +#define DBGWCR0         p14,0,c0,c0,7   /* Watchpoint Control 0 */
> +#define DBGBVR1         p14,0,c0,c1,4   /* Breakpoint Value 1 */
> +#define DBGCR1          p14,0,c0,c1,5   /* Breakpoint Control 1 */
> +#define DBGOSLAR        p14,0,c1,c0,4   /* OS Lock Access */
> +#define DBGOSDLR        p14,0,c1,c3,4   /* OS Double Lock */
> +
>  /* CP14 CR0: */
>  #define TEECR           p14,6,c0,c0,0   /* ThumbEE Configuration Register */
>  



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