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: Wed, 06 May 2015 15:26:33 +0800 Message-ID: <5549C229.7070504@intel.com> References: <1430705771-6744-1-git-send-email-tiejun.chen@intel.com> <55474F790200007800076334@mail.emea.novell.com> <55473873.5030600@citrix.com> <5548E5C9.7020108@oracle.com> <554904B20200007800076D0C@mail.emea.novell.com> <5548EBB7.7070805@oracle.com> <5549DAFF0200007800076EF0@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: <5549DAFF0200007800076EF0@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 , Boris Ostrovsky Cc: kevin.tian@intel.com, keir@xen.org, jinsong.liu@alibaba-inc.com, xen-devel@lists.xen.org, Andrew Cooper , yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2015/5/6 15:12, Jan Beulich wrote: >>>> On 05.05.15 at 18:11, wrote: >> On 05/05/2015 11:58 AM, Jan Beulich wrote: >>>>>> On 05.05.15 at 17:46, wrote: >>>> On 05/04/2015 05:14 AM, Andrew Cooper wrote: >>>>> 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. >>>> https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf >>>> >>>> We really should add support for this. On shutting down a very large >>>> guest (hundreds of GB) we observed *minutes* spent in flushing IOMMU. >>>> This was due to serializing nature of CLFLUSH. >>> But flushing the IOMMU isn't being done via CPU instructions, but >>> rather via commands sent to the IOMMU. I.e. I'm somewhat >>> confused by your reply. >> >> I didn't mean flushing IOMMU itself, sorry. I meant >> __iommu_flush_cache() (or whatever it's equivalent we had in the >> product, which was 4.1-based). > > In that case I wonder how much of that flushing is really necessary Sorry, what is that case? > during IOMMU teardown. VT-d maintainers? > In most cases __iommu_flush_cache() is used to flush any remapping structures into memory then IOMMU can get proper data. Thanks Tiejun