On Wed, Mar 05 2025, Richard Henderson <richard.hender...@linaro.org> wrote:
> On 3/5/25 08:38, Cornelia Huck wrote:
>> Signed-off-by: Cornelia Huck <coh...@redhat.com>
>> ---
>> target/arm/cpu-sysregs.h | 3 +++
>> target/arm/cpu64.c | 25 +++++++++++++++++++++++++
>> target/arm/kvm.c | 12 ++++++++++++
>> 3 files changed, 40 insertions(+)
>>
>> diff --git a/target/arm/cpu-sysregs.h b/target/arm/cpu-sysregs.h
>> index de09ebae91a5..54a4fadbf0c1 100644
>> --- a/target/arm/cpu-sysregs.h
>> +++ b/target/arm/cpu-sysregs.h
>> @@ -128,4 +128,7 @@ static const uint32_t id_register_sysreg[NUM_ID_IDX] = {
>> [CTR_EL0_IDX] = SYS_CTR_EL0,
>> };
>>
>> +int get_sysreg_idx(ARMSysRegs sysreg);
>> +uint64_t idregs_sysreg_to_kvm_reg(ARMSysRegs sysreg);
>> +
>> #endif /* ARM_CPU_SYSREGS_H */
>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
>> index 8188ede5cc8a..9ae78253cb34 100644
>> --- a/target/arm/cpu64.c
>> +++ b/target/arm/cpu64.c
>> @@ -736,6 +736,31 @@ static void aarch64_a53_initfn(Object *obj)
>> define_cortex_a72_a57_a53_cp_reginfo(cpu);
>> }
>>
>> +#ifdef CONFIG_KVM
>> +
>> +int get_sysreg_idx(ARMSysRegs sysreg)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < NUM_ID_IDX; i++) {
>> + if (id_register_sysreg[i] == sysreg) {
>> + return i;
>> + }
>> + }
>> + return -1;
>> +}
>> +
>> +uint64_t idregs_sysreg_to_kvm_reg(ARMSysRegs sysreg)
>> +{
>> + return ARM64_SYS_REG((sysreg & CP_REG_ARM64_SYSREG_OP0_MASK) >>
>> CP_REG_ARM64_SYSREG_OP0_SHIFT,
>> + (sysreg & CP_REG_ARM64_SYSREG_OP1_MASK) >>
>> CP_REG_ARM64_SYSREG_OP1_SHIFT,
>> + (sysreg & CP_REG_ARM64_SYSREG_CRN_MASK) >>
>> CP_REG_ARM64_SYSREG_CRN_SHIFT,
>> + (sysreg & CP_REG_ARM64_SYSREG_CRM_MASK) >>
>> CP_REG_ARM64_SYSREG_CRM_SHIFT,
>> + (sysreg & CP_REG_ARM64_SYSREG_OP2_MASK) >>
>> CP_REG_ARM64_SYSREG_OP2_SHIFT);
>> +}
>> +
>> +#endif
>
> Why are these here, with an ifdef, instead of in kvm.c?
>
> Rather than a loop over an array, you could do
>
> #define DEF(NAME, OP0, OP1, CRN, CRM, OP2) \
> case SYS_##NAME: return NAME##_IDX;
>
> int get_sysreg_idx(ARMSysRegs sysreg)
> {
> switch (sysreg) {
> #include "cpu-sysregs.h.inc"
> }
> g_assert_not_reached();
> }
>
> #undef DEF
Ok, now I see where you're going with the DEF(...) magic.
>
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index da30bdbb2349..2381c87e4ba1 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -246,6 +246,18 @@ static bool kvm_arm_pauth_supported(void)
>> kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
>> }
>>
>> +/* read a sysreg value and store it in the idregs */
>> +static int get_host_cpu_reg(int fd, ARMHostCPUFeatures *ahcf,
>> ARMIDRegisterIdx index)
>> +{
>> + uint64_t *reg;
>> + int ret;
>> +
>> + reg = &ahcf->isar.idregs[index];
>> + ret = read_sys_reg64(fd, reg,
>> +
>> idregs_sysreg_to_kvm_reg(id_register_sysreg[index]));
>> + return ret;
>> +}
>
> Surely this patch doesn't compile by itself,
> because this will Werror for the unused function.
Hm, I'm pretty positive that I did intermediate compiles while sorting
the patches here... let me see if I can improve this.
