Re: [PATCH v3 05/19] KVM: arm64: vgic-debug: Use an xarray mark for debug iterator

[Zenghui Yu <yuzenghui@huawei.com>]

On 2024/8/7 0:21, Marc Zyngier wrote:
> On Tue, 06 Aug 2024 17:00:44 +0100,
> Zenghui Yu <zenghui.yu@linux.dev> 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@huawei.com