From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 6/6] x86: Use clwb in drm_clflush_virt_range Date: Thu, 13 Nov 2014 20:43:53 +0200 Message-ID: <20141113184353.GV10649@intel.com> References: <1415731396-19364-1-git-send-email-ross.zwisler@linux.intel.com> <1415731396-19364-7-git-send-email-ross.zwisler@linux.intel.com> <5464220D.6090204@amacapital.net> <20141113112017.GA14416@pd.tnic> <20141113171133.GD14070@pd.tnic> <20141113173354.GT10649@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <20141113173354.GT10649@intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Borislav Petkov Cc: Andy Lutomirski , intel-gfx@lists.freedesktop.org, X86 ML , "linux-kernel@vger.kernel.org" , DRI , Ingo Molnar , Ross Zwisler , H Peter Anvin , Thomas Gleixner List-Id: intel-gfx@lists.freedesktop.org On Thu, Nov 13, 2014 at 07:33:54PM +0200, Ville Syrj=E4l=E4 wrote: > On Thu, Nov 13, 2014 at 06:11:33PM +0100, Borislav Petkov wrote: > > On Thu, Nov 13, 2014 at 08:38:23AM -0800, Andy Lutomirski wrote: > > > On Nov 13, 2014 3:20 AM, "Borislav Petkov" wrote: > > > > > > > > On Wed, Nov 12, 2014 at 07:14:21PM -0800, Andy Lutomirski wrote= : > > > > > On 11/11/2014 10:43 AM, Ross Zwisler wrote: > > > > > > If clwb is available on the system, use it in drm_clflush_v= irt_range. > > > > > > If clwb is not available, fall back to clflushopt if you ca= n. > > > > > > If clflushopt is not supported, fall all the way back to cl= flush. > > > > > > > > > > I don't know exactly what drm_clflush_virt_range (and the oth= er > > > > > functions you're modifying similarly) are for, but it seems p= lausible to > > > > > me that they're used before reads to make sure that non-coher= ent memory > > > > > sees updated data. If that's true, then this will break it. > > > > > > > > Why would it break it? The updated cachelines will be in memory= and > > > > subsequent reads will be serviced from the cache instead from g= oing to > > > > memory as it is not invalidated as it would be by CLFLUSH. > > > > > > > > /me is puzzled. > > >=20 > > > Suppose you map some device memory WB, and then the device > > > non-coherently updates. If you want the CPU to see it, you need > > > clflush or clflushopt. Some architectures might do this for > > > dma_sync_single_for_cpu with DMA_FROM_DEVICE. > >=20 > > Ah, you're talking about the other way around - the device does the > > writes. Well, the usage sites are all in i915*, maybe we should ask > > them - it looks to me like this is only the CPU making stuff visibl= e in > > the shared buffer but I don't know that code... intel-gfx CCed alth= ough > > dri-devel is already on CC. >=20 > We use it both ways in i915. So please don't break it. Actually to clarify a bit, I think we shouldn't actually need the invalidate part for any modern platform with shared LLC. Apart from the display those tend to be fully coherent, so we only have to care about the display controller not seeing stale data in memory. I have no idea which platforms support this instruction. If Baytrail/Braswell/Cherrytrail are on the list, then we have a problem. Otherwise we should probably be fine with it. But I'm not 100% sure about any future platforms that are still under wraps. Also ttm seems to use some of this stuff. Not sure what they expect from it. --=20 Ville Syrj=E4l=E4 Intel OTC