Re: [Xen-devel] [RFC] xen/arm: Restrict when a physical IRQ can be routed/removed from/to a domain

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 8 Mar 2018, julien.grall@xxxxxxx wrote:
> From: Julien Grall <julien.grall@xxxxxxx>
> 
> Xen is currently allowing to route/remove an interrupt from/to the
> domain while it is running.
> 
> However, we never sync the virtual interrupt state to the physical
> interrupt. This could lead to undesirable effect on the vGIC emulation
> and potentially the hardware.
> 
> One solution would be to sync the interrupt state when routing, but I am
> not sure it is worth the effort as you never really when it is safe to
> route/remove the interrupt when a domain is running.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
> 
> RFC because I am not entirely sure what people are doing with physical
> interrupt today.

I think it is fine to disable dynamic routing. It is not really
something it is supposed to be used today.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/gic.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 968e46fabb..653a815127 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -136,6 +136,14 @@ int gic_route_irq_to_guest(struct domain *d, unsigned 
> int virq,
>      ASSERT(virq < vgic_num_irqs(d));
>      ASSERT(!is_lpi(virq));
>  
> +    /*
> +     * When routing an IRQ to guest, the virtual state is not synced
> +     * back to the physical IRQ. To prevent get unsync, restrict the
> +     * routing to when the Domain is been created.
> +     */
> +    if ( d->creation_finished )
> +        return -EBUSY;
> +
>      ret = vgic_connect_hw_irq(d, NULL, virq, desc, true);
>      if ( ret )
>          return ret;
> @@ -160,25 +168,19 @@ int gic_remove_irq_from_guest(struct domain *d, 
> unsigned int virq,
>      ASSERT(test_bit(_IRQ_GUEST, &desc->status));
>      ASSERT(!is_lpi(virq));
>  
> -    if ( d->is_dying )
> -    {
> -        desc->handler->shutdown(desc);
> +    /*
> +     * Removing an interrupt while the domain is running may have
> +     * undesirable effect on the vGIC emulation.
> +     */
> +    if ( !d->is_dying )
> +        return -EBUSY;
>  
> -        /* EOI the IRQ if it has not been done by the guest */
> -        if ( test_bit(_IRQ_INPROGRESS, &desc->status) )
> -            gic_hw_ops->deactivate_irq(desc);
> -        clear_bit(_IRQ_INPROGRESS, &desc->status);
> -    }
> -    else
> -    {
> -        /*
> -         * TODO: Handle eviction from LRs For now, deny
> -         * remove if the IRQ is inflight or not disabled.
> -         */
> -        if ( test_bit(_IRQ_INPROGRESS, &desc->status) ||
> -             !test_bit(_IRQ_DISABLED, &desc->status) )
> -            return -EBUSY;
> -    }
> +    desc->handler->shutdown(desc);
> +
> +    /* EOI the IRQ if it has not been done by the guest */
> +    if ( test_bit(_IRQ_INPROGRESS, &desc->status) )
> +        gic_hw_ops->deactivate_irq(desc);
> +    clear_bit(_IRQ_INPROGRESS, &desc->status);
>  
>      ret = vgic_connect_hw_irq(d, NULL, virq, desc, false);
>      if ( ret )

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel