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_mo...@quicinc.com> > --- > 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. > + > + 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(...) > +} > +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... -- Kees Cook