Re: [Xen-devel] [PATCH v6 2/5] xen/arm: inflight irqs during migration

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-06-23 at 17:37 +0100, Stefano Stabellini wrote:
> - replace the dsb with smb_wmb and smb_rmb, use them to ensure the order
> of accesses to GIC_IRQ_GUEST_QUEUED and GIC_IRQ_GUEST_MIGRATING.

You access/change those with test_bit et al which already include
appropriate barriers/ordering guarantees, I think.

The __test_foo are the variants without ordering.

IOW I think you can probably do without some/all of those barriers.

> @@ -546,6 +575,8 @@ static int vgic_distr_mmio_write(struct vcpu *v, 
> mmio_info_t *info)
>      int offset = (int)(info->gpa - v->domain->arch.vgic.dbase);
>      int gicd_reg = REG(offset);
>      uint32_t tr;
> +    unsigned long trl;

Can you add /* Need unsigned long for XXX */?

Actually, even better would be to call this variable "target" or
something. (tgt?)

And even better than that would be:

 case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
 {
      unsigned long target;
      ... stuff ...
      return 1;
 }

so the scope is limited to the uses.

> +    int i;
>  
>      switch ( gicd_reg )
>      {
[...]
> @@ -786,9 +837,14 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
> irq)
>  
>      spin_lock_irqsave(&v->arch.vgic.lock, flags);
>  
> +    set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
> +    /* update QUEUED before MIGRATING */
                               ^testing

(otherwise I wonder why you aren't setting it)

> +    smp_wmb();
> +    if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &n->status) )
> +        goto out;
> +
>      if ( !list_empty(&n->inflight) )
>      {
> -        set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
>          gic_raise_inflight_irq(v, irq);
>          goto out;
>      }

Ian.



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