Skip to content

Commit 5ab2496

Browse files
Fuad TabbaMarc Zyngier
authored andcommitted
KVM: arm64: Reimplement vgic-debug XArray iteration
The vgic-debug interface implementation uses XArray marks (`LPI_XA_MARK_DEBUG_ITER`) to "snapshot" LPIs at the start of iteration. This modifies global state for a read-only operation and complicates reference counting, leading to leaks if iteration is aborted or fails. Reimplement the iterator to use dynamic iteration logic: - Remove `lpi_idx` from `struct vgic_state_iter`. - Replace the XArray marking mechanism with dynamic iteration using `xa_find_after(..., XA_PRESENT)`. - Wrap XArray traversals in `rcu_read_lock()`/`rcu_read_unlock()` to ensure safety against concurrent modifications (e.g., LPI unmapping). - Handle potential races where an LPI is removed during iteration by gracefully skipping it in `show()`, rather than warning. - Remove the unused `LPI_XA_MARK_DEBUG_ITER` definition. This simplifies the lifecycle management of the iterator and prevents resource leaks associated with the marking mechanism, and paves the way for using a standard seq_file iterator. Signed-off-by: Fuad Tabba <tabba@google.com> Link: https://patch.msgid.link/20260202085721.3954942-3-tabba@google.com Signed-off-by: Marc Zyngier <maz@kernel.org>
1 parent dcd79ed commit 5ab2496

2 files changed

Lines changed: 20 additions & 49 deletions

File tree

arch/arm64/kvm/vgic/vgic-debug.c

Lines changed: 20 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,9 @@
2525
struct vgic_state_iter {
2626
int nr_cpus;
2727
int nr_spis;
28-
int nr_lpis;
2928
int dist_id;
3029
int vcpu_id;
3130
unsigned long intid;
32-
int lpi_idx;
3331
};
3432

3533
static void iter_next(struct kvm *kvm, struct vgic_state_iter *iter)
@@ -45,13 +43,15 @@ static void iter_next(struct kvm *kvm, struct vgic_state_iter *iter)
4543
* Let the xarray drive the iterator after the last SPI, as the iterator
4644
* has exhausted the sequentially-allocated INTID space.
4745
*/
48-
if (iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS - 1) &&
49-
iter->nr_lpis) {
50-
if (iter->lpi_idx < iter->nr_lpis)
51-
xa_find_after(&dist->lpi_xa, &iter->intid,
52-
VGIC_LPI_MAX_INTID,
53-
LPI_XA_MARK_DEBUG_ITER);
54-
iter->lpi_idx++;
46+
if (iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS - 1)) {
47+
if (iter->intid == VGIC_LPI_MAX_INTID + 1)
48+
return;
49+
50+
rcu_read_lock();
51+
if (!xa_find_after(&dist->lpi_xa, &iter->intid,
52+
VGIC_LPI_MAX_INTID, XA_PRESENT))
53+
iter->intid = VGIC_LPI_MAX_INTID + 1;
54+
rcu_read_unlock();
5555
return;
5656
}
5757

@@ -61,44 +61,21 @@ static void iter_next(struct kvm *kvm, struct vgic_state_iter *iter)
6161
iter->intid = 0;
6262
}
6363

64-
static int iter_mark_lpis(struct kvm *kvm)
64+
static int vgic_count_lpis(struct kvm *kvm)
6565
{
6666
struct vgic_dist *dist = &kvm->arch.vgic;
67-
unsigned long intid, flags;
6867
struct vgic_irq *irq;
68+
unsigned long intid;
6969
int nr_lpis = 0;
7070

71-
xa_lock_irqsave(&dist->lpi_xa, flags);
72-
73-
xa_for_each(&dist->lpi_xa, intid, irq) {
74-
if (!vgic_try_get_irq_ref(irq))
75-
continue;
76-
77-
__xa_set_mark(&dist->lpi_xa, intid, LPI_XA_MARK_DEBUG_ITER);
71+
rcu_read_lock();
72+
xa_for_each(&dist->lpi_xa, intid, irq)
7873
nr_lpis++;
79-
}
80-
81-
xa_unlock_irqrestore(&dist->lpi_xa, flags);
74+
rcu_read_unlock();
8275

8376
return nr_lpis;
8477
}
8578

86-
static void iter_unmark_lpis(struct kvm *kvm)
87-
{
88-
struct vgic_dist *dist = &kvm->arch.vgic;
89-
unsigned long intid, flags;
90-
struct vgic_irq *irq;
91-
92-
xa_for_each_marked(&dist->lpi_xa, intid, irq, LPI_XA_MARK_DEBUG_ITER) {
93-
xa_lock_irqsave(&dist->lpi_xa, flags);
94-
__xa_clear_mark(&dist->lpi_xa, intid, LPI_XA_MARK_DEBUG_ITER);
95-
xa_unlock_irqrestore(&dist->lpi_xa, flags);
96-
97-
/* vgic_put_irq() expects to be called outside of the xa_lock */
98-
vgic_put_irq(kvm, irq);
99-
}
100-
}
101-
10279
static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,
10380
loff_t pos)
10481
{
@@ -108,8 +85,6 @@ static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter,
10885

10986
iter->nr_cpus = nr_cpus;
11087
iter->nr_spis = kvm->arch.vgic.nr_spis;
111-
if (kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
112-
iter->nr_lpis = iter_mark_lpis(kvm);
11388

11489
/* Fast forward to the right position if needed */
11590
while (pos--)
@@ -121,7 +96,7 @@ static bool end_of_vgic(struct vgic_state_iter *iter)
12196
return iter->dist_id > 0 &&
12297
iter->vcpu_id == iter->nr_cpus &&
12398
iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS) &&
124-
(!iter->nr_lpis || iter->lpi_idx > iter->nr_lpis);
99+
iter->intid > VGIC_LPI_MAX_INTID;
125100
}
126101

127102
static void *vgic_debug_start(struct seq_file *s, loff_t *pos)
@@ -178,7 +153,6 @@ static void vgic_debug_stop(struct seq_file *s, void *v)
178153

179154
mutex_lock(&kvm->arch.config_lock);
180155
iter = kvm->arch.vgic.iter;
181-
iter_unmark_lpis(kvm);
182156
kfree(iter);
183157
kvm->arch.vgic.iter = NULL;
184158
mutex_unlock(&kvm->arch.config_lock);
@@ -188,13 +162,14 @@ static void print_dist_state(struct seq_file *s, struct vgic_dist *dist,
188162
struct vgic_state_iter *iter)
189163
{
190164
bool v3 = dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3;
165+
struct kvm *kvm = s->private;
191166

192167
seq_printf(s, "Distributor\n");
193168
seq_printf(s, "===========\n");
194169
seq_printf(s, "vgic_model:\t%s\n", v3 ? "GICv3" : "GICv2");
195170
seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis);
196171
if (v3)
197-
seq_printf(s, "nr_lpis:\t%d\n", iter->nr_lpis);
172+
seq_printf(s, "nr_lpis:\t%d\n", vgic_count_lpis(kvm));
198173
seq_printf(s, "enabled:\t%d\n", dist->enabled);
199174
seq_printf(s, "\n");
200175

@@ -291,16 +266,13 @@ static int vgic_debug_show(struct seq_file *s, void *v)
291266
if (iter->vcpu_id < iter->nr_cpus)
292267
vcpu = kvm_get_vcpu(kvm, iter->vcpu_id);
293268

294-
/*
295-
* Expect this to succeed, as iter_mark_lpis() takes a reference on
296-
* every LPI to be visited.
297-
*/
298269
if (iter->intid < VGIC_NR_PRIVATE_IRQS)
299270
irq = vgic_get_vcpu_irq(vcpu, iter->intid);
300271
else
301272
irq = vgic_get_irq(kvm, iter->intid);
302-
if (WARN_ON_ONCE(!irq))
303-
return -EINVAL;
273+
274+
if (!irq)
275+
return 0;
304276

305277
raw_spin_lock_irqsave(&irq->irq_lock, flags);
306278
print_irq_state(s, irq, vcpu);

include/kvm/arm_vgic.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,6 @@ struct vgic_dist {
300300
*/
301301
u64 propbaser;
302302

303-
#define LPI_XA_MARK_DEBUG_ITER XA_MARK_0
304303
struct xarray lpi_xa;
305304

306305
/* used by vgic-debug */

0 commit comments

Comments
 (0)