Skip to contentSkip to conversation

KVM: arm64: selftests: Cover the ITS MOVALL command

PATCH Applied Fuad Tabba 2 participants 5 messages Oct 1 – Nov 3

Subsystems (3)
lore
Share & tools
lore

Fuad Tabba

author reviewer
#0 lore raw

v2 cover KVM: arm64: selftests: Cover the ITS MOVALL command

Hi folks,

Changes since v1 [1]:
- Make the LPIs pending before they're moved, since pending state is what MOVALL moves. (Marc)
- Put the test in vgic_lpi_stress, which now builds a VM per test.

    (Marc)

- Test MOVI as well.
- Rebased on kvmarm/next.

This series tests MOVI and MOVALL on pending LPIs, in vgic_lpi_stress. The MOVALL case passes only with "KVM: arm64: vgic-its: Fix MOVALL handling of source redistributor", which is in kvmarm/fixes. Under QEMU with pKVM, VHE and nVHE hosts, it failed all 21 runs without the fix, and the test passed all 21 with it and all 5 on top of Oliver's series [2]. MOVI is there because it's what Linux uses to change an LPI's affinity.

The test can also catch the stale LR that Oliver's "KVM: arm64: vgic:
Move IRQ migrations out of vgic_prune_ap_list()" fixes, since the vCPU the LPI is moved from can keep it in its list registers during the move. I've only seen that in the MOVALL case without the MOVALL fix, where 13 of the 21 runs took the LPI on that vCPU. I couldn't reproduce it with MOVI, with the fix, or in 70 runs in a Linux VM on an M4 with Ubuntu's 6.8 kernel, which predates the MOVALL bug. Oliver, this may be the pending-LPI migration test you mentioned.

Turns out the test does run on an M4, unlike what I said in v1, since KVM emulates the ITS whether or not the host has one.

Based on kvmarm/next (8c00199d322b9).

Cheers,
/fuad

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/

Fuad Tabba (3):
KVM: arm64: selftests: Add MOVI and MOVALL commands to the ITS library
KVM: arm64: selftests: Build a VM per test in vgic_lpi_stress
KVM: arm64: selftests: Test MOVI and MOVALL on a pending LPI

 .../selftests/kvm/arm64/vgic_lpi_stress.c     | 258 ++++++++++++++++--
 .../selftests/kvm/include/arm64/gic_v3_its.h  |   2 +
 .../selftests/kvm/lib/arm64/gic_v3_its.c      |  28 ++
 3 files changed, 265 insertions(+), 23 deletions(-)

base-commit: 8c00199d322b9e5e936bb0ff624ac95b01c740fe

Signature · 1 line
2.39.5
author reviewer
#1 lore raw

PATCH v2 1/3 KVM: arm64: selftests: Add MOVI and MOVALL commands to the ITS library

Add its_send_movi_cmd() and its_send_movall_cmd() for a test that moves a pending LPI. MOVALL takes both redistributors as processor numbers, as its_send_mapc_cmd() and its_send_sync_cmd() do, and the new its_encode_target2() puts the second one in RDbase2, in the command's fourth doubleword.

Assisted-by LLM
---
 .../selftests/kvm/include/arm64/gic_v3_its.h  |  2 ++
 .../selftests/kvm/lib/arm64/gic_v3_its.c      | 28 +++++++++++++++++++
 2 files changed, 30 insertions(+)
tools/testing/selftests/kvm/include/arm64/gic_v3_its.h +2 -0
index a43a407e2d5c1..e30a658fbd1c3 100644
@@ -14,6 +14,8 @@ void its_send_mapc_cmd(void *cmdq_base, u32 vcpu_id, u32 collection_id, bool val
 void its_send_mapti_cmd(void *cmdq_base, u32 device_id, u32 event_id,
 			u32 collection_id, u32 intid);
 void its_send_invall_cmd(void *cmdq_base, u32 collection_id);
+void its_send_movi_cmd(void *cmdq_base, u32 device_id, u32 event_id, u32 collection_id);
+void its_send_movall_cmd(void *cmdq_base, u32 from_vcpu, u32 to_vcpu);
 void its_send_sync_cmd(void *cmdq_base, u32 vcpu_id);
 
 #endif // __SELFTESTS_GIC_V3_ITS_H__
tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c +28 -0
index 1188b578121dd..811d858f5def6 100644
@@ -159,6 +159,11 @@ static void its_encode_target(struct its_cmd_block *cmd, u64 target_addr)
 	its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16);
 }
 
+static void its_encode_target2(struct its_cmd_block *cmd, u64 target_addr)
+{
+	its_mask_encode(&cmd->raw_cmd[3], target_addr >> 16, 51, 16);
+}
+
 static void its_encode_collection(struct its_cmd_block *cmd, u16 col)
 {
 	its_mask_encode(&cmd->raw_cmd[2], col, 15, 0);
@@ -253,6 +258,29 @@ void its_send_invall_cmd(void *cmdq_base, u32 collection_id)
 	its_send_cmd(cmdq_base, &cmd);
 }
 
+void its_send_movi_cmd(void *cmdq_base, u32 device_id, u32 event_id, u32 collection_id)
+{
+	struct its_cmd_block cmd = {};
+
+	its_encode_cmd(&cmd, GITS_CMD_MOVI);
+	its_encode_devid(&cmd, device_id);
+	its_encode_event_id(&cmd, event_id);
+	its_encode_collection(&cmd, collection_id);
+
+	its_send_cmd(cmdq_base, &cmd);
+}
+
+void its_send_movall_cmd(void *cmdq_base, u32 from_vcpu, u32 to_vcpu)
+{
+	struct its_cmd_block cmd = {};
+
+	its_encode_cmd(&cmd, GITS_CMD_MOVALL);
+	its_encode_target(&cmd, procnum_to_rdbase(from_vcpu));
+	its_encode_target2(&cmd, procnum_to_rdbase(to_vcpu));
+
+	its_send_cmd(cmdq_base, &cmd);
+}
+
 void its_send_sync_cmd(void *cmdq_base, u32 vcpu_id)
 {
 	struct its_cmd_block cmd = {};
Signature · 1 line
2.39.5
author reviewer
#2 lore raw

PATCH v2 2/3 KVM: arm64: selftests: Build a VM per test in vgic_lpi_stress

In reply to Fuad Tabba

vgic_lpi_stress runs its one test, the stress test, in a VM built from the command-line configuration. Build the VM per test instead, from the test's own configuration, guest code and IRQ handler, so that other ITS tests can go in the same file.

The command-line configuration becomes the stress test's own (stress_args), and each run destroys its thread barrier so the next test can initialise it again. The guest's wait for the stop request moves to guest_wait_for_stop(), for the other tests to share.

No functional change intended.

---
 .../selftests/kvm/arm64/vgic_lpi_stress.c     | 64 ++++++++++++-------
 1 file changed, 41 insertions(+), 23 deletions(-)
tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c +41 -23
index 9a69c439a0f8f..63c725b6b156c 100644
@@ -29,7 +29,7 @@ static struct kvm_vm *vm;
 static struct kvm_vcpu **vcpus;
 static int its_fd;
 
-static struct test_data {
+struct test_data {
 	bool		request_vcpus_stop;
 	u32		nr_cpus;
 	u32		nr_devices;
@@ -43,7 +43,11 @@ static struct test_data {
 
 	gpa_t		lpi_prop_table;
 	gpa_t		lpi_pend_tables;
-} test_data =  {
+};
+
+static struct test_data test_data;
+
+static struct test_data stress_args = {
 	.nr_cpus	= 1,
 	.nr_devices	= 1,
 	.nr_event_ids	= 16,
@@ -124,13 +128,8 @@ static void guest_setup_gic(void)
 		its_send_sync_cmd(test_data.cmdq_base_va, cpuid);
 }
 
-static void guest_code(size_t nr_lpis)
+static void guest_wait_for_stop(void)
 {
-	guest_setup_gic();
-	local_irq_enable();
-
-	GUEST_SYNC(0);
-
 	/*
 	 * Don't use WFI here to avoid blocking the vCPU thread indefinitely and
 	 * never getting the stop signal.
@@ -141,6 +140,16 @@ static void guest_code(size_t nr_lpis)
 	GUEST_DONE();
 }
 
+static void guest_stress_code(size_t nr_lpis)
+{
+	guest_setup_gic();
+	local_irq_enable();
+
+	GUEST_SYNC(0);
+
+	guest_wait_for_stop();
+}
+
 static void setup_memslot(void)
 {
 	size_t pages;
@@ -297,7 +306,7 @@ static void report_stats(struct timespec delta)
 	pr_info("Rate: %.2f LPIs/sec\n", nr_lpis / time);
 }
 
-static void run_test(void)
+static void run_stress_test(void)
 {
 	u32 nr_devices = test_data.nr_devices;
 	u32 nr_vcpus = test_data.nr_cpus;
@@ -329,10 +338,12 @@ static void run_test(void)
 	for (i = 0; i < nr_vcpus; i++)
 		kvm_pthread_join(vcpu_threads[i], NULL);
 
+	pthread_barrier_destroy(&test_setup_barrier);
+
 	report_stats(delta);
 }
 
-static void setup_vm(void)
+static void setup_vm(void *guest_code, handler_fn irq_handler)
 {
 	int i;
 
@@ -345,7 +356,7 @@ static void setup_vm(void)
 	for (i = 0; i < test_data.nr_cpus; i++)
 		vcpu_init_descriptor_tables(vcpus[i]);
 
-	vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, guest_irq_handler);
+	vm_install_exception_handler(vm, VECTOR_IRQ_CURRENT, irq_handler);
 
 	setup_memslot();
 
@@ -361,12 +372,23 @@ static void destroy_vm(void)
 	free(vcpus);
 }
 
+static void test_lpi_stress(void)
+{
+	test_data = stress_args;
+
+	setup_vm(guest_stress_code, guest_irq_handler);
+
+	run_stress_test();
+
+	destroy_vm();
+}
+
 static void pr_usage(const char *name)
 {
 	pr_info("%s [-v NR_VCPUS] [-d NR_DEVICES] [-e NR_EVENTS] [-i ITERS] -h\n", name);
-	pr_info("  -v:\tnumber of vCPUs (default: %u)\n", test_data.nr_cpus);
-	pr_info("  -d:\tnumber of devices (default: %u)\n", test_data.nr_devices);
-	pr_info("  -e:\tnumber of event IDs per device (default: %u)\n", test_data.nr_event_ids);
+	pr_info("  -v:\tnumber of vCPUs (default: %u)\n", stress_args.nr_cpus);
+	pr_info("  -d:\tnumber of devices (default: %u)\n", stress_args.nr_devices);
+	pr_info("  -e:\tnumber of event IDs per device (default: %u)\n", stress_args.nr_event_ids);
 	pr_info("  -i:\tnumber of iterations (default: %lu)\n", nr_iterations);
 }
 
@@ -380,13 +402,13 @@ int main(int argc, char **argv)
 	while ((c = getopt(argc, argv, "hv:d:e:i:")) != -1) {
 		switch (c) {
 		case 'v':
-			test_data.nr_cpus = atoi(optarg);
+			stress_args.nr_cpus = atoi(optarg);
 			break;
 		case 'd':
-			test_data.nr_devices = atoi(optarg);
+			stress_args.nr_devices = atoi(optarg);
 			break;
 		case 'e':
-			test_data.nr_event_ids = atoi(optarg);
+			stress_args.nr_event_ids = atoi(optarg);
 			break;
 		case 'i':
 			nr_iterations = strtoul(optarg, NULL, 0);
@@ -398,16 +420,12 @@ int main(int argc, char **argv)
 		}
 	}
 
-	nr_threads = test_data.nr_cpus + test_data.nr_devices;
+	nr_threads = stress_args.nr_cpus + stress_args.nr_devices;
 	if (nr_threads > get_nprocs())
 		pr_info("WARNING: running %u threads on %d CPUs; performance is degraded.\n",
 			 nr_threads, get_nprocs());
 
-	setup_vm();
-
-	run_test();
-
-	destroy_vm();
+	test_lpi_stress();
 
 	return 0;
 }
Signature · 1 line
2.39.5
author reviewer
#3 lore raw

PATCH v2 3/3 KVM: arm64: selftests: Test MOVI and MOVALL on a pending LPI

In reply to Fuad Tabba

MOVI and MOVALL move the pending state of LPIs between redistributors, so a test of either has to make its LPI pending before the move.

Add one to vgic_lpi_stress. vCPU0 and vCPU1 keep IRQs masked while LPI A is signalled to vCPU0 and LPI B to vCPU1, and each waits until ICC_HPPIR1_EL1 shows its LPI pending at the CPU interface. vCPU1 then moves A to vCPU2, with MOVI or with MAPC and MOVALL of vCPU0's collection. Once unmasked, A must be taken on vCPU2 only and B on vCPU1 only.

KVM retargets A at the MAPC already, so the MOVALL case catches a MOVALL that moves too much: without commit 751f4641560b ("KVM: arm64:
vgic-its: Fix MOVALL handling of source redistributor"), B goes to vCPU2 as well. vCPU1 issues the commands, so its own ITS accesses, which exit to KVM, move a wrongly retargeted B off it before it unmasks.

Both cases run after the stress test, so a kernel that fails them still gets the stress run, and the command-line options still configure only the stress test.

Assisted-by LLM
---
 .../selftests/kvm/arm64/vgic_lpi_stress.c     | 196 +++++++++++++++++-
 1 file changed, 195 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c +195 -1
index 63c725b6b156c..8e4047fa0b6ab 100644
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * vgic_lpi_stress - Stress test for KVM's ITS emulation
+ * vgic_lpi_stress - Tests for KVM's ITS emulation
  *
  * Copyright (c) 2024 Google LLC
  */
@@ -29,11 +29,17 @@ static struct kvm_vm *vm;
 static struct kvm_vcpu **vcpus;
 static int its_fd;
 
+enum move_cmd {
+	MOVE_MOVI,
+	MOVE_MAPC_MOVALL,
+};
+
 struct test_data {
 	bool		request_vcpus_stop;
 	u32		nr_cpus;
 	u32		nr_devices;
 	u32		nr_event_ids;
+	enum move_cmd	move_cmd;
 
 	gpa_t		device_table;
 	gpa_t		collection_table;
@@ -383,6 +389,191 @@ static void test_lpi_stress(void)
 	destroy_vm();
 }
 
+/*
+ * Pending move: A is pending at vCPU0's CPU interface and B at vCPU1's, both
+ * vCPUs with IRQs masked, when vCPU1 moves A to vCPU2. Once unmasked, A must
+ * be taken on vCPU2 only, and B on vCPU1 only.
+ */
+#define MOVE_NR_VCPUS		3
+#define MOVE_NR_LPIS		2
+#define MOVE_DEVICE		0
+
+/* guest_setup_its_mappings() maps event N to vCPU N's collection */
+#define LPI_A			0
+#define LPI_B			1
+#define SRC_VCPU		0
+#define B_VCPU			1
+#define DST_VCPU		2
+
+static atomic_uint lpi_taken[MOVE_NR_VCPUS][MOVE_NR_LPIS];
+static atomic_bool lpi_a_pending, lpi_a_moved, vcpu_unmasked[MOVE_NR_VCPUS];
+
+static void guest_count_irq_handler(struct ex_regs *regs)
+{
+	u32 intid = gic_get_and_ack_irq();
+
+	if (intid == IAR_SPURIOUS)
+		return;
+
+	GUEST_ASSERT(intid >= GIC_LPI_OFFSET && intid < GIC_LPI_OFFSET + MOVE_NR_LPIS);
+	atomic_fetch_add(&lpi_taken[guest_get_vcpuid()][intid - GIC_LPI_OFFSET], 1);
+	gic_set_eoi(intid);
+}
+
+/* HPPIR shows an LPI pending at this CPU interface even with IRQs masked */
+static bool guest_lpi_pending(u32 lpi)
+{
+	return (read_sysreg_s(SYS_ICC_HPPIR1_EL1) & GENMASK(23, 0)) == GIC_LPI_OFFSET + lpi;
+}
+
+static void guest_wait_pending(u32 lpi)
+{
+	while (!guest_lpi_pending(lpi))
+		cpu_relax();
+}
+
+/* An LPI still pending here is taken before the host is told to check */
+static void guest_unmask(u32 lpi)
+{
+	local_irq_enable();
+
+	while (guest_lpi_pending(lpi))
+		cpu_relax();
+
+	atomic_store(&vcpu_unmasked[guest_get_vcpuid()], true);
+}
+
+static void guest_move_lpi_a(void)
+{
+	void *cmdq = test_data.cmdq_base_va;
+
+	if (test_data.move_cmd == MOVE_MOVI) {
+		its_send_movi_cmd(cmdq, MOVE_DEVICE, LPI_A, DST_VCPU);
+	} else {
+		its_send_mapc_cmd(cmdq, DST_VCPU, SRC_VCPU, true);
+		/* The GICv3 spec's MAPC description requires this SYNC before MOVALL */
+		its_send_sync_cmd(cmdq, SRC_VCPU);
+		its_send_movall_cmd(cmdq, SRC_VCPU, DST_VCPU);
+	}
+
+	/* Complete the move at both redistributors before vCPU0 unmasks */
+	its_send_sync_cmd(cmdq, SRC_VCPU);
+	its_send_sync_cmd(cmdq, DST_VCPU);
+}
+
+static void guest_pending_move_code(void)
+{
+	u32 cpuid = guest_get_vcpuid();
+
+	guest_setup_gic();
+
+	if (cpuid == DST_VCPU)
+		local_irq_enable();
+
+	GUEST_SYNC(0);
+
+	if (cpuid == SRC_VCPU) {
+		guest_wait_pending(LPI_A);
+		atomic_store(&lpi_a_pending, true);
+
+		while (!atomic_load(&lpi_a_moved))
+			cpu_relax();
+		guest_unmask(LPI_A);
+	} else if (cpuid == B_VCPU) {
+		guest_wait_pending(LPI_B);
+
+		while (!atomic_load(&lpi_a_pending))
+			cpu_relax();
+		guest_move_lpi_a();
+		atomic_store(&lpi_a_moved, true);
+		guest_unmask(LPI_B);
+	}
+
+	guest_wait_for_stop();
+}
+
+static unsigned int nr_lpis_taken(atomic_uint (*taken)[MOVE_NR_LPIS])
+{
+	unsigned int nr = 0;
+	int vcpu, lpi;
+
+	for (vcpu = 0; vcpu < MOVE_NR_VCPUS; vcpu++)
+		for (lpi = 0; lpi < MOVE_NR_LPIS; lpi++)
+			nr += atomic_load(&taken[vcpu][lpi]);
+
+	return nr;
+}
+
+static bool pending_move_done(atomic_uint (*taken)[MOVE_NR_LPIS], atomic_bool *unmasked)
+{
+	return nr_lpis_taken(taken) >= MOVE_NR_LPIS &&
+	       atomic_load(&unmasked[SRC_VCPU]) && atomic_load(&unmasked[B_VCPU]);
+}
+
+static void check_lpi_taken(atomic_uint (*taken)[MOVE_NR_LPIS], int lpi, int want_vcpu)
+{
+	const char *cmd = test_data.move_cmd == MOVE_MOVI ? "MOVI" : "MAPC+MOVALL";
+	unsigned int nr[MOVE_NR_VCPUS];
+	bool ok = true;
+	int vcpu;
+
+	for (vcpu = 0; vcpu < MOVE_NR_VCPUS; vcpu++) {
+		nr[vcpu] = atomic_load(&taken[vcpu][lpi]);
+		ok &= nr[vcpu] == (vcpu == want_vcpu);
+	}
+
+	TEST_ASSERT(ok, "%s: LPI %c taken %u/%u/%u times on vCPU0/1/2, expected once on vCPU%d",
+		    cmd, 'A' + lpi, nr[0], nr[1], nr[2], want_vcpu);
+}
+
+static void run_pending_move_test(void)
+{
+	atomic_uint (*taken)[MOVE_NR_LPIS] = addr_gva2hva(vm, (gva_t)lpi_taken);
+	atomic_bool *unmasked = addr_gva2hva(vm, (gva_t)vcpu_unmasked);
+	pthread_t vcpu_threads[MOVE_NR_VCPUS];
+	int i;
+
+	pthread_barrier_init(&test_setup_barrier, NULL, MOVE_NR_VCPUS + 1);
+
+	for (i = 0; i < MOVE_NR_VCPUS; i++)
+		kvm_pthread_create(&vcpu_threads[i], NULL, vcpu_worker_thread, vcpus[i]);
+
+	pthread_barrier_wait(&test_setup_barrier);
+
+	signal_lpi(MOVE_DEVICE, LPI_A);
+	signal_lpi(MOVE_DEVICE, LPI_B);
+
+	for (i = 0; i < 10000 && !pending_move_done(taken, unmasked); i++)
+		usleep(1000);
+
+	/* Check first: a vCPU still waiting for its LPI never sees the stop */
+	check_lpi_taken(taken, LPI_A, DST_VCPU);
+	check_lpi_taken(taken, LPI_B, B_VCPU);
+
+	write_guest_global(vm, test_data.request_vcpus_stop, true);
+
+	for (i = 0; i < MOVE_NR_VCPUS; i++)
+		kvm_pthread_join(vcpu_threads[i], NULL);
+
+	pthread_barrier_destroy(&test_setup_barrier);
+}
+
+static void test_pending_move(enum move_cmd move_cmd)
+{
+	test_data = (struct test_data) {
+		.nr_cpus	= MOVE_NR_VCPUS,
+		.nr_devices	= 1,
+		.nr_event_ids	= MOVE_NR_LPIS,
+		.move_cmd	= move_cmd,
+	};
+
+	setup_vm(guest_pending_move_code, guest_count_irq_handler);
+
+	run_pending_move_test();
+
+	destroy_vm();
+}
+
 static void pr_usage(const char *name)
 {
 	pr_info("%s [-v NR_VCPUS] [-d NR_DEVICES] [-e NR_EVENTS] [-i ITERS] -h\n", name);
@@ -427,5 +618,8 @@ int main(int argc, char **argv)
 
 	test_lpi_stress();
 
+	test_pending_move(MOVE_MOVI);
+	test_pending_move(MOVE_MAPC_MOVALL);
+
 	return 0;
 }
Signature · 1 line
2.39.5
maintainer Applied
#4 lore raw
In reply to Fuad Tabba
Quoted 9 lines · Fuad Tabba

Changes since v1 [1]:
- Make the LPIs pending before they're moved, since pending state is what MOVALL moves. (Marc)
- Put the test in vgic_lpi_stress, which now builds a VM per test.

    (Marc)

- Test MOVI as well.
- Rebased on kvmarm/next.

[...]

Applied to fixes, thanks!

[1/3] KVM: arm64: selftests: Add MOVI and MOVALL commands to the ITS library

      commit: 6e4aea4c279c1be2707e5f1d8f94a3afc408b591
[2/3] KVM: arm64: selftests: Build a VM per test in vgic_lpi_stress
      commit: 0edd1f549a1d543fce3e0f6b5c87d182575cb267
[3/3] KVM: arm64: selftests: Test MOVI and MOVALL on a pending LPI
      commit: b5d08dcf25e7a22a7f52d9806c6c6db0c3354a0b

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.