Re: [Xen-devel] [PATCH v10 11/32] ARM: GICv3: forward pending LPIs to guests

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 30/05/17 23:07, Stefano Stabellini wrote:
> On Tue, 30 May 2017, Julien Grall wrote:
> > >      };
> > >  };
> > >
> > > @@ -136,6 +135,80 @@ uint64_t gicv3_get_redist_address(unsigned int cpu,
> > > bool use_pta)
> > >          return per_cpu(lpi_redist, cpu).redist_id << 16;
> > >  }
> > >
> > > +static void vgic_vcpu_inject_lpi(struct domain *d, unsigned int virq)
> > > +{
> > > +    struct pending_irq *p = irq_to_pending(d->vcpu[0], virq);
> > > +    struct vcpu *v = NULL;
> > > +
> > > +    if ( !p )
> > > +        return;
> > > +
> > > +    if ( p->lpi_vcpu_id < d->max_vcpus )
> > > +        v = d->vcpu[read_atomic(&p->lpi_vcpu_id)];
> > Hmmm, what does prevent lpi_vcpu_id to change between the check and the read?
> Supposedly we are going to set lpi_vcpu_id only to good values? Meaning
> that we are going to do the lpi_vcpu_id checks at the time of setting
> lpi_vcpu_id. Thus, even if lpi_vcpu_id changes, it is not a problem. In
> fact, if that is true, can we even drop the if ( p->lpi_vcpu_id <
> d->max_vcpus ) test here?
I am not too confident to say lpi_vcpu_id will always be valid with the 
current locking in the vGIC.
There is a potential race between its_discard_event and this function. 
The former may reset pending_irq whilst reading lpi_vcpu_id as we cannot 
take the vCPU lock yet.
But all of this is racy anyway because of the locking. This will get 
solved by the vGIC rework after the merge.
So For the time being I would keep the check. We can revisit it later if 
necessary.
Cheers,

--
Julien Grall

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