* [PATCH v2 0/5] Read-only memremap()
@ 2019-06-14 20:37 Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 1/5] reserved_mem: Add a devm_memremap_reserved_mem() API Stephen Boyd
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
This patch series implements a read-only version of memremap() via
a new MEMREMAP_RO flag. If this is passed in the mapping call, we'll
try to map the memory region as read-only if it doesn't intersect
with an existing mapping. Otherwise, we'll try to fallback to other
flags to try to map the memory that way.
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. To get there we
introduce a devm version of memremap() for a reserved memory region, add
a memremap() flag, and implement support for that flag on arm64.
Changes from v1:
* Picked up tags and rebased to v5.2-rc3
Cc: Evan Green <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Stephen Boyd (5):
reserved_mem: Add a devm_memremap_reserved_mem() API
soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem()
memremap: Add support for read-only memory mappings
arm64: Add support for arch_memremap_ro()
soc: qcom: cmd-db: Map with read-only mappings
arch/arm64/include/asm/io.h | 1 +
drivers/of/of_reserved_mem.c | 45 +++++++++++++++++++++++++++++++++
drivers/soc/qcom/cmd-db.c | 14 +++-------
include/linux/io.h | 1 +
include/linux/of_reserved_mem.h | 6 +++++
kernel/iomem.c | 15 +++++++++--
6 files changed, 70 insertions(+), 12 deletions(-)
base-commit: f2c7c76c5d0a443053e94adb9f0918fa2fb85c3a
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/5] reserved_mem: Add a devm_memremap_reserved_mem() API
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
@ 2019-06-14 20:37 ` Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 2/5] soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem() Stephen Boyd
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas, Rob Herring
We have a few drivers that need to get a reserved memory region, request
the region, and map the reserved memory with memremap(). Add an API to
do this all in one function call.
Cc: Evan Green <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/of/of_reserved_mem.c | 45 +++++++++++++++++++++++++++++++++
include/linux/of_reserved_mem.h | 6 +++++
2 files changed, 51 insertions(+)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 89e190e94af7..bff932a3b80a 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -12,6 +12,7 @@
#define pr_fmt(fmt) "OF: reserved mem: " fmt
#include <linux/err.h>
+#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_fdt.h>
#include <linux/of_platform.h>
@@ -407,3 +408,47 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
return NULL;
}
EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
+
+/**
+ * devm_memremap_reserved_mem() - acquire reserved_mem from a device node,
+ * request and memremap it
+ * @dev: device with node pointer of the desired reserved-memory region
+ * @flags: flags to pass to memremap()
+ *
+ * This function allows drivers to acquire a reference to the reserved_mem
+ * struct based on the device's device_node handle, request it and then
+ * memremap() it.
+ *
+ * Returns: A remapped reserved memory region, or an error pointer on failure.
+ */
+void *devm_memremap_reserved_mem(struct device *dev, unsigned long flags)
+{
+ void *dest_ptr;
+ struct reserved_mem *rmem;
+ struct resource *res;
+ const char *name;
+
+ rmem = of_reserved_mem_lookup(dev->of_node);
+ if (!rmem) {
+ dev_err(dev, "failed to acquire memory region\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ name = rmem->name ? : dev_name(dev);
+
+ res = devm_request_mem_region(dev, rmem->base, rmem->size, name);
+ if (!res) {
+ dev_err(dev, "can't request region for reserved memory\n");
+ return ERR_PTR(-EBUSY);
+ }
+
+ dest_ptr = devm_memremap(dev, rmem->base, rmem->size, flags);
+ if (!dest_ptr) {
+ dev_err(dev, "memremap failed for reserved memory\n");
+ devm_release_mem_region(dev, rmem->base, rmem->size);
+ dest_ptr = ERR_PTR(-ENOMEM);
+ }
+
+ return dest_ptr;
+}
+EXPORT_SYMBOL_GPL(devm_memremap_reserved_mem);
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index 60f541912ccf..a36be60ef67c 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -39,6 +39,7 @@ void fdt_init_reserved_mem(void);
void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
phys_addr_t base, phys_addr_t size);
struct reserved_mem *of_reserved_mem_lookup(struct device_node *np);
+void *devm_memremap_reserved_mem(struct device *dev, unsigned long flags);
#else
static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
struct device_node *np, int idx)
@@ -54,6 +55,11 @@ static inline struct reserved_mem *of_reserved_mem_lookup(struct device_node *np
{
return NULL;
}
+static inline void *devm_memremap_reserved_mem(struct device *dev,
+ unsigned long flags)
+{
+ return NULL;
+}
#endif
/**
--
Sent by a computer through tubes
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/5] soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem()
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 1/5] reserved_mem: Add a devm_memremap_reserved_mem() API Stephen Boyd
@ 2019-06-14 20:37 ` Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 3/5] memremap: Add support for read-only memory mappings Stephen Boyd
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
This gets rid of some duplicate code, and also makes the reserved memory
region show up as 'cmd-db' memory in /proc/iomem.
Cc: Evan Green <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/soc/qcom/cmd-db.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index f6c3d17b05c7..10a34d26b753 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -238,18 +238,11 @@ EXPORT_SYMBOL(cmd_db_read_slave_id);
static int cmd_db_dev_probe(struct platform_device *pdev)
{
- struct reserved_mem *rmem;
int ret = 0;
- rmem = of_reserved_mem_lookup(pdev->dev.of_node);
- if (!rmem) {
- dev_err(&pdev->dev, "failed to acquire memory region\n");
- return -EINVAL;
- }
-
- cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB);
- if (!cmd_db_header) {
- ret = -ENOMEM;
+ cmd_db_header = devm_memremap_reserved_mem(&pdev->dev, MEMREMAP_WB);
+ if (IS_ERR(cmd_db_header)) {
+ ret = PTR_ERR(cmd_db_header);
cmd_db_header = NULL;
return ret;
}
--
Sent by a computer through tubes
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/5] memremap: Add support for read-only memory mappings
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 1/5] reserved_mem: Add a devm_memremap_reserved_mem() API Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 2/5] soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem() Stephen Boyd
@ 2019-06-14 20:37 ` Stephen Boyd
2019-07-10 14:14 ` Will Deacon
2019-06-14 20:37 ` [PATCH v2 4/5] arm64: Add support for arch_memremap_ro() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 5/5] soc: qcom: cmd-db: Map with read-only mappings Stephen Boyd
4 siblings, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
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 <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
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;
--
Sent by a computer through tubes
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/5] arm64: Add support for arch_memremap_ro()
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
` (2 preceding siblings ...)
2019-06-14 20:37 ` [PATCH v2 3/5] memremap: Add support for read-only memory mappings Stephen Boyd
@ 2019-06-14 20:37 ` Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 5/5] soc: qcom: cmd-db: Map with read-only mappings Stephen Boyd
4 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
Pass in PAGE_KERNEL_RO to the underlying IO mapping mechanism to get a
read-only mapping for the MEMREMAP_RO type of memory mappings that
memremap() supports.
Cc: Evan Green <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
arch/arm64/include/asm/io.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
index b807cb9b517d..cc33f4c8647b 100644
--- a/arch/arm64/include/asm/io.h
+++ b/arch/arm64/include/asm/io.h
@@ -183,6 +183,7 @@ extern void __iomem *ioremap_cache(phys_addr_t phys_addr, size_t size);
#define ioremap_nocache(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
#define ioremap_wc(addr, size) __ioremap((addr), (size), __pgprot(PROT_NORMAL_NC))
#define ioremap_wt(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
+#define arch_memremap_ro(addr, size) __ioremap((addr), (size), PAGE_KERNEL_RO)
#define iounmap __iounmap
/*
--
Sent by a computer through tubes
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 5/5] soc: qcom: cmd-db: Map with read-only mappings
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
` (3 preceding siblings ...)
2019-06-14 20:37 ` [PATCH v2 4/5] arm64: Add support for arch_memremap_ro() Stephen Boyd
@ 2019-06-14 20:37 ` Stephen Boyd
4 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-06-14 20:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
The command DB is read-only already to the kernel because everything is
const marked once we map it. Let's go one step further and try to map
the memory as read-only in the page tables. This should make it harder
for random code to corrupt the database and change the contents.
Cc: Evan Green <evgreen@chromium.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Andy Gross <agross@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/soc/qcom/cmd-db.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c
index 10a34d26b753..6365e8260282 100644
--- a/drivers/soc/qcom/cmd-db.c
+++ b/drivers/soc/qcom/cmd-db.c
@@ -240,7 +240,8 @@ static int cmd_db_dev_probe(struct platform_device *pdev)
{
int ret = 0;
- cmd_db_header = devm_memremap_reserved_mem(&pdev->dev, MEMREMAP_WB);
+ cmd_db_header = devm_memremap_reserved_mem(&pdev->dev,
+ MEMREMAP_RO | MEMREMAP_WB);
if (IS_ERR(cmd_db_header)) {
ret = PTR_ERR(cmd_db_header);
cmd_db_header = NULL;
--
Sent by a computer through tubes
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/5] memremap: Add support for read-only memory mappings
2019-06-14 20:37 ` [PATCH v2 3/5] memremap: Add support for read-only memory mappings Stephen Boyd
@ 2019-07-10 14:14 ` Will Deacon
2019-07-18 18:00 ` Stephen Boyd
0 siblings, 1 reply; 8+ messages in thread
From: Will Deacon @ 2019-07-10 14:14 UTC (permalink / raw)
To: Stephen Boyd
Cc: Dan Williams, linux-kernel, linux-arm-msm, linux-arm-kernel,
devicetree, Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
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 <evgreen@chromium.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 3/5] memremap: Add support for read-only memory mappings
2019-07-10 14:14 ` Will Deacon
@ 2019-07-18 18:00 ` Stephen Boyd
0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-07-18 18:00 UTC (permalink / raw)
To: Will Deacon
Cc: Dan Williams, linux-kernel, linux-arm-msm, linux-arm-kernel,
devicetree, Evan Green, Rob Herring, Bjorn Andersson, Andy Gross,
Will Deacon, Catalin Marinas
Quoting Will Deacon (2019-07-10 07:14:34)
> On Fri, Jun 14, 2019 at 01:37:15PM -0700, Stephen Boyd wrote:
> > @@ -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.
>
I'm also confused about the same thing. I thought it was a "getting
worse via best effort" type of thing based on the comment above the
function.
* In the case of multiple flags, the different
* mapping types will be attempted in the order listed below until one of
* them succeeds.
(I now realize I should have documented the new flag so that this order
would be known. I'll resend this series again with the documentation
fix.)
I also thought that the combination of read-only and write through would
be OK because the flags are more of a best effort approach to making a
mapping. Given that, is there anything to reject? Or do we just keep
trying until we can't try anymore?
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-18 18:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 20:37 [PATCH v2 0/5] Read-only memremap() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 1/5] reserved_mem: Add a devm_memremap_reserved_mem() API Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 2/5] soc: qcom: cmd-db: Migrate to devm_memremap_reserved_mem() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 3/5] memremap: Add support for read-only memory mappings Stephen Boyd
2019-07-10 14:14 ` Will Deacon
2019-07-18 18:00 ` Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 4/5] arm64: Add support for arch_memremap_ro() Stephen Boyd
2019-06-14 20:37 ` [PATCH v2 5/5] soc: qcom: cmd-db: Map with read-only mappings Stephen Boyd
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).