Re: [PATCH 02/15] arm/kvm: add accessors for storing host features into idregs

[Cornelia Huck]

On Tue, Feb 18 2025, Eric Auger <eric.auger@redhat.com> wrote:

> Hi Connie
>
> On 2/7/25 12:02 PM, Cornelia Huck wrote:
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  target/arm/cpu-sysregs.h |  3 +++
>>  target/arm/cpu64.c       | 25 +++++++++++++++++++++++++
>>  target/arm/kvm.c         | 30 ++++++++++++++++++++++++++++++
>>  3 files changed, 58 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) {
> I agree with Richard that if we could get rid of this linear search it
> would be nicer.

FWIW, I have a local branch which reworks the lookup and the macros that
is currently in the "it compiles" stage. Hopefully progressing to the
"it works" stage (provided I'm not preempted by other things again.)

>> +            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
>> +
>>  static void aarch64_host_initfn(Object *obj)
>>  {
>>  #if defined(CONFIG_KVM)
>> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
>> index da30bdbb2349..3b8bb5661f2b 100644
>> --- a/target/arm/kvm.c
>> +++ b/target/arm/kvm.c
>> @@ -246,6 +246,36 @@ static bool kvm_arm_pauth_supported(void)
>>              kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
>>  }
>>  
>> +/* read a 32b sysreg value and store it in the idregs */
>> +static int get_host_cpu_reg32(int fd, ARMHostCPUFeatures *ahcf, ARMSysRegs 
>> sysreg)
> those are defined as static but there is no user so this will break
> compilation locally

Hm, didn't see that, but noticed a few other places that are b0rken. I
think I need to fiddle with my config to broaden compilation coverage.