> On 24 Sep 2026, at 3:27 PM, Eric Auger <[email protected]> wrote:
>
> !-------------------------------------------------------------------|
> CAUTION: External Email
>
> |-------------------------------------------------------------------!
>
>
>
> 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 <[email protected]>
>> ---
>> 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.
Yep, this will fail during write back.
>> +}
>> +
>> +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.
I don’t like the idea of different setters. I will not expose fields which
conflicts with legacy options like GPA/APA which conflicts with pauth.
But we do lose some introspectability with this option.
Thanks,
Khushit
>
> Eric
