Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported

[Richard Henderson <richard.henderson@linaro.org>]

On 8/12/25 17:33, Philippe Mathieu-Daudé wrote:
>>>>> -    if (kvm_arm_pmu_supported()) {
>>>>> +    if (host_cpu_feature_supported(ARM_FEATURE_PMU, false)) {
>>>>
>>>> Why is false correct here?  Alternately, in the next patch, why is it correct to pass 
>>>> true for the EL2 test?
>>>
>>> I think I copied to KVM the HVF use, adapted on top of:
>>> https://lore.kernel.org/qemu-devel/20250808070137.48716-12- mohamed@unpredictable.fr/
>>>
>>>>
>>>> What is the purpose of the can_emulate parameter at all?
>>>
>>> When using split-accel on pre-M3, we might emulate EL2:
>>>
>>>         |   feat            |    can_emulate   |    retval
>>>         +   ----            +      -----       +     ----
>>  > M1/M2  |  ARM_FEATURE_EL2         false            false> M1/M2  | 
>> ARM_FEATURE_EL2         true             true
>>> M3/M4  |  ARM_FEATURE_EL2         any              true
>>
>> For example in hvf.c:
>>
>> static bool hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>> {
>>      ...
>>      if (host_cpu_feature_supported(ARM_FEATURE_EL2, true)) {
>>          ahcf->features |= 1ULL << ARM_FEATURE_EL2;
>>      }
>>
>> and then only when split-accel is not enabled:
>>
>> hv_return_t hvf_arch_vm_create(MachineState *ms, uint32_t pa_range)
>> {
>>      ...
>>      if (host_cpu_feature_supported(ARM_FEATURE_EL2, false)) {
>>          ret = hv_vm_config_set_el2_enabled(config, true);
>>          if (ret != HV_SUCCESS) {
>>              goto cleanup;
>>          }
>>      }
>>
> 
> What I'm looking for:
> 
> - Is this feature supported BY HW?
> 
>    -> hw_init_feature
> 
> - Is this feature supported BY SW?
> 
>    -> sw_init_feature
> 
> - Is this feature supported BY ANY?
> 
>    -> do smth with feature
> 
> With split-accel, this isn't specific to HVF/ARM.
> 
> I can use a tri-state enum { ANY, HW, SW }.

My point, I guess, is:  tcg_enabled() appears to be the only correct setting for 
can_emulate, and since that's the case, it's clearer to not have the parameter and simply 
test can_emulate within any subroutines.


r~