Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: Christoph Hellwig <hch@infradead.org>,
	Will Deacon <will.deacon@arm.com>,
	David Zhou <David1.Zhou@amd.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	amd-gfx@lists.freedesktop.org, Junwei Zhang <Jerry.Zhang@amd.com>,
	Huang Rui <ray.huang@amd.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Alex Deucher <alexander.deucher@amd.com>,
	Sean Paul <sean@poorly.run>,
	Christian Koenig <christian.koenig@amd.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86
Date: Mon, 21 Jan 2019 19:20:02 +0100	[thread overview]
Message-ID: <CAKv+Gu-9VKL5RjScbKss4YfH1Cb8WbXmzU_0UsStnoXJcML7Rg@mail.gmail.com> (raw)
In-Reply-To: <ccc775d3-3053-ab79-dac8-4feb28e8e690@daenzer.net>

On Mon, 21 Jan 2019 at 19:04, Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2019-01-21 6:59 p.m., Ard Biesheuvel wrote:
> > On Mon, 21 Jan 2019 at 18:55, Michel Dänzer <michel@daenzer.net> wrote:
> >>
> >> On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote:
> >>> On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig <hch@infradead.org> wrote:
> >>>
> >>>> Until that happens we should just change the driver ifdefs to default
> >>>> the hacks to off and only enable them on setups where we 100%
> >>>> positively know that they actually work.  And document that fact
> >>>> in big fat comments.
> >>>
> >>> Well, as I mentioned in my commit log as well, if we default to off
> >>> unless CONFIG_X86, we may break working setups on MIPS and Power where
> >>> the device is in fact non-cache coherent, and relies on this
> >>> 'optimization' to get things working.
> >>
> >> FWIW, the amdgpu driver doesn't rely on non-snooped transfers for
> >> correct basic operation (the scenario Christian brought up is a very
> >> specialized use-case), so that shouldn't be an issue.
> >>
> >
> > The point is that this is only true for x86.
> >
> > On other architectures, the use of non-cached mappings on the CPU side
> > means that you /do/ rely on non-snooped transfers, since if those
> > transfers turn out not to snoop inadvertently, the accesses are
> > incoherent with the CPU's view of memory.
>
> The driver generally only uses non-cached mappings if
> drm_arch/device_can_wc_memory returns true.
>

Indeed. And so we should take care to only return 'true' from that
function if it is guaranteed that non-cached CPU mappings are coherent
with the mappings used by the GPU, either because that is always the
case (like on x86), or because we know that the platform in question
implements NoSnoop correctly throughout the interconnect.

What seems to be complicating matters is that in some cases, the
device is non-cache coherent to begin with, so regardless of whether
the NoSnoop attribute is used or not, those accesses will not snoop in
the caches and be coherent with the non-cached mappings used by the
CPU. So if we restrict this optimization [on non-X86] to platforms
that are known to implement NoSnoop correctly, we may break platforms
that are implicitly NoSnoop all the time.