From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/3] xen/vt-d: need barriers to workaround CLFLUSH Date: Mon, 04 May 2015 10:14:27 +0100 Message-ID: <55473873.5030600@citrix.com> References: <1430705771-6744-1-git-send-email-tiejun.chen@intel.com> <55474F790200007800076334@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55474F790200007800076334@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Tiejun Chen Cc: yang.z.zhang@intel.com, jinsong.liu@alibaba-inc.com, kevin.tian@intel.com, keir@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/05/2015 09:52, Jan Beulich wrote: >>>> On 04.05.15 at 04:16, 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). However, the ISA extension document does not indicate which processors will have support for these new instructions. ~Andrew