Hi Andre,
On 06/04/17 00:19, Andre Przywara wrote:
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 142eb64..5128f13 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1451,6 +1451,9 @@ static int vgic_v3_domain_init(struct domain *d)
vgic_v3_its_init_domain(d);
+ rwlock_init(&d->arch.vgic.pend_lpi_tree_lock);
+ radix_tree_init(&d->arch.vgic.pend_lpi_tree);
+
/*
* Domain 0 gets the hardware address.
* Guests get the virtual platform layout.
@@ -1524,14 +1527,33 @@ static int vgic_v3_domain_init(struct domain *d)
static void vgic_v3_domain_free(struct domain *d)
{
vgic_v3_its_free_domain(d);
+ radix_tree_destroy(&d->arch.vgic.pend_lpi_tree, NULL);
xfree(d->arch.vgic.rdist_regions);
}
+/*
+ * Looks up a virtual LPI number in our tree of mapped LPIs. This will return
+ * the corresponding struct pending_irq, which we also use to store the
+ * enabled and pending bit plus the priority.
+ * Returns NULL if an LPI cannot be found (or no LPIs are supported).
I asked few questions on the previous version about when lpi_to_pending
returns NULL and the fact that none of the vGIC code is able to handle
that. I was hoping a bit of documentation/ASSERT in the code rather than
hiding the problem as you seem to do in this version.
Looking at the whole code, I see that you insert the pending_irq in the
radix when on MAPTI. But never remove it. However the command DISCARD or
MAPD (V=0) will remove the mapping from the ITT.
Which means it is never possible to re-use the same vLPI again. However
if you remove from the radix tree you may end up having the LPI still in
LR but lpi_to_pending return NULL and make Xen segfault.
So what will protect Xen against segfault? What is the plan?
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 503a3cf..6ee7538 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -111,6 +111,8 @@ struct arch_domain
uint32_t rdist_stride; /* Re-Distributor stride */
struct rb_root its_devices; /* Devices mapped to an ITS */
spinlock_t its_devices_lock; /* Protects the its_devices tree */
+ struct radix_tree_root pend_lpi_tree; /* Stores struct pending_irq's */
+ rwlock_t pend_lpi_tree_lock; /* Protects the pend_lpi_tree */
#endif
} vgic;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 894c3f1..7c86f5b 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -134,6 +134,8 @@ struct vgic_ops {
void (*domain_free)(struct domain *d);
/* vGIC sysreg/cpregs emulate */
bool (*emulate_reg)(struct cpu_user_regs *regs, union hsr hsr);
+ /* lookup the struct pending_irq for a given LPI interrupt */
+ struct pending_irq *(*lpi_to_pending)(struct domain *d, unsigned int vlpi);
/* Maximum number of vCPU supported */
const unsigned int max_vcpus;
};
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel