LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] pstore: Add mem_type property DT parsing support
@ 2021-03-22 18:42 Mukesh Ojha
  2021-03-30 12:40 ` Mukesh Ojha
  2021-03-31 17:47 ` Kees Cook
  0 siblings, 2 replies; 3+ messages in thread
From: Mukesh Ojha @ 2021-03-22 18:42 UTC (permalink / raw
  To: linux-kernel; +Cc: keescook, anton, ccross, tony.luck, Mukesh Ojha

There could be a scenario where we define some region
in normal memory and use them store to logs which is later
retrieved by bootloader during warm reset.

In this scenario, we wanted to treat this memory as normal
cacheable memory instead of default behaviour which
is an overhead. Making it cacheable could improve
performance.

This commit gives control to change mem_type from Device
tree, and also documents the value for normal memory.

Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
---
Changes in v3:
 - Minor code and documentation update done as per comment given by Kees.

Changes in v2:
 - if-else converted to switch case
 - updated MODULE_PARM_DESC with new memory type.
 - default setting is still intact.

 Documentation/admin-guide/ramoops.rst                  |  4 +++-
 .../devicetree/bindings/reserved-memory/ramoops.txt    | 10 ++++++++--
 fs/pstore/ram.c                                        |  7 ++++++-
 fs/pstore/ram_core.c                                   | 18 ++++++++++++++++--
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/ramoops.rst b/Documentation/admin-guide/ramoops.rst
index b0a1ae7..8f107d8 100644
--- a/Documentation/admin-guide/ramoops.rst
+++ b/Documentation/admin-guide/ramoops.rst
@@ -3,7 +3,7 @@ Ramoops oops/panic logger
 
 Sergiu Iordache <sergiu@chromium.org>
 
-Updated: 17 November 2011
+Updated: 10 Feb 2021
 
 Introduction
 ------------
@@ -30,6 +30,8 @@ mapping to pgprot_writecombine. Setting ``mem_type=1`` attempts to use
 depends on atomic operations. At least on ARM, pgprot_noncached causes the
 memory to be mapped strongly ordered, and atomic operations on strongly ordered
 memory are implementation defined, and won't work on many ARMs such as omaps.
+Setting ``mem_type=2`` attempts to treat the memory region as normal memory,
+which enables full cache on it. This can improve the performance.
 
 The memory area is divided into ``record_size`` chunks (also rounded down to
 power of two) and each kmesg dump writes a ``record_size`` chunk of
diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
index b7886fe..6f1cb20 100644
--- a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
@@ -42,8 +42,14 @@ Optional properties:
 - pmsg-size: size in bytes of log buffer reserved for userspace messages
   (defaults to 0: disabled)
 
-- unbuffered: if present, use unbuffered mappings to map the reserved region
-  (defaults to buffered mappings)
+- mem-type: if present, sets the type of mapping is to be used to map the
+  reserved region. mem-type: 0 = write-combined (default), 1 = unbuffered,
+  2 = cached.
+
+- unbuffered: deprecated, use mem_type instead. if present, and mem_type is
+  not specified, it is equivalent to mem_type = 1 and uses unbuffered mappings
+  to map the reserved region (defaults to buffered mappings mem_type = 0). If
+  both are specified -- "mem_type" overrides "unbuffered".
 
 - max-reason: if present, sets maximum type of kmsg dump reasons to store
   (defaults to 2: log Oopses and Panics). This can be set to INT_MAX to
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ca6d8a8..fefe3d3 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -56,7 +56,7 @@ MODULE_PARM_DESC(mem_size,
 static unsigned int mem_type;
 module_param(mem_type, uint, 0400);
 MODULE_PARM_DESC(mem_type,
-		"set to 1 to try to use unbuffered memory (default 0)");
+		"memory type: 0=write-combined (default), 1=unbuffered, 2=cached");
 
 static int ramoops_max_reason = -1;
 module_param_named(max_reason, ramoops_max_reason, int, 0400);
@@ -648,6 +648,10 @@ static int ramoops_parse_dt(struct platform_device *pdev,
 
 	pdata->mem_size = resource_size(res);
 	pdata->mem_address = res->start;
+	/*
+	 * Setting "unbuffered" is deprecated and will be ignored if
+	 * "mem_type" is also specified.
+	 */
 	pdata->mem_type = of_property_read_bool(of_node, "unbuffered");
 	/*
 	 * Setting "no-dump-oops" is deprecated and will be ignored if
@@ -666,6 +670,7 @@ static int ramoops_parse_dt(struct platform_device *pdev,
 		field = value;						\
 	}
 
+	parse_u32("mem-type", pdata->record_size, pdata->mem_type);
 	parse_u32("record-size", pdata->record_size, 0);
 	parse_u32("console-size", pdata->console_size, 0);
 	parse_u32("ftrace-size", pdata->ftrace_size, 0);
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index fff363b..fe53050 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -396,6 +396,10 @@ void persistent_ram_zap(struct persistent_ram_zone *prz)
 	persistent_ram_update_header_ecc(prz);
 }
 
+#define MEM_TYPE_WCOMBINE	0
+#define MEM_TYPE_NONCACHED	1
+#define MEM_TYPE_NORMAL		2
+
 static void *persistent_ram_vmap(phys_addr_t start, size_t size,
 		unsigned int memtype)
 {
@@ -409,10 +413,20 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
 	page_start = start - offset_in_page(start);
 	page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
 
-	if (memtype)
+	switch (memtype) {
+	case MEM_TYPE_NORMAL:
+		prot = PAGE_KERNEL;
+		break;
+	case MEM_TYPE_NONCACHED:
 		prot = pgprot_noncached(PAGE_KERNEL);
-	else
+		break;
+	case MEM_TYPE_WCOMBINE:
 		prot = pgprot_writecombine(PAGE_KERNEL);
+		break;
+	default:
+		pr_err("invalid mem_type=%d\n", memtype);
+		return NULL;
+	}
 
 	pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL);
 	if (!pages) {
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] pstore: Add mem_type property DT parsing support
  2021-03-22 18:42 [PATCH v3] pstore: Add mem_type property DT parsing support Mukesh Ojha
@ 2021-03-30 12:40 ` Mukesh Ojha
  2021-03-31 17:47 ` Kees Cook
  1 sibling, 0 replies; 3+ messages in thread
From: Mukesh Ojha @ 2021-03-30 12:40 UTC (permalink / raw
  To: linux-kernel; +Cc: keescook, anton, ccross, tony.luck

Hi Kees/All,

Could you please review again?  I have addressed your comment made on 
last patch version.

Thanks,
Mukesh

On 3/23/2021 12:12 AM, Mukesh Ojha wrote:
> There could be a scenario where we define some region
> in normal memory and use them store to logs which is later
> retrieved by bootloader during warm reset.
>
> In this scenario, we wanted to treat this memory as normal
> cacheable memory instead of default behaviour which
> is an overhead. Making it cacheable could improve
> performance.
>
> This commit gives control to change mem_type from Device
> tree, and also documents the value for normal memory.
>
> Signed-off-by: Mukesh Ojha <mojha@codeaurora.org>
> ---
> Changes in v3:
>   - Minor code and documentation update done as per comment given by Kees.
>
> Changes in v2:
>   - if-else converted to switch case
>   - updated MODULE_PARM_DESC with new memory type.
>   - default setting is still intact.
>
>   Documentation/admin-guide/ramoops.rst                  |  4 +++-
>   .../devicetree/bindings/reserved-memory/ramoops.txt    | 10 ++++++++--
>   fs/pstore/ram.c                                        |  7 ++++++-
>   fs/pstore/ram_core.c                                   | 18 ++++++++++++++++--
>   4 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/admin-guide/ramoops.rst b/Documentation/admin-guide/ramoops.rst
> index b0a1ae7..8f107d8 100644
> --- a/Documentation/admin-guide/ramoops.rst
> +++ b/Documentation/admin-guide/ramoops.rst
> @@ -3,7 +3,7 @@ Ramoops oops/panic logger
>   
>   Sergiu Iordache <sergiu@chromium.org>
>   
> -Updated: 17 November 2011
> +Updated: 10 Feb 2021
>   
>   Introduction
>   ------------
> @@ -30,6 +30,8 @@ mapping to pgprot_writecombine. Setting ``mem_type=1`` attempts to use
>   depends on atomic operations. At least on ARM, pgprot_noncached causes the
>   memory to be mapped strongly ordered, and atomic operations on strongly ordered
>   memory are implementation defined, and won't work on many ARMs such as omaps.
> +Setting ``mem_type=2`` attempts to treat the memory region as normal memory,
> +which enables full cache on it. This can improve the performance.
>   
>   The memory area is divided into ``record_size`` chunks (also rounded down to
>   power of two) and each kmesg dump writes a ``record_size`` chunk of
> diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> index b7886fe..6f1cb20 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> +++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.txt
> @@ -42,8 +42,14 @@ Optional properties:
>   - pmsg-size: size in bytes of log buffer reserved for userspace messages
>     (defaults to 0: disabled)
>   
> -- unbuffered: if present, use unbuffered mappings to map the reserved region
> -  (defaults to buffered mappings)
> +- mem-type: if present, sets the type of mapping is to be used to map the
> +  reserved region. mem-type: 0 = write-combined (default), 1 = unbuffered,
> +  2 = cached.
> +
> +- unbuffered: deprecated, use mem_type instead. if present, and mem_type is
> +  not specified, it is equivalent to mem_type = 1 and uses unbuffered mappings
> +  to map the reserved region (defaults to buffered mappings mem_type = 0). If
> +  both are specified -- "mem_type" overrides "unbuffered".
>   
>   - max-reason: if present, sets maximum type of kmsg dump reasons to store
>     (defaults to 2: log Oopses and Panics). This can be set to INT_MAX to
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index ca6d8a8..fefe3d3 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -56,7 +56,7 @@ MODULE_PARM_DESC(mem_size,
>   static unsigned int mem_type;
>   module_param(mem_type, uint, 0400);
>   MODULE_PARM_DESC(mem_type,
> -		"set to 1 to try to use unbuffered memory (default 0)");
> +		"memory type: 0=write-combined (default), 1=unbuffered, 2=cached");
>   
>   static int ramoops_max_reason = -1;
>   module_param_named(max_reason, ramoops_max_reason, int, 0400);
> @@ -648,6 +648,10 @@ static int ramoops_parse_dt(struct platform_device *pdev,
>   
>   	pdata->mem_size = resource_size(res);
>   	pdata->mem_address = res->start;
> +	/*
> +	 * Setting "unbuffered" is deprecated and will be ignored if
> +	 * "mem_type" is also specified.
> +	 */
>   	pdata->mem_type = of_property_read_bool(of_node, "unbuffered");
>   	/*
>   	 * Setting "no-dump-oops" is deprecated and will be ignored if
> @@ -666,6 +670,7 @@ static int ramoops_parse_dt(struct platform_device *pdev,
>   		field = value;						\
>   	}
>   
> +	parse_u32("mem-type", pdata->record_size, pdata->mem_type);
>   	parse_u32("record-size", pdata->record_size, 0);
>   	parse_u32("console-size", pdata->console_size, 0);
>   	parse_u32("ftrace-size", pdata->ftrace_size, 0);
> diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
> index fff363b..fe53050 100644
> --- a/fs/pstore/ram_core.c
> +++ b/fs/pstore/ram_core.c
> @@ -396,6 +396,10 @@ void persistent_ram_zap(struct persistent_ram_zone *prz)
>   	persistent_ram_update_header_ecc(prz);
>   }
>   
> +#define MEM_TYPE_WCOMBINE	0
> +#define MEM_TYPE_NONCACHED	1
> +#define MEM_TYPE_NORMAL		2
> +
>   static void *persistent_ram_vmap(phys_addr_t start, size_t size,
>   		unsigned int memtype)
>   {
> @@ -409,10 +413,20 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size,
>   	page_start = start - offset_in_page(start);
>   	page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE);
>   
> -	if (memtype)
> +	switch (memtype) {
> +	case MEM_TYPE_NORMAL:
> +		prot = PAGE_KERNEL;
> +		break;
> +	case MEM_TYPE_NONCACHED:
>   		prot = pgprot_noncached(PAGE_KERNEL);
> -	else
> +		break;
> +	case MEM_TYPE_WCOMBINE:
>   		prot = pgprot_writecombine(PAGE_KERNEL);
> +		break;
> +	default:
> +		pr_err("invalid mem_type=%d\n", memtype);
> +		return NULL;
> +	}
>   
>   	pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL);
>   	if (!pages) {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] pstore: Add mem_type property DT parsing support
  2021-03-22 18:42 [PATCH v3] pstore: Add mem_type property DT parsing support Mukesh Ojha
  2021-03-30 12:40 ` Mukesh Ojha
@ 2021-03-31 17:47 ` Kees Cook
  1 sibling, 0 replies; 3+ messages in thread
From: Kees Cook @ 2021-03-31 17:47 UTC (permalink / raw
  To: linux-kernel, Mukesh Ojha; +Cc: Kees Cook, ccross, tony.luck, anton

On Tue, 23 Mar 2021 00:12:17 +0530, Mukesh Ojha wrote:
> There could be a scenario where we define some region
> in normal memory and use them store to logs which is later
> retrieved by bootloader during warm reset.
> 
> In this scenario, we wanted to treat this memory as normal
> cacheable memory instead of default behaviour which
> is an overhead. Making it cacheable could improve
> performance.
> 
> [...]

Applied to for-next/pstore, thanks!

[1/1] pstore: Add mem_type property DT parsing support
      https://git.kernel.org/kees/c/9d843e8fafc7

-- 
Kees Cook


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-03-31 17:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-22 18:42 [PATCH v3] pstore: Add mem_type property DT parsing support Mukesh Ojha
2021-03-30 12:40 ` Mukesh Ojha
2021-03-31 17:47 ` Kees Cook

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 read-only IMAP folder(s) and NNTP newsgroup(s).