[PATCH 0/3] various aarch64 fixes for running Hyper-V on TCG

[PATCH 0/3] various aarch64 fixes for running Hyper-V on TCG

[Evgeny Iakovlev]

Small series of changes to aarch64 emulation to better support running
Hyper-V as a TCG guest wtih EL3 firmware.

Evgeny Iakovlev (3):
  target/arm: implement DBGCLAIM registers
  target/arm: provide RAZ/WI stubs for more DCC registers
  target/arm: allow writes to SCR_EL3.HXEn bit when FEAT_HCX is enabled

 target/arm/cpu.h          |  1 +
 target/arm/debug_helper.c | 39 +++++++++++++++++++++++++++++++++++++++
 target/arm/helper.c       |  3 +++
 3 files changed, 43 insertions(+)

-- 
2.34.1

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

[Evgeny Iakovlev]

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) },
+    { .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) },
 };
 
 static const ARMCPRegInfo debug_lpae_cp_reginfo[] = {
-- 
2.34.1

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

[PATCH 2/3] target/arm: provide RAZ/WI stubs for more DCC registers

[Evgeny Iakovlev]

Qemu doesn't implement Debug Communication Channel, however when running
Microsoft Hyper-V in software-emulated ARM64 as a guest, it tries to
access some of the DCM registers during an EL2 context switch.

Provide RAZ/WI stubs for OSDTRRX_EL1, OSDTRTX_EL1 and OSECCR_EL1
registers in the same way the rest of DCM is currently done. Do
account for access traps though with access_tda.

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
---
 target/arm/debug_helper.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index b244e146e2..2a7c3d7e38 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -673,6 +673,18 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
       .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
       .access = PL0_R, .accessfn = access_tda,
       .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "OSDTRRX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 2,
+      .access = PL1_RW, .accessfn = access_tda,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "OSDTRTX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 2,
+      .access = PL1_RW, .accessfn = access_tda,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "OSECCR_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
+      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2,
+      .access = PL1_RW, .accessfn = access_tda,
+      .type = ARM_CP_CONST, .resetvalue = 0 },
     /*
      * DBGDSCRint[15,12,5:2] map to MDSCR_EL1[15,12,5:2].  Map all bits as
      * it is unlikely a guest will care.
-- 
2.34.1

Re: [PATCH 2/3] target/arm: provide RAZ/WI stubs for more DCC registers

[Peter Maydell]

On Thu, 5 Jan 2023 at 22:13, Evgeny Iakovlev
<eiakovlev@linux.microsoft.com> wrote:
>
> Qemu doesn't implement Debug Communication Channel, however when running
> Microsoft Hyper-V in software-emulated ARM64 as a guest, it tries to
> access some of the DCM registers during an EL2 context switch.

I've occasionally thought about implementing the DCC as something
the QEMU user could connect to a QEMU chardev. But that would be
a lot of faff for no very obvious benefit, so making these registers
RAZ makes sense for now.

> Provide RAZ/WI stubs for OSDTRRX_EL1, OSDTRTX_EL1 and OSECCR_EL1
> registers in the same way the rest of DCM is currently done. Do
> account for access traps though with access_tda.

OSECCR_EL1 isn't part of DCC; it's a different bit of the external
debug interface.

> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
> ---
>  target/arm/debug_helper.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index b244e146e2..2a7c3d7e38 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -673,6 +673,18 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>        .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>        .access = PL0_R, .accessfn = access_tda,
>        .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "OSDTRRX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
> +      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 2,
> +      .access = PL1_RW, .accessfn = access_tda,
> +      .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "OSDTRTX_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
> +      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 2,
> +      .access = PL1_RW, .accessfn = access_tda,
> +      .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "OSECCR_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14,
> +      .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 6, .opc2 = 2,
> +      .access = PL1_RW, .accessfn = access_tda,
> +      .type = ARM_CP_CONST, .resetvalue = 0 },

A brief comment or two here would be nice.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 3/3] target/arm: allow writes to SCR_EL3.HXEn bit when FEAT_HCX is enabled

[Evgeny Iakovlev]

ARM trusted firmware, when built with FEAT_HCX support, sets SCR_EL3.HXEn bit
to allow EL2 to modify HCRX_EL2 register without trapping it in EL3. Qemu
uses a valid mask to clear unsupported SCR_EL3 bits when emulating SCR_EL3
write, and that mask doesn't include SCR_EL3.HXEn bit even if FEAT_HCX is
enabled and exposed to the guest. As a result EL3 writes of that bit are
ignored.

Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
---
 target/arm/helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index bac2ea62c4..962affdd52 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1844,6 +1844,9 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
         if (cpu_isar_feature(aa64_sme, cpu)) {
             valid_mask |= SCR_ENTP2;
         }
+        if (cpu_isar_feature(aa64_hcx, cpu)) {
+            valid_mask |= SCR_HXEN;
+        }
     } else {
         valid_mask &= ~(SCR_RW | SCR_ST);
         if (cpu_isar_feature(aa32_ras, cpu)) {
-- 
2.34.1

Re: [PATCH 3/3] target/arm: allow writes to SCR_EL3.HXEn bit when FEAT_HCX is enabled

[Peter Maydell]

On Thu, 5 Jan 2023 at 22:13, Evgeny Iakovlev
<eiakovlev@linux.microsoft.com> wrote:
>
> ARM trusted firmware, when built with FEAT_HCX support, sets SCR_EL3.HXEn bit
> to allow EL2 to modify HCRX_EL2 register without trapping it in EL3. Qemu
> uses a valid mask to clear unsupported SCR_EL3 bits when emulating SCR_EL3
> write, and that mask doesn't include SCR_EL3.HXEn bit even if FEAT_HCX is
> enabled and exposed to the guest. As a result EL3 writes of that bit are
> ignored.
>
> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
> ---
>  target/arm/helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index bac2ea62c4..962affdd52 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1844,6 +1844,9 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>          if (cpu_isar_feature(aa64_sme, cpu)) {
>              valid_mask |= SCR_ENTP2;
>          }
> +        if (cpu_isar_feature(aa64_hcx, cpu)) {
> +            valid_mask |= SCR_HXEN;
> +        }
>      } else {
>          valid_mask &= ~(SCR_RW | SCR_ST);
>          if (cpu_isar_feature(aa32_ras, cpu)) {
> --

Oops. This is worth
Cc: qemu-stable@nongnu.org

I think.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I'll take this 3rd patch into target-arm.next now; I've left
review comments for the other 2.

thanks
-- PMM

Re: [PATCH 3/3] target/arm: allow writes to SCR_EL3.HXEn bit when FEAT_HCX is enabled

[Evgeny Iakovlev]

On 1/13/2023 14:20, Peter Maydell wrote:
> On Thu, 5 Jan 2023 at 22:13, Evgeny Iakovlev
> <eiakovlev@linux.microsoft.com> wrote:
>> ARM trusted firmware, when built with FEAT_HCX support, sets SCR_EL3.HXEn bit
>> to allow EL2 to modify HCRX_EL2 register without trapping it in EL3. Qemu
>> uses a valid mask to clear unsupported SCR_EL3 bits when emulating SCR_EL3
>> write, and that mask doesn't include SCR_EL3.HXEn bit even if FEAT_HCX is
>> enabled and exposed to the guest. As a result EL3 writes of that bit are
>> ignored.
>>
>> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
>> ---
>>   target/arm/helper.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index bac2ea62c4..962affdd52 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -1844,6 +1844,9 @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>>           if (cpu_isar_feature(aa64_sme, cpu)) {
>>               valid_mask |= SCR_ENTP2;
>>           }
>> +        if (cpu_isar_feature(aa64_hcx, cpu)) {
>> +            valid_mask |= SCR_HXEN;
>> +        }
>>       } else {
>>           valid_mask &= ~(SCR_RW | SCR_ST);
>>           if (cpu_isar_feature(aa32_ras, cpu)) {
>> --
> Oops. This is worth
> Cc: qemu-stable@nongnu.org
>
> I think.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I'll take this 3rd patch into target-arm.next now; I've left
> review comments for the other 2.
>
> thanks
> -- PMM


Thanks, Peter! I'll be addressing the comments today and post a v2 with 
changes for the first 2.