Received-SPF: pass (google.com: domain of linux-kernel+bounces-139750-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20240410204337739-0700.eberman@hu-eberman-lv.qualcomm.com>
References: <20240327200917.2576034-1-volodymyr_babchuk@epam.com>
 <e0586d43-284c-4bef-a8be-4ffbc12bf787@linaro.org> <87a5mjz8s3.fsf@epam.com>
 <f4ebe819-9718-42c3-9874-037151587d0c@linaro.org> <cd549ee8-22dc-4bc4-af09-9c5c925ee03a@linaro.org>
 <ZgU_YDUhBeyS5wuh@gerhold.net> <CAE-0n50BGLccCTDfCmOd0Bcbmp7SMwsJd8qTVPWioKvbaD0A0w@mail.gmail.com>
 <20240410204337739-0700.eberman@hu-eberman-lv.qualcomm.com>
From: Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.10
Date: Wed, 10 Apr 2024 21:43:25 -0700
Message-ID: <CAE-0n51e5wm8haqOJgEmE+pEqqFSZOBXvLi-k_+V-Aa0rCXCWA@mail.gmail.com>
Subject: Re: [PATCH] soc: qcom: cmd-db: map shared memory as WT, not WB
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>
Content-Type: text/plain; charset="UTF-8"

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.