Hi, On 9/25/26 2:37 PM, Khushit Shah wrote: > >> On 16 Sep 2026, at 8:15 PM, Eric Auger <[email protected]> wrote: >> >> !-------------------------------------------------------------------| >> CAUTION: External Email >> >> |-------------------------------------------------------------------! >> >> From: Cornelia Huck <[email protected]> >> >> Add an helper to retrieve the writable id reg bitmask. > s/an/a >> then retrieve the writable_map array and dispatch it in the writable_map > s/writable_map/writable_mask >> fields of arm64_id_regs[] elements. > s/fields/field
fixed >> Signed-off-by: Eric Auger <[email protected]> >> Signed-off-by: Cornelia Huck <[email protected]> >> >> --- >> >> v7 -> v8 >> - added kvm_enabled() >> >> v5 -> v6 >> - use arm64_id_regs[].writable_map >> - squashed 2 commits (helper + user) >> - removed stub >> --- >> target/arm/kvm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 61 insertions(+) >> >> diff --git a/target/arm/kvm.c b/target/arm/kvm.c >> index ed99be7fd8..c6eac41991 100644 >> --- a/target/arm/kvm.c >> +++ b/target/arm/kvm.c >> @@ -28,6 +28,7 @@ >> #include "kvm_arm.h" >> #include "cpu.h" >> #include "cpu-sysregs.h" >> +#include "cpu-idregs.h" >> #include "trace.h" >> #include "internals.h" >> #include "hw/pci/pci.h" >> @@ -51,6 +52,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = >> { >> static bool cap_has_mp_state; >> static bool cap_has_inject_serror_esr; >> static bool cap_has_inject_ext_dabt; >> +static bool writable_map_dispatched; >> >> /** >> * ARMHostCPUFeatures: information about the host CPU (identified >> @@ -273,6 +275,21 @@ static uint32_t kvm_arm_sve_get_vls(int fd) >> return vls[0] & MAKE_64BIT_MASK(0, ARM_MAX_VQ); >> } >> >> +static int kvm_feature_idx_to_idregs_idx(int kidx) >> +{ >> + int op1, crm, op2; >> + ARMSysRegs sysreg; >> + >> + op1 = kidx / 64; >> + if (op1 == 2) { >> + op1 = 3; >> + } >> + crm = (kidx % 64) / 8; >> + op2 = kidx % 8; >> + sysreg = ENCODE_ID_REG(3, op1, 0, crm, op2); >> + return get_sysreg_idx(sysreg); >> +} >> + >> static void kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> { >> /* Identify the feature bits corresponding to the host CPU, and >> @@ -480,9 +497,53 @@ static void >> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> ahcf->features = features; >> } >> >> +static int kvm_arm_get_writable_id_regs(uint64_t *idregmap) >> +{ >> + int cap_writable_id_regs; >> + struct reg_mask_range range = { >> + .range = KVM_ARM_FEATURE_ID_RANGE, >> + .reserved = {0}, >> + .addr = (uint64_t)idregmap, >> + }; >> + >> + cap_writable_id_regs = >> + kvm_check_extension(kvm_state, >> KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES); >> + >> + if (!cap_writable_id_regs || >> + !(cap_writable_id_regs & (1 << KVM_ARM_FEATURE_ID_RANGE))) { >> + return -ENOSYS; >> + } >> + >> + if (kvm_vm_ioctl(kvm_state, KVM_ARM_GET_REG_WRITABLE_MASKS, &range)) { >> + return -errno; >> + } > I think just doing return kvm_vm_ioctl(kvm_state, > KVM_ARM_GET_REG_WRITABLE_MASKS, &range) is enough. sure >> + return 0; >> +} >> + >> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) >> { >> CPUARMState *env = &cpu->env; >> + uint64_t *writable_map; >> + >> + writable_map = g_new(uint64_t, KVM_ARM_FEATURE_ID_RANGE_SIZE); >> + >> + if (kvm_enabled() && !writable_map_dispatched && >> + !kvm_arm_get_writable_id_regs(writable_map)) { >> + for (int i = 0; i < KVM_ARM_FEATURE_ID_RANGE_SIZE; i++) { >> + uint64_t mask = writable_map[i]; >> + >> + if (mask) { >> + int idx = kvm_feature_idx_to_idregs_idx(i); >> + >> + if (idx < 0 || idx > ARRAY_SIZE(arm64_id_regs)) { > Bug: >= > Also, this condition will never be true, as get_sysreg_idx() asserts out on > an unknown ID register. Which probably > should not happen. removed > >> + continue; >> + } >> + arm64_id_regs[idx].writable_mask = mask; >> + } >> + } >> + writable_map_dispatched = true; > What is the need of writable_map_dispatched, why can’t we use > arm_host_cpu_features.dtb_compatible as below? Using a dtb related condition to decide whether we shall do the writable mask dispatch looks far-fetched to me, although it would effectively work. I would rather keep it as it and wait for other comments if any. Thanks Eric > > Warm Regards, > Khushit >> + } >> + g_free(writable_map); >> >> if (!arm_host_cpu_features.dtb_compatible) { >> kvm_arm_get_host_cpu_features(&arm_host_cpu_features); >> -- >> 2.53.0 >>
