Message ID | 20230811205839.727373-3-quic_bjorande@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp1384811vqi; Fri, 11 Aug 2023 15:11:51 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF2KdCM7rrboTLBWdOUxCtCRmo62r1G02k42LeP+Yuhar8pFjhEurrDjbobDQrWcll5qhhw X-Received: by 2002:aa7:d84e:0:b0:522:21a1:516b with SMTP id f14-20020aa7d84e000000b0052221a1516bmr3330549eds.24.1691791911604; Fri, 11 Aug 2023 15:11:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691791911; cv=none; d=google.com; s=arc-20160816; b=laEhgyH5dIByX26LFzeSjfpDk/SdRDPZWlAJWDesBvCmhd+nPPRYf0ASPz7yt1GtQK ErtHiNMqvFX9DBOgjdTIng7xoSM+pNOPQqSFwDW5ehI37FBOOgkdPt77vMcMtF6A74wd LewWVOsZO3IZShC/mMEdo8u6AqhNtC0vvbbRMMSEMuWIBxwbl0BdVdPzTXiYXxfYnFaL 4tNFpkO4SNgop79MvDIKNhXR/aGyc28PorBdY+8L2JXCKkb8EG9QunVyA17xjDPZ0YQ8 DCgeFy5JGIKhIKMD2OuXy8SuamLMO0yO8TgvOSm+dml11Nk97QXn/bpokT7iHQPo5Qfo QHqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=rjRUCS76I8CfgS+Dkl3KtXFJE42El+shzDO6P7guxK4=; fh=GHLs1D38WDI8GaSYF7/ttEVaKFjn6AeXObXngbZgRCA=; b=OImT15I4PF0N+CpbxY2oLU1Vk5elgRXyGhHwAifp6AYNbDtpzGPl9I/CB+IgaCXcY9 Y1KAqJccWBHdwDP7y2FMlJ0usdXSCZlg2j7OtLwq+YWHTLsdwpI05N9UsY+BrdnDrxJI OdL6TWWV/xxnWWubWxdSzOzozX8fdjghV1SVagfYLZSeaVPCd3GVXv0K+7tA9StRMYWT F3ko1TisMYfCAq0zV3WfP7ngnHsOmoc4i/p6P+rltrZVHA/Q+2sSoMxl/EgYA1ieC1Te nXF/Pyrk9dSugZhj/6IsD8ePeNmSgrMA3vIOVrncaRUZjEtERV7jfIuZ4UsQIhitSGvY vASA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=kABoSGnW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p5-20020aa7cc85000000b005233e4f1ff5si3922199edt.591.2023.08.11.15.11.25; Fri, 11 Aug 2023 15:11:51 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=kABoSGnW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236302AbjHKU7G (ORCPT <rfc822;lanlanxiyiji@gmail.com> + 99 others); Fri, 11 Aug 2023 16:59:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50396 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233798AbjHKU67 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 11 Aug 2023 16:58:59 -0400 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 20394E7E; Fri, 11 Aug 2023 13:58:57 -0700 (PDT) Received: from pps.filterd (m0279865.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 37BFv8c5030614; Fri, 11 Aug 2023 20:58:47 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=qcppdkim1; bh=rjRUCS76I8CfgS+Dkl3KtXFJE42El+shzDO6P7guxK4=; b=kABoSGnWg5EZDiBaxlLu80Wiw/qyfTzbaQFKJJJSSa5QolHIGxIM24WyNOGu86VY0Owf dtdDhHdA7pceH//HeT57DCO+0Xpzo/jUE4aqvVFa7y/a09U1aa4fA8YGWAZfp207NPdS 00rvZi2vk5LqwQnJf6tu51qDolPwDTdL3o5PctHXsrV3h7A0HkHFbrBmhqpVXRw1QynF amR8vLTbIksWF5nAhjtGC+d8Y9J1IlS95anKFzfi8X0BszON9QrW0Dk6AM+cj5XfyIem rfJRpjDvw1oM+H6+i3E7S3COlNeNId/y2MUx8IP6Rf/9kY9ppjaD/QQ94a8MjhFuvqrO Gg== Received: from nalasppmta03.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3sd8ybtr2d-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 11 Aug 2023 20:58:46 +0000 Received: from nalasex01c.na.qualcomm.com (nalasex01c.na.qualcomm.com [10.47.97.35]) by NALASPPMTA03.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 37BKwkbM008697 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 11 Aug 2023 20:58:46 GMT Received: from hu-bjorande-lv.qualcomm.com (10.49.16.6) by nalasex01c.na.qualcomm.com (10.47.97.35) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.30; Fri, 11 Aug 2023 13:58:45 -0700 From: Bjorn Andersson <quic_bjorande@quicinc.com> To: Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Chris Lew <quic_clew@quicinc.com> CC: Alex Elder <elder@kernel.org>, "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <linux-remoteproc@vger.kernel.org>, Andrew Lunn <andrew@lunn.ch> Subject: [PATCH v2 2/4] soc: qcom: aoss: Add debugfs interface for sending messages Date: Fri, 11 Aug 2023 13:58:37 -0700 Message-ID: <20230811205839.727373-3-quic_bjorande@quicinc.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230811205839.727373-1-quic_bjorande@quicinc.com> References: <20230811205839.727373-1-quic_bjorande@quicinc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [10.49.16.6] X-ClientProxiedBy: nalasex01c.na.qualcomm.com (10.47.97.35) To nalasex01c.na.qualcomm.com (10.47.97.35) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: GJ_kAZJX-pBjPuBdBOheuX1VWHS2gg32 X-Proofpoint-GUID: GJ_kAZJX-pBjPuBdBOheuX1VWHS2gg32 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-08-11_12,2023-08-10_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 phishscore=0 lowpriorityscore=0 malwarescore=0 adultscore=0 clxscore=1015 priorityscore=1501 suspectscore=0 impostorscore=0 spamscore=0 mlxlogscore=999 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2308110193 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773969567632662449 X-GMAIL-MSGID: 1773972395573719470 |
Series |
soc: qcom: aoss: Introduce debugfs interface and cleanup things
|
|
Commit Message
Bjorn Andersson
Aug. 11, 2023, 8:58 p.m. UTC
From: Chris Lew <clew@codeaurora.org> In addition to the normal runtime commands, the Always On Processor (AOP) provides a number of debug commands which can be used during system debugging for things such as preventing power collapse or placing floor votes for certain resources. Some of these are documented in the Robotics RB5 "Debug AOP ADB" linked below. Provide a debugfs interface for the developer/tester to send these commands to the AOP. Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html Signed-off-by: Chris Lew <clew@codeaurora.org> [bjorn: Dropped debugfs guards, improve error codes, rewrote commit message] Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> --- drivers/soc/qcom/qcom_aoss.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)
Comments
On Fri, Aug 11, 2023 at 11:01:50PM +0200, Andrew Lunn wrote: > > +static ssize_t qmp_debugfs_write(struct file *file, const char __user *userstr, > > + size_t len, loff_t *pos) > > +{ > > + struct qmp *qmp = file->private_data; > > + char buf[QMP_MSG_LEN]; > > + int ret; > > + > > + if (!len || len >= QMP_MSG_LEN) > > + return -EINVAL; > > + > > + if (copy_from_user(buf, userstr, len)) > > + return -EFAULT; > > + buf[len] = '\0'; > > + > > + ret = qmp_send(qmp, buf); > > + if (ret < 0) > > + return ret; > > Sorry, but you still appear to be sending binary blobs from userspace > to the firmware. This is not liked. > As mentioned in the cover letter, I do recognize your concern here. I don't see it as a realistic way to work around the kernel for reasons of being proprietary - given that we don't have debugfs mounted in the vast majority of product. I do however recognize the benefit of this interface for fellow upstream engineers. > The documentation you pointed to has three commands. Please implement > three debugfs files, one per command. > The documentation pointed to has 4 classes ("class"), but this is not the full set, each class has N resources ("res") and each resource has a varying value space - "off", "mol", "enabled", "disabled", "max" to take the examples from the documentation, other classes takes integers as argument. Some classes has a fourth key... Further more, the list of classes, resources and values varies from target to target. We're composing the lists of commands, but I'm not sure that it will be feasible to spell out all the valid commands, on a per-target basis. It is just a debug feature, I don't want it to take up a significant portion of the driver. The alternative is to continue carrying this as an out-of-tree patch, the only people suffering from that are the ones working exclusively in on the upstream kernel. Regards, Bjorn
On Fri, Aug 11, 2023 at 01:58:37PM -0700, Bjorn Andersson wrote: > From: Chris Lew <clew@codeaurora.org> > > In addition to the normal runtime commands, the Always On Processor > (AOP) provides a number of debug commands which can be used during > system debugging for things such as preventing power collapse or placing > floor votes for certain resources. Some of these are documented in the > Robotics RB5 "Debug AOP ADB" linked below. > > Provide a debugfs interface for the developer/tester to send these > commands to the AOP. > > Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html > Signed-off-by: Chris Lew <clew@codeaurora.org> > [bjorn: Dropped debugfs guards, improve error codes, rewrote commit message] > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com> > --- > drivers/soc/qcom/qcom_aoss.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c > index 880fe234ca0a..13bf13ab78d6 100644 > --- a/drivers/soc/qcom/qcom_aoss.c > +++ b/drivers/soc/qcom/qcom_aoss.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2019, Linaro Ltd > */ > #include <linux/clk-provider.h> > +#include <linux/debugfs.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/mailbox_client.h> > @@ -82,6 +83,7 @@ struct qmp { > > struct clk_hw qdss_clk; > struct qmp_cooling_device *cooling_devs; > + struct dentry *debugfs_file; Simon Horman pointed out in the previous version that this isn't added to kernel-doc, and I missed correcting this. Regards, Bjorn
> As mentioned in the cover letter, I do recognize your concern here. I > don't see it as a realistic way to work around the kernel for reasons of > being proprietary - given that we don't have debugfs mounted in the vast > majority of product. Look around. How many drivers do you see which allow passing binary blobs to firmware? Andrew
On Fri, 11 Aug 2023 16:32:28 -0700 Bjorn Andersson <quic_bjorande@quicinc.com> wrote: > On Fri, Aug 11, 2023 at 11:01:50PM +0200, Andrew Lunn wrote: > > > +static ssize_t qmp_debugfs_write(struct file *file, const char __user *userstr, > > > + size_t len, loff_t *pos) > > > +{ > > > + struct qmp *qmp = file->private_data; > > > + char buf[QMP_MSG_LEN]; > > > + int ret; > > > + > > > + if (!len || len >= QMP_MSG_LEN) > > > + return -EINVAL; > > > + > > > + if (copy_from_user(buf, userstr, len)) > > > + return -EFAULT; > > > + buf[len] = '\0'; > > > + > > > + ret = qmp_send(qmp, buf); > > > + if (ret < 0) > > > + return ret; > > > > Sorry, but you still appear to be sending binary blobs from userspace > > to the firmware. This is not liked. > > > > As mentioned in the cover letter, I do recognize your concern here. I > don't see it as a realistic way to work around the kernel for reasons of > being proprietary - given that we don't have debugfs mounted in the vast > majority of product. Anyone who cares about security, and has things like kernel lockdown turned on is going to be scared by this. If you allow API to tell firmware to do any arbitrary thing it means you could be telling firmware "please read this area of kernel memory for me"
diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c index 880fe234ca0a..13bf13ab78d6 100644 --- a/drivers/soc/qcom/qcom_aoss.c +++ b/drivers/soc/qcom/qcom_aoss.c @@ -3,6 +3,7 @@ * Copyright (c) 2019, Linaro Ltd */ #include <linux/clk-provider.h> +#include <linux/debugfs.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/mailbox_client.h> @@ -82,6 +83,7 @@ struct qmp { struct clk_hw qdss_clk; struct qmp_cooling_device *cooling_devs; + struct dentry *debugfs_file; }; static void qmp_kick(struct qmp *qmp) @@ -475,6 +477,32 @@ void qmp_put(struct qmp *qmp) } EXPORT_SYMBOL(qmp_put); +static ssize_t qmp_debugfs_write(struct file *file, const char __user *userstr, + size_t len, loff_t *pos) +{ + struct qmp *qmp = file->private_data; + char buf[QMP_MSG_LEN]; + int ret; + + if (!len || len >= QMP_MSG_LEN) + return -EINVAL; + + if (copy_from_user(buf, userstr, len)) + return -EFAULT; + buf[len] = '\0'; + + ret = qmp_send(qmp, buf); + if (ret < 0) + return ret; + + return len; +} + +static const struct file_operations qmp_debugfs_fops = { + .open = simple_open, + .write = qmp_debugfs_write, +}; + static int qmp_probe(struct platform_device *pdev) { struct qmp *qmp; @@ -523,6 +551,9 @@ static int qmp_probe(struct platform_device *pdev) platform_set_drvdata(pdev, qmp); + qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL, + qmp, &qmp_debugfs_fops); + return 0; err_close_qmp: @@ -537,6 +568,8 @@ static int qmp_remove(struct platform_device *pdev) { struct qmp *qmp = platform_get_drvdata(pdev); + debugfs_remove(qmp->debugfs_file); + qmp_qdss_clk_remove(qmp); qmp_cooling_devices_remove(qmp);