Re: [PATCH] KVM: selftests: vgic_init kvm selftests fixup
On Wed, 7 Apr 2021 15:59:37 +0200, Eric Auger wrote: > Bring some improvements/rationalization over the first version > of the vgic_init selftests: > > - ucall_init is moved in run_cpu() > - vcpu_args_set is not called as not needed > - whenever a helper is supposed to succeed, call the non "_" version > - helpers do not return -errno, instead errno is checked by the caller > - vm_gic struct is used whenever possible, as well as vm_gic_destroy > - _kvm_create_device takes an addition fd parameter Applied to kvm-arm64/vgic-5.13, thanks! [1/1] KVM: selftests: vgic_init kvm selftests fixup commit: 4cffb2df4260ed38c7ae4105f6913ad2d71a16ec Cheers, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH] KVM: selftests: vgic_init kvm selftests fixup
Hi Eric, If $SUBJECT started with 'fixup!' and otherwise matched the original subject then this could be automagically squashed in wherever the original commit already is. Anyway, Reviewed-by: Andrew Jones Thanks, drew On Wed, Apr 07, 2021 at 03:59:37PM +0200, Eric Auger wrote: > Bring some improvements/rationalization over the first version > of the vgic_init selftests: > > - ucall_init is moved in run_cpu() > - vcpu_args_set is not called as not needed > - whenever a helper is supposed to succeed, call the non "_" version > - helpers do not return -errno, instead errno is checked by the caller > - vm_gic struct is used whenever possible, as well as vm_gic_destroy > - _kvm_create_device takes an addition fd parameter > > Signed-off-by: Eric Auger > Suggested-by: Andrew Jones > > --- > > Applies on top of [PATCH v6 0/9] KVM/ARM: Some vgic fixes > and init sequence KVM selftests, put on kvm-arm64/vgic-5.13 > > The whole patchset can be found at > https://github.com/eauger/linux/tree/vgic_kvmselftests_v7 > --- > .../testing/selftests/kvm/aarch64/vgic_init.c | 275 -- > .../testing/selftests/kvm/include/kvm_util.h | 2 +- > tools/testing/selftests/kvm/lib/kvm_util.c| 30 +- > 3 files changed, 136 insertions(+), 171 deletions(-) > > diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c > b/tools/testing/selftests/kvm/aarch64/vgic_init.c > index be1a7c0d0527..682e753fdc59 100644 > --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c > +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c > @@ -27,7 +27,7 @@ struct vm_gic { > int gic_fd; > }; > > -int max_ipa_bits; > +static int max_ipa_bits; > > /* helper to access a redistributor register */ > static int access_redist_reg(int gicv3_fd, int vcpu, int offset, > @@ -51,12 +51,8 @@ static void guest_code(void) > /* we don't want to assert on run execution, hence that helper */ > static int run_vcpu(struct kvm_vm *vm, uint32_t vcpuid) > { > - int ret; > - > - vcpu_args_set(vm, vcpuid, 1); > - ret = _vcpu_ioctl(vm, vcpuid, KVM_RUN, NULL); > - get_ucall(vm, vcpuid, NULL); > - > + ucall_init(vm, NULL); > + int ret = _vcpu_ioctl(vm, vcpuid, KVM_RUN, NULL); > if (ret) > return -errno; > return 0; > @@ -68,7 +64,6 @@ static struct vm_gic vm_gic_create(void) > > v.vm = vm_create_default_with_vcpus(NR_VCPUS, 0, 0, guest_code, NULL); > v.gic_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, false); > - TEST_ASSERT(v.gic_fd > 0, "GICv3 device created"); > > return v; > } > @@ -91,66 +86,62 @@ static void subtest_dist_rdist(struct vm_gic *v) > uint64_t addr; > > /* Check existing group/attributes */ > - ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > - KVM_VGIC_V3_ADDR_TYPE_DIST); > - TEST_ASSERT(!ret, "KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_DIST > supported"); > + kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_V3_ADDR_TYPE_DIST); > > - ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > - KVM_VGIC_V3_ADDR_TYPE_REDIST); > - TEST_ASSERT(!ret, > "KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST supported"); > + kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, > + KVM_VGIC_V3_ADDR_TYPE_REDIST); > > /* check non existing attribute */ > ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, 0); > - TEST_ASSERT(ret == -ENXIO, "attribute not supported"); > + TEST_ASSERT(ret && errno == ENXIO, "attribute not supported"); > > /* misaligned DIST and REDIST address settings */ > addr = 0x1000; > ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >KVM_VGIC_V3_ADDR_TYPE_DIST, , true); > - TEST_ASSERT(ret == -EINVAL, "GICv3 dist base not 64kB aligned"); > + TEST_ASSERT(ret && errno == EINVAL, "GICv3 dist base not 64kB aligned"); > > ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >KVM_VGIC_V3_ADDR_TYPE_REDIST, , true); > - TEST_ASSERT(ret == -EINVAL, "GICv3 redist base not 64kB aligned"); > + TEST_ASSERT(ret && errno == EINVAL, "GICv3 redist base not 64kB > aligned"); > > /* out of range address */ > if (max_ipa_bits) { > addr = 1ULL << max_ipa_bits; > ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >KVM_VGIC_V3_ADDR_TYPE_DIST, , > true); > - TEST_ASSERT(ret == -E2BIG, "dist address beyond IPA limit"); > + TEST_ASSERT(ret && errno == E2BIG, "dist address beyond IPA > limit"); > > ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, >
[PATCH] KVM: selftests: vgic_init kvm selftests fixup
Bring some improvements/rationalization over the first version of the vgic_init selftests: - ucall_init is moved in run_cpu() - vcpu_args_set is not called as not needed - whenever a helper is supposed to succeed, call the non "_" version - helpers do not return -errno, instead errno is checked by the caller - vm_gic struct is used whenever possible, as well as vm_gic_destroy - _kvm_create_device takes an addition fd parameter Signed-off-by: Eric Auger Suggested-by: Andrew Jones --- Applies on top of [PATCH v6 0/9] KVM/ARM: Some vgic fixes and init sequence KVM selftests, put on kvm-arm64/vgic-5.13 The whole patchset can be found at https://github.com/eauger/linux/tree/vgic_kvmselftests_v7 --- .../testing/selftests/kvm/aarch64/vgic_init.c | 275 -- .../testing/selftests/kvm/include/kvm_util.h | 2 +- tools/testing/selftests/kvm/lib/kvm_util.c| 30 +- 3 files changed, 136 insertions(+), 171 deletions(-) diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c index be1a7c0d0527..682e753fdc59 100644 --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c @@ -27,7 +27,7 @@ struct vm_gic { int gic_fd; }; -int max_ipa_bits; +static int max_ipa_bits; /* helper to access a redistributor register */ static int access_redist_reg(int gicv3_fd, int vcpu, int offset, @@ -51,12 +51,8 @@ static void guest_code(void) /* we don't want to assert on run execution, hence that helper */ static int run_vcpu(struct kvm_vm *vm, uint32_t vcpuid) { - int ret; - - vcpu_args_set(vm, vcpuid, 1); - ret = _vcpu_ioctl(vm, vcpuid, KVM_RUN, NULL); - get_ucall(vm, vcpuid, NULL); - + ucall_init(vm, NULL); + int ret = _vcpu_ioctl(vm, vcpuid, KVM_RUN, NULL); if (ret) return -errno; return 0; @@ -68,7 +64,6 @@ static struct vm_gic vm_gic_create(void) v.vm = vm_create_default_with_vcpus(NR_VCPUS, 0, 0, guest_code, NULL); v.gic_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, false); - TEST_ASSERT(v.gic_fd > 0, "GICv3 device created"); return v; } @@ -91,66 +86,62 @@ static void subtest_dist_rdist(struct vm_gic *v) uint64_t addr; /* Check existing group/attributes */ - ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, - KVM_VGIC_V3_ADDR_TYPE_DIST); - TEST_ASSERT(!ret, "KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_DIST supported"); + kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_V3_ADDR_TYPE_DIST); - ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, - KVM_VGIC_V3_ADDR_TYPE_REDIST); - TEST_ASSERT(!ret, "KVM_DEV_ARM_VGIC_GRP_ADDR/KVM_VGIC_V3_ADDR_TYPE_REDIST supported"); + kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, + KVM_VGIC_V3_ADDR_TYPE_REDIST); /* check non existing attribute */ ret = _kvm_device_check_attr(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, 0); - TEST_ASSERT(ret == -ENXIO, "attribute not supported"); + TEST_ASSERT(ret && errno == ENXIO, "attribute not supported"); /* misaligned DIST and REDIST address settings */ addr = 0x1000; ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_DIST, , true); - TEST_ASSERT(ret == -EINVAL, "GICv3 dist base not 64kB aligned"); + TEST_ASSERT(ret && errno == EINVAL, "GICv3 dist base not 64kB aligned"); ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST, , true); - TEST_ASSERT(ret == -EINVAL, "GICv3 redist base not 64kB aligned"); + TEST_ASSERT(ret && errno == EINVAL, "GICv3 redist base not 64kB aligned"); /* out of range address */ if (max_ipa_bits) { addr = 1ULL << max_ipa_bits; ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_DIST, , true); - TEST_ASSERT(ret == -E2BIG, "dist address beyond IPA limit"); + TEST_ASSERT(ret && errno == E2BIG, "dist address beyond IPA limit"); ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, KVM_VGIC_V3_ADDR_TYPE_REDIST, , true); - TEST_ASSERT(ret == -E2BIG, "redist address beyond IPA limit"); + TEST_ASSERT(ret && errno == E2BIG, "redist address beyond IPA limit"); } /* set REDIST base address @0x0*/ addr = 0x0; - ret = _kvm_device_access(v->gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR, -KVM_VGIC_V3_ADDR_TYPE_REDIST, ,