linux-kernel - Re: [PATCH V1 2/6] soc: qcom: dcc: Add driver support for Data Capture and Compare unit(DCC)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Mar 2021 10:31:16 -0600
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Sai Prakash Ranjan <saiprakash.ranjan@...eaurora.org>
Cc:     Souradeep Chowdhury <schowdhu@...eaurora.org>,
        Rob Herring <robh+dt@...nel.org>,
        Andy Gross <agross@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        sibis@...eaurora.org, Rajendra Nayak <rnayak@...eaurora.org>,
        vkoul@...nel.org
Subject: Re: [PATCH V1 2/6] soc: qcom: dcc: Add driver support for Data
 Capture and Compare unit(DCC)

On Thu 11 Mar 00:19 CST 2021, Sai Prakash Ranjan wrote:

> Hi Bjorn,
> 
> On 2021-03-11 04:49, Bjorn Andersson wrote:
> > On Wed 10 Mar 10:46 CST 2021, Souradeep Chowdhury wrote:
> > 
> > > The DCC is a DMA Engine designed to capture and store data
> > > during system crash or software triggers. The DCC operates
> > > based on link list entries which provides it with data and
> > > addresses and the function it needs to perform. These
> > > functions are read, write and loop. Added the basic driver
> > > in this patch which contains a probe method which instantiates
> > > the resources needed by the driver. DCC has it's own SRAM which
> > > needs to be instantiated at probe time as well.
> > > 
> > 
> > So to summarize, the DCC will upon a crash copy the configured region
> > into the dcc-ram, where it can be retrieved either by dumping the memory
> > over USB or from sysfs on the next boot?
> > 
> 
> Not just the next boot, but also for the current boot via /dev/dcc_sram,
> more below.
> 

Interesting!

> > > Signed-off-by: Souradeep Chowdhury <schowdhu@...eaurora.org>
> > > ---
> > >  drivers/soc/qcom/Kconfig  |   8 +
> > >  drivers/soc/qcom/Makefile |   1 +
> > >  drivers/soc/qcom/dcc.c    | 388
> > > ++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 397 insertions(+)
> > >  create mode 100644 drivers/soc/qcom/dcc.c
> > > 
> 
> <snip>...
> 
> > 
> > How about implementing this using pstore instead of exposing it through
> > a custom /dev/dcc_sram (if I read the code correclty)
> > 
> 
> Using pstore is definitely a good suggestion, we have been thinking of
> adding it as a separate change once the basic support for DCC gets in.
> But pstore ram backend again depends on warm reboot which is present only
> in chrome compute platforms but android platforms do not officially support
> warm reboot. Pstore with block devices as a backend would be ideal but it
> is still a work in progress to use mmc as the backend [1].
> 

I was under the impression that we can have multiple pstore
implementations active, so we would have ramoops and dcc presented
side by side after such restart. Perhaps that's a misunderstanding on my
part?

> Now the other reason as to why we need this dev interface in addition to
> pstore,
> 
>  * Pstore contents are retrieved on the next boot, but DCC SRAM contents
>    can be collected via dev interface for the current boot via SW trigger.
>    Lets say we have some non-fatal errors in one of the subsystems and we
>    want to analyze the register values, it becomes as simple as configuring
>    that region, trigger dcc and collect the sram contents and parse it.
> 
>    echo "addr" > /sys/bus/platform/devices/***.dcc/config
>    echo  1 > /sys/bus/platform/devices/***.dcc/trigger
>    cat /dev/dcc_sram > dcc_sram.bin
>    python dcc_parser.py -s dcc_sram.bin --v2 -o output/
> 
> We don't have to reboot at all for SW triggers. This is very useful and
> widely used internally.
> 
> Is the custom /dev/dcc_sram not recommended because of the dependency on
> the userspace component being not available openly? If so, we already have
> the dcc parser upstream which we use to extract the sram contents [2].
> 

My concern is that we come up with a custom chardev implementation for
something that already exists or should exist in a more generic form.


In the runtime sequence above, why don't you have trigger just generate
a devcoredump? But perhaps the answer is that we want a unified
interface between the restart and runtime use cases?


It would be nice to have some more details of how I can use this and how
to operate the sysfs interface to achieve that, would you be able to
elaborate on this?

Regards,
Bjorn

> [1]
> https://lore.kernel.org/lkml/20210120121047.2601-1-bbudiredla@marvell.com/
> [2] https://source.codeaurora.org/quic/la/platform/vendor/qcom-opensource/tools/tree/dcc_parser
> 
> Thanks,
> Sai
> 
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation