On 9/28/21 8:47 PM, Ricardo Koller wrote:
> As a preparation for the next commits which will add some tests for
> GICv2, make aarch64/vgic_init GIC version agnostic. Add a new generic
> run_tests function(gic_dev_type) that starts all applicable tests using
> GICv3 or GICv2. GICv2 tests are attempted if GICv3 is not available in
> the system. There are currently no GICv2 tests, but the test passes now
> in GICv2 systems.
>
> Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
> ---
> .../testing/selftests/kvm/aarch64/vgic_init.c | 156 +++++++++++-------
> 1 file changed, 95 insertions(+), 61 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/aarch64/vgic_init.c b/tools/testing/selftests/kvm/aarch64/vgic_init.c
> index 623f31a14326..896a29f2503d 100644
> --- a/tools/testing/selftests/kvm/aarch64/vgic_init.c
> +++ b/tools/testing/selftests/kvm/aarch64/vgic_init.c
> @@ -22,6 +22,9 @@
>
> #define GICR_TYPER 0x8
>
> +#define VGIC_DEV_IS_V2(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V2)
> +#define VGIC_DEV_IS_V3(_d) ((_d) == KVM_DEV_TYPE_ARM_VGIC_V3)
> +
> struct vm_gic {
> struct kvm_vm *vm;
> int gic_fd;
> @@ -30,8 +33,8 @@ struct vm_gic {
> static int max_ipa_bits;
>
> /* helper to access a redistributor register */
> -static int access_redist_reg(int gicv3_fd, int vcpu, int offset,
> - uint32_t *val, bool write)
> +static int access_v3_redist_reg(int gicv3_fd, int vcpu, int offset,
> + uint32_t *val, bool write)
> {
> uint64_t attr = REG_OFFSET(vcpu, offset);
>
> @@ -58,7 +61,7 @@ static int run_vcpu(struct kvm_vm *vm, uint32_t vcpuid)
> return 0;
> }
>
> -static struct vm_gic vm_gic_create(void)
> +static struct vm_gic vm_gic_v3_create(void)
> {
> struct vm_gic v;
>
> @@ -80,7 +83,7 @@ static void vm_gic_destroy(struct vm_gic *v)
> * device gets created, a legacy RDIST region is set at @0x0
> * and a DIST region is set @0x60000
> */
> -static void subtest_dist_rdist(struct vm_gic *v)
> +static void subtest_v3_dist_rdist(struct vm_gic *v)
> {
> int ret;
> uint64_t addr;
> @@ -145,7 +148,7 @@ static void subtest_dist_rdist(struct vm_gic *v)
> }
>
> /* Test the new REDIST region API */
> -static void subtest_redist_regions(struct vm_gic *v)
> +static void subtest_v3_redist_regions(struct vm_gic *v)
> {
> uint64_t addr, expected_addr;
> int ret;
> @@ -249,7 +252,7 @@ static void subtest_redist_regions(struct vm_gic *v)
> * VGIC KVM device is created and initialized before the secondary CPUs
> * get created
> */
> -static void test_vgic_then_vcpus(void)
> +static void test_v3_vgic_then_vcpus(uint32_t gic_dev_type)
> {
> struct vm_gic v;
> int ret, i;
> @@ -257,7 +260,7 @@ static void test_vgic_then_vcpus(void)
> v.vm = vm_create_default(0, 0, guest_code);
> v.gic_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);
>
> - subtest_dist_rdist(&v);
> + subtest_v3_dist_rdist(&v);
>
> /* Add the rest of the VCPUs */
> for (i = 1; i < NR_VCPUS; ++i)
> @@ -270,14 +273,14 @@ static void test_vgic_then_vcpus(void)
> }
>
> /* All the VCPUs are created before the VGIC KVM device gets initialized */
> -static void test_vcpus_then_vgic(void)
> +static void test_v3_vcpus_then_vgic(uint32_t gic_dev_type)
> {
> struct vm_gic v;
> int ret;
>
> - v = vm_gic_create();
> + v = vm_gic_v3_create();
>
> - subtest_dist_rdist(&v);
> + subtest_v3_dist_rdist(&v);
>
> ret = run_vcpu(v.vm, 3);
> TEST_ASSERT(ret == -EINVAL, "dist/rdist overlap detected on 1st vcpu run");
> @@ -285,15 +288,15 @@ static void test_vcpus_then_vgic(void)
> vm_gic_destroy(&v);
> }
>
> -static void test_new_redist_regions(void)
> +static void test_v3_new_redist_regions(void)
> {
> void *dummy = NULL;
> struct vm_gic v;
> uint64_t addr;
> int ret;
>
> - v = vm_gic_create();
> - subtest_redist_regions(&v);
> + v = vm_gic_v3_create();
> + subtest_v3_redist_regions(&v);
> kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
> KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
>
> @@ -303,8 +306,8 @@ static void test_new_redist_regions(void)
>
> /* step2 */
>
> - v = vm_gic_create();
> - subtest_redist_regions(&v);
> + v = vm_gic_v3_create();
> + subtest_v3_redist_regions(&v);
>
> addr = REDIST_REGION_ATTR_ADDR(1, 0x280000, 0, 2);
> kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
> @@ -317,8 +320,8 @@ static void test_new_redist_regions(void)
>
> /* step 3 */
>
> - v = vm_gic_create();
> - subtest_redist_regions(&v);
> + v = vm_gic_v3_create();
> + subtest_v3_redist_regions(&v);
>
> _kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
> KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, dummy, true);
> @@ -338,7 +341,7 @@ static void test_new_redist_regions(void)
> vm_gic_destroy(&v);
> }
>
> -static void test_typer_accesses(void)
> +static void test_v3_typer_accesses(void)
> {
> struct vm_gic v;
> uint64_t addr;
> @@ -351,12 +354,12 @@ static void test_typer_accesses(void)
>
> vm_vcpu_add_default(v.vm, 3, guest_code);
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(ret && errno == EINVAL, "attempting to read GICR_TYPER of non created vcpu");
>
> vm_vcpu_add_default(v.vm, 1, guest_code);
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(ret && errno == EBUSY, "read GICR_TYPER before GIC initialized");
>
> vm_vcpu_add_default(v.vm, 2, guest_code);
> @@ -365,7 +368,7 @@ static void test_typer_accesses(void)
> KVM_DEV_ARM_VGIC_CTRL_INIT, NULL, true);
>
> for (i = 0; i < NR_VCPUS ; i++) {
> - ret = access_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && !val, "read GICR_TYPER before rdist region setting");
> }
>
> @@ -374,10 +377,10 @@ static void test_typer_accesses(void)
> KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr, true);
>
> /* The 2 first rdists should be put there (vcpu 0 and 3) */
> - ret = access_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && !val, "read typer of rdist #0");
>
> - ret = access_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x310, "read typer of rdist #1");
>
> addr = REDIST_REGION_ATTR_ADDR(10, 0x100000, 0, 1);
> @@ -385,11 +388,11 @@ static void test_typer_accesses(void)
> KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr, true);
> TEST_ASSERT(ret && errno == EINVAL, "collision with previous rdist region");
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x100,
> "no redist region attached to vcpu #1 yet, last cannot be returned");
>
> - ret = access_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x200,
> "no redist region attached to vcpu #2, last cannot be returned");
>
> @@ -397,10 +400,10 @@ static void test_typer_accesses(void)
> kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
> KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr, true);
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x100, "read typer of rdist #1");
>
> - ret = access_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x210,
> "read typer of rdist #1, last properly returned");
>
> @@ -417,7 +420,7 @@ static void test_typer_accesses(void)
> * rdist region #2 @0x200000 2 rdist capacity
> * rdists: 1, 2
> */
> -static void test_last_bit_redist_regions(void)
> +static void test_v3_last_bit_redist_regions(void)
> {
> uint32_t vcpuids[] = { 0, 3, 5, 4, 1, 2 };
> struct vm_gic v;
> @@ -444,29 +447,29 @@ static void test_last_bit_redist_regions(void)
> kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
> KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION, &addr, true);
>
> - ret = access_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x000, "read typer of rdist #0");
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x100, "read typer of rdist #1");
>
> - ret = access_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x200, "read typer of rdist #2");
>
> - ret = access_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x310, "read typer of rdist #3");
>
> - ret = access_redist_reg(v.gic_fd, 5, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 5, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x500, "read typer of rdist #5");
>
> - ret = access_redist_reg(v.gic_fd, 4, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 4, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x410, "read typer of rdist #4");
>
> vm_gic_destroy(&v);
> }
>
> /* Test last bit with legacy region */
> -static void test_last_bit_single_rdist(void)
> +static void test_v3_last_bit_single_rdist(void)
> {
> uint32_t vcpuids[] = { 0, 3, 5, 4, 1, 2 };
> struct vm_gic v;
> @@ -485,28 +488,32 @@ static void test_last_bit_single_rdist(void)
> kvm_device_access(v.gic_fd, KVM_DEV_ARM_VGIC_GRP_ADDR,
> KVM_VGIC_V3_ADDR_TYPE_REDIST, &addr, true);
>
> - ret = access_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 0, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x000, "read typer of rdist #0");
>
> - ret = access_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 3, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x300, "read typer of rdist #1");
>
> - ret = access_redist_reg(v.gic_fd, 5, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 5, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x500, "read typer of rdist #2");
>
> - ret = access_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 1, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x100, "read typer of rdist #3");
>
> - ret = access_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> + ret = access_v3_redist_reg(v.gic_fd, 2, GICR_TYPER, &val, false);
> TEST_ASSERT(!ret && val == 0x210, "read typer of rdist #3");
>
> vm_gic_destroy(&v);
> }
>
> -void test_kvm_device(void)
> +/*
> + * Returns 0 if it's possible to create GIC device of a given type (V2 or V3).
> + */
> +int test_kvm_device(uint32_t gic_dev_type)
> {
> struct vm_gic v;
> int ret, fd;
> + uint32_t other;
>
> v.vm = vm_create_default_with_vcpus(NR_VCPUS, 0, 0, guest_code, NULL);
>
> @@ -514,38 +521,65 @@ void test_kvm_device(void)
> ret = _kvm_create_device(v.vm, 0, true, &fd);
> TEST_ASSERT(ret && errno == ENODEV, "unsupported device");
>
> - /* trial mode with VGIC_V3 device */
> - ret = _kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, true, &fd);
> - if (ret) {
> - print_skip("GICv3 not supported");
> - exit(KSFT_SKIP);
> - }
> - v.gic_fd = kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, false);
> + /* trial mode */
> + ret = _kvm_create_device(v.vm, gic_dev_type, true, &fd);
> + if (ret)
> + return ret;
> + v.gic_fd = kvm_create_device(v.vm, gic_dev_type, false);
> +
> + ret = _kvm_create_device(v.vm, gic_dev_type, false, &fd);
> + TEST_ASSERT(ret && errno == EEXIST, "create GIC device twice");
>
> - ret = _kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, false, &fd);
> - TEST_ASSERT(ret && errno == EEXIST, "create GICv3 device twice");
> + kvm_create_device(v.vm, gic_dev_type, true);
>
> - kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V3, true);
> + /* try to create the other gic_dev_type */
> + other = VGIC_DEV_IS_V2(gic_dev_type) ? KVM_DEV_TYPE_ARM_VGIC_V3
> + : KVM_DEV_TYPE_ARM_VGIC_V2;
>
> - if (!_kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V2, true, &fd)) {
> - ret = _kvm_create_device(v.vm, KVM_DEV_TYPE_ARM_VGIC_V2, false, &fd);
> - TEST_ASSERT(ret && errno == EINVAL, "create GICv2 while v3 exists");
> + if (!_kvm_create_device(v.vm, other, true, &fd)) {
> + ret = _kvm_create_device(v.vm, other, false, &fd);
> + TEST_ASSERT(ret && errno == EINVAL,
> + "create GIC device while other version exists");
> }
>
> vm_gic_destroy(&v);
> +
> + return 0;
> +}
> +
> +void run_tests(uint32_t gic_dev_type)
> +{
> + if (VGIC_DEV_IS_V3(gic_dev_type)) {
> + test_v3_vcpus_then_vgic(gic_dev_type);
> + test_v3_vgic_then_vcpus(gic_dev_type);
> + test_v3_new_redist_regions();
> + test_v3_typer_accesses();
> + test_v3_last_bit_redist_regions();
> + test_v3_last_bit_single_rdist();
> + }
> }
>
> int main(int ac, char **av)
> {
> + int ret;
> +
> max_ipa_bits = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
>
> - test_kvm_device();
> - test_vcpus_then_vgic();
> - test_vgic_then_vcpus();
> - test_new_redist_regions();
> - test_typer_accesses();
> - test_last_bit_redist_regions();
> - test_last_bit_single_rdist();
> + ret = test_kvm_device(KVM_DEV_TYPE_ARM_VGIC_V3);
> + if (!ret) {
> + pr_info("Running GIC_v3 tests.\n");
> + run_tests(KVM_DEV_TYPE_ARM_VGIC_V3);
> + return 0;
If the GICv3 supports compat with GICv2, I think you could be able to
run both tests consecutively. So maybe don't return?
Besides looks good
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
Eric
> + }
> +
> + ret = test_kvm_device(KVM_DEV_TYPE_ARM_VGIC_V2);
> + if (!ret) {
> + pr_info("Running GIC_v2 tests.\n");
> + run_tests(KVM_DEV_TYPE_ARM_VGIC_V2);
> + return 0;
> + }
>
> + print_skip("No GICv2 nor GICv3 support");
> + exit(KSFT_SKIP);
> return 0;
> }
