Re: [PATCH] KVM: selftests: Add SYNC after guest ITS setup in vgic_lpi_stress

[Marc Zyngier <maz@kernel.org>]

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.