From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [PATCH 1/3] xen/vt-d: need barriers to workaround CLFLUSH Date: Tue, 05 May 2015 10:45:45 +0800 Message-ID: <55482ED9.8050208@intel.com> References: <1430705771-6744-1-git-send-email-tiejun.chen@intel.com> <55474F790200007800076334@mail.emea.novell.com> <55474C7C.4000909@intel.com> <554769870200007800076468@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <554769870200007800076468@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 , andrew.cooper3@citrix.com, kevin.tian@intel.com, yang.z.zhang@intel.com, Konrad Rzeszutek Wilk , boris.ostrovsky@oracle.com Cc: jinsong.liu@alibaba-inc.com, keir@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2015/5/4 18:43, Jan Beulich wrote: >>>> On 04.05.15 at 12:39, wrote: >> On 2015/5/4 16: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 >> >> 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 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