From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D2A78EB64D7 for ; Fri, 30 Jun 2023 03:42:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TdMwost16mJywqfxw6u0olQaSrFX31eP4RrNe5I7N9M=; b=OLUqmf8D2Knssn GZb4jc060pKZaFpUa6xmUgh5z4ZG3GGTyfPnGMwYhtFsj3HWyuz1FicqG6+aFsMn39wzC2RzoYZkJ CSX7e+I1d6s8/0ITzbb/wNBKvDyziIO9Eb2Sgt5G/qUFMdYPzN52l96wFhjAy/RvVGM93ClUezyB5 D0TIoYKZNkWEBmM+Yek5KCensWhTXDEaYYl8/peITPEI/M1BNC5B6sd7wCknTfWjM4i8AHMLtBfpH BAx0T3KJF9Evsaue45c3itbYI4Js886yoSxTN7Lb6jtULarMUXn+43Pgqkcly6Cwynnrx5z0a+GA/ 4Rq9itjGEXEBghS6Qmwg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qF51P-002mJm-2S; Fri, 30 Jun 2023 03:42:11 +0000 Received: from mx0b-0031df01.pphosted.com ([205.220.180.131]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qF51N-002mIk-08 for linux-arm-kernel@lists.infradead.org; Fri, 30 Jun 2023 03:42:10 +0000 Received: from pps.filterd (m0279870.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 35U3dYZa018520; Fri, 30 Jun 2023 03:41:47 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=qcppdkim1; bh=seQ2j0gbJ727u3DPmbrPvnp+QMoIsDD6zoTAV/LLxGs=; b=k7/67pWnKxiivIUzpZLXHAvhD9aOhk1TEi12M1YYgewLaU2xMvnyILcWV2fIVpLNyNip zVWgOPbthu/NaQ/D0mWU7TTrtPgU2KF7t0aFi4g9k40CGPbTaHbtIPyg9xaDG2s37cJq FJQe/ESik6zPQqGhuHBCGxPuJb8RsQw8zVUqBu+X/7iHWm78tLWYlJAExZzzRP/DOFUM /YPL+vUXfpE1otUzLL+EnpqXEMwpv0u2itErpRTMNPRHPoo/7ehiV4A52wiFZwT8x7hM BpdH8FpsYeNRh5sNXmFgKR/eTNVjdooUFLW6oOdOquMAhdsZ1nGQWP1F5Jh//U2iMEFQ Dg== Received: from nalasppmta04.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3rh0aajpd6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 30 Jun 2023 03:41:47 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 35U3fjTk004090 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 30 Jun 2023 03:41:45 GMT Received: from hu-pkondeti-hyd.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.7; Thu, 29 Jun 2023 20:41:38 -0700 Date: Fri, 30 Jun 2023 09:11:34 +0530 From: Pavan Kondeti To: Mukesh Ojha CC: Pavan Kondeti , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH v4 13/21] remoterproc: qcom: refactor to leverage exported minidump symbol Message-ID: <5976b2a5-6ff4-4d1d-8d8a-dca783f49799@quicinc.com> References: <1687955688-20809-1-git-send-email-quic_mojha@quicinc.com> <1687955688-20809-14-git-send-email-quic_mojha@quicinc.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: fcztJk2Fzuuq4shNAq7buAclqPcMp1p1 X-Proofpoint-ORIG-GUID: fcztJk2Fzuuq4shNAq7buAclqPcMp1p1 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-06-30_01,2023-06-27_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 priorityscore=1501 phishscore=0 spamscore=0 lowpriorityscore=0 suspectscore=0 mlxscore=0 clxscore=1015 impostorscore=0 mlxlogscore=842 malwarescore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306300030 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230629_204209_220931_B1F31D22 X-CRM114-Status: GOOD ( 33.51 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jun 29, 2023 at 02:50:34PM +0530, Mukesh Ojha wrote: > > > On 6/28/2023 9:21 PM, Pavan Kondeti wrote: > > On Wed, Jun 28, 2023 at 06:04:40PM +0530, Mukesh Ojha wrote: > > > -static int qcom_add_minidump_segments(struct rproc *rproc, struct minidump_subsystem *subsystem, > > > - void (*rproc_dumpfn_t)(struct rproc *rproc, struct rproc_dump_segment *segment, > > > - void *dest, size_t offset, size_t size)) > > > +void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, > > > + void (*rproc_dumpfn_t)(struct rproc *rproc, > > > + struct rproc_dump_segment *segment, void *dest, size_t offset, > > > + size_t size)) > > > { > > > - struct minidump_region __iomem *ptr; > > > - struct minidump_region region; > > > - int seg_cnt, i; > > > dma_addr_t da; > > > size_t size; > > > + int seg_cnt; > > > char *name; > > > + void *ptr; > > > + int ret; > > > + int i; > > > if (WARN_ON(!list_empty(&rproc->dump_segments))) { > > > dev_err(&rproc->dev, "dump segment list already populated\n"); > > > - return -EUCLEAN; > > > + return; > > > } > > > - seg_cnt = le32_to_cpu(subsystem->region_count); > > > - ptr = ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr), > > > - seg_cnt * sizeof(struct minidump_region)); > > > + ptr = qcom_ss_md_mapped_base(minidump_id, &seg_cnt); > > > if (!ptr) > > > - return -EFAULT; > > > + return; > > > for (i = 0; i < seg_cnt; i++) { > > > - memcpy_fromio(®ion, ptr + i, sizeof(region)); > > > - if (le32_to_cpu(region.valid) == MINIDUMP_REGION_VALID) { > > > - name = kstrndup(region.name, MAX_REGION_NAME_LENGTH - 1, GFP_KERNEL); > > > - if (!name) { > > > - iounmap(ptr); > > > - return -ENOMEM; > > > - } > > > - da = le64_to_cpu(region.address); > > > - size = le64_to_cpu(region.size); > > > - rproc_coredump_add_custom_segment(rproc, da, size, rproc_dumpfn_t, name); > > > + ret = qcom_ss_valid_segment_info(ptr, i, &name, &da, &size); > > > + if (ret < 0) { > > > + iounmap(ptr); > > > + dev_err(&rproc->dev, > > > + "Failed with error: %d while adding minidump entries\n", > > > + ret); > > > + goto clean_minidump; > > > } > > > - } > > > - > > > - iounmap(ptr); > > > - return 0; > > > -} > > > - > > > -void qcom_minidump(struct rproc *rproc, unsigned int minidump_id, > > > - void (*rproc_dumpfn_t)(struct rproc *rproc, > > > - struct rproc_dump_segment *segment, void *dest, size_t offset, > > > - size_t size)) > > > -{ > > > - 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"); > > > - return; > > > - } > > > - /* Get subsystem table of contents using the minidump id */ > > > - subsystem = &toc->subsystems[minidump_id]; > > > - > > > - /** > > > - * 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; > > > + /* if it is a valid segment */ > > > + if (!ret) > > > + rproc_coredump_add_custom_segment(rproc, da, size, > > > + rproc_dumpfn_t, name); > > > } > > > - ret = qcom_add_minidump_segments(rproc, subsystem, rproc_dumpfn_t); > > > - if (ret) { > > > - dev_err(&rproc->dev, "Failed with error: %d while adding minidump entries\n", ret); > > > - goto clean_minidump; > > > - } > > > + iounmap(ptr); > > > rproc_coredump_using_sections(rproc); > > > + > > > clean_minidump: > > > qcom_minidump_cleanup(rproc); > > > } > > > > I like the idea of moving minidump pieces to drivers/soc/qcom/*minidump*. > > > > Is it possible to accept one function callback from remoteproc and do > > all of this in one function exported by minidump? > > > > qcom_ss_valid_segment_info() seems a low level function to be exported.. > > It was ending up with circular dependency due to > rproc_coredump_add_custom_segment() > > > rproc_coredump => qcom_common => qcom_minidump_smem => rproc_coredump > Where is the circular dependency here? Any API accepting callback would end up like below caller -> API(callback) -> (*callback) -> [in caller code ...] May be I am missing something here. AFAICS, the minidump could do everything except it does not know how to create a segment and add it to rproc::dump_segments list. Is it not possible to write a minidump API that accepts a callback and a list head? we may not probably re-use rproc_coredump_add_custom_segment() as is, but some refactoring allows/covers both cases..Pls give it a thought. This way we don't need to create an internal API like below int qcom_ss_valid_segment_info(void *ptr, int i, char **name, dma_addr_t *da, size_t *size) Thanks, Pavan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel