Re: [Xen-devel] [PATCH 2/2] xen/arm: observe itarget setting in vgic_enable_irqs and vgic_disable_irqs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2014-05-25 at 19:06 +0100, Stefano Stabellini wrote:
> vgic_enable_irqs should enable irq delivery to the vcpu specified by
> GICD_ITARGETSR, rather than the vcpu that wrote to GICD_ISENABLER.
> Similarly vgic_disable_irqs should use the target vcpu specified by
> itarget to disable irqs.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
>  xen/arch/arm/vgic.c |   42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index e4f38a0..0f0ba1d 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -376,12 +376,25 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t 
> r, int n)
>      unsigned int irq;
>      unsigned long flags;
>      int i = 0;
> +    int target;
> +    struct vcpu *v_target;
> +    struct vgic_irq_rank *rank;
>  
>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
> -        p = irq_to_pending(v, irq);
> +        rank = vgic_irq_rank(v, 1, irq/32);
> +        vgic_lock_rank(v, rank);
> +        if ( irq >= 32 )
> +        {
> +            target = rank->itargets[(irq%32)/4] >> (8*(irq % 4));
> +            target &= 0xff;

This is byte_read(), isn't it?

> +            v_target = v->domain->vcpu[target];

There needs to be some sort of range check here I think. Else you are
setting a trap for whoever implements itargets properly.

> +        } else
> +            v_target = v;
> +        vgic_unlock_rank(v, rank);
> +        p = irq_to_pending(v_target, irq);
>          clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> -        gic_remove_from_queues(v, irq);
> +        gic_remove_from_queues(v_target, irq);
>          if ( p->desc != NULL )
>          {
>              spin_lock_irqsave(&p->desc->lock, flags);
> @@ -399,21 +412,34 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t 
> r, int n)
>      unsigned int irq;
>      unsigned long flags;
>      int i = 0;
> +    int target;
> +    struct vcpu *v_target;
> +    struct vgic_irq_rank *rank;
>  
>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
> -        p = irq_to_pending(v, irq);
> +        rank = vgic_irq_rank(v, 1, irq/32);
> +        vgic_lock_rank(v, rank);
> +        if ( irq >= 32 )
> +        {
> +            target = rank->itargets[(irq%32)/4] >> (8*(irq % 4));
> +            target &= 0xff;
> +            v_target = v->domain->vcpu[target];
> +        } else
> +            v_target = v;

This is the same code as above -- there should be a helper
(get_target_vcpu or some such).

> +        vgic_unlock_rank(v, rank);
> +        p = irq_to_pending(v_target, irq);
>          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> -        if ( irq == v->domain->arch.evtchn_irq &&
> +        if ( irq == v_target->domain->arch.evtchn_irq &&
>               vcpu_info(current, evtchn_upcall_pending) &&
>               list_empty(&p->inflight) )
> -            vgic_vcpu_inject_irq(v, irq);
> +            vgic_vcpu_inject_irq(v_target, irq);
>          else {
>              unsigned long flags;
> -            spin_lock_irqsave(&v->arch.vgic.lock, flags);
> +            spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
>              if ( !list_empty(&p->inflight) && 
> !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
> -                gic_raise_guest_irq(v, irq, p->priority);
> -            spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> +                gic_raise_guest_irq(v_target, irq, p->priority);
> +            spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
>          }
>          if ( p->desc != NULL )
>          {



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel