Re: [PATCH 15/19] xen: arm: Annotate registers trapped by MDCR_EL2.TDA

[Julien Grall <julien.grall@citrix.com>]

Hi Ian,

On 31/03/2015 12:07, Ian Campbell wrote:
> Gather the affected handlers in a single place per trap type.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>   xen/arch/arm/traps.c |   71 ++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 518f047..0ab5e56 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1816,6 +1816,28 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr)
>       case HSR_CPREG32(DBGOSDLR):
>           return handle_raz_wi(regs, r, 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

Listing unhandled registers doesn't bring much information. It's more 
useful to explain that we choose to implement a dummy Debug support.

Also, it's very confusing that you are mixing AArch64 name with AArch32 
name. The former is used for the traps register while the latter is used 
for register name.

> +     */
>       case HSR_CPREG32(DBGDIDR):
>           /*
>            * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis
> @@ -1925,6 +1947,17 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, const union hsr hsr)
>           return;
>       }
>
> +    /*
> +     * MDCR_EL2.TDOSA

Did you mean MDCR_EL2.TDA?

[..]

>       case HSR_SYSREG_MDSCR_EL1:
> -    /*  - Breakpoints */

[..]

> -    /*  - Watchpoints */

I think this 2 comments was useful in order to split the list of registers.

regards,

-- 
Julien Grall