Re: [PATCH] soc: qcom: cmd-db: map shared memory as WT, not WB

From: Stephen Boyd <swboyd@chromium.org>
To: Elliot Berman <quic_eberman@quicinc.com>
Cc: Caleb Connolly <caleb.connolly@linaro.org>,
	Stephan Gerhold <stephan@gerhold.net>,
	 Konrad Dybcio <konrad.dybcio@linaro.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	 Bjorn Andersson <andersson@kernel.org>,
	 "linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Douglas Anderson <dianders@chromium.org>,
	 Rob Clark <robdclark@gmail.com>
Subject: Re: [PATCH] soc: qcom: cmd-db: map shared memory as WT, not WB
Date: Wed, 10 Apr 2024 21:43:25 -0700	[thread overview]
Message-ID: <CAE-0n51e5wm8haqOJgEmE+pEqqFSZOBXvLi-k_+V-Aa0rCXCWA@mail.gmail.com> (raw)
In-Reply-To: <20240410204337739-0700.eberman@hu-eberman-lv.qualcomm.com>

Quoting Elliot Berman (2024-04-10 20:54:24)
> On Thu, Mar 28, 2024 at 07:40:49PM -0500, Stephen Boyd wrote:
> > Quoting Stephan Gerhold (2024-03-28 02:58:56)
> > >
> > > FWIW: This old patch series from Stephen Boyd is closely related:
> > > https://lore.kernel.org/linux-arm-msm/20190910160903.65694-1-swboyd@chromium.org/
> > >
> > > > The main use case I have is to map the command-db memory region on
> > > > Qualcomm devices with a read-only mapping. It's already a const marked
> > > > pointer and the API returns const pointers as well, so this series
> > > > makes sure that even stray writes can't modify the memory.
> > >
> > > Stephen, what was the end result of that patch series? Mapping the
> > > cmd-db read-only sounds cleaner than trying to be lucky with the right
> > > set of cache flags.
> > >
> >
> > I dropped it because I got busy with other stuff. Feel free to pick it
> > back up. It looks like the part where I left off was where we had to
> > make the API fallback to mapping the memory as writeable if read-only
> > isn't supported on the architecture.
> [...]
> > The other weird thing was that we passed both MEMREMAP_RO and
> > MEMREMAP_WB to indicate what sort of fallback we want. Perhaps that can
> > be encoded in the architecture layer so that you get the closest thing
> > to read-only memory (i.e. any sort of write side caching is removed) and
> > you don't have to pass a fallback mapping type.
>
> Was there any motivation for having the fallback? I suspect driver
> owners that want to use MEMREMAP_RO will know which architectures the
> driver is applicable to and also know whether MEMREMAP_RO would work.

I don't think there was any motivation, but it has been many years so
maybe I forgot. I suspect you're suggesting that we just don't do that
and driver writers can call the memremap API another time if they want
to implement a fallback? Sounds OK to me.

>
> > I also wanted to return a const pointer.
>
> The stash looks mostly complete. Do you think it should be part of the
> series or submitted separately?
>

Make it part of the series so this topic can be finished? For example,
it seems like iounmap() should have always taken a const pointer.