Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <af23d650-3dec-9a51-566c-1eec2fa4b84b@quicinc.com>
Date:   Fri, 14 Apr 2023 16:44:02 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v2 6/6] remoterproc: qcom: refactor to leverage exported
 minidump symbol
Content-Language: en-US
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        <agross@kernel.org>, <andersson@kernel.org>,
        <konrad.dybcio@linaro.org>, <corbet@lwn.net>,
        <keescook@chromium.org>, <tony.luck@intel.com>,
        <gpiccoli@igalia.com>, <catalin.marinas@arm.com>, <will@kernel.org>
CC:     <linux-arm-msm@vger.kernel.org>,
        <linux-remoteproc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-hardening@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <linux-doc@vger.kernel.org>
References: <1679491817-2498-1-git-send-email-quic_mojha@quicinc.com>
 <1679491817-2498-7-git-send-email-quic_mojha@quicinc.com>
 <6c01d2fc-3155-0dcd-f473-9cbd75dd69ec@linaro.org>
From:   Mukesh Ojha <quic_mojha@quicinc.com>
In-Reply-To: <6c01d2fc-3155-0dcd-f473-9cbd75dd69ec@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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@quicinc.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 .


-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