diff mbox series

[v2,3/3] arm/kvm: convert to read_sys_reg64

Message ID 20231010142453.224369-4-cohuck@redhat.com (mailing list archive)
State New, archived
Headers show
Series arm/kvm: use kvm_{get,set}_one_reg | expand

Commit Message

Cornelia Huck Oct. 10, 2023, 2:24 p.m. UTC
We can use read_sys_reg64 to get the SVE_VLS register instead of
calling GET_ONE_REG directly.

Suggested-by: Gavin Shan <gshan@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/arm/kvm64.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Gavin Shan Oct. 11, 2023, 12:02 a.m. UTC | #1
On 10/11/23 00:24, Cornelia Huck wrote:
> We can use read_sys_reg64 to get the SVE_VLS register instead of
> calling GET_ONE_REG directly.
> 
> Suggested-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>   target/arm/kvm64.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 558c0b88dd69..d40c89a84752 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>               .target = -1,
>               .features[0] = (1 << KVM_ARM_VCPU_SVE),
>           };
> -        struct kvm_one_reg reg = {
> -            .id = KVM_REG_ARM64_SVE_VLS,
> -            .addr = (uint64_t)&vls[0],
> -        };
>           int fdarray[3], ret;
>   
>           probed = true;
> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>               error_report("failed to create scratch VCPU with SVE enabled");
>               abort();
>           }
> -        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &reg);
> +        ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
>           kvm_arm_destroy_scratch_host_vcpu(fdarray);
>           if (ret) {
>               error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
Peter Maydell Oct. 17, 2023, 12:09 p.m. UTC | #2
On Tue, 10 Oct 2023 at 15:25, Cornelia Huck <cohuck@redhat.com> wrote:
>
> We can use read_sys_reg64 to get the SVE_VLS register instead of
> calling GET_ONE_REG directly.
>
> Suggested-by: Gavin Shan <gshan@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  target/arm/kvm64.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 558c0b88dd69..d40c89a84752 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>              .target = -1,
>              .features[0] = (1 << KVM_ARM_VCPU_SVE),
>          };
> -        struct kvm_one_reg reg = {
> -            .id = KVM_REG_ARM64_SVE_VLS,
> -            .addr = (uint64_t)&vls[0],
> -        };
>          int fdarray[3], ret;
>
>          probed = true;
> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>              error_report("failed to create scratch VCPU with SVE enabled");
>              abort();
>          }
> -        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &reg);
> +        ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
>          kvm_arm_destroy_scratch_host_vcpu(fdarray);
>          if (ret) {
>              error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",

read_sys_reg64() asserts that the register you're trying to
read is 64 bits, but KVM_REG_ARM64_SVE_VLS is not, it's 512 bits:

#define KVM_REG_ARM64_SVE_VLS           (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
                                         KVM_REG_SIZE_U512 | 0xffff)

So this change would trip the assert on a host where SVE
is supported and enabled.

thanks
-- PMM
Cornelia Huck Oct. 17, 2023, 3:28 p.m. UTC | #3
On Tue, Oct 17 2023, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Tue, 10 Oct 2023 at 15:25, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> We can use read_sys_reg64 to get the SVE_VLS register instead of
>> calling GET_ONE_REG directly.
>>
>> Suggested-by: Gavin Shan <gshan@redhat.com>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  target/arm/kvm64.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index 558c0b88dd69..d40c89a84752 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>>              .target = -1,
>>              .features[0] = (1 << KVM_ARM_VCPU_SVE),
>>          };
>> -        struct kvm_one_reg reg = {
>> -            .id = KVM_REG_ARM64_SVE_VLS,
>> -            .addr = (uint64_t)&vls[0],
>> -        };
>>          int fdarray[3], ret;
>>
>>          probed = true;
>> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>>              error_report("failed to create scratch VCPU with SVE enabled");
>>              abort();
>>          }
>> -        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &reg);
>> +        ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
>>          kvm_arm_destroy_scratch_host_vcpu(fdarray);
>>          if (ret) {
>>              error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
>
> read_sys_reg64() asserts that the register you're trying to
> read is 64 bits, but KVM_REG_ARM64_SVE_VLS is not, it's 512 bits:
>
> #define KVM_REG_ARM64_SVE_VLS           (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
>                                          KVM_REG_SIZE_U512 | 0xffff)
>
> So this change would trip the assert on a host where SVE
> is supported and enabled.

Whoops, it seems that I misread this. (And my test environment didn't
have that enabled...)
diff mbox series

Patch

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 558c0b88dd69..d40c89a84752 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -500,10 +500,6 @@  uint32_t kvm_arm_sve_get_vls(CPUState *cs)
             .target = -1,
             .features[0] = (1 << KVM_ARM_VCPU_SVE),
         };
-        struct kvm_one_reg reg = {
-            .id = KVM_REG_ARM64_SVE_VLS,
-            .addr = (uint64_t)&vls[0],
-        };
         int fdarray[3], ret;
 
         probed = true;
@@ -512,7 +508,7 @@  uint32_t kvm_arm_sve_get_vls(CPUState *cs)
             error_report("failed to create scratch VCPU with SVE enabled");
             abort();
         }
-        ret = ioctl(fdarray[2], KVM_GET_ONE_REG, &reg);
+        ret = read_sys_reg64(fdarray[2], &vls[0], KVM_REG_ARM64_SVE_VLS);
         kvm_arm_destroy_scratch_host_vcpu(fdarray);
         if (ret) {
             error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",