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

["Zhang, Yang Z" <yang.z.zhang@xxxxxxxxx>]

Jan Beulich wrote on 2015-05-04:
>>>> On 04.05.15 at 11:14, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 04/05/2015 09: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 think "serializing" needs to be changed to
>>> "fencing", as serialization is not what we really care about here.
>>> If you and the maintainers agree, I could certainly fix up both aspects 
>>> while committing.
>> 
>> On the subject of writebacks, we should get around to alternating-up
>> the use of clflushopt and clwb, either of which would be better than
>> a clflush in this case (avoiding the need for the leading mfence).
> 
> Plus the barrier would perhaps rather sit around the loop invoking
> cacheline_flush() in __iommu_flush_cache(), and I wonder whether VT-d

Agree. It's better to batch the flush operations, like:

@@ -167,11 +167,15 @@ static void __iommu_flush_cache(void *addr, unsigned int 
size)
     if ( !iommus_incoherent )
         return;
 
+    mb();
+
     if ( clflush_size == 0 )
         clflush_size = get_cache_line_size();
 
     for ( i = 0; i < size; i += clflush_size )
         cacheline_flush((char *)addr + i);
+
+    mb();
 }

> code shouldn't use available flushing code elsewhere in the system,
> and whether that code then wouldn't need barriers added (or use
> clflushopt/clwb as you
> suggest) instead.


> 
> Jan


Best regards,
Yang


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