* [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress
@ 2025-11-14 14:39 Maximilian Dittgen
2025-11-14 15:42 ` Marc Zyngier
2025-11-19 13:57 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Maximilian Dittgen
0 siblings, 2 replies; 6+ messages in thread
From: Maximilian Dittgen @ 2025-11-14 14:39 UTC (permalink / raw)
To: maz, oliver.upton
Cc: pbonzini, shuah, linux-arm-kernel, kvmarm, linux-kselftest, kvm,
mdittgen, lilitj, nh-open-source
vgic_lpi_stress sends MAPTI and MAPC commands during guest GIC
setup to map interrupt events to ITT entries and collection IDs
to redistributors, respectively.
Theoretically, we have no guarantee that the ITS will
finish handling these mapping commands before the selftest
calls KVM_SIGNAL_MSI to inject LPIs to the guest. If LPIs
are injected before ITS mapping completes, the ITS cannot
properly pass the interrupt on to the redistributor.
In practice, KVM processes ITS commands synchronously, so
SYNC calls are functionally unnecessary and ignored in
vgic_its_handle_command().
However, selftests should test based on ARM specification and
be blind to KVM-specific implementation optimizations. Thus,
we must update the test to be architecturally compliant and
logically correct.
Fix by adding a SYNC command to the selftests ITS library,
then calling SYNC after ITS mapping to ensure mapping
completes before signal_lpi() writes to GITS_TRANSLATER.
This patch depends on commit a24f7afce048 ("KVM: selftests:
fix MAPC RDbase target formatting in vgic_lpi_stress"), which
is queued in kvmarm/fixes.
Signed-off-by: Maximilian Dittgen <mdittgen@amazon.de>
---
Validated by the following debug logging to the GITS_CMD_SYNC handler
in vgic_its_handle_command():
kvm_info("ITS SYNC command: %016llx %016llx %016llx %016llx\n",
its_cmd[0], its_cmd[1], its_cmd[2], its_cmd[3]);
Initialized a selftest guest with 4 vCPUs by:
./vgic_lpi_stress -v 4
Confirmed that an ITS SYNC was successfully called for all 4 vCPUs:
kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000000000 0000000000000000
kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000010000 0000000000000000
kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000020000 0000000000000000
kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000030000 0000000000000000
---
tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c | 4 ++++
.../testing/selftests/kvm/include/arm64/gic_v3_its.h | 1 +
tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c | 11 +++++++++++
3 files changed, 16 insertions(+)
diff --git a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
index 687d04463983..e857a605f577 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
@@ -118,6 +118,10 @@ static void guest_setup_gic(void)
guest_setup_its_mappings();
guest_invalidate_all_rdists();
+
+ /* SYNC to ensure ITS setup is complete */
+ for (cpuid = 0; cpuid < test_data.nr_cpus; cpuid++)
+ its_send_sync_cmd(test_data.cmdq_base_va, cpuid);
}
static void guest_code(size_t nr_lpis)
diff --git a/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h b/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
index 3722ed9c8f96..58feef3eb386 100644
--- a/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
+++ b/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
@@ -15,5 +15,6 @@ 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_sync_cmd(void *cmdq_base, u32 vcpu_id);
#endif // __SELFTESTS_GIC_V3_ITS_H__
diff --git a/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c b/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
index 0e2f8ed90f30..d9ee331074ea 100644
--- a/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
+++ b/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
@@ -253,3 +253,14 @@ void its_send_invall_cmd(void *cmdq_base, u32 collection_id)
its_send_cmd(cmdq_base, &cmd);
}
+
+void its_send_sync_cmd(void *cmdq_base, u32 vcpu_id)
+{
+ struct its_cmd_block cmd = {};
+
+ its_encode_cmd(&cmd, GITS_CMD_SYNC);
+ its_encode_target(&cmd, procnum_to_rdbase(vcpu_id));
+
+ its_send_cmd(cmdq_base, &cmd);
+}
+
--
2.50.1 (Apple Git-155)
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Christof Hellmis
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress
2025-11-14 14:39 [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
@ 2025-11-14 15:42 ` Marc Zyngier
2025-11-14 20:26 ` Oliver Upton
2025-11-19 13:57 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Maximilian Dittgen
1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2025-11-14 15:42 UTC (permalink / raw)
To: Maximilian Dittgen
Cc: oliver.upton, pbonzini, shuah, linux-arm-kernel, kvmarm,
linux-kselftest, kvm, lilitj, nh-open-source
On Fri, 14 Nov 2025 14:39:02 +0000,
Maximilian Dittgen <mdittgen@amazon.de> wrote:
>
> vgic_lpi_stress sends MAPTI and MAPC commands during guest GIC
> setup to map interrupt events to ITT entries and collection IDs
> to redistributors, respectively.
>
> Theoretically, we have no guarantee that the ITS will
> finish handling these mapping commands before the selftest
> calls KVM_SIGNAL_MSI to inject LPIs to the guest. If LPIs
> are injected before ITS mapping completes, the ITS cannot
> properly pass the interrupt on to the redistributor.
>
> In practice, KVM processes ITS commands synchronously, so
> SYNC calls are functionally unnecessary and ignored in
> vgic_its_handle_command().
>
> However, selftests should test based on ARM specification and
> be blind to KVM-specific implementation optimizations. Thus,
That's hardly an optimisation. Quite the opposite, really. This is an
implementation choice to make it simple (well, simple for an ITS
emulation...) and not racy.
> we must update the test to be architecturally compliant and
> logically correct.
>
> Fix by adding a SYNC command to the selftests ITS library,
> then calling SYNC after ITS mapping to ensure mapping
> completes before signal_lpi() writes to GITS_TRANSLATER.
>
> This patch depends on commit a24f7afce048 ("KVM: selftests:
> fix MAPC RDbase target formatting in vgic_lpi_stress"), which
> is queued in kvmarm/fixes.
This sentence has no place in a commit message.
>
Signed-off-by: Maximilian Dittgen <mdittgen@amazon.de>
> ---
> Validated by the following debug logging to the GITS_CMD_SYNC handler
> in vgic_its_handle_command():
>
> kvm_info("ITS SYNC command: %016llx %016llx %016llx %016llx\n",
> its_cmd[0], its_cmd[1], its_cmd[2], its_cmd[3]);
>
> Initialized a selftest guest with 4 vCPUs by:
>
> ./vgic_lpi_stress -v 4
>
> Confirmed that an ITS SYNC was successfully called for all 4 vCPUs:
>
> kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000000000 0000000000000000
> kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000010000 0000000000000000
> kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000020000 0000000000000000
> kvm [5094]: ITS SYNC command: 0000000000000005 0000000000000000 0000000000030000 0000000000000000
> ---
> tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c | 4 ++++
> .../testing/selftests/kvm/include/arm64/gic_v3_its.h | 1 +
> tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c | 11 +++++++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> index 687d04463983..e857a605f577 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> @@ -118,6 +118,10 @@ static void guest_setup_gic(void)
>
> guest_setup_its_mappings();
> guest_invalidate_all_rdists();
> +
> + /* SYNC to ensure ITS setup is complete */
> + for (cpuid = 0; cpuid < test_data.nr_cpus; cpuid++)
> + its_send_sync_cmd(test_data.cmdq_base_va, cpuid);
You are making an implementation assumption here. There is nothing in
the spec that says that the GICR_TYPER.Processor_Number associated
with a given CPU is the same thing as the CPU number that the
selftests infrastructure give you.
It turns out that KVM makes it so that vcpu_id and Processor_Number
are the same thing. But given the blurb above about sticking to the
architecture and not relying on implementation details, this is not
what I'd expect.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress
2025-11-14 15:42 ` Marc Zyngier
@ 2025-11-14 20:26 ` Oliver Upton
0 siblings, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2025-11-14 20:26 UTC (permalink / raw)
To: Marc Zyngier
Cc: Maximilian Dittgen, oliver.upton, pbonzini, shuah,
linux-arm-kernel, kvmarm, linux-kselftest, kvm, lilitj,
nh-open-source
On Fri, Nov 14, 2025 at 03:42:30PM +0000, Marc Zyngier wrote:
> On Fri, 14 Nov 2025 14:39:02 +0000,
> Maximilian Dittgen <mdittgen@amazon.de> wrote:
> > diff --git a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> > index 687d04463983..e857a605f577 100644
> > --- a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> > +++ b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
> > @@ -118,6 +118,10 @@ static void guest_setup_gic(void)
> >
> > guest_setup_its_mappings();
> > guest_invalidate_all_rdists();
> > +
> > + /* SYNC to ensure ITS setup is complete */
> > + for (cpuid = 0; cpuid < test_data.nr_cpus; cpuid++)
> > + its_send_sync_cmd(test_data.cmdq_base_va, cpuid);
>
> You are making an implementation assumption here. There is nothing in
> the spec that says that the GICR_TYPER.Processor_Number associated
> with a given CPU is the same thing as the CPU number that the
> selftests infrastructure give you.
This is already rather widely assumed in the selftests GIC library as
well as this test (ex. guest_invalidate_all_rdists()). I'd be happy with
an assertion in gicv3_cpu_init() that ensures
GICR_TYPER.Processor_Number == cpu.
Thanks,
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number
2025-11-14 14:39 [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
2025-11-14 15:42 ` Marc Zyngier
@ 2025-11-19 13:57 ` Maximilian Dittgen
2025-11-19 13:57 ` [PATCH v2 2/2] KVM: selftests: SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
2025-11-19 22:35 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Oliver Upton
1 sibling, 2 replies; 6+ messages in thread
From: Maximilian Dittgen @ 2025-11-19 13:57 UTC (permalink / raw)
To: maz, oliver.upton
Cc: pbonzini, shuah, linux-arm-kernel, kvmarm, linux-kselftest, kvm,
mdittgen, lilitj, nh-open-source
The selftests GIC library and tests assume that the
GICR_TYPER.Processor_number associated with a given CPU is the same as
the CPU's selftest index.
Since this assumption is not guaranteed by specification, add an assert
in gicv3_cpu_init() that validates this is true.
Signed-off-by: Maximilian Dittgen <mdittgen@amazon.de>
---
tools/testing/selftests/kvm/lib/arm64/gic_v3.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/kvm/lib/arm64/gic_v3.c b/tools/testing/selftests/kvm/lib/arm64/gic_v3.c
index 66d05506f78b..50158d08117b 100644
--- a/tools/testing/selftests/kvm/lib/arm64/gic_v3.c
+++ b/tools/testing/selftests/kvm/lib/arm64/gic_v3.c
@@ -304,6 +304,9 @@ static void gicv3_cpu_init(unsigned int cpu)
redist_base_cpu = gicr_base_cpu(cpu);
sgi_base = sgi_base_from_redist(redist_base_cpu);
+ /* Verify assumption that GICR_TYPER.Processor_number == cpu */
+ GUEST_ASSERT(((readq_relaxed(redist_base_cpu + GICR_TYPER) >> 8) & 0xffff) == cpu);
+
gicv3_enable_redist(redist_base_cpu);
/*
--
2.50.1 (Apple Git-155)
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Christof Hellmis
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] KVM: selftests: SYNC after guest ITS setup in vgic_lpi_stress
2025-11-19 13:57 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Maximilian Dittgen
@ 2025-11-19 13:57 ` Maximilian Dittgen
2025-11-19 22:35 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Oliver Upton
1 sibling, 0 replies; 6+ messages in thread
From: Maximilian Dittgen @ 2025-11-19 13:57 UTC (permalink / raw)
To: maz, oliver.upton
Cc: pbonzini, shuah, linux-arm-kernel, kvmarm, linux-kselftest, kvm,
mdittgen, lilitj, nh-open-source
vgic_lpi_stress sends MAPTI and MAPC commands during guest GIC setup to
map interrupt events to ITT entries and collection IDs to
redistributors, respectively.
We have no guarantee that the ITS will finish handling these mapping
commands before the selftest calls KVM_SIGNAL_MSI to inject LPIs to the
guest. If LPIs are injected before ITS mapping completes, the ITS cannot
properly pass the interrupt on to the redistributor.
Fix by adding a SYNC command to the selftests ITS library, then calling
SYNC after ITS mapping to ensure mapping completes before signal_lpi()
writes to GITS_TRANSLATER.
Signed-off-by: Maximilian Dittgen <mdittgen@amazon.de>
---
tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c | 4 ++++
.../testing/selftests/kvm/include/arm64/gic_v3_its.h | 1 +
tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c | 11 +++++++++++
3 files changed, 16 insertions(+)
diff --git a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
index 687d04463983..e857a605f577 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_lpi_stress.c
@@ -118,6 +118,10 @@ static void guest_setup_gic(void)
guest_setup_its_mappings();
guest_invalidate_all_rdists();
+
+ /* SYNC to ensure ITS setup is complete */
+ for (cpuid = 0; cpuid < test_data.nr_cpus; cpuid++)
+ its_send_sync_cmd(test_data.cmdq_base_va, cpuid);
}
static void guest_code(size_t nr_lpis)
diff --git a/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h b/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
index 3722ed9c8f96..58feef3eb386 100644
--- a/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
+++ b/tools/testing/selftests/kvm/include/arm64/gic_v3_its.h
@@ -15,5 +15,6 @@ 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_sync_cmd(void *cmdq_base, u32 vcpu_id);
#endif // __SELFTESTS_GIC_V3_ITS_H__
diff --git a/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c b/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
index 0e2f8ed90f30..d9ee331074ea 100644
--- a/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
+++ b/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
@@ -253,3 +253,14 @@ void its_send_invall_cmd(void *cmdq_base, u32 collection_id)
its_send_cmd(cmdq_base, &cmd);
}
+
+void its_send_sync_cmd(void *cmdq_base, u32 vcpu_id)
+{
+ struct its_cmd_block cmd = {};
+
+ its_encode_cmd(&cmd, GITS_CMD_SYNC);
+ its_encode_target(&cmd, procnum_to_rdbase(vcpu_id));
+
+ its_send_cmd(cmdq_base, &cmd);
+}
+
--
2.50.1 (Apple Git-155)
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Christof Hellmis
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number
2025-11-19 13:57 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Maximilian Dittgen
2025-11-19 13:57 ` [PATCH v2 2/2] KVM: selftests: SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
@ 2025-11-19 22:35 ` Oliver Upton
1 sibling, 0 replies; 6+ messages in thread
From: Oliver Upton @ 2025-11-19 22:35 UTC (permalink / raw)
To: maz, oliver.upton, Maximilian Dittgen
Cc: Oliver Upton, pbonzini, shuah, linux-arm-kernel, kvmarm,
linux-kselftest, kvm, lilitj, nh-open-source
On Wed, 19 Nov 2025 14:57:43 +0100, Maximilian Dittgen wrote:
> The selftests GIC library and tests assume that the
> GICR_TYPER.Processor_number associated with a given CPU is the same as
> the CPU's selftest index.
>
> Since this assumption is not guaranteed by specification, add an assert
> in gicv3_cpu_init() that validates this is true.
>
> [...]
I did a small cleanup to patch #1 to get rid of open-coded shifts / masks.
Please new versions of a series as a separate thread, using a cover letter
for anything more than a single patch. This is helpful for tracking patches
on my end.
Applied to next, thanks!
[1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number
https://git.kernel.org/kvmarm/kvmarm/c/31df012da496
[2/2] KVM: selftests: SYNC after guest ITS setup in vgic_lpi_stress
https://git.kernel.org/kvmarm/kvmarm/c/85f329df2931
--
Best,
Oliver
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-19 22:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-14 14:39 [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
2025-11-14 15:42 ` Marc Zyngier
2025-11-14 20:26 ` Oliver Upton
2025-11-19 13:57 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Maximilian Dittgen
2025-11-19 13:57 ` [PATCH v2 2/2] KVM: selftests: SYNC after guest ITS setup in vgic_lpi_stress Maximilian Dittgen
2025-11-19 22:35 ` [PATCH v2 1/2] KVM: selftests: Assert GICR_TYPER.Processor_Number matches selftest CPU number Oliver Upton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).