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

Philippe Mathieu-Daudé posted 11 patches 3 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, Alexander Graf <agraf@csgraf.de>, Mads Ynddal <mads@ynddal.dk>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Philippe Mathieu-Daudé 3 months ago
Use the generic host_cpu_feature_supported() helper to
check for the PMU feature support. This will allow to
expand to non-KVM accelerators such HVF.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/kvm_arm.h  | 13 -------------
 target/arm/cpu.c      |  4 ++--
 target/arm/kvm-stub.c |  5 -----
 target/arm/kvm.c      |  9 ++-------
 4 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 6a9b6374a6d..364578c50d6 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -177,14 +177,6 @@ void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp);
  */
 bool kvm_arm_aarch32_supported(void);
 
-/**
- * kvm_arm_pmu_supported:
- *
- * Returns: true if KVM can enable the PMU
- * and false otherwise.
- */
-bool kvm_arm_pmu_supported(void);
-
 /**
  * kvm_arm_sve_supported:
  *
@@ -212,11 +204,6 @@ static inline bool kvm_arm_aarch32_supported(void)
     return false;
 }
 
-static inline bool kvm_arm_pmu_supported(void)
-{
-    return false;
-}
-
 static inline bool kvm_arm_sve_supported(void)
 {
     return false;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1dc2a8330d8..c78a3c9cda8 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1564,8 +1564,8 @@ static void arm_set_pmu(Object *obj, bool value, Error **errp)
     ARMCPU *cpu = ARM_CPU(obj);
 
     if (value) {
-        if (kvm_enabled() && !kvm_arm_pmu_supported()) {
-            error_setg(errp, "'pmu' feature not supported by KVM on this host");
+        if (host_cpu_feature_supported(ARM_FEATURE_PMU, false)) {
+            error_setg(errp, "'pmu' feature not supported by this host accelerator");
             return;
         }
         set_feature(&cpu->env, ARM_FEATURE_PMU);
diff --git a/target/arm/kvm-stub.c b/target/arm/kvm-stub.c
index c93462c5b9b..3beb336416d 100644
--- a/target/arm/kvm-stub.c
+++ b/target/arm/kvm-stub.c
@@ -32,11 +32,6 @@ bool kvm_arm_aarch32_supported(void)
     return false;
 }
 
-bool kvm_arm_pmu_supported(void)
-{
-    return false;
-}
-
 bool kvm_arm_sve_supported(void)
 {
     return false;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 82853e68d8d..0fe0f89f931 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -288,7 +288,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                              1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
     }
 
-    if (kvm_arm_pmu_supported()) {
+    if (host_cpu_feature_supported(ARM_FEATURE_PMU, false)) {
         init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
         pmu_supported = true;
         features |= 1ULL << ARM_FEATURE_PMU;
@@ -506,11 +506,6 @@ void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
                                     "Set off to disable KVM steal time.");
 }
 
-bool kvm_arm_pmu_supported(void)
-{
-    return kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
-}
-
 int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
 {
     KVMState *s = KVM_STATE(ms->accelerator);
@@ -1783,7 +1778,7 @@ bool arm_hw_accel_cpu_feature_supported(enum arm_features feat, bool can_emulate
     case ARM_FEATURE_GENERIC_TIMER:
         return true;
     case ARM_FEATURE_PMU:
-        return kvm_arm_pmu_supported();
+        return kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
     case ARM_FEATURE_EL2:
         return kvm_arm_el2_supported();
     case ARM_FEATURE_EL3:
-- 
2.49.0


Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Richard Henderson 3 months ago
On 8/12/25 03:06, Philippe Mathieu-Daudé wrote:
> +++ b/target/arm/kvm.c
> @@ -288,7 +288,7 @@ static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>                                1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>       }
>   
> -    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?

What is the purpose of the can_emulate parameter at all?


r~

Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Philippe Mathieu-Daudé 3 months ago
On 12/8/25 02:48, Richard Henderson wrote:
> On 8/12/25 03:06, Philippe Mathieu-Daudé wrote:
>> +++ b/target/arm/kvm.c
>> @@ -288,7 +288,7 @@ static bool 
>> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>                                1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>>       }
>> -    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

Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Philippe Mathieu-Daudé 3 months ago
On 12/8/25 06:49, Philippe Mathieu-Daudé wrote:
> On 12/8/25 02:48, Richard Henderson wrote:
>> On 8/12/25 03:06, Philippe Mathieu-Daudé wrote:
>>> +++ b/target/arm/kvm.c
>>> @@ -288,7 +288,7 @@ static bool 
>>> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>                                1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>>>       }
>>> -    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;
         }
     }


Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Philippe Mathieu-Daudé 3 months ago
On 12/8/25 08:03, Philippe Mathieu-Daudé wrote:
> On 12/8/25 06:49, Philippe Mathieu-Daudé wrote:
>> On 12/8/25 02:48, Richard Henderson wrote:
>>> On 8/12/25 03:06, Philippe Mathieu-Daudé wrote:
>>>> +++ b/target/arm/kvm.c
>>>> @@ -288,7 +288,7 @@ static bool 
>>>> kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>>>>                                1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
>>>>       }
>>>> -    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 }.

Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Richard Henderson 3 months ago
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~

Re: [RFC PATCH 07/11] target/arm: Replace kvm_arm_pmu_supported by host_cpu_feature_supported
Posted by Philippe Mathieu-Daudé 3 months ago
On 12/8/25 14:42, Richard Henderson wrote:
> 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.

Got it!