On Fri, Jun 14, 2019 at 01:37:15PM -0700, Stephen Boyd wrote: > Sometimes we have memories that are supposed to be read-only, but when > we map these regions the best we can do is map them as write-back with > MEMREMAP_WB. Introduce a read-only memory mapping (MEMREMAP_RO) that > allows us to map reserved memory regions as read-only. This way, we're > less likely to see these special memory regions become corrupted by > stray writes to them. > > Cc: Evan Green <[email protected]> > Cc: Rob Herring <[email protected]> > Cc: Bjorn Andersson <[email protected]> > Cc: Andy Gross <[email protected]> > Cc: Will Deacon <[email protected]> > Cc: Catalin Marinas <[email protected]> > Cc: Dan Williams <[email protected]> > Reviewed-by: Bjorn Andersson <[email protected]> > Signed-off-by: Stephen Boyd <[email protected]> > --- > include/linux/io.h | 1 + > kernel/iomem.c | 15 +++++++++++++-- > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/include/linux/io.h b/include/linux/io.h > index 32e30e8fb9db..16c7f4498869 100644 > --- a/include/linux/io.h > +++ b/include/linux/io.h > @@ -159,6 +159,7 @@ enum { > MEMREMAP_WC = 1 << 2, > MEMREMAP_ENC = 1 << 3, > MEMREMAP_DEC = 1 << 4, > + MEMREMAP_RO = 1 << 5, > }; > > void *memremap(resource_size_t offset, size_t size, unsigned long flags); > diff --git a/kernel/iomem.c b/kernel/iomem.c > index 93c264444510..10d5ef0ff09e 100644 > --- a/kernel/iomem.c > +++ b/kernel/iomem.c > @@ -19,6 +19,13 @@ static void *arch_memremap_wb(resource_size_t offset, > unsigned long size) > } > #endif > > +#ifndef arch_memremap_ro > +static void *arch_memremap_ro(resource_size_t offset, unsigned long size) > +{ > + return NULL; > +} > +#endif > + > #ifndef arch_memremap_can_ram_remap > static bool arch_memremap_can_ram_remap(resource_size_t offset, size_t size, > unsigned long flags) > @@ -84,7 +91,10 @@ void *memremap(resource_size_t offset, size_t size, > unsigned long flags) > } > > /* Try all mapping types requested until one returns non-NULL */ > - if (flags & MEMREMAP_WB) { > + if ((flags & MEMREMAP_RO) && is_ram != REGION_INTERSECTS) > + addr = arch_memremap_ro(offset, size); > + > + if (!addr && (flags & MEMREMAP_WB)) { > /* > * MEMREMAP_WB is special in that it can be satisfied > * from the direct map. Some archs depend on the > @@ -103,7 +113,8 @@ void *memremap(resource_size_t offset, size_t size, > unsigned long flags) > * address mapping. Enforce that this mapping is not aliasing > * System RAM. > */ > - if (!addr && is_ram == REGION_INTERSECTS && flags != MEMREMAP_WB) { > + if (!addr && is_ram == REGION_INTERSECTS && > + (flags != MEMREMAP_WB || flags != MEMREMAP_RO)) { > WARN_ONCE(1, "memremap attempted on ram %pa size: %#lx\n", > &offset, (unsigned long) size); > return NULL;
This function seems a little confused about whether 'flags' is really a bitmap of flags, or whether it is equal to exactly one entry in the enum. Given that I think it's sensible for somebody to specify 'MEMREMAP_RO | MEMREMAP_WT', then we probably need to start checking these things a bit more thoroughly so we can reject unsupported combinations at the very least. Will
