Thanks for the review.
On 9/14/2023 4:54 AM, Kees Cook wrote:
On Mon, Sep 11, 2023 at 04:23:52PM +0530, Mukesh Ojha wrote:
There are users like Qualcomm minidump which is interested in
knowing the pstore frontend addresses and sizes from the backend
(ram) to be able to register it with firmware to finally collect
them during crash for debugging.
Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
---
fs/pstore/platform.c | 15 +++++++++++++++
fs/pstore/ram.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pstore.h | 6 ++++++
3 files changed, 63 insertions(+)
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index e5bca9a004cc..fdac951059c1 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -139,6 +139,21 @@ enum pstore_type_id pstore_name_to_type(const char *name)
}
EXPORT_SYMBOL_GPL(pstore_name_to_type);
+int pstore_region_defined(struct pstore_record *record,
+ void **virt, phys_addr_t *phys,
+ size_t *size, unsigned int *max_dump_cnt)
+{
+ if (!psinfo)
+ return -EINVAL;
+
+ record->psi = psinfo;
Uh, this makes no sense to me. If this is a real pstore_record, you
cannot just assign psi here.
Ok.
+
+ return psinfo->region_info ?
+ psinfo->region_info(record, virt, phys, size, max_dump_cnt) :
+ -EINVAL;
Common code style for this kind of thing is usually like this:
if (!psinfo->region_info)
return -EINVAL;
return psinfo->region_info(...)
Thanks.
-Mukesh
+}
+EXPORT_SYMBOL_GPL(pstore_region_defined);
+
static void pstore_timer_kick(void)
{
if (pstore_update_ms < 0)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ab551caa1d2a..62202f3ddf63 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -437,6 +437,47 @@ static int ramoops_pstore_erase(struct pstore_record *record)
return 0;
}
+static int ramoops_region_info(struct pstore_record *record,
+ void **virt, phys_addr_t *phys,
+ size_t *size, unsigned int *max_dump_cnt)
But there's a larger problem here -- "virt", "phys" and likely
"max_dump_cnt" are aspects _specific to the ram backend_. This can't be
a generic pstore interface.
I'm not opposed to it being exposed only from ramoops, though.
But I think you'll want a pstore generic way to iterate over the
records..
