On 2024/8/7 0:21, Marc Zyngier wrote:
On Tue, 06 Aug 2024 17:00:44 +0100,
Zenghui Yu <zenghui.yu@xxxxxxxxx> wrote:
>
> On 2024/8/6 22:11, Zenghui Yu wrote:
> > @@ -112,7 +113,7 @@ static bool end_of_vgic(struct vgic_state_iter *iter)
> > return iter->dist_id > 0 &&
> > iter->vcpu_id == iter->nr_cpus &&
> > iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS) &&
> > - iter->lpi_idx > iter->nr_lpis;
> > + (iter->lpi_idx > iter->nr_lpis || !iter->nr_lpis);
>
> And this should actually be written as:
>
> iter->lpi_idx >= iter->nr_lpis
>
> even in the first commit adding the LPI status in debugfs (e294cb3a6d1a)
> if I understand it correctly. I will give it a bit more tests tomorrow..
Yup, this looks like a long-standing bug (/me pleads guilty).
Maybe worth fixing them independently in order to facilitate the
inevitable backports?
I'm sorry that I misread the code again (shouldn't have sent spam late
at night :-( ).
Consider the last LPI:
|vgic_debug_next() {
| iter_next() // get the last valid LPI intid
| end_of_vgic() // lpi_idx == nr_lpis
|}
We need to go ahead to print this LPI's state and go through one more
vgic_debug_next() to exit the iterator. So there's no problem in the
current implementation for LPI, it's just that the code is a bit hard to
follow.
I've sent the "easiest approach" [*] out now.
Thanks,
Zenghui
[*]
https://lore.kernel.org/kvmarm/20240807052024.2084-1-yuzenghui@xxxxxxxxxx
