target/arm/kvm: Unify kvm_arm_FOO() API

[PATCH-for-9.0 05/16] target/arm/kvm: Have kvm_arm_sve_get_vls take a ARMCPU argument

Posted by Philippe Mathieu-Daudé 1 year ago

Unify the "kvm_arm.h" API: All functions related to ARM vCPUs
take a ARMCPU* argument. Use the CPU() QOM cast macro When
calling the generic vCPU API from "sysemu/kvm.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/kvm_arm.h | 6 +++---
 target/arm/cpu64.c   | 2 +-
 target/arm/kvm.c     | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 6fb8a5f67e..84f87f5ed7 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -129,13 +129,13 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray);
 
 /**
  * kvm_arm_sve_get_vls:
- * @cs: CPUState
+ * @cpu: ARMCPU
  *
  * Get all the SVE vector lengths supported by the KVM host, setting
  * the bits corresponding to their length in quadwords minus one
  * (vq - 1) up to ARM_MAX_VQ.  Return the resulting map.
  */
-uint32_t kvm_arm_sve_get_vls(CPUState *cs);
+uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu);
 
 /**
  * kvm_arm_set_cpu_features_from_host:
@@ -278,7 +278,7 @@ static inline void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
     g_assert_not_reached();
 }
 
-static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
+static inline uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
 {
     g_assert_not_reached();
 }
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 1e9c6c85ae..8e30a7993e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -66,7 +66,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
      */
     if (kvm_enabled()) {
         if (kvm_arm_sve_supported()) {
-            cpu->sve_vq.supported = kvm_arm_sve_get_vls(CPU(cpu));
+            cpu->sve_vq.supported = kvm_arm_sve_get_vls(cpu);
             vq_supported = cpu->sve_vq.supported;
         } else {
             assert(!cpu_isar_feature(aa64_sve, cpu));
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 71833a845a..766a077bcf 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1803,7 +1803,7 @@ bool kvm_arm_sve_supported(void)
 
 QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
 
-uint32_t kvm_arm_sve_get_vls(CPUState *cs)
+uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
 {
     /* Only call this function if kvm_arm_sve_supported() returns true. */
     static uint64_t vls[KVM_ARM64_SVE_VLS_WORDS];
-- 
2.41.0

Re: [PATCH-for-9.0 05/16] target/arm/kvm: Have kvm_arm_sve_get_vls take a ARMCPU argument

Posted by Gavin Shan 1 year ago

Hi Phil,

On 11/24/23 05:35, Philippe Mathieu-Daudé wrote:
> Unify the "kvm_arm.h" API: All functions related to ARM vCPUs
> take a ARMCPU* argument. Use the CPU() QOM cast macro When
> calling the generic vCPU API from "sysemu/kvm.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/arm/kvm_arm.h | 6 +++---
>   target/arm/cpu64.c   | 2 +-
>   target/arm/kvm.c     | 2 +-
>   3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 6fb8a5f67e..84f87f5ed7 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -129,13 +129,13 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray);
>   
>   /**
>    * kvm_arm_sve_get_vls:
> - * @cs: CPUState
> + * @cpu: ARMCPU
>    *
>    * Get all the SVE vector lengths supported by the KVM host, setting
>    * the bits corresponding to their length in quadwords minus one
>    * (vq - 1) up to ARM_MAX_VQ.  Return the resulting map.
>    */
> -uint32_t kvm_arm_sve_get_vls(CPUState *cs);
> +uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu);
>   

Either @cs or @cpu isn't dereferenced in kvm_arm_sve_get_vls(). So I guess
the argument can be simply droped?

>   /**
>    * kvm_arm_set_cpu_features_from_host:
> @@ -278,7 +278,7 @@ static inline void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
>       g_assert_not_reached();
>   }
>   
> -static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> +static inline uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
>   {
>       g_assert_not_reached();
>   }
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 1e9c6c85ae..8e30a7993e 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -66,7 +66,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp)
>        */
>       if (kvm_enabled()) {
>           if (kvm_arm_sve_supported()) {
> -            cpu->sve_vq.supported = kvm_arm_sve_get_vls(CPU(cpu));
> +            cpu->sve_vq.supported = kvm_arm_sve_get_vls(cpu);
>               vq_supported = cpu->sve_vq.supported;
>           } else {
>               assert(!cpu_isar_feature(aa64_sve, cpu));
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 71833a845a..766a077bcf 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -1803,7 +1803,7 @@ bool kvm_arm_sve_supported(void)
>   
>   QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
>   
> -uint32_t kvm_arm_sve_get_vls(CPUState *cs)
> +uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu)
>   {
>       /* Only call this function if kvm_arm_sve_supported() returns true. */
>       static uint64_t vls[KVM_ARM64_SVE_VLS_WORDS];

Thanks,
Gavin

Re: [PATCH-for-9.0 05/16] target/arm/kvm: Have kvm_arm_sve_get_vls take a ARMCPU argument

Posted by Philippe Mathieu-Daudé 12 months ago

Hi Gavin,

On 27/11/23 05:12, Gavin Shan wrote:
> Hi Phil,
> 
> On 11/24/23 05:35, Philippe Mathieu-Daudé wrote:
>> Unify the "kvm_arm.h" API: All functions related to ARM vCPUs
>> take a ARMCPU* argument. Use the CPU() QOM cast macro When
>> calling the generic vCPU API from "sysemu/kvm.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/arm/kvm_arm.h | 6 +++---
>>   target/arm/cpu64.c   | 2 +-
>>   target/arm/kvm.c     | 2 +-
>>   3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index 6fb8a5f67e..84f87f5ed7 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -129,13 +129,13 @@ void kvm_arm_destroy_scratch_host_vcpu(int 
>> *fdarray);
>>   /**
>>    * kvm_arm_sve_get_vls:
>> - * @cs: CPUState
>> + * @cpu: ARMCPU
>>    *
>>    * Get all the SVE vector lengths supported by the KVM host, setting
>>    * the bits corresponding to their length in quadwords minus one
>>    * (vq - 1) up to ARM_MAX_VQ.  Return the resulting map.
>>    */
>> -uint32_t kvm_arm_sve_get_vls(CPUState *cs);
>> +uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu);
> 
> Either @cs or @cpu isn't dereferenced in kvm_arm_sve_get_vls(). So I guess
> the argument can be simply droped?

If KVM eventually supports heterogeneous vCPUs such big.LITTLE, we'd
de-reference. But then we'd have a major rework of the code.

Peter, do you have a preference?

Thanks,

Phil.