Re: [Xen-devel] [PATCH v10 01/32] ARM: vGIC: avoid rank lock when reading priority

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 6 Jun 2017, Julien Grall wrote:
> Hi Andre,
> 
> On 06/06/17 18:20, Andre Przywara wrote:
> > > > 
> > > > What about this one (in xen/arch/arm/vgic-v[23].c):
> > > > 
> > > > static int vgic_v2_distr_mmio_write(struct vcpu *v, ...
> > > > {
> > > > -    uint32_t *ipriorityr;
> > > > +    uint32_t *ipriorityr, priority;
> > > > ....
> > > >      vgic_lock_rank(v, rank, flags);
> > > >      ipriorityr = &rank->ipriorityr[REG_RANK_INDEX(8,
> > > >                                             gicd_reg - GICD_IPRIORITYR,
> > > >                                             DABT_WORD)];
> > > > -    vgic_reg32_update(ipriorityr, r, info);
> > > > +    priority = ACCESS_ONCE(*ipriorityr);
> > > > +    vgic_reg32_update(&priority, r, info);
> > > > +    ACCESS_ONCE(*ipriorityr) = priority;
> > > >      vgic_unlock_rank(v, rank, flags);
> > > > ....
> > > 
> > > What you do is very similar to my patch but open-code it. It might be
> > > possible to have other place which could take advantage of the a generic
> > > solution, so I am not convinced this would be the right way.
> > 
> > My idea was to confine this fix to what we care for right now.
> > I guess eventually it doesn't matter as we will probably get rid of the
> > rank and its lock anyway.
> 
> That's a good point. So we may want to wait until the vGIC rework to see what
> we can do here. Stefano, any opinion?

Either would work. Since we are at it, I would have done what Julien has
done, but also doing the minimum required now is also OK. I don't think
the order of changes is important in this case given that the end result
will be the same.

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