Re: [PATCH v3] arm64: mm: fix pass user prot to ioremap_prot in generic_access_phys

[Catalin Marinas <catalin.marinas@arm.com>]

On Thu, Feb 05, 2026 at 06:25:36PM +0000, Catalin Marinas wrote:
> On Thu, Feb 05, 2026 at 05:36:01PM +0000, Will Deacon wrote:
> > On Thu, Feb 05, 2026 at 02:31:47PM +0000, Catalin Marinas wrote:
> > > On Thu, Feb 05, 2026 at 03:23:27PM +0800, Jinjiang Tu wrote:
> > > > 在 2026/2/3 17:23, Will Deacon 写道:
> > > > > On Tue, Feb 03, 2026 at 11:38:15AM +0800, Jinjiang Tu wrote:
> > > > > > 在 2026/2/2 22:55, Will Deacon 写道:
> > > > > > > On Fri, Jan 30, 2026 at 03:38:07PM +0800, Jinjiang Tu wrote:
> > > > > > > > +#define arch_mk_kernel_prot arch_mk_kernel_prot
> > > > > > > > +static inline pgprot_t arch_mk_kernel_prot(pgprot_t user_prot)
> > > > > > > > +{
> > > > > > > > +	ptdesc_t mem_type = pgprot_val(user_prot) & PTE_ATTRINDX_MASK;
> > > > > > > > +
> > > > > > > > +	return __pgprot_modify(PAGE_KERNEL, PTE_ATTRINDX_MASK, mem_type);
> > > > > > > > +}
> > > > > > > 
> > > > > > > Do we really need another arch helper here?
> > > [...]
> > > > > My point is that we already have the helper: ioremap_prot(). Just fix
> > > > > that for arm64 and cc the other arch maintainers if you're not sure how
> > > > > to fix it for them. What we don't need to do is add an additional helper.
> > > > 
> > > > ioremap_prot() may be called outside of arch/arm64 in the future, and I think
> > > > most of the cases will not pass a user prot to ioremap_prot().
> > > > 
> > > > generic_access_phys() is a special case, so I want to limit the modification to
> > > > generic_access_phys() only.
> > > 
> > > Or we can just have an ioremap_user_prot() (or some more meaningful
> > > name), defined by default as ioremap_prot(). It's still introducing a
> > > new macro though, unless we go and rename it on all architectures.
> > 
> > ioremap_prot() has exactly one caller outside of arch code and that is
> > generic_access_phys(). We should just fix the arm64 implementation of
> > ioremap_prot() and not introduce any new macros. If a new caller comes
> > along later, we can figure out what to do then. We could shout if the
> > prot isn't a user prot so we detect the problem.
> 
> I was more worried about out of tree drivers using it since it's an
> EXPORT_SYMBOL(). We should remove the export anyway given that we have
> only a fixed number of memory types programmed in MAIR and all have
> corresponding ioremap wrappers already.
> 
> So yes, just fixing it in ioremap_prot() works for me if we also remove
> the export, just in case there are dodgy drivers out there.

Ah, removing the export would break KMI if backported (unless GKI won't
merge it) since all the other ioremap_* macros use ioremap_prot(). Well,
not a problem for stable/LTS in general, just for GKI.

I would still introduce a new ioremap_user_prot() to make the intent
clearer. In its implementation we could skip the ioremap_prot_hook().
For generic_access_phys(), do we even care about encrypted/decrypted
pgprot?

-- 
Catalin