Re: [RFC PATCH v7 11/18] arm/kvm: Initialize all writable ID registers from host

> On 26 Jul 2026, at 8:59 PM, Eric Auger <[email protected]> wrote:
> 
> !-------------------------------------------------------------------|
>  CAUTION: External Email
> 
> |-------------------------------------------------------------------!
> 
> We want to allow overwriting writable fields of some ID registers.
> However currently some of them are never touched, neither read nor
> w. Examples are CLIDR_EL1, CTR_EL0, REVIDR_EL1, MIDR_EL1.
> 
> We want to initialize them from the host value, allow overwrite
> and write back for kvm afterwards. This patch implements the
> initialization.
> 
> Introduce a new get_host_cpu_idregs() helper that gets the host
> values for all writable ID regs and store them in isar.idregs[].
> 
> Signed-off-by: Eric Auger <[email protected]>
> Signed-off-by: Cornelia Huck <[email protected]>
> 
> ---
> 
> v6 -> v7
> - remove vcpu from prototypes (Khushit)
> 
> v5 -> v6
> - do not check writable_map anymore
> ---
> target/arm/kvm.c        | 96 ++++++++++++++++++++++++++++++++++++++++-
> target/arm/trace-events |  2 +
> 2 files changed, 96 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index d270f9870e..4f9b0a559d 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -43,6 +43,7 @@
> #include "hw/acpi/ghes.h"
> #include "target/arm/gtimer.h"
> #include "migration/blocker.h"
> +#include "cpu-idregs.h"
> 
> const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>     KVM_CAP_INFO(DEVICE_CTRL),
> @@ -290,7 +291,44 @@ static int kvm_feature_idx_to_idregs_idx(int kidx)
>     return get_sysreg_idx(sysreg);
> }
> 
> -static void kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> +/*
> + * get_host_cpu_idregs: Read all the writable ID reg host values
> + *
> + * Need to be called once the writable mask has been populated
> + * Note we may want to read all the known id regs but some of them are not
> + * writable and return an error, hence the choice of reading only those which
> + * are writable. Those are also readable!
> + */
> +static int get_host_cpu_idregs(int fd, ARMHostCPUFeatures *ahcf)
> +{
> +    int err = 0;
> +    int i;
> +
> +    for (i = 0; i < NUM_ID_IDX; i++) {
> +        ARM64SysReg *sysregdesc = &arm64_id_regs[i];
> +        ARMSysRegs sysreg = id_register_sysreg[i];
> +        uint64_t *reg;
> +        int ret;
> +
> +        if (!sysregdesc->writable_mask) {
> +            continue;
> +        }
> +
> +        reg = &ahcf->isar.idregs[i];
> +        ret = read_sys_reg64(fd, reg, idregs_sysreg_to_kvm_reg(sysreg));
> +        trace_get_host_cpu_idregs(sysregdesc->name, *reg);
> +        if (ret) {
> +            error_report("%s error reading value of host %s register (%m)",
> +                         __func__, sysregdesc->name);
> +
> +            err = ret;
> +        }
> +    }
> +    return err;
> +}
> +
> +static void
> +kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
> {
>     /* Identify the feature bits corresponding to the host CPU, and
>      * fill out the ARMHostCPUClass fields accordingly. To do this
> @@ -376,6 +414,16 @@ static void kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>         SET_IDREG(&ahcf->isar, ID_AA64PFR0, 0x00000011); /* EL1&0, AArch64 only */
>         err = 0;
>     } else {
> +        /* Make sure all writable ID reg values are initialized */
> +        err |= get_host_cpu_idregs(fd, ahcf);
> +
> +        /*
> +         * temporarily override the CLIDR_EL1 value since some host values
> +         * trigger "Unified type is not implemented at level n" error in
> +         * fdt_add_cpu_nodes()
> +         */
> +        SET_IDREG(&ahcf->isar, CLIDR, 0x0);
> +
>         err |= get_host_cpu_reg(fd, ahcf, ID_AA64PFR1_EL1_IDX);
>         err |= get_host_cpu_reg(fd, ahcf, ID_AA64PFR2_EL1_IDX);
>         err |= get_host_cpu_reg(fd, ahcf, ID_AA64SMFR0_EL1_IDX);
> @@ -1161,6 +1209,34 @@ bool kvm_arm_cpu_post_load(ARMCPU *cpu)
>     return true;
> }
> 
> +/*
> + * Copy writable ID regs from isar.idregs[] to cpreg_list
> + * in case their value differs from the original init cpreg value
> + */
> +static void kvm_arm_writable_idregs_to_cpreg_list(ARMCPU *cpu)
> +{
> +    for (int i = 0; i < NUM_ID_IDX; i++) {
> +        ARM64SysReg *sysregdesc = &arm64_id_regs[i];
> +        ARMSysRegs sysreg = id_register_sysreg[i];
> +        uint64_t previous, new;
> +        uint64_t *cpreg;
> +
> +        if (!sysregdesc->writable_mask) {
> +            continue;
> +        }
> +
> +        cpreg = kvm_arm_get_cpreg_ptr(cpu, idregs_sysreg_to_kvm_reg(sysreg));
> +        previous = *cpreg;
> +        new = cpu->isar.idregs[i];
> +
> +        if (previous != new) {
> +            *cpreg = new;
> +            trace_kvm_arm_writable_idregs_to_cpreg_list(sysregdesc->name,
> +                                                        previous, new);
> +         }
> +    }
> +}
> +
> void kvm_arm_reset_vcpu(ARMCPU *cpu)
> {
>     int ret;
> @@ -2115,7 +2191,23 @@ int kvm_arch_init_vcpu(CPUState *cs)
>     }
>     cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
> 
> -    return kvm_arm_init_cpreg_list(cpu);
> +    ret = kvm_arm_init_cpreg_list(cpu);
> +    if (ret) {
> +        return ret;
> +    }
> +    /* overwrite writable ID regs with their updated property values */
> +    kvm_arm_writable_idregs_to_cpreg_list(cpu);
> +    ret = write_list_to_kvmstate(cpu, KVM_PUT_FULL_STATE);
> +    if (!ret) {
> +        return -1;
> +    }
> +    /*
> +     * modified values may have changed the visibility of some regs,
> +     * reinitialize the cpreg_list accordingly
> +     */
> +     ret = kvm_arm_init_cpreg_list(cpu);
nit: indent
> +
> +    return ret;
> }
> 
> int kvm_arch_destroy_vcpu(CPUState *cs)
> diff --git a/target/arm/trace-events b/target/arm/trace-events
> index 8502fb3265..c25d2a1191 100644
> --- a/target/arm/trace-events
> +++ b/target/arm/trace-events
> @@ -13,6 +13,8 @@ arm_gt_update_irq(int timer, int irqstate) "gt_update_irq: timer %d irqstate %d"
> 
> # kvm.c
> kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" is translated into 0x%"PRIx64
> +get_host_cpu_idregs(const char *name, uint64_t value) "scratch vcpu host value for %s is 0x%"PRIx64
> +kvm_arm_writable_idregs_to_cpreg_list(const char *name, uint64_t previous, uint64_t new) "%s overwrite default 0x%"PRIx64" with 0x%"PRIx64
> 
> # cpu.c
> arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64
> -- 
> 2.53.0
> 

Reviewed-by: Khushit Shah <[email protected]>