KVM: arm64: vgic-v5: Random sparse fixes
Marc Zyngier
maintainercover KVM: arm64: vgic-v5: Random sparse fixes
The LKP bot has been reporting a bunch of sparse warnings in the newly merged vgic-v5 code (mostly in the table code) that we could do without. I've therefore investigate those, fixed them, and some more.
I'm queuing this on top of the existing branch.
Marc Zyngier (4):
KVM: arm64: vgic-v5: Drop __iomem attribute from {vmd,vpet}_base
KVM: arm64: vgic-v5: Correctly reset h_lpi_ist to NULL
KVM: arm64: vgic-v5: Tidy-up programming of vpe descriptor address
KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
arch/arm64/kvm/vgic/vgic-v5-tables.c | 24 +++++++++++------------- arch/arm64/kvm/vgic/vgic-v5-tables.h | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-)
Signature · 1 line
2.47.3
Marc Zyngier
maintainerPATCH 1/4 KVM: arm64: vgic-v5: Drop __iomem attribute from {vmd,vpet}_base
Both vmd_base and vpet_base are pointers to kmalloc'd memory, and therefore cannot carry the __iomem attribute, which designates pointers to MMIO.
Drop the abusive qualifier.
--- arch/arm64/kvm/vgic/vgic-v5-tables.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/kvm/vgic/vgic-v5-tables.h +2 -2
@@ -23,8 +23,8 @@ struct vmtl2_entry {typedef __le64 vpe_entry ;struct vgic_v5_vm_info { -void __iomem *vmd_base; -vpe_entry __iomem *vpet_base; +void *vmd_base; +vpe_entry *vpet_base;void *vped_base;u8 vpe_id_bits;
Signature · 1 line
2.47.3
Re: [PATCH 1/4] KVM: arm64: vgic-v5: Drop __iomem attribute from {vmd,vpet}_base
Quoted 10 lines · Marc Zyngier
Both
vmd_baseandvpet_baseare pointers to kmalloc'd memory, and therefore cannot carry the__iomemattribute, which designates pointers to MMIO.Drop the abusive qualifier.
Reported-by kernel test robot <[email protected]>Signed-off-by Marc Zyngier <[email protected]>
Cheers,
/fuad
Quoted 22 lines
--- arch/arm64/kvm/vgic/vgic-v5-tables.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm64/kvm/vgic/vgic-v5-tables.h +2
-3@@ -23,8 +23,8 @@ struct vmtl2_entry {typedef __le64 vpe_entry ;struct vgic_v5_vm_info { -void __iomem *vmd_base; -vpe_entry __iomem *vpet_base; +void *vmd_base; +vpe_entry *vpet_base;void *vped_base;u8 vpe_id_bits; --2.47.3
Marc Zyngier
maintainerPATCH 2/4 KVM: arm64: vgic-v5: Correctly reset h_lpi_ist to NULL
In reply to Marc ZyngierResetting a pointer to 0 is not completely wrong, but sparse legitimately shouts about it integer/pointer mix.
Use NULL as expected.
--- arch/arm64/kvm/vgic/vgic-v5-tables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic-v5-tables.c +1 -1
@@ -1066,7 +1066,7 @@ static int vgic_v5_alloc_two_level_lpi_ist(struct kvm *kvm, unsigned int id_bits/* Free the L1 IST again */ vmi =xa_load (&vm_info, vm_id);kfree (vmi->h_lpi_ist); - vmi->h_lpi_ist =0 ; + vmi->h_lpi_ist =NULL ;return ret; }
Signature · 1 line
2.47.3
Re: [PATCH 2/4] KVM: arm64: vgic-v5: Correctly reset h_lpi_ist to NULL
Quoted 9 lines · Marc Zyngier
Resetting a pointer to 0 is not completely wrong, but sparse legitimately shouts about it integer/pointer mix.
Use NULL as expected.
Reported-by kernel test robot <[email protected]>Signed-off-by Marc Zyngier <[email protected]>
Cheers,
/fuad
Quoted 21 lines
--- arch/arm64/kvm/vgic/vgic-v5-tables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm64/kvm/vgic/vgic-v5-tables.c +1
-2@@ -1066,7 +1066,7 @@ static int vgic_v5_alloc_two_level_lpi_ist(struct kvm *kvm, unsigned int id_bits/* Free the L1 IST again */ vmi =xa_load (&vm_info, vm_id);kfree (vmi->h_lpi_ist); - vmi->h_lpi_ist =0 ; + vmi->h_lpi_ist =NULL ;return ret; } --2.47.3
Marc Zyngier
maintainerPATCH 3/4 KVM: arm64: vgic-v5: Tidy-up programming of vpe descriptor address
In reply to Marc ZyngierAssigning an address calculation to a vpe_entry is poor practice, and results in sparse moaning loudly. Tidy things up by computing the physical address as such, and performing the cpu_to_le64() on that.
Whilst at it, simplify the computing of that address, avoiding the shifting back and forth, and kill the pointless GICV5_VPED_ADDR_SHIFT definition.
--- arch/arm64/kvm/vgic/vgic-v5-tables.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
arch/arm64/kvm/vgic/vgic-v5-tables.c +3 -5
@@ -63,7 +63,6 @@ static DEFINE_XARRAY(vm_info);/* Virtual PE Table Entry */ #define GICV5_VPE_VALID BIT_ULL(0)/* Note that there is no shift for the address by design. */ -#define GICV5_VPED_ADDR_SHIFT 3ULL#define GICV5_VPED_ADDR GENMASK_ULL(55, 3)/* L2 Interrupt State Table Entry */
@@ -725,7 +724,7 @@ int vgic_v5_vmte_alloc_vpe(struct kvm_vcpu *vcpu)u32 vm_id =vgic_v5_vm_id (vcpu->kvm);u16 vpe_id =vgic_v5_vpe_id (vcpu);struct vgic_v5_vm_info *vmi; -vpe_entry tmp, *vpet_base; +vpe_entry *vpet_base;void *vped;/* Make sure we're not over what the hardware supports */
@@ -751,10 +750,9 @@ int vgic_v5_vmte_alloc_vpe(struct kvm_vcpu *vcpu) vped = (u8 *)vmi->vped_base + (size_t )vcpu->vcpu_idx * vmt_info->vped_size; - tmp =FIELD_PREP (GICV5_VPED_ADDR ,virt_to_phys (vped) >>GICV5_VPED_ADDR_SHIFT ); - scoped_guard(raw_spinlock_irqsave, &vgic_v5_irs_lock) { -WRITE_ONCE (vpet_base[vpe_id],cpu_to_le64 (tmp)); +phys_addr_t paddr =virt_to_phys (vped) &GICV5_VPED_ADDR ; +WRITE_ONCE (vpet_base[vpe_id],cpu_to_le64 (paddr));vgic_v5_clean_inval (vpet_base + vpe_id,sizeof (vpe_entry)); }
Signature · 1 line
2.47.3
Re: [PATCH 3/4] KVM: arm64: vgic-v5: Tidy-up programming of vpe descriptor address
Quoted 10 lines · Marc Zyngier
Assigning an address calculation to a
vpe_entryis poor practice, and results in sparse moaning loudly. Tidy things up by computing the physical address as such, and performing thecpu_to_le64()on that.Whilst at it, simplify the computing of that address, avoiding the shifting back and forth, and kill the pointless
GICV5_VPED_ADDR_SHIFTdefinition.Signed-off-by Marc Zyngier <[email protected]>
Cheers,
/fuad
Quoted 41 lines
--- arch/arm64/kvm/vgic/vgic-v5-tables.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)arch/arm64/kvm/vgic/vgic-v5-tables.c +3
-6@@ -63,7 +63,6 @@ static DEFINE_XARRAY(vm_info);/* Virtual PE Table Entry */ #define GICV5_VPE_VALID BIT_ULL(0)/* Note that there is no shift for the address by design. */ -#define GICV5_VPED_ADDR_SHIFT 3ULL#define GICV5_VPED_ADDR GENMASK_ULL(55, 3)/* L2 Interrupt State Table Entry */ @@ -725,7 +724,7 @@ int vgic_v5_vmte_alloc_vpe(struct kvm_vcpu *vcpu)u32 vm_id =vgic_v5_vm_id (vcpu->kvm);u16 vpe_id =vgic_v5_vpe_id (vcpu);struct vgic_v5_vm_info *vmi; -vpe_entry tmp, *vpet_base; +vpe_entry *vpet_base;void *vped;/* Make sure we're not over what the hardware supports */ @@ -751,10 +750,9 @@ int vgic_v5_vmte_alloc_vpe(struct kvm_vcpu *vcpu) vped = (u8 *)vmi->vped_base + (size_t )vcpu->vcpu_idx * vmt_info->vped_size; - tmp =FIELD_PREP (GICV5_VPED_ADDR ,virt_to_phys (vped) >>GICV5_VPED_ADDR_SHIFT ); - scoped_guard(raw_spinlock_irqsave, &vgic_v5_irs_lock) { -WRITE_ONCE (vpet_base[vpe_id],cpu_to_le64 (tmp)); +phys_addr_t paddr =virt_to_phys (vped) &GICV5_VPED_ADDR ; +WRITE_ONCE (vpet_base[vpe_id],cpu_to_le64 (paddr));vgic_v5_clean_inval (vpet_base + vpe_id,sizeof (vpe_entry)); } --2.47.3
Marc Zyngier
maintainerPATCH 4/4 KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
In reply to Marc ZyngierThe vgic-v5 defines the host ISTE (h_iste) as a __le32, but uses get_user/put_user on a userspace buffer declared as u32 *. This leads to sparse having yet another fit.
Instead, define h_iste as a u32, and perform the conversion at the point of doing the access on the architectural state.
--- arch/arm64/kvm/vgic/vgic-v5-tables.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
arch/arm64/kvm/vgic/vgic-v5-tables.c +7 -7
@@ -1457,7 +1457,6 @@ static int vgic_v5_get_lpi_ist_desc(struct kvm *kvm,static int vgic_v5_save_linear_ist (const struct vgic_v5_ist_desc *ist,u32 __user *uaddr,size_t nr_entries) { -__le32 h_iste;size_t index;int ret;
@@ -1466,8 +1465,9 @@ static int vgic_v5_save_linear_ist(const struct vgic_v5_ist_desc *ist,for (index =0 ; index < nr_entries; index++) {__le32 *h_iste_addr = ist->base + index * ist->iste_size; +u32 h_iste; - h_iste =READ_ONCE (*h_iste_addr); + h_iste =le32_to_cpu (READ_ONCE (*h_iste_addr)); ret =put_user (h_iste, uaddr);if (ret)return ret;
@@ -1520,7 +1520,7 @@ static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, h_iste = *(__le32 *)(h_l2_ist_base + h_l2_index * ist->iste_size); - ret =put_user (h_iste, uaddr); + ret =put_user (le32_to_cpu (h_iste), uaddr);if (ret)return ret;
@@ -1710,19 +1710,19 @@ static int vgic_v5_restore_linear_ist(struct kvm *kvm, u32 __user *uaddr, size_t nr_entries, u32 intid_type) { - __le32 h_iste; size_t index;int ret;for (index =0 ; index < nr_entries; index++) {void *h_iste_addr = ist->base + index * ist->iste_size; +u32 h_iste; ret =get_user (h_iste, uaddr);if (ret)return ret; ret =vgic_v5_restore_ist_entry (kvm, ist, h_iste_addr, - h_iste, index, intid_type); +cpu_to_le32 (h_iste), index, intid_type);if (ret)return ret;
@@ -1744,7 +1744,6 @@ static int vgic_v5_restore_two_level_ist(struct kvm *kvm,struct vgic_v5_two_level_ist_shape shape;size_t h_l1_index, h_l2_index;void *h_l2_ist_base; -__le32 h_iste;int ret; shape =vgic_v5_two_level_ist_shape (ist);
@@ -1771,13 +1770,14 @@ static int vgic_v5_restore_two_level_ist(struct kvm *kvm,void *h_iste_addr = h_l2_ist_base + h_l2_index * ist->iste_size;u32 intid = h_l1_index * shape.l2_entries + h_l2_index; +u32 h_iste; ret =get_user (h_iste, uaddr);if (ret)return ret; ret =vgic_v5_restore_ist_entry (kvm, ist, h_iste_addr, - h_iste, intid, +cpu_to_le32 (h_iste), intid, intid_type);if (ret)return ret;
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 4/4] KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] kvm: arm64: code and commit message mismatch regarding h_iste type --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: arm64: code and commit message mismatch regarding h_iste type --
commit a916f514228ceaaf50a5546f6df29cea59ab15f1
Author: Marc Zyngier <[email protected]>
KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
This commit addresses a sparse warning by modifying the endianness conversion of the host ISTE. The intent is to define h_iste as a u32 instead of __le32, doing the conversion at the point of access.
Quoted 4 lines
arch/arm64/kvm/vgic/vgic-v5-tables.c +0
-0
[ ... ]
arch/arm64/kvm/vgic/vgic-v5-tables.c static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, · line 1525
Quoted 8 lines
arch/arm64/kvm/vgic/vgic-v5-tables.c +1
-1@@ -1520,7 +1520,7 @@ static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, h_iste = *(__le32 *)(h_l2_ist_base + h_l2_index * ist->iste_size); - ret =put_user (h_iste, uaddr); + ret =put_user (le32_to_cpu (h_iste), uaddr);if (ret)return ret;
[Severity: Medium]
Is the refactoring in vgic_v5_save_two_level_ist() incomplete?
The commit message explicitly states "define h_iste as a u32", and this pattern was applied to the other three functions modified in the file. However, in vgic_v5_save_two_level_ist(), the h_iste variable retains its __le32 type at the top of the function, and the conversion is instead done inline within the put_user() call.
Does this variable also need to be redefined as a u32 to remain consistent with the commit description and the rest of the patch?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Marc Zyngier
maintainerRe: [PATCH 4/4] KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
Quoted 35 lines · On Sat, 19 Sep 2026 13:39:04 +0100, [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: arm64: code and commit message mismatch regardingh_istetype --commit
a916f514228ceaaf50a5546f6df29cea59ab15f1
Author: Marc Zyngier <[email protected]>KVM: arm64: vgic-v5: Correctly handle host ISTE
__le32conversionThis commit addresses a sparse warning by modifying the endianness conversion of the host ISTE. The intent is to define
h_isteas a u32 instead of__le32, doing the conversion at the point of access.Quoted 4 lines
arch/arm64/kvm/vgic/vgic-v5-tables.c +0
-0[ ... ]
arch/arm64/kvm/vgic/vgic-v5-tables.c static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, · line 1525
Quoted 8 lines
arch/arm64/kvm/vgic/vgic-v5-tables.c +1
-1@@ -1520,7 +1520,7 @@ static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, h_iste = *(__le32 *)(h_l2_ist_base + h_l2_index * ist->iste_size); - ret =put_user (h_iste, uaddr); + ret =put_user (le32_to_cpu (h_iste), uaddr);if (ret)return ret;[Severity: Medium]
Is the refactoring invgic_v5_save_two_level_ist()incomplete?
Quoted 8 lines
The commit message explicitly states "define
h_isteas a u32", and this pattern was applied to the other three functions modified in the file. However, invgic_v5_save_two_level_ist(), theh_istevariable retains its__le32type at the top of the function, and the conversion is instead done inline within theput_user()call.Does this variable also need to be redefined as a u32 to remain consistent with the commit description and the rest of the patch?
I'll add this to this patch.
M.
arch/arm64/kvm/vgic/vgic-v5-tables.c +3 -4
@@ -1490,7 +1490,6 @@ static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist,struct vgic_v5_two_level_ist_shape shape;size_t h_l1_index, h_l2_index;void *h_l2_ist_base; -__le32 h_iste;int ret; shape =vgic_v5_two_level_ist_shape (ist);
@@ -1517,10 +1516,10 @@ static int vgic_v5_save_two_level_ist(const struct vgic_v5_ist_desc *ist, shape.l2_entries * ist->iste_size);for (h_l2_index =0 ; h_l2_index < shape.l2_entries; h_l2_index++) { - h_iste = *(__le32 *)(h_l2_ist_base + - h_l2_index * ist->iste_size); +void *h_iste_addr = h_l2_ist_base + h_l2_index * ist->iste_size; +u32 h_iste =le32_to_cpu (*(__le32 *)h_iste_addr); - ret =put_user (le32_to_cpu (h_iste), uaddr); + ret =put_user (h_iste, uaddr);if (ret)return ret;
Signature · 1 line
Jazz isn't dead. It just smells funny.
Re: [PATCH 4/4] KVM: arm64: vgic-v5: Correctly handle host ISTE __le32 conversion
In reply to Marc ZyngierHi Marc,
On Sat, 19 Sep 2026 13:31:50 +0100, Marc Zyngier <[email protected]> wrote:
[...]
Quoted 1 line
arch/arm64/kvm/vgic/vgic-v5-tables.c +0
-0
[...]
arch/arm64/kvm/vgic/vgic-v5-tables.c static int vgic_v5_save_linear_ist(const struct vgic_v5_ist_desc *ist, · line 1471
Quoted 9 lines
arch/arm64/kvm/vgic/vgic-v5-tables.c +2
-1@@ -1466,8 +1465,9 @@ static int vgic_v5_save_linear_ist(const struct vgic_v5_ist_desc *ist,for (index =0 ; index < nr_entries; index++) {__le32 *h_iste_addr = ist->base + index * ist->iste_size; +u32 h_iste; - h_iste =READ_ONCE (*h_iste_addr); + h_iste =le32_to_cpu (READ_ONCE (*h_iste_addr)); ret =put_user (h_iste, uaddr);
arm-vgic-v5.rst documents the IST buffers as little-endian entries, which is what the raw __le32 copy gave userspace. With le32_to_cpu() here a BE host would hand out native-endian entries instead. Making uaddr a __le32 __user * (it's a u32 __user * today) keeps the documented format, and sparse is clean with it too.
With that:
Cheers,
/fuad
Hi Marc,
On Sat, 19 Sep 2026 13:31:46 +0100, Marc Zyngier <[email protected]> wrote:
[...]
Quoted 1 line
I'm queuing this on top of the existing branch.
For the four as queued in kvmarm/next, fixup included:
Sparse on vgic-v5-tables.c goes to no warnings, and the vgic_v5 selftest passes on the FVP GICv5 model with a VHE host, before and after.
The same selftest on an nVHE host oopses in __vgic_v5_restore_ppi_state() at the first KVM_RUN, with and without this series. That's the EL1 access to the GICv5 CPU interface that 6/7 of [1] fixes, not new here.
Cheers,
/fuad