On Thu, 11 May 2017, Andre Przywara wrote:
> So far irq_to_pending() is just a convenience function to lookup
> statically allocated arrays. This will change with LPIs, which are
> more dynamic.
> The proper answer to the issue of preventing stale pointers is
> ref-counting, which requires more rework and will be introduced with
> a later rework.
> For now move the irq_to_pending() calls that are used with LPIs under the
> VGIC VCPU lock, and only use the returned pointer while holding the lock.
> This prevents the memory from being freed while we use it.
I don't like the idea of doing this just for the functions that are used
by LPIs and not the other. Specifically, we are leaving out:
[a]:
- vgic_migrate_irq
- vgic_enable_irqs
- vgic_disable_irqs
[b]:
- arch_move_irqs
Those in group [a] are easy to fix, please do. Just introduce a spinlock
in vgic_disable_irqs (it is safe because gic_remove_from_queues already
takes the vgic vcpu lock).
[b] is not easy to fix, just add a comment.
> Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> ---
> xen/arch/arm/gic.c | 5 ++++-
> xen/arch/arm/vgic.c | 4 +++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index da19130..dcb1783 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -402,10 +402,13 @@ static inline void gic_add_to_lr_pending(struct vcpu
> *v, struct pending_irq *n)
>
> void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
> {
> - struct pending_irq *p = irq_to_pending(v, virtual_irq);
> + struct pending_irq *p;
> unsigned long flags;
>
> spin_lock_irqsave(&v->arch.vgic.lock, flags);
> +
> + p = irq_to_pending(v, virtual_irq);
> +
> if ( !list_empty(&p->lr_queue) )
> list_del_init(&p->lr_queue);
> spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 83569b0..d30f324 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -466,7 +466,7 @@ void vgic_clear_pending_irqs(struct vcpu *v)
> void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq)
> {
> uint8_t priority;
> - struct pending_irq *iter, *n = irq_to_pending(v, virq);
> + struct pending_irq *iter, *n;
> unsigned long flags;
> bool running;
>
> @@ -474,6 +474,8 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int
> virq)
>
> spin_lock_irqsave(&v->arch.vgic.lock, flags);
>
> + n = irq_to_pending(v, virq);
> +
> /* vcpu offline */
> if ( test_bit(_VPF_down, &v->pause_flags) )
> {
> --
> 2.9.0
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
