Re: [Xen-devel] [PATCH 1/3] xen/vt-d: need barriers to workaround CLFLUSH

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/5/4 18:43, Jan Beulich wrote:
> > > > On 04.05.15 at 12:39, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2015/5/4 16:52, Jan Beulich wrote:
> > > > > > On 04.05.15 at 04:16, <tiejun.chen@xxxxxxxxx> wrote:
> > > > --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> > > > +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> > > > @@ -56,7 +56,9 @@ unsigned int get_cache_line_size(void)
> > > >
> > > >    void cacheline_flush(char * addr)
> > > >    {
> > > > +    mb();
> > > >        clflush(addr);
> > > > +    mb();
> > > >    }
> > > I think the purpose of the flush is to force write back, not to evict
> > > the cache line, and if so wmb() would appear to be sufficient. As
> > > the SDM says that's not the case, a comment explaining why wmb()
> > > is not sufficient would seem necessary. Plus in the description I
> > Seems wmb() is not sufficient here.
> >
> > "CLFLUSH is only ordered by the MFENCE instruction. It is not guaranteed
> > to be ordered by any other fencing, serializing or other CLFLUSH
> > instruction."
> Right - that's what I said in the second sentence.
Thanks for all guys' comments.

Does this work for everyone?

    xen/vt-d: need barriers to workaround CLFLUSH

    clflush is a light weight but not fencing instruction, so
    hence memory fence is necessary to make sure all load/store
    visible before flush cache line.

    Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index c7bda73..1248a17 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -170,8 +170,15 @@ static void __iommu_flush_cache(void *addr, 
unsigned int size)
     if ( clflush_size == 0 )
         clflush_size = get_cache_line_size();

+    /*
+     * CLFLUSH is only ordered by the MFENCE instruction.
+     * It is not guaranteed to be ordered by any other fencing,
+     * serializing or other CLFLUSH instruction.
+     */
+    mb();
     for ( i = 0; i < size; i += clflush_size )
         cacheline_flush((char *)addr + i);
+    mb();
 }

 void iommu_flush_cache_entry(void *addr, unsigned int size)

Thanks
Tiejun

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