Re: [PATCH 1/3] target/arm: implement DBGCLAIM registers

[Peter Maydell]

On Thu, 5 Jan 2023 at 22:12, Evgeny Iakovlev
<eiakovlev@linux.microsoft.com> wrote:
>
> The architecture does not define any functionality for the CLAIM tag bits.
> So we will just keep the raw bits, as per spec.
>
> Helps Hyper-V boot on aarch64-tcg because it context-switches DBGCLAIM
> on EL2 entry/exit.
>
> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
> ---
>  target/arm/cpu.h          |  1 +
>  target/arm/debug_helper.c | 27 +++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 2b4bd20f9d..eddec155b0 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -494,6 +494,7 @@ typedef struct CPUArchState {
>          uint64_t dbgbcr[16]; /* breakpoint control registers */
>          uint64_t dbgwvr[16]; /* watchpoint value registers */
>          uint64_t dbgwcr[16]; /* watchpoint control registers */
> +        uint64_t dbgclaim;   /* DBGCLAIM bits */
>          uint64_t mdscr_el1;
>          uint64_t oslsr_el1; /* OS Lock Status */
>          uint64_t osdlr_el1; /* OS DoubleLock status */
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index c21739242c..b244e146e2 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -629,6 +629,18 @@ static void osdlr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>      }
>  }
>
> +static void dbgclaimset_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                              uint64_t value)
> +{
> +    env->cp15.dbgclaim |= (value & 0xFF);
> +}
> +
> +static void dbgclaimclr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                              uint64_t value)
> +{
> +    env->cp15.dbgclaim &= ~(value & 0xFF);
> +}
> +
>  static const ARMCPRegInfo debug_cp_reginfo[] = {
>      /*
>       * DBGDRAR, DBGDSAR: always RAZ since we don't implement memory mapped
> @@ -712,6 +724,21 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>        .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 0,
>        .access = PL1_RW, .accessfn = access_tda,
>        .type = ARM_CP_NOP },
> +    /*
> +     * Dummy DBGCLAIM registers.
> +     * "The architecture does not define any functionality for the CLAIM tag 
> bits.",
> +     * so we only keep the raw bits
> +     */
> +    { .name = "DBGCLAIMSET_EL1", .state = ARM_CP_STATE_BOTH,
> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 6,
> +      .access = PL1_RW, .accessfn = access_tda,
> +      .writefn = dbgclaimset_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.dbgclaim) },

DBGCLAIMSET_EL1 CLAIM bits are supposed to RAO. (In v7 this
was done so software could identify how many claim bits are
implemented by writing all-1s and then reading back.) So we
need a readfn that just returns 0xff, and can skip the .fieldoffset.

We should mark DBGCLAIMSET_EL1 as ARM_CP_ALIAS, because the
actual state is handled by DBGCLAIMCLR_EL1.

> +    { .name = "DBGCLAIMCLR_EL1", .state = ARM_CP_STATE_BOTH,
> +      .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 7, .crm = 9, .opc2 = 6,
> +      .access = PL1_RW, .accessfn = access_tda,
> +      .writefn = dbgclaimclr_write,
> +      .fieldoffset = offsetof(CPUARMState, cp15.dbgclaim) },

This also needs .raw_writefn = raw_write,
so that on migration restore we can write the value in
and not have it go via the "clear these bits" writefn.

thanks
-- PMM