diff mbox

target-arm: Add MDCR_EL2

Message ID 1443436639-6603-1-git-send-email-serge.fdrv@gmail.com
State New
Headers show

Commit Message

Sergey Fedorov Sept. 28, 2015, 10:37 a.m. UTC
Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
---

This patch is a prerequisite for a debug exception routing patch:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html

 target-arm/cpu-qom.h |  1 +
 target-arm/cpu.c     |  1 +
 target-arm/cpu.h     |  1 +
 target-arm/cpu64.c   |  1 +
 target-arm/helper.c  | 13 +++++++++++++
 5 files changed, 17 insertions(+)

Comments

Alex Bennée Sept. 29, 2015, 6 a.m. UTC | #1
Sergey Fedorov <serge.fdrv@gmail.com> writes:

> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> ---
>
> This patch is a prerequisite for a debug exception routing patch:
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html
>
>  target-arm/cpu-qom.h |  1 +
>  target-arm/cpu.c     |  1 +
>  target-arm/cpu.h     |  1 +
>  target-arm/cpu64.c   |  1 +
>  target-arm/helper.c  | 13 +++++++++++++
>  5 files changed, 17 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 25fb1ce..d2b0769 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -167,6 +167,7 @@ typedef struct ARMCPU {
>      uint64_t id_aa64mmfr0;
>      uint64_t id_aa64mmfr1;
>      uint32_t dbgdidr;
> +    uint32_t mdcr;
>      uint32_t clidr;
>      uint64_t mp_affinity; /* MP ID without feature bits */
>      /* The elements of this array are the CCSIDR values for each cache,
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index d7b4445..6474c0d 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -1125,6 +1125,7 @@ static void cortex_a15_initfn(Object *obj)
>      cpu->id_isar3 = 0x11112131;
>      cpu->id_isar4 = 0x10011142;
>      cpu->dbgdidr = 0x3515f021;
> +    cpu->mdcr = 0x00000006;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 1b80516..d57ed20 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -380,6 +380,7 @@ typedef struct CPUARMState {
>          uint64_t dbgwvr[16]; /* watchpoint value registers */
>          uint64_t dbgwcr[16]; /* watchpoint control registers */
>          uint64_t mdscr_el1;
> +        uint64_t mdcr_el2;

Given we already have banked el3 regs shouldn't this be:

           uint64_6 mdcr_el[4]

?
>          /* If the counter is enabled, this stores the last time the counter
>           * was reset. Otherwise it stores the counter value
>           */
> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
> index 63c8b1c..a41cd28 100644
> --- a/target-arm/cpu64.c
> +++ b/target-arm/cpu64.c
> @@ -136,6 +136,7 @@ static void aarch64_a57_initfn(Object *obj)
>      cpu->id_aa64isar0 = 0x00011120;
>      cpu->id_aa64mmfr0 = 0x00001124;
>      cpu->dbgdidr = 0x3516d000;
> +    cpu->mdcr = 0x00000006;
>      cpu->clidr = 0x0a200023;
>      cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
>      cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 12ea88f..5fe1291 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3221,6 +3221,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>      { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
>        .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>  
> @@ -3870,6 +3873,13 @@ static void define_debug_regs(ARMCPU *cpu)
>          .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
>          .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
>      };
> +    ARMCPRegInfo mdcr_el2 = {
> +        .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
> +        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
> +        .access = PL2_RW,
> +        .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
> +        .resetvalue = cpu->mdcr,
> +    };
>  
>      /* Note that all these register fields hold "number of Xs minus 1". */
>      brps = extract32(cpu->dbgdidr, 24, 4);
> @@ -3894,6 +3904,9 @@ static void define_debug_regs(ARMCPU *cpu)
>      if (arm_feature(&cpu->env, ARM_FEATURE_LPAE)) {
>          define_arm_cp_regs(cpu, debug_lpae_cp_reginfo);
>      }
> +    if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) {
> +        define_one_arm_cp_reg(cpu, &mdcr_el2);
> +    }
>  
>      for (i = 0; i < brps + 1; i++) {
>          ARMCPRegInfo dbgregs[] = {
Peter Maydell Sept. 29, 2015, 9:25 a.m. UTC | #2
On 29 September 2015 at 07:00, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Sergey Fedorov <serge.fdrv@gmail.com> writes:
>
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> ---
>>
>> This patch is a prerequisite for a debug exception routing patch:
>> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html
>>
>>  target-arm/cpu-qom.h |  1 +
>>  target-arm/cpu.c     |  1 +
>>  target-arm/cpu.h     |  1 +
>>  target-arm/cpu64.c   |  1 +
>>  target-arm/helper.c  | 13 +++++++++++++
>>  5 files changed, 17 insertions(+)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 25fb1ce..d2b0769 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -167,6 +167,7 @@ typedef struct ARMCPU {
>>      uint64_t id_aa64mmfr0;
>>      uint64_t id_aa64mmfr1;
>>      uint32_t dbgdidr;
>> +    uint32_t mdcr;
>>      uint32_t clidr;
>>      uint64_t mp_affinity; /* MP ID without feature bits */
>>      /* The elements of this array are the CCSIDR values for each cache,
>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>> index d7b4445..6474c0d 100644
>> --- a/target-arm/cpu.c
>> +++ b/target-arm/cpu.c
>> @@ -1125,6 +1125,7 @@ static void cortex_a15_initfn(Object *obj)
>>      cpu->id_isar3 = 0x11112131;
>>      cpu->id_isar4 = 0x10011142;
>>      cpu->dbgdidr = 0x3515f021;
>> +    cpu->mdcr = 0x00000006;
>>      cpu->clidr = 0x0a200023;
>>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 1b80516..d57ed20 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -380,6 +380,7 @@ typedef struct CPUARMState {
>>          uint64_t dbgwvr[16]; /* watchpoint value registers */
>>          uint64_t dbgwcr[16]; /* watchpoint control registers */
>>          uint64_t mdscr_el1;
>> +        uint64_t mdcr_el2;
>
> Given we already have banked el3 regs shouldn't this be:
>
>            uint64_6 mdcr_el[4]
>
> ?

You could argue either way, but since there's only an
MDCR_EL2 and an MDCR_EL3 and they're not really the same
field format there won't be any code that wants to do
mdcr_el[x], so I think calling the field mdcr_el2 is ok.

-- PMM
Peter Maydell Sept. 29, 2015, 9:33 a.m. UTC | #3
On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> ---
>
> This patch is a prerequisite for a debug exception routing patch:
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html
>
>  target-arm/cpu-qom.h |  1 +
>  target-arm/cpu.c     |  1 +
>  target-arm/cpu.h     |  1 +
>  target-arm/cpu64.c   |  1 +
>  target-arm/helper.c  | 13 +++++++++++++
>  5 files changed, 17 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 25fb1ce..d2b0769 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -167,6 +167,7 @@ typedef struct ARMCPU {
>      uint64_t id_aa64mmfr0;
>      uint64_t id_aa64mmfr1;
>      uint32_t dbgdidr;
> +    uint32_t mdcr;

This field should be named mdcr_el2 if we have it, but:
the reset value for this register is defined architecturally,
so we don't need to specify it per CPU. (It's "all 0s, except
the bottom field resets to the same value as PMCR.N". Strictly
speaking some fields are defined to be architecturally
unknown and so might differ per CPU, but only in ways which
a guest doesn't care about. We typically model these in
the same way for all guest CPUs in QEMU.)

thanks
-- PMM
Sergey Fedorov Sept. 29, 2015, 5:14 p.m. UTC | #4
On 29.09.2015 12:33, Peter Maydell wrote:
> On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> ---
>>
>> This patch is a prerequisite for a debug exception routing patch:
>> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html
>>
>>  target-arm/cpu-qom.h |  1 +
>>  target-arm/cpu.c     |  1 +
>>  target-arm/cpu.h     |  1 +
>>  target-arm/cpu64.c   |  1 +
>>  target-arm/helper.c  | 13 +++++++++++++
>>  5 files changed, 17 insertions(+)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 25fb1ce..d2b0769 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -167,6 +167,7 @@ typedef struct ARMCPU {
>>      uint64_t id_aa64mmfr0;
>>      uint64_t id_aa64mmfr1;
>>      uint32_t dbgdidr;
>> +    uint32_t mdcr;
> This field should be named mdcr_el2 if we have it, but:
> the reset value for this register is defined architecturally,
> so we don't need to specify it per CPU. (It's "all 0s, except
> the bottom field resets to the same value as PMCR.N". Strictly
> speaking some fields are defined to be architecturally
> unknown and so might differ per CPU, but only in ways which
> a guest doesn't care about. We typically model these in
> the same way for all guest CPUs in QEMU.)
>

We reset PMCR_EL0 to cpu->midr & 0xff000000. So if we want to reset
MDCR_EL2.N with PMCR_EL0.N we should reset PMCR_EL0.N to the proper
value somehow first. I think we could reset PMCR_EL0 with its own reset
value from a dedicated ARMCPU structure field independent from MIDR_EL1
reset value. This makes sense considering that most of PMCR_EL0's fields
are RO/UNKNOWN. What do you think?

Best regards,
Sergey
Peter Maydell Sept. 29, 2015, 5:19 p.m. UTC | #5
On 29 September 2015 at 18:14, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> On 29.09.2015 12:33, Peter Maydell wrote:
>> On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> This field should be named mdcr_el2 if we have it, but:
>> the reset value for this register is defined architecturally,
>> so we don't need to specify it per CPU. (It's "all 0s, except
>> the bottom field resets to the same value as PMCR.N". Strictly
>> speaking some fields are defined to be architecturally
>> unknown and so might differ per CPU, but only in ways which
>> a guest doesn't care about. We typically model these in
>> the same way for all guest CPUs in QEMU.)
>>
>
> We reset PMCR_EL0 to cpu->midr & 0xff000000. So if we want to reset
> MDCR_EL2.N with PMCR_EL0.N we should reset PMCR_EL0.N to the proper
> value somehow first. I think we could reset PMCR_EL0 with its own reset
> value from a dedicated ARMCPU structure field independent from MIDR_EL1
> reset value. This makes sense considering that most of PMCR_EL0's fields
> are RO/UNKNOWN. What do you think?

At the moment we don't implement any perf counters except
the cycle counter, so I think PMCR_EL0.N is already being
reset to the proper value...

-- PMM
Sergey Fedorov Sept. 29, 2015, 5:24 p.m. UTC | #6
On 29.09.2015 20:19, Peter Maydell wrote:
> On 29 September 2015 at 18:14, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>> On 29.09.2015 12:33, Peter Maydell wrote:
>>> On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>>> This field should be named mdcr_el2 if we have it, but:
>>> the reset value for this register is defined architecturally,
>>> so we don't need to specify it per CPU. (It's "all 0s, except
>>> the bottom field resets to the same value as PMCR.N". Strictly
>>> speaking some fields are defined to be architecturally
>>> unknown and so might differ per CPU, but only in ways which
>>> a guest doesn't care about. We typically model these in
>>> the same way for all guest CPUs in QEMU.)
>>>
>> We reset PMCR_EL0 to cpu->midr & 0xff000000. So if we want to reset
>> MDCR_EL2.N with PMCR_EL0.N we should reset PMCR_EL0.N to the proper
>> value somehow first. I think we could reset PMCR_EL0 with its own reset
>> value from a dedicated ARMCPU structure field independent from MIDR_EL1
>> reset value. This makes sense considering that most of PMCR_EL0's fields
>> are RO/UNKNOWN. What do you think?
> At the moment we don't implement any perf counters except
> the cycle counter, so I think PMCR_EL0.N is already being
> reset to the proper value...

Seems like I should just simply use zero as a reset value for MDCR_EL2
register :)
Alex Bennée Oct. 2, 2015, 2:39 p.m. UTC | #7
Peter Maydell <peter.maydell@linaro.org> writes:

> On 29 September 2015 at 07:00, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Sergey Fedorov <serge.fdrv@gmail.com> writes:
>>
>>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>>> ---
>>>
>>> This patch is a prerequisite for a debug exception routing patch:
>>> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg03542.html
>>>
>>>  target-arm/cpu-qom.h |  1 +
>>>  target-arm/cpu.c     |  1 +
>>>  target-arm/cpu.h     |  1 +
>>>  target-arm/cpu64.c   |  1 +
>>>  target-arm/helper.c  | 13 +++++++++++++
>>>  5 files changed, 17 insertions(+)
>>>
>>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>>> index 25fb1ce..d2b0769 100644
>>> --- a/target-arm/cpu-qom.h
>>> +++ b/target-arm/cpu-qom.h
>>> @@ -167,6 +167,7 @@ typedef struct ARMCPU {
>>>      uint64_t id_aa64mmfr0;
>>>      uint64_t id_aa64mmfr1;
>>>      uint32_t dbgdidr;
>>> +    uint32_t mdcr;
>>>      uint32_t clidr;
>>>      uint64_t mp_affinity; /* MP ID without feature bits */
>>>      /* The elements of this array are the CCSIDR values for each cache,
>>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
>>> index d7b4445..6474c0d 100644
>>> --- a/target-arm/cpu.c
>>> +++ b/target-arm/cpu.c
>>> @@ -1125,6 +1125,7 @@ static void cortex_a15_initfn(Object *obj)
>>>      cpu->id_isar3 = 0x11112131;
>>>      cpu->id_isar4 = 0x10011142;
>>>      cpu->dbgdidr = 0x3515f021;
>>> +    cpu->mdcr = 0x00000006;
>>>      cpu->clidr = 0x0a200023;
>>>      cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
>>>      cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index 1b80516..d57ed20 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -380,6 +380,7 @@ typedef struct CPUARMState {
>>>          uint64_t dbgwvr[16]; /* watchpoint value registers */
>>>          uint64_t dbgwcr[16]; /* watchpoint control registers */
>>>          uint64_t mdscr_el1;
>>> +        uint64_t mdcr_el2;
>>
>> Given we already have banked el3 regs shouldn't this be:
>>
>>            uint64_6 mdcr_el[4]
>>
>> ?
>
> You could argue either way, but since there's only an
> MDCR_EL2 and an MDCR_EL3 and they're not really the same
> field format there won't be any code that wants to do
> mdcr_el[x], so I think calling the field mdcr_el2 is ok.


Fair enough that makes sense.
Sergey Fedorov Oct. 8, 2015, 9:56 a.m. UTC | #8
On 29.09.2015 20:24, Sergey Fedorov wrote:
> On 29.09.2015 20:19, Peter Maydell wrote:
>> On 29 September 2015 at 18:14, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>>> On 29.09.2015 12:33, Peter Maydell wrote:
>>>> On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>>>> This field should be named mdcr_el2 if we have it, but:
>>>> the reset value for this register is defined architecturally,
>>>> so we don't need to specify it per CPU. (It's "all 0s, except
>>>> the bottom field resets to the same value as PMCR.N". Strictly
>>>> speaking some fields are defined to be architecturally
>>>> unknown and so might differ per CPU, but only in ways which
>>>> a guest doesn't care about. We typically model these in
>>>> the same way for all guest CPUs in QEMU.)
>>>>
>>> We reset PMCR_EL0 to cpu->midr & 0xff000000. So if we want to reset
>>> MDCR_EL2.N with PMCR_EL0.N we should reset PMCR_EL0.N to the proper
>>> value somehow first. I think we could reset PMCR_EL0 with its own reset
>>> value from a dedicated ARMCPU structure field independent from MIDR_EL1
>>> reset value. This makes sense considering that most of PMCR_EL0's fields
>>> are RO/UNKNOWN. What do you think?
>> At the moment we don't implement any perf counters except
>> the cycle counter, so I think PMCR_EL0.N is already being
>> reset to the proper value...
> Seems like I should just simply use zero as a reset value for MDCR_EL2
> register :)

Peter, would be okay to use zero as a reset value for MDCR_EL2 in this
patch?

Best regards,
Sergey
Peter Maydell Oct. 8, 2015, 10:10 a.m. UTC | #9
On 8 October 2015 at 10:56, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
> On 29.09.2015 20:24, Sergey Fedorov wrote:
>> On 29.09.2015 20:19, Peter Maydell wrote:
>>> On 29 September 2015 at 18:14, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>>>> On 29.09.2015 12:33, Peter Maydell wrote:
>>>>> On 28 September 2015 at 11:37, Sergey Fedorov <serge.fdrv@gmail.com> wrote:
>>>>> This field should be named mdcr_el2 if we have it, but:
>>>>> the reset value for this register is defined architecturally,
>>>>> so we don't need to specify it per CPU. (It's "all 0s, except
>>>>> the bottom field resets to the same value as PMCR.N". Strictly
>>>>> speaking some fields are defined to be architecturally
>>>>> unknown and so might differ per CPU, but only in ways which
>>>>> a guest doesn't care about. We typically model these in
>>>>> the same way for all guest CPUs in QEMU.)
>>>>>
>>>> We reset PMCR_EL0 to cpu->midr & 0xff000000. So if we want to reset
>>>> MDCR_EL2.N with PMCR_EL0.N we should reset PMCR_EL0.N to the proper
>>>> value somehow first. I think we could reset PMCR_EL0 with its own reset
>>>> value from a dedicated ARMCPU structure field independent from MIDR_EL1
>>>> reset value. This makes sense considering that most of PMCR_EL0's fields
>>>> are RO/UNKNOWN. What do you think?
>>> At the moment we don't implement any perf counters except
>>> the cycle counter, so I think PMCR_EL0.N is already being
>>> reset to the proper value...
>> Seems like I should just simply use zero as a reset value for MDCR_EL2
>> register :)
>
> Peter, would be okay to use zero as a reset value for MDCR_EL2 in this
> patch?

Yes; you could add a brief comment that this assumes there are no
other performance counters.

-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 25fb1ce..d2b0769 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -167,6 +167,7 @@  typedef struct ARMCPU {
     uint64_t id_aa64mmfr0;
     uint64_t id_aa64mmfr1;
     uint32_t dbgdidr;
+    uint32_t mdcr;
     uint32_t clidr;
     uint64_t mp_affinity; /* MP ID without feature bits */
     /* The elements of this array are the CCSIDR values for each cache,
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index d7b4445..6474c0d 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1125,6 +1125,7 @@  static void cortex_a15_initfn(Object *obj)
     cpu->id_isar3 = 0x11112131;
     cpu->id_isar4 = 0x10011142;
     cpu->dbgdidr = 0x3515f021;
+    cpu->mdcr = 0x00000006;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32K L1 dcache */
     cpu->ccsidr[1] = 0x201fe00a; /* 32K L1 icache */
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 1b80516..d57ed20 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -380,6 +380,7 @@  typedef struct CPUARMState {
         uint64_t dbgwvr[16]; /* watchpoint value registers */
         uint64_t dbgwcr[16]; /* watchpoint control registers */
         uint64_t mdscr_el1;
+        uint64_t mdcr_el2;
         /* If the counter is enabled, this stores the last time the counter
          * was reset. Otherwise it stores the counter value
          */
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 63c8b1c..a41cd28 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -136,6 +136,7 @@  static void aarch64_a57_initfn(Object *obj)
     cpu->id_aa64isar0 = 0x00011120;
     cpu->id_aa64mmfr0 = 0x00001124;
     cpu->dbgdidr = 0x3516d000;
+    cpu->mdcr = 0x00000006;
     cpu->clidr = 0x0a200023;
     cpu->ccsidr[0] = 0x701fe00a; /* 32KB L1 dcache */
     cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 12ea88f..5fe1291 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3221,6 +3221,9 @@  static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
     { .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
       .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
+      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -3870,6 +3873,13 @@  static void define_debug_regs(ARMCPU *cpu)
         .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
         .access = PL0_R, .type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
     };
+    ARMCPRegInfo mdcr_el2 = {
+        .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
+        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
+        .access = PL2_RW,
+        .fieldoffset = offsetof(CPUARMState, cp15.mdcr_el2),
+        .resetvalue = cpu->mdcr,
+    };
 
     /* Note that all these register fields hold "number of Xs minus 1". */
     brps = extract32(cpu->dbgdidr, 24, 4);
@@ -3894,6 +3904,9 @@  static void define_debug_regs(ARMCPU *cpu)
     if (arm_feature(&cpu->env, ARM_FEATURE_LPAE)) {
         define_arm_cp_regs(cpu, debug_lpae_cp_reginfo);
     }
+    if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) {
+        define_one_arm_cp_reg(cpu, &mdcr_el2);
+    }
 
     for (i = 0; i < brps + 1; i++) {
         ARMCPRegInfo dbgregs[] = {