[PATCH] pstore/ram: Allow ramoops.ecc to override DT and platform_data

[Raul E Rangel]

When ramoops is instantiated via DeviceTree (ramoops_parse_dt()) or
platform data (e.g. chromeos_pstore), the ramoops.ecc kernel
command-line and module parameter is currently ignored. Depthcharge
and firmware often create the /reserved-memory/ramoops node without
setting the "ecc-size" property, leaving users unable to configure
or override Reed-Solomon ECC from the kernel command line without
modifying firmware tables or DTBs.

Initialize ramoops_ecc to -1 to differentiate between an unspecified
parameter and an explicit override. Snapshot ramoops_ecc_param in
ramoops_init() before ramoops_probe() overwrites ramoops_ecc with the
active ECC size for sysfs parameter visibility. In ramoops_probe(),
if ramoops_ecc_param >= 0, let it override cxt->ecc_info.ecc_size
(retaining the backwards-compatible mapping of 1 to 16 bytes, and 0
disabling ECC). Also update Documentation/admin-guide/ramoops.rst to
document this override and modernize the struct ramoops_platform_data
example.

Signed-off-by: Raul E Rangel <rrangel@xxxxxxxxxxxx>
Assisted-by: Gemini
---
Tested on an x86_64 Alder Lake Chromebook (brya / osiris) running Linux
6.18 with ACPI / platform_data (chromeos_pstore, default ecc_size=4):
- Default (ramoops.ecc=4): ramoops initializes with ecc: 4, and console
  and pmsg logs in /sys/fs/pstore/ persist across warm reboot.
- Positive override (ramoops.ecc=16): overrides platform_data ecc_size=4,
  initializing ramoops with ecc: 16 and updating
  /sys/module/ramoops/parameters/ecc to 16.
- Zero override (ramoops.ecc=0): overrides platform_data ecc_size=4,
  disabling ECC (ecc: 0) and updating /sys/module/ramoops/parameters/ecc
  to 0.

 Documentation/admin-guide/ramoops.rst |  9 +++++++--
 fs/pstore/ram.c                       | 22 ++++++++++++++++------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/ramoops.rst b/Documentation/admin-guide/ramoops.rst
index 2eabef31220d..2c512b718717 100644
--- a/Documentation/admin-guide/ramoops.rst
+++ b/Documentation/admin-guide/ramoops.rst
@@ -54,7 +54,10 @@ on restart (i.e. new dumps after the restart will overwrite old ones).
 Ramoops also supports software ECC protection of persistent memory regions.
 This might be useful when a hardware reset was used to bring the machine back
 to life (i.e. a watchdog triggered). In such cases, RAM may be somewhat
-corrupt, but usually it is restorable.
+corrupt, but usually it is restorable. Software ECC can be configured via the
+``ecc-size`` Device Tree property or ``.ecc_info.ecc_size`` in platform data,
+and can be overridden via the ``ramoops.ecc`` kernel command-line / module
+parameter (where ``0`` disables ECC).
 
 Setting the parameters
 ----------------------
@@ -101,7 +104,9 @@ Setting the ramoops parameters can be done in several different manners:
         .mem_type               = <...>,
         .record_size            = <...>,
         .max_reason             = <...>,
-        .ecc                    = <...>,
+        .ecc_info               = {
+                .ecc_size       = <...>,
+        },
   };
 
   static struct platform_device ramoops_dev = {
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 2eb0d4fa2186..3eac84701fb2 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -69,12 +69,11 @@ module_param_named(max_reason, ramoops_max_reason, int, 0400);
 MODULE_PARM_DESC(max_reason,
 		 "maximum reason for kmsg dump (default 2: Oops and Panic) ");
 
-static int ramoops_ecc;
+static int ramoops_ecc = -1;
+static int ramoops_ecc_param = -1;
 module_param_named(ecc, ramoops_ecc, int, 0400);
 MODULE_PARM_DESC(ecc,
-		"if non-zero, the option enables ECC support and specifies "
-		"ECC buffer size in bytes (1 is a special value, means 16 "
-		"bytes ECC)");
+		"if positive, the option enables ECC support and specifies ECC buffer size in bytes (1 is a special value, means 16 bytes ECC; 0 disables ECC)");
 
 static int ramoops_dump_oops = -1;
 module_param_named(dump_oops, ramoops_dump_oops, int, 0400);
@@ -780,6 +779,15 @@ static int ramoops_probe(struct platform_device *pdev)
 	cxt->pmsg_size = pdata->pmsg_size;
 	cxt->flags = pdata->flags;
 	cxt->ecc_info = pdata->ecc_info;
+	/*
+	 * If "ramoops.ecc" was explicitly specified on the kernel command line
+	 * or module load options, let it override the Device Tree "ecc-size"
+	 * property or platform data (with 1 mapped to 16 bytes for backwards
+	 * compatibility, and 0 disabling ECC).
+	 */
+	if (ramoops_ecc_param >= 0)
+		cxt->ecc_info.ecc_size =
+			ramoops_ecc_param == 1 ? 16 : ramoops_ecc_param;
 
 	paddr = cxt->phys_addr;
 
@@ -864,7 +872,7 @@ static int ramoops_probe(struct platform_device *pdev)
 	ramoops_pmsg_size = pdata->pmsg_size;
 	ramoops_ftrace_size = pdata->ftrace_size;
 	mem_type = pdata->mem_type;
-	ramoops_ecc = pdata->ecc_info.ecc_size;
+	ramoops_ecc = cxt->ecc_info.ecc_size;
 
 	pr_info("using 0x%lx@0x%llx, ecc: %d\n",
 		cxt->size, (unsigned long long)cxt->phys_addr,
@@ -963,7 +971,8 @@ static void __init ramoops_register_dummy(void)
 	 * For backwards compatibility ramoops.ecc=1 means 16 bytes ECC
 	 * (using 1 byte for ECC isn't much of use anyway).
 	 */
-	pdata.ecc_info.ecc_size = ramoops_ecc == 1 ? 16 : ramoops_ecc;
+	pdata.ecc_info.ecc_size = ramoops_ecc < 0 ? 0 :
+				  (ramoops_ecc == 1 ? 16 : ramoops_ecc);
 
 	dummy = platform_device_register_data(NULL, "ramoops", -1,
 			&pdata, sizeof(pdata));
@@ -978,6 +987,7 @@ static int __init ramoops_init(void)
 {
 	int ret;
 
+	ramoops_ecc_param = ramoops_ecc;
 	ramoops_register_dummy();
 	ret = platform_driver_register(&ramoops_driver);
 	if (ret != 0)
-- 
2.56.0.317.g454f8302e1-goog