Re: [Xen-devel] [XEN PATCH v3 07/11] xen: arm: vgic: allow delivery of PPIs to guests

[Julien Grall <julien@xen.org>]

Hi,

On 15/11/2019 20:10, Stewart Hildebrand wrote:
> Allow vgic_get_hw_irq_desc to be called with a vcpu argument.
> 
> Use vcpu argument in vgic_connect_hw_irq.
> 
> vgic_connect_hw_irq is called for PPIs and SPIs, not SGIs. Enforce with
> ASSERTs.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@dornerworks.com>
> 
> ---
> v3: new patch
> 
> ---
> Note: I have only modified the old vgic to allow delivery of PPIs.

The new vGIC should also be modified to support delivery of PPIs.

> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 82f524a35c..c3933c2687 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -410,10 +410,10 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>               irq_set_affinity(p->desc, cpumask_of(v_target->processor));
>               spin_lock_irqsave(&p->desc->lock, flags);
>               /*
> -             * The irq cannot be a PPI, we only support delivery of SPIs
> -             * to guests.
> +             * The irq cannot be a SGI, we only support delivery of SPIs
> +             * and PPIs to guests.
>                */
> -            ASSERT(irq >= 32);
> +            ASSERT(irq >= NR_SGIS);

We usually put ASSERT() in place we know that code wouldn't be able to 
work correctly if there ASSERT were hit. In this particular case:

>               if ( irq_type_set_by_domain(d) )
>                   gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));

1) We don't want to allow any domain (including Dom0) to modify the 
interrupt type (i.e. level/edge) for PPIs as this is shared. You will 
also most likely need to modify the counterpart in setup_guest_irq().

>               p->desc->handler->enable(p->desc);

2) On GICv3, the re-distributor of vCPU A is accessible by vCPU B. So 
vCPU B could enable the SGI for vCPU A. But this would be called on the 
wrong pCPU leading to inconsistency between the hardware state of the 
internal vGIC state.

Cheers,

-- 
Julien Grall

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