| Message ID | 20231010142453.224369-4-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm/kvm: use kvm_{get,set}_one_reg | expand |
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, ®); > + 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",
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, ®); > + 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
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, ®); >> + 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 --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, ®); + 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",
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(-)