On Tue, May 09, 2023 at 06:06:26PM +0200, Luca Stefani wrote: > > On 03/05/23 19:02, Mukesh Ojha wrote: > > This driver was inspired from the fact pstore ram region should be > > fixed and boot firmware need to have awarness about this region, > > so that it will be persistent across boot. But, there are many > > QCOM SoC which does not support warm boot from hardware but they > > have minidump support from the software, and for them, there is > > no need of this pstore ram region to be fixed, but at the same > > time have interest in the pstore frontends. So, this driver > > get the dynamic reserved region from the ram and register the > > ramoops platform device. > > > > +---------+ +---------+ +--------+ +---------+ > > | console | | pmsg | | ftrace | | dmesg | > > +---------+ +---------+ +--------+ +---------+ > > | | | | > > | | | | > > +------------------------------------------+ > > | > > \ / > > +----------------+ > > (1) |pstore frontends| > > +----------------+ > > | > > \ / > > +------------------- + > > (2) | pstore backend(ram)| > > +--------------------+ > > | > > \ / > > +--------------------+ > > (3) |qcom_pstore_minidump| > > +--------------------+ > > | > > \ / > > +---------------+ > > (4) | qcom_minidump | > > +---------------+ > > > > This driver will route all the pstore front data to the stored > > in qcom pstore reserved region and the reason of showing an > > arrow from (3) to (4) as qcom_pstore_minidump driver will register > > all the available frontends region with qcom minidump driver > > in upcoming patch. > > > > Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx> > [...] > > +static struct qcom_ramoops_config default_ramoops_config = { > > + .mem_type = 2, > > + .record_size = 0x0, > > + .console_size = 0x200000, > > + .ftrace_size = 0x0, > > + .pmsg_size = 0x0, > > +}; > > This is effectively hard-cording the configuration of ramoops. > > Since the memory range is dynamic and by itself doesn't impose any > limitation this should be configurable in the device-tree, like a standard > ramoops entry backed by a memory range. > > I think this should provide the same interface/knobs as pstore-ram does, > unless there's some known limitations to minidump, in which case those > should be expressed. Yeah, I had the same thought reading this myself. Beyond that, it looks fine as a way to let pstore know about a new RAM backend. -Kees -- Kees Cook