[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5a872001-dfcb-5c00-3b13-3c29e6be213b@linaro.org>
Date: Fri, 14 Apr 2023 12:40:58 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Mukesh Ojha <quic_mojha@...cinc.com>, agross@...nel.org,
andersson@...nel.org, konrad.dybcio@...aro.org, corbet@....net,
keescook@...omium.org, tony.luck@...el.com, gpiccoli@...lia.com,
catalin.marinas@....com, will@...nel.org
Cc: linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 6/6] remoterproc: qcom: refactor to leverage exported
minidump symbol
On 14/04/2023 12:14, Mukesh Ojha wrote:
>
>
> On 4/14/2023 4:14 PM, Srinivas Kandagatla wrote:
>>
>>
>> On 22/03/2023 13:30, Mukesh Ojha wrote:
>>> qcom_minidump driver provides qcom_minidump_subsystem_desc()
>>> exported API which other driver can use it query subsystem
>>> descriptor. Refactor qcom_minidump() to use this symbol.
>>>
>>> Signed-off-by: Mukesh Ojha <quic_mojha@...cinc.com>
>>> ---
>>> drivers/remoteproc/qcom_common.c | 13 ++-----------
>>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/qcom_common.c
>>> b/drivers/remoteproc/qcom_common.c
>>> index 88fc984..240e9f7 100644
>>> --- a/drivers/remoteproc/qcom_common.c
>>> +++ b/drivers/remoteproc/qcom_common.c
>>> @@ -94,19 +94,10 @@ void qcom_minidump(struct rproc *rproc, unsigned
>>> int minidump_id,
>>> {
>>> int ret;
>>> struct minidump_subsystem *subsystem;
>>> - struct minidump_global_toc *toc;
>>> - /* Get Global minidump ToC*/
>>> - toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID,
>>> NULL);
>>> -
>>> - /* check if global table pointer exists and init is set */
>>> - if (IS_ERR(toc) || !toc->status) {
>>> - dev_err(&rproc->dev, "Minidump TOC not found in SMEM\n");
>>> + subsystem = qcom_minidump_subsystem_desc(minidump_id);
>>> + if (IS_ERR(subsystem))
>>> return;
>>
>> Sorry If I am missing something but I got lost looking at the below
>> code snippet in drivers/remoteproc/qcom_common.c
>>
>>
>> -------------------->cut<-----------------------------
>> subsystem = qcom_minidump_subsystem_desc(minidump_id);
>> if (IS_ERR(subsystem))
>> return;
>>
>> /**
>> * Collect minidump if SS ToC is valid and segment table
>> * is initialized in memory and encryption status is set.
>> */
>> if (subsystem->regions_baseptr == 0 ||
>> le32_to_cpu(subsystem->status) != 1 ||
>> le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED ||
>> le32_to_cpu(subsystem->encryption_status) !=
>> MINIDUMP_SS_ENCR_DONE) {
>> dev_err(&rproc->dev, "Minidump not ready, skipping\n");
>> return;
>> }
>> -------------------->cut<-----------------------------
>>
>> where does "subsystem->regions_baseptr" for this ADSP minidump
>> descriptor get set?
>
> Other co-processor such as adsp/cdsp/Mpss has their own way of
> registering their region/segment (mostly they are static known
> regions) with minidump global infra and which could be happening
> from firmware side .
If its happening from firmware side, then that ram phys address range
should be reserved from kernel usage I guess.
Do you have more details on where exactly is this reserved from within
linux kernel?
--srini
>
>
> -Mukesh
>
>>
>>
>> --srini
>>
>>> - }
>>> -
>>> - /* Get subsystem table of contents using the minidump id */
>>> - subsystem = &toc->subsystems[minidump_id];
>>> /**
>>> * Collect minidump if SS ToC is valid and segment table
Powered by blists - more mailing lists