Re: [PATCH v2 6/6] remoterproc: qcom: refactor to leverage exported minidump symbol

From: Srinivas Kandagatla
Date: Fri Apr 14 2023 - 08:22:57 EST




On 14/04/2023 12:49, Mukesh Ojha wrote:


On 4/14/2023 5:10 PM, Srinivas Kandagatla wrote:


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@xxxxxxxxxxx>
---
  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?

These regions are inside remoteproc memory carve-out.
like.

adsp_mem: memory@85e00000 {
    reg = <0x0 0x85e00000 0x0 0x2100000>;
    no-map;
};

thanks for explaining.

--srini



remoteproc_adsp: remoteproc@30000000 {
    compatible = "qcom,sm8450-adsp-pas";
    reg = <0 0x30000000 0 0x100>;
           ...
           ...
    memory-region = <&adsp_mem>; <==

-Mukesh



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