Re: [PATCH] target/arm: Correct the encoding of MDCCSR_EL0

[Peter Maydell]

On Fri, 2 Jul 2021 at 16:01, Nick Hudson <hnick@vmware.com> wrote:
> Maybe I’m misreading the ARM ARM and the qemu use of mdscr_el1, but I think
> this is good enough / more correct.  I’m somewhat confused by AA64 MDSCR_EL1
> vs DBGSCRint vs DBGSCRext, however.

Yeah, it is confusing and we generally haven't modeled this very
well, because we've kind of only cared when guests don't work and
some of the purpose of these registers is for external debug
which we don't model at all.

Looking more closely at the Arm ARM:
 * MDSCR_EL1 is the AArch64 register
 * DBGDSCRext is the AArch32 version of that
We model these basically correctly, I think
 * MDCCSR_EL0 is supposed to be an AArch64 read-only register which
has bits [30:29] of EDSCR (ie the JTAG-debugger-view of the TX/RX
connection)
 * DBGDSCRint is similar for AArch32, but it also has various
bits that are read-only views of bits in DBGDSCRext/MDSCR_EL1

Bits [30:29] of MDSCR_EL1 are sort-of-but-not-quite the same
bits as EDSCR [30:29], but they're close enough for our purposes.

>     /* MDCCSR_EL0[30:29] map to DBGDSCRint[30:29]. Simply RAZ.

(QEMU coding style requires "/*" on a line of its own.)

>      * We don't implement the configurable EL0 access.
>      */
>     { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
>       .cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>       .type = ARM_CP_CONST, .resetvalue = 0 },

This seems reasonable; we don't implement the external debug
Debug Communication Channel so we might as well model
RXfull/TXfull as 0. It might be nice to mention in the comment
that the reason we RAZ is because we don't implement the DCC.

>     /* DBGDSCRint[15,12,5:2] map to MDSCR_EL1[15,12,5:2] */
>     { .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
>       .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>       .type = ARM_CP_ALIAS,
>       .access = PL1_R, .accessfn = access_tda,
>       .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },

Here we're taking advantage of the fact tha MDSCR_EL1[30:29] are
close enough that we can get away with using those. It's not
consistent with how we modelled MDCCSR_EL0's version of those
flags but it's unlikely any guest code will care.

> Please let me know if you want me to submit a new patch.

Yes, please do.

thanks
-- PMM