Re: [PATCH v2 4/4] KVM: arm64: selftests: Add ITS table save tests

On Fri, 21 Aug 2026 at 08:00, <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Unsafe DMA Buffer Configuration (ITT Size Miscalculation)

The oversized scan stays inside the memory the test allocated for the
two ITTs, and both cases pass. If the series needs a respin I'll tidy
this up, but I won't respin just for this.

> - [Medium] Incorrect Endianness Conversion for L1 Device Table

CPU_BIG_ENDIAN depends on BROKEN on arm64, being deprecated [1]

/fuad

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

> --
>
> --- Patch [4]: [PATCH v2 4/4] KVM: arm64: selftests: Add ITS table save tests ---
> commit 00c3ff8ee303e906c6d0f346f17949dbb0abf5fc
> Author: Fuad Tabba <[email protected]>
> Subject: KVM: arm64: selftests: Add ITS table save tests
>
> This commit adds selftests for KVM_DEV_ARM_ITS_SAVE_TABLES to cover cases
> where a guest alters tables. It tests handling of a GITS_BASER<coll> write
> that changes the table, and a device whose L2 block the guest invalidated.
> Each case is then reset and restored to verify the save functionality.
>
> > diff --git a/tools/testing/selftests/kvm/arm64/vgic_its_save.c b/tools/testing/selftests/kvm/arm64/vgic_its_save.c
> > new file mode 100644
> > index 0000000000000..864da01539f3f
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/arm64/vgic_its_save.c
> > @@ -0,0 +1,441 @@
> [ ... ]
> > +/*
> > + * its_send_mapd_cmd() encodes ilog2(itt_size) - 1 as num_eventid_bits, and
> > + * vgic_its_restore_itt() scans BIT_ULL(num_eventid_bits) * ESZ, so the size
> > + * handed to MAPD has to match the ITT allocated for it.
> > + */
> > +#define ITT_EVENTID_BITS     13
> > +#define ITT_MAPD_SIZE                BIT_ULL(ITT_EVENTID_BITS + 1)
>
> [Severity: Medium]
> Does this calculation configure the hardware with an ITT size of 128KB instead
> of the intended 64KB?
>
> The test calculates ITT_MAPD_SIZE as BIT_ULL(ITT_EVENTID_BITS + 1) (evaluating
> to 16,384) to compensate for its_send_mapd_cmd() subtracting 1. However, KVM's
> its_cmd_get_size() adds 1 back, resulting in 14 bits of event IDs.
>
> During restore, KVM computes the scan size as BIT_ULL(14) * 8 = 128KB, causing
> it to read 64KB past the end of the allocated ITT memory.
>
> > +#define ITT_SZ                       (BIT_ULL(ITT_EVENTID_BITS) * ESZ)
> [ ... ]
> > +static void guest_unreachable_device(void)
> > +{
> > +     u64 *l1;
> [ ... ]
> > +     guest_make_device_table_indirect();
> > +
> > +     /* Both L2 blocks present, so both MAPDs are in range. */
> > +     l1 = test_data.device_l1_va;
> > +     l1[0] = L1E_VALID_MASK | (test_data.device_l2[0] & L1E_ADDR_MASK);
> > +     l1[1] = L1E_VALID_MASK | (test_data.device_l2[1] & L1E_ADDR_MASK);
>
> [Severity: Medium]
> Will this fail on big-endian hosts? The L1 entries are being populated using
> native CPU endianness.
>
> Since the ARM GICv3 ITS architecture and KVM emulator require these tables to
> be little-endian (using le64_to_cpu on read), shouldn't these use htole64()
> when writing them?
>
> > +
> > +     its_send_mapd_cmd(test_data.cmdq_base_va, DEVICE_A_ID,
> > +                       test_data.itt_tables, ITT_MAPD_SIZE, true);
> > +     its_send_mapd_cmd(test_data.cmdq_base_va, DEVICE_B_ID,
> > +                       test_data.itt_tables + ITT_SZ, ITT_MAPD_SIZE, true);
>
> [Severity: Medium]
> This isn't a separate bug, but as a result of the ITT_MAPD_SIZE miscalculation
> above, doesn't this send MAPD commands configuring overlapping, out-of-bounds
> ITT tables for the two devices?
>
> > +
> > +     /*
> > +      * Drop the block holding device B. No ITS command and no GITS_BASER
> > +      * write is involved, so nothing tells KVM the device is now
> > +      * unreachable.
> > +      */
> > +     l1[1] = 0;
> > +
> > +     GUEST_DONE();
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4