Re: [Xen-devel] [PATCH v2 2/4] xen/x86: Drop unnecessary barriers

[Tim Deegan <tim@xxxxxxx>]

At 14:55 +0100 on 18 Aug (1503068128), Tim Deegan wrote:
> At 12:22 +0100 on 16 Aug (1502886128), Andrew Cooper wrote:
> > diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
> > index c9c2252..1e3dfaf 100644
> > --- a/xen/arch/x86/mm/shadow/multi.c
> > +++ b/xen/arch/x86/mm/shadow/multi.c
> > @@ -3112,7 +3112,6 @@ static int sh_page_fault(struct vcpu *v,
> >       * will make sure no inconsistent mapping being translated into
> >       * shadow page table. */
> >      version = atomic_read(&d->arch.paging.shadow.gtable_dirty_version);
> > -    rmb();
> >      walk_ok = sh_walk_guest_tables(v, va, &gw, error_code);
> 
> Nack.  We must read the version before reading the tables, or we might
> accidentally use out-of-date tables.
> 
> If anything, this needs more barriers!  There ought to be a read
> barrier before we re-read the version in shadow_check_gwalk(), but
> taking the paging lock DTRT.  And there ought to be a wmb() before we
> increment the version later on, which I guess I'll make a patch for.

These can be smp_*mb(), though, to align with the rest of the series.

Tim.

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