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

[Tim Deegan <tim@xxxxxxx>]

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.

Cheers,

Tim.

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