Hi Oliver,
On 2024/4/23 4:01, Oliver Upton wrote:
The vgic debug iterator is the final user of vgic_copy_lpi_list(), but
is a bit more complicated to transition to something else. Use a mark
in the LPI xarray to record the indices 'known' to the debug iterator.
Protect against the LPIs from being freed by associating an additional
reference with the xarray mark.
Rework iter_next() to let the xarray walk 'drive' the iteration after
visiting all of the SGIs, PPIs, and SPIs.
Signed-off-by: Oliver Upton <oliver.upton@xxxxxxxxx>
---
arch/arm64/kvm/vgic/vgic-debug.c | 82 +++++++++++++++++++++++---------
arch/arm64/kvm/vgic/vgic-its.c | 4 +-
arch/arm64/kvm/vgic/vgic.h | 1 +
include/kvm/arm_vgic.h | 2 +
4 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-debug.c b/arch/arm64/kvm/vgic/vgic-debug.c
index 389025ce7749..bcbc8c986b1d 100644
--- a/arch/arm64/kvm/vgic/vgic-debug.c
+++ b/arch/arm64/kvm/vgic/vgic-debug.c
@@ -28,27 +28,65 @@ struct vgic_state_iter {
int nr_lpis;
int dist_id;
int vcpu_id;
- int intid;
+ unsigned long intid;
int lpi_idx;
- u32 *lpi_array;
};
-static void iter_next(struct vgic_state_iter *iter)
+static void iter_next(struct kvm *kvm, struct vgic_state_iter *iter)
{
+ struct vgic_dist *dist = &kvm->arch.vgic;
+
if (iter->dist_id == 0) {
iter->dist_id++;
return;
}
+ /*
+ * Let the xarray drive the iterator after the last SPI, as the iterator
+ * has exhausted the sequentially-allocated INTID space.
+ */
+ if (iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS - 1)) {
+ if (iter->lpi_idx < iter->nr_lpis)
+ xa_find_after(&dist->lpi_xa, &iter->intid,
+ VGIC_LPI_MAX_INTID,
+ LPI_XA_MARK_DEBUG_ITER);
+ iter->lpi_idx++;
+ return;
+ }
What's the purpose of moving the LPI handling up here?
+
iter->intid++;
if (iter->intid == VGIC_NR_PRIVATE_IRQS &&
++iter->vcpu_id < iter->nr_cpus)
iter->intid = 0;
In case the guest *doesn't* have any LPI, we previously relied on the
iterator setting
'intid = nr_spis + VGIC_NR_PRIVATE_IRQS' && 'lpi_idx = 1'
to exit the iterator. But it was broken with this refactor -- the intid
remains at 'nr_spis + VGIC_NR_PRIVATE_IRQS - 1', and vgic_debug_show()
endlessly prints the last SPI's state.
The following diff seems work for me.
diff --git a/arch/arm64/kvm/vgic/vgic-debug.c
b/arch/arm64/kvm/vgic/vgic-debug.c
index 6faa1d16c9ce..f56f74c8cf54 100644
--- a/arch/arm64/kvm/vgic/vgic-debug.c
+++ b/arch/arm64/kvm/vgic/vgic-debug.c
@@ -41,11 +41,16 @@ static void iter_next(struct kvm *kvm, struct
vgic_state_iter *iter)
return;
}
+ iter->intid++;
+ if (iter->intid == VGIC_NR_PRIVATE_IRQS &&
+ ++iter->vcpu_id < iter->nr_cpus)
+ iter->intid = 0;
+
/*
* Let the xarray drive the iterator after the last SPI, as the iterator
* has exhausted the sequentially-allocated INTID space.
*/
- if (iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS - 1)) {
+ if (iter->intid >= (iter->nr_spis + VGIC_NR_PRIVATE_IRQS)) {
if (iter->lpi_idx < iter->nr_lpis)
xa_find_after(&dist->lpi_xa, &iter->intid,
VGIC_LPI_MAX_INTID,
@@ -53,11 +58,6 @@ static void iter_next(struct kvm *kvm, struct
vgic_state_iter *iter)
iter->lpi_idx++;
return;
}
-
- iter->intid++;
- if (iter->intid == VGIC_NR_PRIVATE_IRQS &&
- ++iter->vcpu_id < iter->nr_cpus)
- iter->intid = 0;
}
static int iter_mark_lpis(struct kvm *kvm)
