Re: [RFC PATCH v4 08/25] target/arm/kvm: handle writeback for special ID register fields

[Eric Auger <eric.auger@redhat.com>]

On 9/21/26 2:32 PM, Khushit Shah wrote:
> Some fields should not be written back to KVM for various reasons:
>  - ID_AA64PFR0_EL1.GIC / ID_PFR1_EL1.GIC are fabricated by KVM based on
>    the gic-version property.
>  - KVM does not allow writing 0 to ID_DFR0_EL1.CopDbg, which breaks
>    booting an AArch64-only guest on a host that also supports AArch32.
>  - ID_DFR0_EL1.PerfMon is populated by KVM even for AArch64-only guests
>    but is not writable there.
>  - MPIDR_EL1 is managed by KVM based on vCPU index.
>  - Hold off writing back CLIDR_EL1 until we properly support exposing
>    cache topology to a KVM guest.
>
> Signed-off-by: Khushit Shah <khushit.shah@nutanix.com>
> ---
>  target/arm/cpu-idregs.c |  6 ++++++
>  target/arm/cpu-idregs.h |  2 ++
>  target/arm/kvm.c        | 40 +++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu-idregs.c b/target/arm/cpu-idregs.c
> index d58b8ed063..1f7ebfe9bb 100644
> --- a/target/arm/cpu-idregs.c
> +++ b/target/arm/cpu-idregs.c
> @@ -107,3 +107,9 @@ bool arm_field_is_writable(const ARM64SysRegField *field)
>      return (arm64_id_regs[field->index].writable_mask & field_mask)
>             == field_mask;
>  }
> +
> +bool field_matches(const ARM64SysRegField *field, ARMIDRegisterIdx index,
> +                   const char *name)
> +{
> +    return field->index == index && !strcmp(field->name, name);
> +}
> diff --git a/target/arm/cpu-idregs.h b/target/arm/cpu-idregs.h
> index aab1a79db1..8ef3f9350e 100644
> --- a/target/arm/cpu-idregs.h
> +++ b/target/arm/cpu-idregs.h
> @@ -35,4 +35,6 @@ extern ARM64SysReg arm64_id_regs[NUM_ID_IDX];
>  
>  uint64_t arm_get_field_mask(const ARM64SysRegField *field);
>  bool arm_field_is_writable(const ARM64SysRegField *field);
> +bool field_matches(const ARM64SysRegField *field, ARMIDRegisterIdx index,
> +                   const char *name);
>  #endif
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 2d94b3a884..70d9a07580 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -1317,10 +1317,40 @@ bool kvm_arm_cpu_post_load(ARMCPU *cpu)
>      return true;
>  }
>  
> +static bool arm_field_skip_writeback_always(const ARM64SysRegField *field)
> +{
> +    /*
> +     * GIC is controlled by the gic-version property and fabricated by KVM
> +     * when the vGIC device is created, so a named CPU model must not touch
> +     * it. KVM also rejects writing 0 to ID_DFR0_EL1.CopDbg, which breaks
> +     * booting a model that lacks AArch32 support on a host that supports
> +     * it. MPIDR_EL1 is managed by KVM based on vCPU index, skip writing it
> +     * back. Similarly, skip writing back CLIDR_EL1 till we properly
> +     * support exposing cache for KVM guest.
> +     */
> +    return field_matches(field, ID_AA64PFR0_EL1_IDX, "GIC")
> +        || field_matches(field, ID_PFR1_EL1_IDX, "GIC")
> +        || field_matches(field, ID_DFR0_EL1_IDX, "CopDbg")
> +        || field->index == MPIDR_EL1_IDX
> +        || field->index == CLIDR_EL1_IDX;

what does happen for SMIDR_EL1 and GMID_EL1 whose value could not be
initialized from host. The setter for any of their field shall fail.
> +}
> +
> +static bool
> +arm_field_skip_writeback_if_not_writable(const ARM64SysRegField *field)
> +{
> +    /*
> +     * KVM populates ID_DFR0_EL1.PerfMon even for AArch64-only guests but
> +     * does not expose it as writable there, so skip it when it is not
> +     * writable.
> +     */
> +    return field_matches(field, ID_DFR0_EL1_IDX, "PerfMon");
> +}
> +
>  /*
>   * Copy writable ID reg fields from isar.idregs[] into the KVM cpreg list,
>   * so the subsequent write_list_to_kvmstate() pushes them to KVM.
> - * Only writable fields are copied.
> + * Only writable fields are copied; fields that must not be written back
> + * (see arm_field_skip_writeback_*) are skipped.
>   * Returns -1 if any vCPU's ID reg value differs from the host value and the
>   * field is not writable.
>   */
> @@ -1357,6 +1387,14 @@ static int kvm_arm_write_idregs_to_cpreg_list(ARMCPU *cpu)
>                  continue;
>              }
>  
> +            if (arm_field_skip_writeback_always(field) ||
> +                (!arm_field_is_writable(field) &&
> +                 arm_field_skip_writeback_if_not_writable(field))) {
> +                /* Never write this field back; keep KVM's value. */
> +                writable_mask &= ~field_mask;
> +                continue;
> +            }
> +
>              if (!arm_field_is_writable(field)) {
>                  error_report("%s.%s is not writable: host=0x%" PRIx64
>                               ", requested=0x%" PRIx64,
v9 has changed the way new field values are stored. writable field props
and non writable field props could have different setters. A non
writable field setter just needs to check the new value is the same as
the current host value. No need to record the new value in the hashtable
for further commitment.

Eric