[RFC][PATCH 00/10] pstore: directly mapped regions

[RFC][PATCH 00/10] pstore: directly mapped regions

[Eugen Hristev]

Hello,

This series comes as an RFC proposed solution to enhance pstore and
devcoredump with the following functionality:
It is interesting to mark specific memory regions in the kernel
as core areas, such that in the even of a cataclysmic event like
panic, deadlock, hardware fault, those areas can be retrieved by
an external agent, that can be JTAG, firmware, additional debug
processor, etc.
Even though pstore panic handlers attempt to copy data into the
persistent storage, this is not always working, because the kernel
could be in a deadlock, have irqs disabled, panic handler could
crash itself, or even some memory areas could be corrupted badly
enough that the recovery mechanism doesn't work.
This patch series attempts to solve this by reusing existing
infrastructure in pstore and devcoredump, and provide a copy-free
possibility to have core kernel data exposed for creating debug
information like a coredump or list of binary elements with data.

How this works:
Drivers and key kernel infrastructure, like for example the dmesg
buffer (ftrace buffer is still work in progress and not in the series),
would register themselves into pstore, with a simple function
call taking a name, a pointer to the area and size.
That would be all, and the patch labeled "EXAMPLE" shows a driver
doing exactly that, registering it's own dev struct with string markers
that can help identifying the data later once it's being retrieved.
Further, pstore and pstore/zone would keep track of all registered
areas inside the intermediary back-end. There would not be any copy of
the data.
Pstore would require a new type of back-end, named directly mapped
(because there is no copy, and each zone directly maps the incoming
buffer). If this back-end is available, then pstore further calls
the back-end to register the area.
The back-end itself is then free to do whatever it wishes with the
received area. It can (possibly) do a copy to store data, it can
have a timer to check the area every x time periods, or store the
pointers inside a table of contents (which is what the qcom smem is
doing).
The qcom smem driver registers itself into pstore as a new type of
back-end called shared memory. It's very similar with the pstore/blk
but much simplified, to cope with just a shared memory storage.
The areas metadata are stored in a shared memory that is being managed
by the qcom_smem driver. It's called shared memory because it's being
shared between the main processor running the kernel and additional
secondary remote processors running firmware.
In this qcom smem backend case, the firmware runs when there is a
panic event, of watchdog timeout, and uses the table of contents to
retrieve useful kernel debug data.

The advantages of this proposed idea are: no kernel panic handler
intervention. Everything is being marked and prepared ahead of time,
before there is an actual crash. There is no overhead associated,
simply the areas are being marked and metadata created, no buffers
associated, no memory areas reserved for it. Memory areas being marked,
there is no possibility of getting older useless data, that was copied
before the actual events leading to a crash would occur, so the data
would be the real actual data the kernel was working on.
Of course, this would not work if there is no external agent to parse
the areas and get the useful data. However as the hardware becomes
more and more advanced, in the cases where this is available, kernel
debugging had no way of using this advantage. As the hardware evolves,
so is the kernel.

There is another aspect to underline: the directly mapped zones
change the semantics of pstore using them. Pstore relies on user being
able to read the data from a previous kernel run once the system
has rebooted. This is not valid with the directly mapped zones unless
the JTAG/firmware would copy this data into a persistent area and make
it available upon reboot (which is not impossible).

I would like to thank everyone reviewing and chiping in on the series.

The back-end implementation for qcom_smem is based on the minidump
patch series and driver written by Mukesh Ojha, thanks:
https://lore.kernel.org/lkml/20240131110837.14218-1-quic_mojha@quicinc.com/

Eugen

Eugen Hristev (10):
  pstore/zone: move pstore_device_info into zone header
  pstore/smem: add new pstore/smem type of pstore
  pstore/zone: introduce directly mapped zones
  qcom: smem: add pstore smem backend
  pstore: implement core area registration
  qcom: smem: enable smem pstore backend
  printk: export symbols for buffer address and length functions
  pstore: register kmsg into directly mapped zones if available
  devcoredump: add devcd_{un}register_core_area API
  rng: qcom_rng: EXAMPLE: registering dev structure

 drivers/base/devcoredump.c     |  13 ++
 drivers/crypto/qcom-rng.c      |  11 ++
 drivers/soc/qcom/Kconfig       |   9 +
 drivers/soc/qcom/Makefile      |   6 +-
 drivers/soc/qcom/smem.c        |  10 ++
 drivers/soc/qcom/smem_md.c     | 306 +++++++++++++++++++++++++++++++++
 drivers/soc/qcom/smem_pstore.c | 112 ++++++++++++
 fs/pstore/Kconfig              |  13 ++
 fs/pstore/Makefile             |   3 +
 fs/pstore/platform.c           |  84 +++++++++
 fs/pstore/smem.c               | 115 +++++++++++++
 fs/pstore/zone.c               | 122 +++++++++++--
 include/linux/devcoredump.h    |   3 +
 include/linux/pstore.h         |  20 +++
 include/linux/pstore_blk.h     |  14 --
 include/linux/pstore_smem.h    |   9 +
 include/linux/pstore_zone.h    |  17 ++
 include/linux/soc/qcom/smem.h  |  43 +++++
 kernel/printk/printk.c         |   2 +
 19 files changed, 885 insertions(+), 27 deletions(-)
 create mode 100644 drivers/soc/qcom/smem_md.c
 create mode 100644 drivers/soc/qcom/smem_pstore.c
 create mode 100644 fs/pstore/smem.c
 create mode 100644 include/linux/pstore_smem.h

-- 
2.43.0

[RFC][PATCH 01/10] pstore/zone: move pstore_device_info into zone header

[Eugen Hristev]

Struct pstore_device_info is used in pstore/zone, and it's
not really connected to anything related to pstore/blk.
To further improve on the pstore zone and allow other types of
drivers to connect, it is logical to have this struct into
pstore_zone.h file.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 include/linux/pstore_blk.h  | 14 --------------
 include/linux/pstore_zone.h | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/pstore_blk.h b/include/linux/pstore_blk.h
index 924ca07aafbd..d0c8fb40c46c 100644
--- a/include/linux/pstore_blk.h
+++ b/include/linux/pstore_blk.h
@@ -7,20 +7,6 @@
 #include <linux/pstore.h>
 #include <linux/pstore_zone.h>
 
-/**
- * struct pstore_device_info - back-end pstore/blk driver structure.
- *
- * @flags:	Refer to macro starting with PSTORE_FLAGS defined in
- *		linux/pstore.h. It means what front-ends this device support.
- *		Zero means all backends for compatible.
- * @zone:	The struct pstore_zone_info details.
- *
- */
-struct pstore_device_info {
-	unsigned int flags;
-	struct pstore_zone_info zone;
-};
-
 int  register_pstore_device(struct pstore_device_info *dev);
 void unregister_pstore_device(struct pstore_device_info *dev);
 
diff --git a/include/linux/pstore_zone.h b/include/linux/pstore_zone.h
index 1e35eaa33e5e..284364234162 100644
--- a/include/linux/pstore_zone.h
+++ b/include/linux/pstore_zone.h
@@ -54,6 +54,20 @@ struct pstore_zone_info {
 	pstore_zone_write_op panic_write;
 };
 
+/**
+ * struct pstore_device_info - back-end pstore device driver structure.
+ *
+ * @flags:	Refer to macro starting with PSTORE_FLAGS defined in
+ *		linux/pstore.h. It means what front-ends this device support.
+ *		Zero means all backends for compatible.
+ * @zone:	The struct pstore_zone_info details.
+ *
+ */
+struct pstore_device_info {
+	unsigned int flags;
+	struct pstore_zone_info zone;
+};
+
 extern int register_pstore_zone(struct pstore_zone_info *info);
 extern void unregister_pstore_zone(struct pstore_zone_info *info);
 
-- 
2.43.0

[RFC][PATCH 02/10] pstore/smem: add new pstore/smem type of pstore

[Eugen Hristev]

Add shared memory type of pstore.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 fs/pstore/Kconfig           |  13 ++++
 fs/pstore/Makefile          |   3 +
 fs/pstore/smem.c            | 115 ++++++++++++++++++++++++++++++++++++
 include/linux/pstore_smem.h |   9 +++
 4 files changed, 140 insertions(+)
 create mode 100644 fs/pstore/smem.c
 create mode 100644 include/linux/pstore_smem.h

diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
index 3acc38600cd1..84f87edf9b8f 100644
--- a/fs/pstore/Kconfig
+++ b/fs/pstore/Kconfig
@@ -81,6 +81,19 @@ config PSTORE_RAM
 
 	  For more information, see Documentation/admin-guide/ramoops.rst.
 
+config PSTORE_SMEM
+	tristate "Log panic/oops to a shared memory buffer"
+	depends on PSTORE
+	select PSTORE_ZONE
+	help
+	  This enables panic and oops messages to be logged to memory
+	  that is shared between different hardware blocks in the system.
+	  This shared memory can be a static ram, a part of dynamic RAM,
+	  a dedicated cache or memory area specific for crash dumps,
+	  or even a memory on an attached device.
+
+	  if unsure, say N.
+
 config PSTORE_ZONE
 	tristate
 	depends on PSTORE
diff --git a/fs/pstore/Makefile b/fs/pstore/Makefile
index c270467aeece..f2a314ca03a0 100644
--- a/fs/pstore/Makefile
+++ b/fs/pstore/Makefile
@@ -18,3 +18,6 @@ obj-$(CONFIG_PSTORE_ZONE)	+= pstore_zone.o
 
 pstore_blk-objs += blk.o
 obj-$(CONFIG_PSTORE_BLK)	+= pstore_blk.o
+
+pstore_smem-objs += smem.o
+obj-$(CONFIG_PSTORE_SMEM)	+= pstore_smem.o
diff --git a/fs/pstore/smem.c b/fs/pstore/smem.c
new file mode 100644
index 000000000000..9eedd7df5446
--- /dev/null
+++ b/fs/pstore/smem.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Implements pstore backend driver for shared memory devices,
+ * using the pstore/zone API.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/blkdev.h>
+#include <linux/string.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pstore_smem.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/init_syscalls.h>
+#include <linux/mount.h>
+
+/*
+ * All globals must only be accessed under the pstore_smem_lock
+ * during the register/unregister functions.
+ */
+static DEFINE_MUTEX(pstore_smem_lock);
+static struct pstore_device_info *pstore_device_info;
+
+static int __register_pstore_device(struct pstore_device_info *dev)
+{
+	int ret;
+
+	lockdep_assert_held(&pstore_smem_lock);
+
+	if (!dev) {
+		pr_err("NULL device info\n");
+		return -EINVAL;
+	}
+	if (!dev->zone.total_size) {
+		pr_err("zero sized device\n");
+		return -EINVAL;
+	}
+	if (!dev->zone.read) {
+		pr_err("no read handler for device\n");
+		return -EINVAL;
+	}
+	if (!dev->zone.write) {
+		pr_err("no write handler for device\n");
+		return -EINVAL;
+	}
+
+	/* someone already registered before */
+	if (pstore_device_info)
+		return -EBUSY;
+
+	/* zero means not limit on which backends to attempt to store. */
+	if (!dev->flags)
+		dev->flags = UINT_MAX;
+
+	/* Initialize required zone ownership details. */
+	dev->zone.name = KBUILD_MODNAME;
+	dev->zone.owner = THIS_MODULE;
+
+	ret = register_pstore_zone(&dev->zone);
+	if (ret == 0)
+		pstore_device_info = dev;
+
+	return ret;
+}
+/**
+ * register_pstore_smem_device() - register smem device to pstore
+ *
+ * @dev: smem device information
+ *
+ * Return:
+ * * 0		- OK
+ * * Others	- some error.
+ */
+int register_pstore_smem_device(struct pstore_device_info *dev)
+{
+	int ret;
+
+	mutex_lock(&pstore_smem_lock);
+	ret = __register_pstore_device(dev);
+	mutex_unlock(&pstore_smem_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(register_pstore_smem_device);
+
+static void __unregister_pstore_device(struct pstore_device_info *dev)
+{
+	lockdep_assert_held(&pstore_smem_lock);
+	if (pstore_device_info && pstore_device_info == dev) {
+		unregister_pstore_zone(&dev->zone);
+		pstore_device_info = NULL;
+	}
+}
+
+/**
+ * unregister_pstore_smem_device() - unregister smem device from pstore
+ *
+ * @dev: smem device information
+ */
+void unregister_pstore_smem_device(struct pstore_device_info *dev)
+{
+	mutex_lock(&pstore_smem_lock);
+	__unregister_pstore_device(dev);
+	mutex_unlock(&pstore_smem_lock);
+}
+EXPORT_SYMBOL_GPL(unregister_pstore_smem_device);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Eugen Hristev <eugen.hristev@linaro.org>");
+MODULE_DESCRIPTION("pstore backend for smem devices");
diff --git a/include/linux/pstore_smem.h b/include/linux/pstore_smem.h
new file mode 100644
index 000000000000..f0ad23e117c4
--- /dev/null
+++ b/include/linux/pstore_smem.h
@@ -0,0 +1,9 @@
+#ifndef PSTORE_SMEM_H
+#define PSTORE_SMEM_H
+
+#include <linux/pstore_zone.h>
+
+int register_pstore_smem_device(struct pstore_device_info *dev);
+void unregister_pstore_smem_device(struct pstore_device_info *dev);
+
+#endif
-- 
2.43.0

[RFC][PATCH 03/10] pstore/zone: introduce directly mapped zones

[Eugen Hristev]

Directly mapped zones have a different semantic from usual pstore zones.
Such zones use a pointer to data in their buffer struct, instead of
keeping the buffer locally.
The data pointer and size is then passed to the backend for further use.
Having a different semantics, backends supporting only these do not offer
read/write ops, and only new register_dmr and unregister_dmr ops.
Ofcourse, a backend could support both classic zones and directly mapped.
Directly mapped zones have the advantage of not being passed through
in the event of a crashed, but rather at registration time.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 fs/pstore/platform.c        |  1 +
 fs/pstore/smem.c            |  6 ++---
 fs/pstore/zone.c            | 45 ++++++++++++++++++++++++++++---------
 include/linux/pstore.h      |  4 ++++
 include/linux/pstore_zone.h |  3 +++
 5 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index f56b066ab80c..e20e60b88727 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -51,6 +51,7 @@ static const char * const pstore_type_names[] = {
 	"powerpc-common",
 	"pmsg",
 	"powerpc-opal",
+	"directly-mapped",
 };
 
 static int pstore_new_entry;
diff --git a/fs/pstore/smem.c b/fs/pstore/smem.c
index 9eedd7df5446..684ecc20cae5 100644
--- a/fs/pstore/smem.c
+++ b/fs/pstore/smem.c
@@ -36,15 +36,15 @@ static int __register_pstore_device(struct pstore_device_info *dev)
 		pr_err("NULL device info\n");
 		return -EINVAL;
 	}
-	if (!dev->zone.total_size) {
+	if (!dev->zone.total_size && !dev->zone.dmapped_cnt) {
 		pr_err("zero sized device\n");
 		return -EINVAL;
 	}
-	if (!dev->zone.read) {
+	if (!dev->zone.read && !dev->zone.dmapped_cnt) {
 		pr_err("no read handler for device\n");
 		return -EINVAL;
 	}
-	if (!dev->zone.write) {
+	if (!dev->zone.write && !dev->zone.dmapped_cnt) {
 		pr_err("no write handler for device\n");
 		return -EINVAL;
 	}
diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index 5fa2fa2e7aa7..affa4370208c 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -113,6 +113,7 @@ struct psz_context {
 	struct pstore_zone *ppsz;
 	struct pstore_zone *cpsz;
 	struct pstore_zone **fpszs;
+	struct pstore_zone **dmszs;
 	unsigned int kmsg_max_cnt;
 	unsigned int kmsg_read_cnt;
 	unsigned int kmsg_write_cnt;
@@ -120,6 +121,7 @@ struct psz_context {
 	unsigned int console_read_cnt;
 	unsigned int ftrace_max_cnt;
 	unsigned int ftrace_read_cnt;
+	unsigned int dmapped_max_cnt;
 	/*
 	 * These counters should be calculated during recovery.
 	 * It records the oops/panic times after crashes rather than boots.
@@ -1148,6 +1150,8 @@ static void psz_free_all_zones(struct psz_context *cxt)
 		psz_free_zone(&cxt->cpsz);
 	if (cxt->fpszs)
 		psz_free_zones(&cxt->fpszs, &cxt->ftrace_max_cnt);
+	if (cxt->dmszs)
+		psz_free_zones(&cxt->dmszs, &cxt->dmapped_max_cnt);
 }
 
 static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
@@ -1160,9 +1164,9 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
 	if (!size)
 		return NULL;
 
-	if (*off + size > info->total_size) {
-		pr_err("no room for %s (0x%zx@0x%llx over 0x%lx)\n",
-			name, size, *off, info->total_size);
+	if (*off + size > info->total_size && type != PSTORE_TYPE_DMAPPED) {
+		pr_err("no room for %s type %d (0x%zx@0x%llx over 0x%lx)\n",
+			name, type, size, *off, info->total_size);
 		return ERR_PTR(-ENOMEM);
 	}
 
@@ -1170,7 +1174,8 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
 	if (!zone)
 		return ERR_PTR(-ENOMEM);
 
-	zone->buffer = kmalloc(size, GFP_KERNEL);
+	zone->buffer = kmalloc(type == PSTORE_TYPE_DMAPPED ?
+			       sizeof(struct psz_buffer) : size, GFP_KERNEL);
 	if (!zone->buffer) {
 		kfree(zone);
 		return ERR_PTR(-ENOMEM);
@@ -1179,7 +1184,10 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
 	zone->off = *off;
 	zone->name = name;
 	zone->type = type;
-	zone->buffer_size = size - sizeof(struct psz_buffer);
+	if (zone->type == PSTORE_TYPE_DMAPPED)
+		zone->buffer_size = 0;
+	else
+		zone->buffer_size = size - sizeof(struct psz_buffer);
 	zone->buffer->sig = type ^ PSZ_SIG;
 	zone->oldbuf = NULL;
 	atomic_set(&zone->dirty, 0);
@@ -1188,8 +1196,9 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type,
 
 	*off += size;
 
-	pr_debug("pszone %s: off 0x%llx, %zu header, %zu data\n", zone->name,
-			zone->off, sizeof(*zone->buffer), zone->buffer_size);
+	pr_debug("pszone %s: off 0x%llx, %zu header, %zu data %s\n", zone->name,
+			zone->off, sizeof(*zone->buffer), zone->buffer_size,
+			zone->type == PSTORE_TYPE_DMAPPED ? " dmapped " : "");
 	return zone;
 }
 
@@ -1206,7 +1215,7 @@ static struct pstore_zone **psz_init_zones(enum pstore_type_id type,
 	if (!total_size || !record_size)
 		return NULL;
 
-	if (*off + total_size > info->total_size) {
+	if (*off + total_size > info->total_size && type != PSTORE_TYPE_DMAPPED) {
 		pr_err("no room for zones %s (0x%zx@0x%llx over 0x%lx)\n",
 			name, total_size, *off, info->total_size);
 		return ERR_PTR(-ENOMEM);
@@ -1245,6 +1254,15 @@ static int psz_alloc_zones(struct psz_context *cxt)
 	int err;
 	size_t off_size = 0;
 
+	cxt->dmszs = psz_init_zones(PSTORE_TYPE_DMAPPED, &off,
+			info->dmapped_cnt,
+			1, &cxt->dmapped_max_cnt);
+	if (IS_ERR(cxt->dmszs)) {
+		err = PTR_ERR(cxt->dmszs);
+		cxt->dmszs = NULL;
+		goto free_out;
+	}
+
 	off_size += info->pmsg_size;
 	cxt->ppsz = psz_init_zone(PSTORE_TYPE_PMSG, &off, info->pmsg_size);
 	if (IS_ERR(cxt->ppsz)) {
@@ -1302,7 +1320,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	int err = -EINVAL;
 	struct psz_context *cxt = &pstore_zone_cxt;
 
-	if (info->total_size < 4096) {
+	if (info->total_size < 4096 && !info->dmapped_cnt) {
 		pr_warn("total_size must be >= 4096\n");
 		return -EINVAL;
 	}
@@ -1312,7 +1330,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	}
 
 	if (!info->kmsg_size && !info->pmsg_size && !info->console_size &&
-	    !info->ftrace_size) {
+	    !info->ftrace_size && !info->dmapped_cnt) {
 		pr_warn("at least one record size must be non-zero\n");
 		return -EINVAL;
 	}
@@ -1345,7 +1363,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	 * if no @read, pstore may mount failed.
 	 * if no @write, pstore do not support to remove record file.
 	 */
-	if (!info->read || !info->write) {
+	if (!info->dmapped_cnt && (!info->read || !info->write)) {
 		pr_err("no valid general read/write interface\n");
 		return -EINVAL;
 	}
@@ -1365,6 +1383,7 @@ int register_pstore_zone(struct pstore_zone_info *info)
 	pr_debug("\tpmsg size : %ld Bytes\n", info->pmsg_size);
 	pr_debug("\tconsole size : %ld Bytes\n", info->console_size);
 	pr_debug("\tftrace size : %ld Bytes\n", info->ftrace_size);
+	pr_debug("\tdmapped areas : %ld\n", info->dmapped_cnt);
 
 	err = psz_alloc_zones(cxt);
 	if (err) {
@@ -1406,6 +1425,10 @@ int register_pstore_zone(struct pstore_zone_info *info)
 		cxt->pstore.flags |= PSTORE_FLAGS_FTRACE;
 		pr_cont(" ftrace");
 	}
+	if (info->dmapped_cnt) {
+		cxt->pstore.flags |= PSTORE_FLAGS_DMAPPED;
+		pr_cont(" dmapped");
+	}
 	pr_cont("\n");
 
 	err = pstore_register(&cxt->pstore);
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index fed601053c51..8360d94c96b6 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -39,6 +39,7 @@ enum pstore_type_id {
 	PSTORE_TYPE_PMSG	= 7,
 	PSTORE_TYPE_PPC_OPAL	= 8,
 
+	PSTORE_TYPE_DMAPPED	= 9,
 	/* End of the list */
 	PSTORE_TYPE_MAX
 };
@@ -199,6 +200,8 @@ struct pstore_info {
 	int		(*write_user)(struct pstore_record *record,
 				      const char __user *buf);
 	int		(*erase)(struct pstore_record *record);
+	int		(*register_dmr)(struct pstore_record *record);
+	int		(*unregister_dmr)(struct pstore_record *record);
 };
 
 /* Supported frontends */
@@ -206,6 +209,7 @@ struct pstore_info {
 #define PSTORE_FLAGS_CONSOLE	BIT(1)
 #define PSTORE_FLAGS_FTRACE	BIT(2)
 #define PSTORE_FLAGS_PMSG	BIT(3)
+#define PSTORE_FLAGS_DMAPPED	BIT(4)
 
 extern int pstore_register(struct pstore_info *);
 extern void pstore_unregister(struct pstore_info *);
diff --git a/include/linux/pstore_zone.h b/include/linux/pstore_zone.h
index 284364234162..a74d0cc75577 100644
--- a/include/linux/pstore_zone.h
+++ b/include/linux/pstore_zone.h
@@ -48,10 +48,13 @@ struct pstore_zone_info {
 	unsigned long pmsg_size;
 	unsigned long console_size;
 	unsigned long ftrace_size;
+	unsigned long dmapped_cnt;
 	pstore_zone_read_op read;
 	pstore_zone_write_op write;
 	pstore_zone_erase_op erase;
 	pstore_zone_write_op panic_write;
+	int (*register_dmr)(char *, int, void *, size_t);
+	int (*unregister_dmr)(void *, size_t);
 };
 
 /**
-- 
2.43.0

[RFC][PATCH 04/10] qcom: smem: add pstore smem backend

[Eugen Hristev]

Add support for pstore smem backend in the qcom smem driver.
This backend resorts to minidump regions behind the scenes.

Co-developed-by: Mukesh Ojha <quic_mojha@quicinc.com>
Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 drivers/soc/qcom/Kconfig       |   9 +
 drivers/soc/qcom/Makefile      |   6 +-
 drivers/soc/qcom/smem_md.c     | 306 +++++++++++++++++++++++++++++++++
 drivers/soc/qcom/smem_pstore.c | 112 ++++++++++++
 include/linux/soc/qcom/smem.h  |  43 +++++
 5 files changed, 474 insertions(+), 2 deletions(-)
 create mode 100644 drivers/soc/qcom/smem_md.c
 create mode 100644 drivers/soc/qcom/smem_pstore.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 58e63cf0036b..e1c71f713c05 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -180,6 +180,15 @@ config QCOM_SMEM
 	  The driver provides an interface to items in a heap shared among all
 	  processors in a Qualcomm platform.
 
+config QCOM_SMEM_PSTORE
+	bool "Qualcomm Shared Memory(SMEM) Pstore backend"
+	depends on QCOM_SMEM
+	select PSTORE
+	select PSTORE_SMEM
+	help
+	  Say y here to enable the shared memory driver to register itself
+	  as a pstore backend.
+
 config QCOM_SMD_RPM
 	tristate "Qualcomm Resource Power Manager (RPM) over SMD"
 	depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index acbca2ab5cc2..304b031ed70e 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -23,8 +23,10 @@ obj-$(CONFIG_QCOM_RPMH)		+= qcom_rpmh.o
 qcom_rpmh-y			+= rpmh-rsc.o
 qcom_rpmh-y			+= rpmh.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= rpm-proc.o smd-rpm.o
-obj-$(CONFIG_QCOM_SMEM) +=	smem.o
-obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
+obj-$(CONFIG_QCOM_SMEM) += qcom_smem.o
+qcom_smem-y += smem.o
+qcom_smem-$(CONFIG_QCOM_SMEM_PSTORE) += smem_pstore.o smem_md.o
+obj-$(CONFIG_QCOM_SMEM_STATE)	+= smem_state.o
 CFLAGS_smp2p.o := -I$(src)
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
diff --git a/drivers/soc/qcom/smem_md.c b/drivers/soc/qcom/smem_md.c
new file mode 100644
index 000000000000..2b665ecc5695
--- /dev/null
+++ b/drivers/soc/qcom/smem_md.c
@@ -0,0 +1,306 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/hwspinlock.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/devcoredump.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/socinfo.h>
+#include <linux/dma-mapping.h>
+#include <linux/pstore_smem.h>
+#include <linux/pstore_zone.h>
+#include <linux/pstore.h>
+
+#define MAX_NUM_ENTRIES	  201
+#define MAX_STRTBL_SIZE	  (MAX_NUM_ENTRIES * MAX_REGION_NAME_LENGTH)
+
+#define MAX_NUM_OF_SS           10
+#define MAX_REGION_NAME_LENGTH  16
+#define SBL_MINIDUMP_SMEM_ID	602
+#define MINIDUMP_REGION_VALID	   ('V' << 24 | 'A' << 16 | 'L' << 8 | 'I' << 0)
+#define MINIDUMP_SS_ENCR_DONE	   ('D' << 24 | 'O' << 16 | 'N' << 8 | 'E' << 0)
+#define MINIDUMP_SS_ENABLED	   ('E' << 24 | 'N' << 16 | 'B' << 8 | 'L' << 0)
+#define MINIDUMP_REGION_INVALID	   ('I' << 24 | 'N' << 16 | 'V' << 8 | 'A' << 0)
+#define MINIDUMP_REGION_INIT	   ('I' << 24 | 'N' << 16 | 'I' << 8 | 'T' << 0)
+#define MINIDUMP_REGION_NOINIT	   0
+
+#define MINIDUMP_SS_ENCR_REQ	   (0 << 24 | 'Y' << 16 | 'E' << 8 | 'S' << 0)
+#define MINIDUMP_SS_ENCR_NOTREQ	   (0 << 24 | 0 << 16 | 'N' << 8 | 'R' << 0)
+#define MINIDUMP_SS_ENCR_START	   ('S' << 24 | 'T' << 16 | 'R' << 8 | 'T' << 0)
+
+#define MINIDUMP_APSS_DESC	   0
+
+/**
+ * struct minidump - Minidump driver data information
+ * @apss_data: APSS driver data
+ * @md_lock: Lock to protect access to APSS minidump table
+ */
+struct minidump {
+	struct device		*dev;
+	struct minidump_ss_data	*apss_data;
+	struct mutex		md_lock;
+};
+
+/**
+ * struct minidump_region - Minidump region
+ * @name		: Name of the region to be dumped
+ * @seq_num:		: Use to differentiate regions with same name.
+ * @valid		: This entry to be dumped (if set to 1)
+ * @address		: Physical address of region to be dumped
+ * @size		: Size of the region
+ */
+struct minidump_region {
+	char	name[MAX_REGION_NAME_LENGTH];
+	__le32	seq_num;
+	__le32	valid;
+	__le64	address;
+	__le64	size;
+};
+
+/**
+ * struct minidump_subsystem - Subsystem's SMEM Table of content
+ * @status : Subsystem toc init status
+ * @enabled : if set to 1, this region would be copied during coredump
+ * @encryption_status: Encryption status for this subsystem
+ * @encryption_required : Decides to encrypt the subsystem regions or not
+ * @region_count : Number of regions added in this subsystem toc
+ * @regions_baseptr : regions base pointer of the subsystem
+ */
+struct minidump_subsystem {
+	__le32	status;
+	__le32	enabled;
+	__le32	encryption_status;
+	__le32	encryption_required;
+	__le32	region_count;
+	__le64	regions_baseptr;
+};
+
+/**
+ * struct minidump_global_toc - Global Table of Content
+ * @status : Global Minidump init status
+ * @md_revision : Minidump revision
+ * @enabled : Minidump enable status
+ * @subsystems : Array of subsystems toc
+ */
+struct minidump_global_toc {
+	__le32				status;
+	__le32				md_revision;
+	__le32				enabled;
+	struct minidump_subsystem	subsystems[MAX_NUM_OF_SS];
+};
+/**
+ * struct minidump_ss_data - Minidump subsystem private data
+ * @md_ss_toc: Application Subsystem TOC pointer
+ * @md_regions: Application Subsystem region base pointer
+ */
+struct minidump_ss_data {
+	struct minidump_subsystem *md_ss_toc;
+	struct minidump_region	  *md_regions;
+};
+
+static struct minidump *md;
+
+static void qcom_md_add_region(const struct qcom_minidump_region *region)
+{
+	struct minidump_subsystem *mdss_toc = md->apss_data->md_ss_toc;
+	struct minidump_region *mdr;
+	unsigned int region_cnt;
+
+	region_cnt = le32_to_cpu(mdss_toc->region_count);
+	mdr = &md->apss_data->md_regions[region_cnt];
+	strscpy(mdr->name, region->name, sizeof(mdr->name));
+	mdr->address = cpu_to_le64(region->phys_addr);
+	mdr->size = cpu_to_le64(region->size);
+	mdr->valid = cpu_to_le32(MINIDUMP_REGION_VALID);
+	region_cnt++;
+	mdss_toc->region_count = cpu_to_le32(region_cnt);
+}
+
+static int qcom_md_get_region_index(struct minidump_ss_data *mdss_data,
+				    const struct qcom_minidump_region *region)
+{
+	struct minidump_subsystem *mdss_toc = mdss_data->md_ss_toc;
+	struct minidump_region *mdr;
+	unsigned int i;
+	unsigned int count;
+
+	count = le32_to_cpu(mdss_toc->region_count);
+	for (i = 0; i < count; i++) {
+		mdr = &mdss_data->md_regions[i];
+		if (!strcmp(mdr->name, region->name))
+			return i;
+	}
+
+	return -ENOENT;
+}
+
+static int qcom_md_region_unregister(const struct qcom_minidump_region *region)
+{
+	struct minidump_ss_data *mdss_data = md->apss_data;
+	struct minidump_subsystem *mdss_toc = mdss_data->md_ss_toc;
+	struct minidump_region *mdr;
+	unsigned int region_cnt;
+	unsigned int idx;
+	int ret;
+
+	ret = qcom_md_get_region_index(mdss_data, region);
+	if (ret < 0) {
+		dev_err(md->dev, "%s region is not present\n", region->name);
+		return ret;
+	}
+
+	idx = ret;
+	mdr = &mdss_data->md_regions[0];
+	region_cnt = le32_to_cpu(mdss_toc->region_count);
+	/*
+	 * Left shift all the regions exist after this removed region
+	 * index by 1 to fill the gap and zero out the last region
+	 * present at the end.
+	 */
+	memmove(&mdr[idx], &mdr[idx + 1], (region_cnt - idx - 1) * sizeof(*mdr));
+	memset(&mdr[region_cnt - 1], 0, sizeof(*mdr));
+	region_cnt--;
+	mdss_toc->region_count = cpu_to_le32(region_cnt);
+
+	return 0;
+}
+
+static int qcom_md_region_register(const struct qcom_minidump_region *region)
+{
+	struct minidump_ss_data *mdss_data = md->apss_data;
+	struct minidump_subsystem *mdss_toc = mdss_data->md_ss_toc;
+	unsigned int num_region;
+	int ret;
+
+	ret = qcom_md_get_region_index(mdss_data, region);
+	if (ret >= 0) {
+		dev_info(md->dev, "%s region is already registered\n", region->name);
+		return -EEXIST;
+	}
+
+	/* Check if there is a room for a new entry */
+	num_region = le32_to_cpu(mdss_toc->region_count);
+	if (num_region >= MAX_NUM_ENTRIES) {
+		dev_err(md->dev, "maximum region limit %u reached\n", num_region);
+		return -ENOSPC;
+	}
+
+	qcom_md_add_region(region);
+
+	return 0;
+}
+
+static bool qcom_minidump_valid_region(const struct qcom_minidump_region *region)
+{
+	return region &&
+		strnlen(region->name, MINIDUMP_MAX_NAME_LENGTH) < MINIDUMP_MAX_NAME_LENGTH &&
+			region->virt_addr &&
+			region->size &&
+			IS_ALIGNED(region->size, 4);
+}
+
+/**
+ * qcom_minidump_region_register() - Register region in APSS Minidump table.
+ * @region: minidump region.
+ *
+ * Return: On success, it returns 0 and negative error value on failure.
+ */
+int qcom_minidump_region_register(const struct qcom_minidump_region *region)
+{
+	int ret;
+
+	if (!qcom_minidump_valid_region(region))
+		return -EINVAL;
+
+	mutex_lock(&md->md_lock);
+	ret = qcom_md_region_register(region);
+
+	mutex_unlock(&md->md_lock);
+	return ret;
+}
+
+/**
+ * qcom_minidump_region_unregister() - Unregister region from APSS Minidump table.
+ * @region: minidump region.
+ *
+ * Return: On success, it returns 0 and negative error value on failure.
+ */
+int qcom_minidump_region_unregister(const struct qcom_minidump_region *region)
+{
+	int ret;
+
+	if (!qcom_minidump_valid_region(region))
+		return -EINVAL;
+
+	mutex_lock(&md->md_lock);
+	ret = qcom_md_region_unregister(region);
+
+	mutex_unlock(&md->md_lock);
+	return ret;
+}
+
+
+static int qcom_apss_md_table_init(struct minidump_subsystem *mdss_toc)
+{
+	struct minidump_ss_data *mdss_data;
+
+	mdss_data = devm_kzalloc(md->dev, sizeof(*mdss_data), GFP_KERNEL);
+	if (!mdss_data)
+		return -ENOMEM;
+
+	mdss_data->md_ss_toc = mdss_toc;
+	mdss_data->md_regions = devm_kcalloc(md->dev, MAX_NUM_ENTRIES,
+					     sizeof(*mdss_data->md_regions),
+					     GFP_KERNEL);
+	if (!mdss_data->md_regions)
+		return -ENOMEM;
+
+	mdss_toc = mdss_data->md_ss_toc;
+	mdss_toc->regions_baseptr = cpu_to_le64(virt_to_phys(mdss_data->md_regions));
+	mdss_toc->enabled = cpu_to_le32(MINIDUMP_SS_ENABLED);
+	mdss_toc->status = cpu_to_le32(1);
+	mdss_toc->region_count = cpu_to_le32(0);
+
+	/* Tell bootloader not to encrypt the regions of this subsystem */
+	mdss_toc->encryption_status = cpu_to_le32(MINIDUMP_SS_ENCR_DONE);
+	mdss_toc->encryption_required = cpu_to_le32(MINIDUMP_SS_ENCR_NOTREQ);
+
+	md->apss_data = mdss_data;
+
+	return 0;
+}
+
+int qcom_smem_md_init(struct device *dev)
+{
+	struct minidump_global_toc *mdgtoc;
+	size_t size;
+	int ret;
+
+	md = devm_kzalloc(dev, sizeof(*md), GFP_KERNEL);
+
+	md->dev = dev;
+
+	mdgtoc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, &size);
+	if (IS_ERR(mdgtoc)) {
+		ret = PTR_ERR(mdgtoc);
+		dev_err(md->dev, "Couldn't find minidump smem item %d\n", ret);
+	}
+
+	if (size < sizeof(*mdgtoc) || !mdgtoc->status) {
+		ret = -EINVAL;
+		dev_err(md->dev, "minidump table is not initialized %d\n", ret);
+	}
+
+	mutex_init(&md->md_lock);
+
+	ret = qcom_apss_md_table_init(&mdgtoc->subsystems[MINIDUMP_APSS_DESC]);
+	if (ret)
+		dev_err(md->dev, "apss minidump initialization failed %d\n", ret);
+	return ret;
+}
diff --git a/drivers/soc/qcom/smem_pstore.c b/drivers/soc/qcom/smem_pstore.c
new file mode 100644
index 000000000000..a322a320e435
--- /dev/null
+++ b/drivers/soc/qcom/smem_pstore.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/hwspinlock.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/devcoredump.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/socinfo.h>
+#include <linux/dma-mapping.h>
+#include <linux/pstore_smem.h>
+#include <linux/pstore_zone.h>
+#include <linux/pstore.h>
+
+static LIST_HEAD(apss_md_rlist);
+struct md_region_list {
+	struct qcom_minidump_region md_region;
+	struct list_head list;
+};
+
+static struct qcom_smem_pstore_context {
+	struct pstore_device_info dev;
+} oops_ctx;
+
+static int register_smem_region(const char *name, int id, void *vaddr,
+				   phys_addr_t paddr, size_t size)
+{
+	struct qcom_minidump_region *md_region;
+	int ret;
+
+	struct md_region_list *mdr_list =
+		kzalloc(sizeof(*mdr_list), GFP_KERNEL);
+	if (!mdr_list)
+		return -ENOMEM;
+	md_region = &mdr_list->md_region;
+
+	scnprintf(md_region->name, sizeof(md_region->name), "K%d%.8s", id, name);
+	md_region->virt_addr = vaddr;
+	md_region->phys_addr = paddr;
+	md_region->size = size;
+	ret = qcom_minidump_region_register(md_region);
+	if (ret < 0) {
+		pr_err("failed to register region in minidump: err: %d\n", ret);
+		return ret;
+	}
+
+	list_add(&mdr_list->list, &apss_md_rlist);
+	return 0;
+}
+
+static int unregister_smem_region(void *vaddr,
+					phys_addr_t paddr, size_t size)
+{
+	int ret = -ENOENT;
+	struct md_region_list *mdr_list;
+	struct md_region_list *tmp;
+
+	list_for_each_entry_safe(mdr_list, tmp, &apss_md_rlist, list) {
+		struct qcom_minidump_region *region;
+
+		region = &mdr_list->md_region;
+		if (region->virt_addr == vaddr) {
+			ret = qcom_minidump_region_unregister(region);
+			list_del(&mdr_list->list);
+			goto unregister_smem_region_exit;
+		}
+	}
+
+unregister_smem_region_exit:
+	pr_err("failed to unregister region in minidump: err: %d\n", ret);
+
+	return ret;
+}
+
+static int qcom_smem_register_dmr(char *name, int id, void *area, size_t size)
+{
+	return register_smem_region(name, id, area, virt_to_phys(area), size);
+}
+
+static int qcom_smem_unregister_dmr(void *area, size_t size)
+{
+	return unregister_smem_region(area, virt_to_phys(area), size);
+}
+
+int qcom_register_pstore_smem(struct device *dev)
+{
+	int ret;
+
+	struct qcom_smem_pstore_context *ctx = &oops_ctx;
+
+	ctx->dev.flags = PSTORE_FLAGS_DMAPPED;
+	ctx->dev.zone.register_dmr = qcom_smem_register_dmr;
+	ctx->dev.zone.unregister_dmr = qcom_smem_unregister_dmr;
+	ctx->dev.zone.dmapped_cnt = 2;
+
+	ret = register_pstore_smem_device(&ctx->dev);
+	if (ret)
+		dev_warn(dev, "Could not register pstore smem device.");
+
+	return 0;
+}
+
+void qcom_unregister_pstore_smem(void)
+{
+	struct qcom_smem_pstore_context *ctx = &oops_ctx;
+
+	unregister_pstore_smem_device(&ctx->dev);
+}
diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
index f946e3beca21..376822f13a4f 100644
--- a/include/linux/soc/qcom/smem.h
+++ b/include/linux/soc/qcom/smem.h
@@ -17,4 +17,47 @@ int qcom_smem_get_feature_code(u32 *code);
 
 int qcom_smem_bust_hwspin_lock_by_host(unsigned int host);
 
+#ifdef CONFIG_QCOM_SMEM_PSTORE
+int qcom_register_pstore_smem(struct device *dev);
+void qcom_unregister_pstore_smem(void);
+
+#define MINIDUMP_MAX_NAME_LENGTH	12
+
+/**
+ * struct qcom_minidump_region - Minidump region information
+ *
+ * @name:	Minidump region name
+ * @virt_addr:  Virtual address of the entry.
+ * @phys_addr:	Physical address of the entry to dump.
+ * @size:	Number of bytes to dump from @address location,
+ *		and it should be 4 byte aligned.
+ */
+struct qcom_minidump_region {
+	char		name[MINIDUMP_MAX_NAME_LENGTH];
+	void		*virt_addr;
+	phys_addr_t	phys_addr;
+	size_t		size;
+};
+
+int qcom_minidump_region_unregister(const struct qcom_minidump_region *region);
+int qcom_minidump_region_register(const struct qcom_minidump_region *region);
+
+int qcom_smem_md_init(struct device *dev);
+
+#else
+
+static inline int qcom_register_pstore_smem(struct device *dev)
+{
+	return 0;
+}
+
+static inline void qcom_unregister_pstore_smem(void)
+{
+}
+
+static inline int qcom_smem_md_init(struct device *dev)
+{
+	return 0;
+}
+#endif
 #endif
-- 
2.43.0

[RFC][PATCH 05/10] pstore: implement core area registration

[Eugen Hristev]

Implement core area registration mechanism.
Implement directly mapped zone corespoding to core areas.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 fs/pstore/platform.c   | 67 ++++++++++++++++++++++++++++++++++++
 fs/pstore/zone.c       | 77 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pstore.h | 16 +++++++++
 3 files changed, 160 insertions(+)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index e20e60b88727..32448d9dd316 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -68,6 +68,7 @@ static DECLARE_WORK(pstore_work, pstore_dowork);
  * the filesystem mount/unmount routines.
  */
 static DEFINE_MUTEX(psinfo_lock);
+static DEFINE_MUTEX(ps_dmr_lock);
 struct pstore_info *psinfo;
 
 static char *backend;
@@ -99,6 +100,12 @@ MODULE_PARM_DESC(kmsg_bytes, "amount of kernel log to snapshot (in bytes)");
 
 static void *compress_workspace;
 
+static LIST_HEAD(rec_list);
+struct rec_list_t {
+	struct pstore_record rec;
+	struct list_head list;
+};
+
 /*
  * Compression is only used for dmesg output, which consists of low-entropy
  * ASCII text, and so we can assume worst-case 60%.
@@ -271,6 +278,66 @@ void pstore_record_init(struct pstore_record *record,
 	record->time = ns_to_timespec64(ktime_get_real_fast_ns());
 }
 
+int pstore_register_core_area(const char *handle, void *area, size_t size)
+{
+	struct rec_list_t *rec_element = kzalloc(sizeof (*rec_element), GFP_KERNEL);
+	struct pstore_record *record = &rec_element->rec;
+	int ret;
+
+	if (!psinfo || !psinfo->register_dmr) {
+		pr_err("No pstore available ! Bailing out.\n");
+		return -EAGAIN;
+	}
+
+	pstore_record_init(record, psinfo);
+	record->type = PSTORE_TYPE_DMAPPED;
+	record->buf = area;
+	record->size = size;
+
+	if (handle) {
+		record->priv = kmalloc(8, GFP_KERNEL);
+		strncpy(record->priv, handle, 8);
+	}
+
+	mutex_lock(&ps_dmr_lock);
+
+	ret = psinfo->register_dmr(record);
+	if (!ret)
+		list_add(&rec_element->list, &rec_list);
+
+	mutex_unlock(&ps_dmr_lock);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pstore_register_core_area);
+
+int pstore_unregister_core_area(const char *handle, void *area, size_t size)
+{
+	struct rec_list_t *rec_element, *tmp;
+	int ret;
+
+	if (!psinfo || !psinfo->unregister_dmr)
+		return -EAGAIN;
+
+	mutex_lock(&ps_dmr_lock);
+	list_for_each_entry_safe(rec_element, tmp, &rec_list, list) {
+		struct pstore_record *record;
+
+		record = &rec_element->rec;
+
+		if (record->buf == area) {
+			ret = psinfo->unregister_dmr(record);
+			list_del(&rec_element->list);
+			mutex_unlock(&ps_dmr_lock);
+			return 0;
+		}
+	}
+
+	mutex_unlock(&ps_dmr_lock);
+	return 0;
+
+}
+EXPORT_SYMBOL_GPL(pstore_unregister_core_area);
+
 /*
  * callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the
  * end of the buffer.
diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c
index affa4370208c..f9e2dc4252ea 100644
--- a/fs/pstore/zone.c
+++ b/fs/pstore/zone.c
@@ -35,6 +35,7 @@ struct psz_buffer {
 	uint32_t sig;
 	atomic_t datalen;
 	atomic_t start;
+	void *data_ptr;
 	uint8_t data[];
 };
 
@@ -822,6 +823,38 @@ static int notrace psz_kmsg_write(struct psz_context *cxt,
 	return 0;
 }
 
+static int notrace psz_register_dmr_record(struct pstore_zone *zone,
+			struct pstore_record *record)
+{
+	struct pstore_zone_info *info = pstore_zone_cxt.pstore_zone_info;
+	int ret;
+
+	if (!info->register_dmr)
+		return -ENOTSUPP;
+
+	zone->buffer->data_ptr = record->buf;
+	atomic_set(&zone->buffer->datalen, record->size);
+
+	ret = info->register_dmr(record->priv, record->id, record->buf,
+				 record->size);
+	if (!ret)
+		atomic_set(&zone->dirty, true);
+	return ret;
+}
+
+static int psz_unregister_dmr_zone(struct pstore_zone *zone)
+{
+	struct pstore_zone_info *info = pstore_zone_cxt.pstore_zone_info;
+	if (!info->unregister_dmr)
+		return -ENOTSUPP;
+
+	info->unregister_dmr(zone->buffer->data_ptr,
+			     atomic_read(&zone->buffer->datalen));
+
+	atomic_set(&zone->dirty, false);
+	return 0;
+}
+
 static int notrace psz_record_write(struct pstore_zone *zone,
 		struct pstore_record *record)
 {
@@ -906,6 +939,48 @@ static int notrace psz_pstore_write(struct pstore_record *record)
 	}
 }
 
+static int pstore_unregister_dmr(struct pstore_record *record)
+{
+	struct psz_context *cxt = record->psi->data;
+	int c = 0;
+
+	if (!cxt->dmszs)
+		return -ENODEV;
+
+	while (c < cxt->dmapped_max_cnt) {
+		if (!atomic_read(&cxt->dmszs[c]->dirty))
+			continue;
+
+		if (cxt->dmszs[c]->buffer->data_ptr == record->buf)
+			return psz_unregister_dmr_zone(cxt->dmszs[c]);
+		c++;
+	}
+
+	return -ENOENT;
+}
+
+static int pstore_register_dmr(struct pstore_record *record)
+{
+	struct psz_context *cxt = record->psi->data;
+	int c = 0;
+
+	if (!cxt->dmszs)
+		return -ENODEV;
+
+	while (c < cxt->dmapped_max_cnt) {
+		if (!atomic_read(&cxt->dmszs[c]->dirty))
+			break;
+		c++;
+	}
+
+	if (c == cxt->dmapped_max_cnt)
+		return -ENOSPC;
+
+	record->id = c;
+
+	return psz_register_dmr_record(cxt->dmszs[c], record);
+}
+
 static struct pstore_zone *psz_read_next_zone(struct psz_context *cxt)
 {
 	struct pstore_zone *zone = NULL;
@@ -1110,6 +1185,8 @@ static struct psz_context pstore_zone_cxt = {
 		.read = psz_pstore_read,
 		.write = psz_pstore_write,
 		.erase = psz_pstore_erase,
+		.register_dmr = pstore_register_dmr,
+		.unregister_dmr = pstore_unregister_dmr,
 	},
 };
 
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 8360d94c96b6..85f3f964b268 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -290,4 +290,20 @@ pstore_ftrace_write_timestamp(struct pstore_ftrace_record *rec, u64 val)
 }
 #endif
 
+#ifdef CONFIG_PSTORE
+int pstore_register_core_area(const char *handle, void *area, size_t size);
+int pstore_unregister_core_area(const char *handle, void *area, size_t size);
+#else
+static inline int pstore_register_core_area(const char *handle, void *area,
+					    size_t size)
+{
+	return 0;
+}
+static inline int pstore_unregister_core_area(const char *handle, void *area,
+				       size_t size)
+{
+	return 0;
+}
+#endif
+
 #endif /*_LINUX_PSTORE_H*/
-- 
2.43.0

[RFC][PATCH 06/10] qcom: smem: enable smem pstore backend

[Eugen Hristev]

Enable smem pstore backend that registers regions into the minidump
table.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 drivers/soc/qcom/smem.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 592819701809..f4d2646fcc0b 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -1236,11 +1236,21 @@ static int qcom_smem_probe(struct platform_device *pdev)
 	if (IS_ERR(smem->socinfo))
 		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
 
+	ret = qcom_smem_md_init(&pdev->dev);
+	if (ret)
+		dev_info(&pdev->dev, "smem minidump backend init failed\n");
+
+	ret = qcom_register_pstore_smem(&pdev->dev);
+	if (ret)
+		dev_info(&pdev->dev, "smem pstore backend registration failed\n");
+
 	return 0;
 }
 
 static void qcom_smem_remove(struct platform_device *pdev)
 {
+	qcom_unregister_pstore_smem();
+
 	platform_device_unregister(__smem->socinfo);
 
 	hwspin_lock_free(__smem->hwlock);
-- 
2.43.0

[RFC][PATCH 07/10] printk: export symbols for buffer address and length functions

[Eugen Hristev]

log_buf_addr_get() and log_buf_len_get() can be reused in another module,
export the symbols.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 kernel/printk/printk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 07668433644b..022947f9b61d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -588,12 +588,14 @@ char *log_buf_addr_get(void)
 {
 	return log_buf;
 }
+EXPORT_SYMBOL_GPL(log_buf_addr_get);
 
 /* Return log buffer size */
 u32 log_buf_len_get(void)
 {
 	return log_buf_len;
 }
+EXPORT_SYMBOL_GPL(log_buf_len_get);
 
 /*
  * Define how much of the log buffer we could take at maximum. The value
-- 
2.43.0

[RFC][PATCH 08/10] pstore: register kmsg into directly mapped zones if available

[Eugen Hristev]

If dmapped zones are available, register the log buffer into one zone.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 fs/pstore/platform.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 32448d9dd316..9a5c1d6d5031 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -452,11 +452,22 @@ static void pstore_register_kmsg(void)
 	kmsg_dump_register(&pstore_dumper);
 }
 
+static int pstore_register_kmsg_dmapped(void)
+{
+	return pstore_register_core_area("dmesg", log_buf_addr_get(),
+					 log_buf_len_get());
+}
+
 static void pstore_unregister_kmsg(void)
 {
 	kmsg_dump_unregister(&pstore_dumper);
 }
 
+static int pstore_unregister_kmsg_dmapped(void)
+{
+	return pstore_unregister_core_area("dmesg", log_buf_addr_get(),
+					   log_buf_len_get());
+}
 #ifdef CONFIG_PSTORE_CONSOLE
 static void pstore_console_write(struct console *con, const char *s, unsigned c)
 {
@@ -582,6 +593,9 @@ int pstore_register(struct pstore_info *psi)
 		pstore_dumper.max_reason = psinfo->max_reason;
 		pstore_register_kmsg();
 	}
+	if (psi->flags & PSTORE_FLAGS_DMAPPED)
+		if (pstore_register_kmsg_dmapped())
+			pr_warn("Registering kmsg as dmapped failed.\n");
 	if (psi->flags & PSTORE_FLAGS_CONSOLE)
 		pstore_register_console();
 	if (psi->flags & PSTORE_FLAGS_FTRACE)
@@ -628,6 +642,8 @@ void pstore_unregister(struct pstore_info *psi)
 		pstore_unregister_console();
 	if (psi->flags & PSTORE_FLAGS_DMESG)
 		pstore_unregister_kmsg();
+	if (psi->flags & PSTORE_FLAGS_DMAPPED)
+		pstore_unregister_kmsg_dmapped();
 
 	/* Stop timer and make sure all work has finished. */
 	del_timer_sync(&pstore_timer);
-- 
2.43.0

[RFC][PATCH 09/10] devcoredump: add devcd_{un}register_core_area API

[Eugen Hristev]

Add API for registering device core area for saving into pstore.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 drivers/base/devcoredump.c  | 13 +++++++++++++
 include/linux/devcoredump.h |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index 2a0e0b2fdb98..2cf17b148c5a 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -9,6 +9,7 @@
 #include <linux/device.h>
 #include <linux/devcoredump.h>
 #include <linux/list.h>
+#include <linux/pstore.h>
 #include <linux/slab.h>
 #include <linux/fs.h>
 #include <linux/workqueue.h>
@@ -76,6 +77,18 @@ static struct devcd_entry *dev_to_devcd(struct device *dev)
 	return container_of(dev, struct devcd_entry, devcd_dev);
 }
 
+int devcd_register_core_area(struct device *dev, void *area, size_t size)
+{
+	return pstore_register_core_area(dev->driver->name, area, size);
+}
+EXPORT_SYMBOL_GPL(devcd_register_core_area);
+
+int devcd_unregister_core_area(struct device *dev, void *area, size_t size)
+{
+	return pstore_unregister_core_area(dev->driver->name, area, size);
+}
+EXPORT_SYMBOL_GPL(devcd_unregister_core_area);
+
 static void devcd_dev_release(struct device *dev)
 {
 	struct devcd_entry *devcd = dev_to_devcd(dev);
diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
index 377892604ff4..9d80e4ac91ff 100644
--- a/include/linux/devcoredump.h
+++ b/include/linux/devcoredump.h
@@ -69,6 +69,9 @@ void dev_coredumpsg(struct device *dev, struct scatterlist *table,
 		    size_t datalen, gfp_t gfp);
 
 void dev_coredump_put(struct device *dev);
+
+int devcd_register_core_area(struct device *, void *, size_t);
+int devcd_unregister_core_area(struct device *, void *, size_t);
 #else
 static inline void dev_coredumpv(struct device *dev, void *data,
 				 size_t datalen, gfp_t gfp)
-- 
2.43.0

[RFC][PATCH 10/10] rng: qcom_rng: EXAMPLE: registering dev structure

[Eugen Hristev]

Proof of concept on how devcd register core area works.

Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
 drivers/crypto/qcom-rng.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index 0685ba122e8a..a1509609f50c 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -7,6 +7,7 @@
 #include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/crypto.h>
+#include <linux/devcoredump.h>
 #include <linux/hw_random.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
@@ -32,11 +33,13 @@
 #define QCOM_TRNG_QUALITY	1024
 
 struct qcom_rng {
+	char start[10];
 	struct mutex lock;
 	void __iomem *base;
 	struct clk *clk;
 	struct hwrng hwrng;
 	struct qcom_rng_match_data *match_data;
+	char end[10];
 };
 
 struct qcom_rng_ctx {
@@ -192,6 +195,10 @@ static int qcom_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->base))
 		return PTR_ERR(rng->base);
 
+	/* Setting some markers to easily recognize them afterwards */
+	strcpy(rng->start, "MD_RNG_ST");
+	strcpy(rng->end, "MD_RNG_en");
+
 	rng->clk = devm_clk_get_optional(&pdev->dev, "core");
 	if (IS_ERR(rng->clk))
 		return PTR_ERR(rng->clk);
@@ -218,6 +225,8 @@ static int qcom_rng_probe(struct platform_device *pdev)
 		}
 	}
 
+	devcd_register_core_area(&pdev->dev, rng, sizeof(*rng));
+
 	return ret;
 fail:
 	crypto_unregister_rng(&qcom_rng_alg);
@@ -228,6 +237,8 @@ static void qcom_rng_remove(struct platform_device *pdev)
 {
 	crypto_unregister_rng(&qcom_rng_alg);
 
+	devcd_unregister_core_area(&pdev->dev, qcom_rng_dev,
+				   sizeof(*qcom_rng_dev));
 	qcom_rng_dev = NULL;
 }
 
-- 
2.43.0

Re: [RFC][PATCH 00/10] pstore: directly mapped regions

[Johannes Berg]

On Mon, 2025-02-17 at 12:16 +0200, Eugen Hristev wrote:

> This series comes as an RFC proposed solution to enhance pstore and
> devcoredump with the following functionality:

...

> This patch series attempts to solve this by reusing existing
> infrastructure in pstore and devcoredump, and provide a copy-free

...

You mention devcoredump multiple times, but it almost seems like you
don't even know what devcoredump does? I don't see how there's any
relation at all, and the code added to it seems to have no relation to
the actual functionality of devcoredump either?

johannes

Re: [RFC][PATCH 00/10] pstore: directly mapped regions

[Eugen Hristev]

On 2/17/25 12:23, Johannes Berg wrote:
> On Mon, 2025-02-17 at 12:16 +0200, Eugen Hristev wrote:
> 
>> This series comes as an RFC proposed solution to enhance pstore and
>> devcoredump with the following functionality:
> 
> ...
> 
>> This patch series attempts to solve this by reusing existing
>> infrastructure in pstore and devcoredump, and provide a copy-free
> 
> ...
> 
> You mention devcoredump multiple times, but it almost seems like you
> don't even know what devcoredump does? I don't see how there's any
> relation at all, and the code added to it seems to have no relation to
> the actual functionality of devcoredump either?

At this moment going through devcoredump is not something that impacts
the idea of the implementation.
The whole reason of going through it (because things work without it as
well), is to see whether this has any kind of impact or not, and if
there is any kind of fit/reason of going through it.
Devcoredump is involved because the whole core registration is similar
to a core area that devcoredump could use.
For example, would it be interesting to have a handler going through all
devices, and have the dump areas already registered ?
Meaning, when there is a request to generate a core dump, one could
directly dump this area instead of calling back the driver, and provide
that to the userspace instead of the driver calling the dev_coredumpv by
its own.

> 
> johannes

Re: [RFC][PATCH 00/10] pstore: directly mapped regions

[Johannes Berg]

On Mon, 2025-02-17 at 12:44 +0200, Eugen Hristev wrote:
> 
> At this moment going through devcoredump is not something that impacts
> the idea of the implementation.

Yeah. I don't think it _should_ go through it at all.

> The whole reason of going through it (because things work without it as
> well), is to see whether this has any kind of impact or not, and if
> there is any kind of fit/reason of going through it.

So it's just a trial balloon?

> Devcoredump is involved because the whole core registration is similar
> to a core area that devcoredump could use.

Yeah but ... 

> For example, would it be interesting to have a handler going through all
> devices, and have the dump areas already registered ?
> Meaning, when there is a request to generate a core dump, one could
> directly dump this area instead of calling back the driver, and provide
> that to the userspace instead of the driver calling the dev_coredumpv by
> its own.

I'll be blunt ... so you _really_ haven't understood devcoredump then?

It's really not doing this. It's not meant to do this... It's intended
to dump data from inside the device when the device crashes.

Please remove devcoredump involvement from this series.

johannes

Re: [RFC][PATCH 00/10] pstore: directly mapped regions

[Eugen Hristev]

On 2/17/25 13:19, Johannes Berg wrote:
> On Mon, 2025-02-17 at 12:44 +0200, Eugen Hristev wrote:
>>
>> At this moment going through devcoredump is not something that impacts
>> the idea of the implementation.
> 
> Yeah. I don't think it _should_ go through it at all.
> 
>> The whole reason of going through it (because things work without it as
>> well), is to see whether this has any kind of impact or not, and if
>> there is any kind of fit/reason of going through it.
> 
> So it's just a trial balloon?

Yes, that's why it's marked as an RFC.
> 
>> Devcoredump is involved because the whole core registration is similar
>> to a core area that devcoredump could use.
> 
> Yeah but ... 
> 
>> For example, would it be interesting to have a handler going through all
>> devices, and have the dump areas already registered ?
>> Meaning, when there is a request to generate a core dump, one could
>> directly dump this area instead of calling back the driver, and provide
>> that to the userspace instead of the driver calling the dev_coredumpv by
>> its own.
> 
> I'll be blunt ... so you _really_ haven't understood devcoredump then?
> 
> It's really not doing this. It's not meant to do this... It's intended
> to dump data from inside the device when the device crashes.
> 
> Please remove devcoredump involvement from this series.

Thank you for your feedback.

> 
> johannes

Re: [RFC][PATCH 00/10] pstore: directly mapped regions

[Johannes Berg]

On Mon, 2025-02-17 at 13:39 +0200, Eugen Hristev wrote:
> 
> On 2/17/25 13:19, Johannes Berg wrote:
> > On Mon, 2025-02-17 at 12:44 +0200, Eugen Hristev wrote:
> > > 
> > > At this moment going through devcoredump is not something that impacts
> > > the idea of the implementation.
> > 
> > Yeah. I don't think it _should_ go through it at all.
> > 
> > > The whole reason of going through it (because things work without it as
> > > well), is to see whether this has any kind of impact or not, and if
> > > there is any kind of fit/reason of going through it.
> > 
> > So it's just a trial balloon?
> 
> Yes, that's why it's marked as an RFC.
> 

(IMHO) that doesn't make it acceptable to just randomly go modify code
you don't understand the purpose of, certainly not without actually
stating such. But anyway, I'll leave it at that.

johannes

Re: [RFC][PATCH 07/10] printk: export symbols for buffer address and length functions

[Christoph Hellwig]

On Mon, Feb 17, 2025 at 12:17:03PM +0200, Eugen Hristev wrote:
> log_buf_addr_get() and log_buf_len_get() can be reused in another module,
> export the symbols.

Err, no way.

Re: [RFC][PATCH 07/10] printk: export symbols for buffer address and length functions

[Sergey Senozhatsky]

On (25/02/18 00:26), Christoph Hellwig wrote:
> On Mon, Feb 17, 2025 at 12:17:03PM +0200, Eugen Hristev wrote:
> > log_buf_addr_get() and log_buf_len_get() can be reused in another module,
> > export the symbols.
> 
> Err, no way.

Yeah I think we've been there before [1]

[1] https://lore.kernel.org/all/20230905081902.321778-1-hch@lst.de/

Re: [RFC][PATCH 07/10] printk: export symbols for buffer address and length functions

[Eugen Hristev]

On 2/18/25 10:58, Sergey Senozhatsky wrote:
> On (25/02/18 00:26), Christoph Hellwig wrote:
>> On Mon, Feb 17, 2025 at 12:17:03PM +0200, Eugen Hristev wrote:
>>> log_buf_addr_get() and log_buf_len_get() can be reused in another module,
>>> export the symbols.
>>
>> Err, no way.
> 
> Yeah I think we've been there before [1]
> 
> [1] https://lore.kernel.org/all/20230905081902.321778-1-hch@lst.de/

Okay... in the line of the purpose of this patch series, I would like to
be able to have the dmesg ready to be retrieved out of the kernel in
case the kernel becomes unusable (due to various reasons as described in
the cover letter). However to know exactly 'where' is the buffer stored
I would need some way to point to it.
Do you have any other suggestion on how to do this ? Using the
kmsg_dump() API works only if the kernel/CPU can run code.

Thanks for looking into the patches,
Eugen