* [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress
@ 2025-10-17 16:19 Maximilian Dittgen
2025-10-17 17:06 ` Marc Zyngier
0 siblings, 1 reply; 5+ messages in thread
From: Maximilian Dittgen @ 2025-10-17 16:19 UTC (permalink / raw)
To: maz, oliver.upton
Cc: pbonzini, shuah, linux-arm-kernel, kvmarm, linux-kselftest, kvm,
mdittgen, epetron, nh-open-source, Maximilian Dittgen
When mapping guest ITS collections, vgic_lpi_stress iterates over
integers in the range [0, nr_cpus), passing them as the target_addr
parameter to its_send_mapc_cmd(). These integers correspond to the
selftest userspace vCPU IDs that we intend to map each ITS collection
to.
However, its_encode_target() within its_send_mapc_cmd() expects a
vCPU's redistributor address--not the vCPU ID--as the target_addr
parameter. This is evident from how its_encode_target() encodes the
target_addr parameter as:
its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16)
This shows that we right-shift the input target_addr parameter by 16
bits before encoding it. This makes sense when the parameter refers to
redistributor addresses (e.g., 0x20000, 0x30000) but not vCPU IDs
(e.g., 0x2, 0x3).
The current impact of passing vCPU IDs to its_send_mapc_cmd() is that
all vCPU IDs become 0x0 after the bit shift. Thus, when
vgic_its_cmd_handle_mapc() receives the ITS command in vgic-its.c, it
always interprets the collection's target_vcpu as 0. All interrupts
sent to collections will be processed by vCPU 0, which defeats the
purpose of this multi-vCPU test.
Fix by left-shifting the vCPU parameter received by its_send_mapc_cmd
16 bits before passing it into its_encode_target for encoding.
Signed-off-by: Maximilian Dittgen <mdittgen@amazon.com>
---
To validate the patch, I added the following debug code at the top of vgic_its_cmd_handle_mapc:
u64 raw_cmd2 = le64_to_cpu(its_cmd[2]);
u32 target_addr = its_cmd_get_target_addr(its_cmd);
kvm_info("MAPC: coll_id=%d, raw_cmd[2]=0x%llx, parsed_target=%u\n",
coll_id, raw_cmd2, target_addr);
vcpu = kvm_get_vcpu_by_id(kvm, its_cmd_get_target_addr(its_cmd));
kvm_info("MAPC: coll_id=%d, vcpu_id=%d\n", coll_id, vcpu ? vcpu->vcpu_id : -1);
I then ran `./vgic_lpi_stress -v 3` to trigger the stress selftest with 3 vCPUs.
Before the patch, the debug logs read:
kvm [20832]: MAPC: coll_id=0, raw_cmd[2]=0x8000000000000000, parsed_target=0
kvm [20832]: MAPC: coll_id=0, vcpu_id=0
kvm [20832]: MAPC: coll_id=1, raw_cmd[2]=0x8000000000000001, parsed_target=0
kvm [20832]: MAPC: coll_id=1, vcpu_id=0
kvm [20832]: MAPC: coll_id=2, raw_cmd[2]=0x8000000000000002, parsed_target=0
kvm [20832]: MAPC: coll_id=2, vcpu_id=0
Note the last bit of the cmd string reflects the collection ID, but the rest of the cmd string reads 0. The handler parses out vCPU 0 for all 3 mapc calls.
After the patch, the debug logs read:
kvm [20019]: MAPC: coll_id=0, raw_cmd[2]=0x8000000000000000, parsed_target=0
kvm [20019]: MAPC: coll_id=0, vcpu_id=0
kvm [20019]: MAPC: coll_id=1, raw_cmd[2]=0x8000000000010001, parsed_target=1
kvm [20019]: MAPC: coll_id=1, vcpu_id=1
kvm [20019]: MAPC: coll_id=2, raw_cmd[2]=0x8000000000020002, parsed_target=2
kvm [20019]: MAPC: coll_id=2, vcpu_id=2
Note that the target vcpu and target collection are both visible in the cmd string. The handler parses out the correct vCPU for all 3 mapc calls.
---
tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
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 09f270545646..23c46ad17221 100644
--- a/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
+++ b/tools/testing/selftests/kvm/lib/arm64/gic_v3_its.c
@@ -15,6 +15,8 @@
#include "gic_v3.h"
#include "processor.h"
+#define GITS_COLLECTION_TARGET_SHIFT 16
+
static u64 its_read_u64(unsigned long offset)
{
return readq_relaxed(GITS_BASE_GVA + offset);
@@ -217,7 +219,7 @@ void its_send_mapc_cmd(void *cmdq_base, u32 vcpu_id, u32 collection_id, bool val
its_encode_cmd(&cmd, GITS_CMD_MAPC);
its_encode_collection(&cmd, collection_id);
- its_encode_target(&cmd, vcpu_id);
+ its_encode_target(&cmd, vcpu_id << GITS_COLLECTION_TARGET_SHIFT);
its_encode_valid(&cmd, valid);
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
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress
2025-10-17 16:19 [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress Maximilian Dittgen
@ 2025-10-17 17:06 ` Marc Zyngier
2025-10-20 12:12 ` Maximilian Dittgen
0 siblings, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2025-10-17 17:06 UTC (permalink / raw)
To: Maximilian Dittgen
Cc: oliver.upton, pbonzini, shuah, linux-arm-kernel, kvmarm,
linux-kselftest, kvm, epetron, nh-open-source, Maximilian Dittgen
On Fri, 17 Oct 2025 17:19:18 +0100,
Maximilian Dittgen <mdittgen@amazon.de> wrote:
>
> When mapping guest ITS collections, vgic_lpi_stress iterates over
> integers in the range [0, nr_cpus), passing them as the target_addr
> parameter to its_send_mapc_cmd(). These integers correspond to the
> selftest userspace vCPU IDs that we intend to map each ITS collection
> to.
>
> However, its_encode_target() within its_send_mapc_cmd() expects a
> vCPU's redistributor address--not the vCPU ID--as the target_addr
> parameter. This is evident from how its_encode_target() encodes the
> target_addr parameter as:
>
> its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16)
>
> This shows that we right-shift the input target_addr parameter by 16
> bits before encoding it. This makes sense when the parameter refers to
> redistributor addresses (e.g., 0x20000, 0x30000) but not vCPU IDs
> (e.g., 0x2, 0x3).
From the KVM ITS emulation code:
* We use linear CPU numbers for redistributor addressing,
* so GITS_TYPER.PTA is 0.
It is not an address.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress
2025-10-17 17:06 ` Marc Zyngier
@ 2025-10-20 12:12 ` Maximilian Dittgen
2025-10-20 13:01 ` Marc Zyngier
0 siblings, 1 reply; 5+ messages in thread
From: Maximilian Dittgen @ 2025-10-20 12:12 UTC (permalink / raw)
To: maz
Cc: epetron, kvm, kvmarm, linux-arm-kernel, linux-kselftest, mdittgen,
mdittgen, nh-open-source, oliver.upton, pbonzini, shuah
On Fri, 17 Oct 2025 19:06:25 +0200,
Marc Zyngier <maz@kernel.org> wrote:
>
> * We use linear CPU numbers for redistributor addressing,
> * so GITS_TYPER.PTA is 0.
>
> It is not an address.
The issue is that its_encode_target in selftests is designed for
physical redistriubtor addresses (GITS_TYPER.PTA = 1) and thus
performs a right shift by 16 bits:
its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16);
When the vgic_lpi_stress selftest passes in a linear vCPU id as
the redistributor address (GITS_TYPER.PTA = 0 behavior),
The its_encode_target function shifts the CPU numbers 16 bits right,
functionally zeroing them.
We need to either:
- Align this specific selftest with GITS_TYPER.PTA = 0 and not use
its_encode_target to encode the target vCPU id. Instead have a
dedicated encode function for the use case without a bit shift.
- Align all selftests with GITS_TYPER.PTA = 0 and refactor
its_encode_target to skip the bit shift altogether.
- Align selftests with GITS_TYPER.PTA = 1 and pass a redistributor
address, not a vCPU id, into its_send_mapc_cmd().
Otherwise, the selftest's current behavior incorrectly maps all
collections to target vCPU 0.
Thanks,
Maximilian
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress
2025-10-20 12:12 ` Maximilian Dittgen
@ 2025-10-20 13:01 ` Marc Zyngier
2025-10-20 15:13 ` Maximilian Dittgen
0 siblings, 1 reply; 5+ messages in thread
From: Marc Zyngier @ 2025-10-20 13:01 UTC (permalink / raw)
To: Maximilian Dittgen
Cc: epetron, kvm, kvmarm, linux-arm-kernel, linux-kselftest, mdittgen,
nh-open-source, oliver.upton, pbonzini, shuah
On Mon, 20 Oct 2025 13:12:20 +0100,
Maximilian Dittgen <mdittgen@amazon.de> wrote:
>
> On Fri, 17 Oct 2025 19:06:25 +0200,
> Marc Zyngier <maz@kernel.org> wrote:
> >
> > * We use linear CPU numbers for redistributor addressing,
> > * so GITS_TYPER.PTA is 0.
> >
> > It is not an address.
>
> The issue is that its_encode_target in selftests is designed for
> physical redistriubtor addresses (GITS_TYPER.PTA = 1) and thus
No. Please read the spec.
> performs a right shift by 16 bits:
>
> its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16);
>
> When the vgic_lpi_stress selftest passes in a linear vCPU id as
> the redistributor address (GITS_TYPER.PTA = 0 behavior),
> The its_encode_target function shifts the CPU numbers 16 bits right,
> functionally zeroing them.
>
> We need to either:
> - Align this specific selftest with GITS_TYPER.PTA = 0 and not use
> its_encode_target to encode the target vCPU id. Instead have a
> dedicated encode function for the use case without a bit shift.
> - Align all selftests with GITS_TYPER.PTA = 0 and refactor
> its_encode_target to skip the bit shift altogether.
> - Align selftests with GITS_TYPER.PTA = 1 and pass a redistributor
> address, not a vCPU id, into its_send_mapc_cmd().
What part of "GITS_TYPER.PTA is 0" did you miss?
>
> Otherwise, the selftest's current behavior incorrectly maps all
> collections to target vCPU 0.
To be clear: I don't object to the patch. I object to the nonsensical
commit message.
You cannot say "I'm replacing the vcpu_id with an address". That's not
for you to decide, as the emulated HW is *imposing* that decision on
you. You don't even have the addresses at which the RDs are. You are
merely *reformatting* the vcpu_id to fit a field that can *also*
contain a 64kB-aligned address *when GITS_TYPER.PTA==1*. See 5.3.1 in
the GICv3 spec.
And yes, what you have is the correct fix. Just wrap it as a helper
(I'd suggest procnum_to_rdbase(), if you need a name for it).
This test also violate the architecture by not performing a SYNC after
any command, which would also require the use of a properly formatted
target field. But hey, correctness is overrated.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress
2025-10-20 13:01 ` Marc Zyngier
@ 2025-10-20 15:13 ` Maximilian Dittgen
0 siblings, 0 replies; 5+ messages in thread
From: Maximilian Dittgen @ 2025-10-20 15:13 UTC (permalink / raw)
To: maz
Cc: epetron, kvm, kvmarm, linux-arm-kernel, linux-kselftest, mdittgen,
mdittgen, nh-open-source, oliver.upton, pbonzini, shuah
On Mon, 20 Oct 2025 15:01:15 +0200,
Marc Zyngier <maz@kernel.org> wrote:
>
> To be clear: I don't object to the patch. I object to the nonsensical
> commit message.
>
> And yes, what you have is the correct fix. Just wrap it as a helper
> I'd suggest procnum_to_rdbase(), if you need a name for it).
Thank you for the feedback, I've created a PATCH v2 with revised commit
message and a procnum_to_rdbase() helper.
>
> This test also violate the architecture by not performing a SYNC after
> any command, which would also require the use of a properly formatted
> target field. But hey, correctness is overrated.
I am currently working on a RFC patchset for per-vCPU vLPI enablement
that extends this selftest to test the feature. I will package in SYNCs
as a patch on the RFC.
Thanks,
Maximilian
Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-20 15:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-17 16:19 [PATCH] KVM: selftests: fix ITS collection target addresses in vgic_lpi_stress Maximilian Dittgen
2025-10-17 17:06 ` Marc Zyngier
2025-10-20 12:12 ` Maximilian Dittgen
2025-10-20 13:01 ` Marc Zyngier
2025-10-20 15:13 ` Maximilian Dittgen
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).