On Fri, May 23 2025, Shameerali Kolothum Thodi
<shameerali.kolothum.th...@huawei.com> wrote:
>> -----Original Message-----
>> From: Cornelia Huck <coh...@redhat.com>
>> Sent: Monday, April 14, 2025 5:39 PM
>> To: eric.auger....@gmail.com; eric.au...@redhat.com; qemu-
>> de...@nongnu.org; qemu-...@nongnu.org; kvm...@lists.linux.dev;
>> peter.mayd...@linaro.org; richard.hender...@linaro.org;
>> alex.ben...@linaro.org; m...@kernel.org; oliver.up...@linux.dev;
>> seb...@redhat.com; Shameerali Kolothum Thodi
>> <shameerali.kolothum.th...@huawei.com>; arm...@redhat.com;
>> berra...@redhat.com; abolo...@redhat.com; jdene...@redhat.com
>> Cc: ag...@csgraf.de; shahu...@redhat.com; mark.rutl...@arm.com;
>> phi...@linaro.org; pbonz...@redhat.com; Cornelia Huck
>> <coh...@redhat.com>
>> Subject: [PATCH v3 06/10] arm/kvm: Allow reading all the writable ID
>> registers
>>
>> From: Eric Auger <eric.au...@redhat.com>
>>
>> At the moment kvm_arm_get_host_cpu_features() reads a subset of the
>> ID regs. As we want to introduce properties for all writable ID reg
>> fields, we want more genericity and read more default host register
>> values.
>>
>> Introduce a new get_host_cpu_idregs() helper and add a new exhaustive
>> boolean parameter to kvm_arm_get_host_cpu_features() and
>> kvm_arm_set_cpu_features_from_host() to select the right behavior.
>> The host cpu model will keep the legacy behavior unless the writable
>> id register interface is available.
>>
>> A writable_map IdRegMap is introduced in the CPU object. A subsequent
>> patch will populate it.
>>
>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>> Signed-off-by: Cornelia Huck <coh...@redhat.com>
>> ---
>> target/arm/cpu-sysregs.h | 2 ++
>> target/arm/cpu.h | 3 ++
>> target/arm/cpu64.c | 2 +-
>> target/arm/kvm.c | 78 ++++++++++++++++++++++++++++++++++++++--
>> target/arm/kvm_arm.h | 9 +++--
>> target/arm/trace-events | 1 +
>> 6 files changed, 89 insertions(+), 6 deletions(-)
(...)
>> +/*
>> + * 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(ARMCPU *cpu, 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 = sysregdesc->sysreg;
>> + uint64_t writable_mask = cpu->writable_map-
>> >regs[idregs_idx_to_kvm_idx(i)];
>> + uint64_t *reg;
>> + int ret;
>> +
>> + if (!writable_mask) {
>> + continue;
>> + }
>> +
>> + reg = &ahcf->isar.idregs[i];
>> + ret = read_sys_reg64(fd, reg, idregs_sysreg_to_kvm_reg(sysreg));
>
> I think we can use get_host_cpu_reg() here.
We'd need to rejiggle the tracing a bit, though.
Looking at this, maybe we have too many cross-indexing variants, I'll
check and see if we can simplify that a bit.
>
>> + 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 bool
>> +kvm_arm_get_host_cpu_features(ARMCPU *cpu, ARMHostCPUFeatures
>> *ahcf,
>> + bool exhaustive)
>> {
>> /* Identify the feature bits corresponding to the host CPU, and
>> * fill out the ARMHostCPUClass fields accordingly. To do this
>> @@ -398,6 +465,11 @@ static bool
>> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>> err |= get_host_cpu_reg(fd, ahcf, ID_DFR1_EL1_IDX);
>> err |= get_host_cpu_reg(fd, ahcf, ID_MMFR5_EL1_IDX);
>>
>> + /* Make sure writable ID reg values are read */
>> + if (exhaustive) {
>> + err |= get_host_cpu_idregs(cpu, fd, ahcf);
>> + }
>
>
> Also if we do this a bit above can we avoid reading the ID registers twice
> if "exhaustive=true" ?
It's probably not performance critical as we just do this once, but
let's try that anyway.
