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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/05/2015 16:22, Konrad Rzeszutek Wilk wrote:
> On Mon, May 04, 2015 at 06:39:56PM +0800, Chen, Tiejun wrote:
>> On 2015/5/4 16:52, Jan Beulich wrote:
>>>>>> On 04.05.15 at 04:16, <tiejun.chen@intel.com> 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."
> That is incorrect. We have observed that CLFLUSH instruction do serialize each
> other. That is if on a core you send a bunch of CLFLUSH it stalls the pipeline.
>
> Cc-ing Boris who discovered this.

Simply stalling the pipeline says nothing about its ordering with
respect to other instructions.

It is plausible that certain processor pipelines employ stricter
restrictions on CLFLUSH, but Xen must not assume that this is the case
in general, especially as it is in direct contradiction to both the
Intel and AMD instruction manuals.

~Andrew