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
