On Tue, May 09, 2023 at 06:06:26PM +0200, Luca Stefani wrote:

On 03/05/23 19:02, Mukesh Ojha wrote:

quoted

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 <redacted>

[...]

quoted

+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