On 9/25/26 3:27 PM, Khushit Shah wrote: > >> On 16 Sep 2026, at 8:15 PM, Eric Auger <[email protected]> wrote: >> >> !-------------------------------------------------------------------| >> CAUTION: External Email >> >> |-------------------------------------------------------------------! >> >> kvm_arm_get_host_cpu_features() already retrieves the values of some >> host id registers to analyze the available vcpu features. >> >> Eventually we want to be able to allow the user to change the value >> of writable ID reg fields. So we need to store the initial value of all >> writable ID regs into isar.idregs[]. >> >> Introduce a new get_host_cpu_idregs() helper that populates isar.idregs[] >> and call it from kvm_arm_get_host_cpu_features. >> >> This patch has no impact on cpreg values. isar.idregs[] is only >> used as a temporary storage for register values to be updated in place >> later. Also it does not handle the writeback of isar.idregs[] into cpreg >> value list after user update. This will be handled separately. >> >> Note the host values are retrieved from the scratch vcpu initialized with >> all the available capabilities exposed by KVM, independently on the >> final choice made by the suer. >> >> Signed-off-by: Eric Auger <[email protected]> >> Signed-off-by: Cornelia Huck <[email protected]> > Hi Eric, > > As we discussed, in v10, there will be an option to also read all the ID > registers, not only writable. yes we can do that. reviewers may advocate the leave as is though as the only call site in this series will use writable mode. >> --- >> v8 -> v9: >> - improve the commit msg. Only initialize in this patch and move the >> write back to cpreg value list in a different patch. >> - Do not hack CLIDR here. The correct way to fix that temporarily is to >> do that in the machine instead. >> >> v7 -> v8 >> - fix indent and added Khushit's R-b >> >> v6 -> v7 >> - remove vcpu from prototypes (Khushit) >> >> v5 -> v6 >> - do not check writable_map anymore >> --- >> target/arm/kvm.c | 43 ++++++++++++++++++++++++++++++++++++++++- >> target/arm/trace-events | 2 ++ >> 2 files changed, 44 insertions(+), 1 deletion(-) >> >> diff --git a/target/arm/kvm.c b/target/arm/kvm.c >> index c6eac41991..958b25e550 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,9 @@ 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); >> + > Perhaps if we read “all” the ID registers, we should be able to get rid of > all the manual reads below. Yes if all are read, we can remove this. But for now this is depends on the host capabilities, better to leave as is I think. > >> 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); >> 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 > ^^ leaked into this commit. yup. Thanks! Eric > > Warm Regards, > Khushit > >> # cpu.c >> arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64 >> -- >> 2.53.0 >>
