[PATCH v2] hvf: arm: Handle unknown ID registers as RES0

[PATCH v2] hvf: arm: Handle unknown ID registers as RES0

[Alexander Graf]

Recent Linux versions added support to read ID_AA64ISAR2_EL1. On M1,
those reads trap into QEMU which handles them as faults.

However, AArch64 ID registers should always read as RES0. Let's
handle them accordingly.

This fixes booting Linux 5.17 guests.

Cc: qemu-stable@nongnu.org
Reported-by: Ivan Babrou <ivan@cloudflare.com>
Signed-off-by: Alexander Graf <agraf@csgraf.de>
---
 target/arm/hvf/hvf.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 92ad0d29c4..39c3e0d85f 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -729,6 +729,17 @@ static bool hvf_handle_psci_call(CPUState *cpu)
     return true;
 }
 
+static bool is_id_sysreg(uint32_t reg)
+{
+    uint32_t op0 = (reg >> 20) & 0x3;
+    uint32_t op1 = (reg >> 14) & 0x7;
+    uint32_t crn = (reg >> 10) & 0xf;
+    uint32_t crm = (reg >> 1) & 0xf;
+    uint32_t op2 = (reg >> 7) & 0x7;
+
+    return op0 == 3 && op1 == 0 && crn == 0 && crm >= 1 && crm < 8 && op2 < 8;
+}
+
 static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
 {
     ARMCPU *arm_cpu = ARM_CPU(cpu);
@@ -781,6 +792,11 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, uint32_t rt)
         /* Dummy register */
         break;
     default:
+        if (is_id_sysreg(reg)) {
+            /* ID system registers read as RES0 */
+            val = 0;
+            break;
+        }
         cpu_synchronize_state(cpu);
         trace_hvf_unhandled_sysreg_read(env->pc, reg,
                                         (reg >> 20) & 0x3,
-- 
2.32.0 (Apple Git-132)

Re: [PATCH v2] hvf: arm: Handle unknown ID registers as RES0

[Peter Maydell]

On Tue, 8 Feb 2022 at 10:27, Alexander Graf <agraf@csgraf.de> wrote:
>
> Recent Linux versions added support to read ID_AA64ISAR2_EL1. On M1,
> those reads trap into QEMU which handles them as faults.
>
> However, AArch64 ID registers should always read as RES0. Let's
> handle them accordingly.
>
> This fixes booting Linux 5.17 guests.
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Ivan Babrou <ivan@cloudflare.com>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> ---
>  target/arm/hvf/hvf.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 92ad0d29c4..39c3e0d85f 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -729,6 +729,17 @@ static bool hvf_handle_psci_call(CPUState *cpu)
>      return true;
>  }
>
> +static bool is_id_sysreg(uint32_t reg)
> +{
> +    uint32_t op0 = (reg >> 20) & 0x3;
> +    uint32_t op1 = (reg >> 14) & 0x7;
> +    uint32_t crn = (reg >> 10) & 0xf;
> +    uint32_t crm = (reg >> 1) & 0xf;
> +    uint32_t op2 = (reg >> 7) & 0x7;

This is now the fifth place where we unpack the fields
of a bad-sysreg syndrome register value (we already do
it in the tracing for handled and unhandled sysreg reads
and writes). Seems like a good time to define some
abstractions for it rather than using a lot of hard-coded
constant values.

To demonstrate the value of this, you have the shift value
for op2 wrong -- it starts at bit 17, not 7.

> +
> +    return op0 == 3 && op1 == 0 && crn == 0 && crm >= 1 && crm < 8 && op2 < 8;

The last clause in this condition can never be false,
because op2 is only a 3 bit field.


thanks
-- PMM