On Wed, Feb 22, 2023 at 08:13:59PM +0800, Leo Yan wrote: > Hi Souradeep, > > On Wed, Feb 22, 2023 at 04:46:07PM +0530, Souradeep Chowdhury wrote: > > On 2/22/2023 7:13 AM, Leo Yan wrote: > > > On Wed, Feb 15, 2023 at 04:05:36PM +0100, Arnd Bergmann wrote: > > [...] > > > > > If the possible use is purely for saving some state across > > > > a reboot, as opposed to other events, I wonder if there is > > > > a good way to integrate it into the fs/pstore/ code, which > > > > already has a way to multiplex various kinds of input (log > > > > buffer, ftrace call chain, userspace strings, ...) into > > > > various kinds of persistent buffers (sram, blockdev, mtd, > > > > efivars, ...) with the purpose of helping analyze the > > > > state after a reboot. > > > > > > Good point! > > > > > > I understand pstore/ramoops is somehow like a sink which routes the > > > tracing data (software tracing data but not hadware tracing data) to > > > persistent memory. This is why we also can route these software > > > tracing data to STM (hardware sink!). > > > > > > Seems to me, Arnd suggests to connect two sinks between DCC and > > > pstore (to persistent memory). But I cannot give an example code in > > > kernel for doing this way, sorry if I miss something. > > > > > > Essentially, a good user case is to keep a persistent memory for the > > > tracing data, then after rebooting cycle we can retrieve the tracing > > > data via user space interface (like sysfs node). > > > > Hi Leo/Arnd, > > > > Just wanted to let you know that the justification of not using PStore was > > already given in the version 1 of this patch series as below > > > > https://lore.kernel.org/linux-arm-msm/ab30490c016f906fd9bc5d789198530b@xxxxxxxxxxxxxx/#r > > > > PStore/Ramoops only persists across warm-reboots which is present for chrome > > devices but not for android ones. > > Thanks for the info. Just remind a subtle difference of reboots. > > Besides warm reboot, kernel can reboot system after panic (see kernel > command line option `panic`) and watchdog can reboot the system as well. > > Even though Android doesn't support warm reboot, system still can reboot > on panic or by watchdog (in particular after bus lockup), pstore/ramoops > also can support these cases. > The fact that DDR content isn't maintained across a typical reboot is certainly causing issues for using pstore. But afaiu, with pstore we capture various system state into the pstore and then make that available after reboot. DCC provides a mechanism for capturing hardware state (by reading register content) and storing that in DCC-specific SRAM. This can be triggered either by a HW reset, or on request from the user. As such it doesn't seem to be a conceptual fit, we perhaps could present the data as if it was pstore data, but I don't see how the reports created at runtime would fit into that model. > > Also the dcc_sram contents can > > also be collected by going for a software trigger after loading the kernel > > and the dcc_sram is parsed to get the register values with the > > opensource parser as below > > > > https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/tools/tree/dcc_parser > > To be clear, current driver is fine for me (TBH, I didn't spend much > time to read it but it's very neat after quickly went through it), I > just share some info in case it's helpful for the discussion. > My take is still that /dev/dcc looks to provide a reasonable ABI, and the control mechanism was pushed to debugfs for the purpose of enabling users to play with the interface, and improve it as necessary. Regards, Bjorn