Re: [PATCH V2 2/4] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On 8/30/2024 1:11 PM, Dmitry Baryshkov wrote:
On Thu, Aug 29, 2024 at 07:10:19PM GMT, Gokul Sriram Palanisamy wrote:
From: Vignesh Viswanathan
Add support to bring up hexagon based WCSS secure PIL remoteproc.
IPQ5332, IPQ9574 supports secure PIL remoteproc.
Could you please clarify, why this can't be handled by the qcom_q6v5_pas
driver.
Hi Dmitry,
Apologies for this delayed response. Currently we came with this secure
PIL driver is based on Bjon's suggestion [1] for not to bloat the
existing non-secure wcss PIL driver with secure-pil and to write a
separate wcss-secire-pil driver. The PAS driver already looked bloated
with several MSM platform. Also it has a required clock field which is
already taken care by TrustZone for IPQ chipsets.
The recent development is to add mailbox driver that will take care of
handling secure call with both TMELCOM used by recent IPQ5424 based
platform and TrustZone SCM for older IPQ platforms. This will mean, the
current wcss-sec-pil driver will only need to use mbox_send_message to
handle secure calls.
[1]
https://patchwork.kernel.org/project/linux-arm-msm/patch/1611984013-10201-3-git-send-email-gokul...@codeaurora.org/
Regards,
Gokul
Signed-off-by: Vignesh Viswanathan
Signed-off-by: Manikanta Mylavarapu
Signed-off-by: Gokul Sriram Palanisamy
---
changes since v1: Addressed comments by Krzysztof
- moved of_node_put( ) before return value check to avoid
leaking refcount.
- simplified if/else for error handling.
- implemented 'devm_clk_get_enabled' instead of using
'devm_clk_get' and 'clk_prepare_enable' conscutively.
- implemented 'dev_err_probe' for error handling.
drivers/remoteproc/Kconfig | 22 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5_wcss_sec.c | 354
3 files changed, 377 insertions(+)
create mode 100644 drivers/remoteproc/qcom_q6v5_wcss_sec.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 0f0862e20a93..3e7c6fc62ca1 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -255,6 +255,28 @@ config QCOM_Q6V5_WCSS
Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
a non-TrustZone wireless subsystem.
+config QCOM_Q6V5_WCSS_SEC
+ tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
+ depends on OF && ARCH_QCOM
+ depends on QCOM_SMEM
+ depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n
+ depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+ depends on QCOM_SYSMON || QCOM_SYSMON=n
+ depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
+ depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
+ select QCOM_MDT_LOADER
+ select QCOM_PIL_INFO
+ select QCOM_Q6V5_COMMON
+ select QCOM_RPROC_COMMON
+ select QCOM_SCM
+ help
+ Say y here to support the Qualcomm Secure Peripheral Image Loader
+ for the Hexagon based remote processors on e.g. IPQ5332.
+
+ This is TrustZone wireless subsystem. The firmware is
+ verified and booted with the help of the Peripheral Authentication
+ System (PAS) in TrustZone.
+
config QCOM_SYSMON
tristate "Qualcomm sysmon driver"
depends on RPMSG
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 5ff4e2fee4ab..d4971b672812 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
+obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
qcom_wcnss_pil-y += qcom_wcnss.o
diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c
b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
new file mode 100644
index ..3c8bb2639567
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2018 Linaro Ltd.
+ * Copyright (C) 2014 Sony Mobile Communications AB
+ * Copyright (c) 2012-2018, 2024 The Linux Foundation. All rights reserved.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "qcom_common.h"
+#include "qcom_q6v5.h"
+
+#include "remoteproc_internal.h"
+
+#define WCSS_CRASH_REASON 421
+
+#define WCSS_PAS_ID0x6
+#define MPD_WCSS_PAS_ID0xD
+
+struct wcss_sec {
+ struct device *dev;
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_ssr ssr
Re: [PATCH V2 2/4] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On 10/11/2024 12:28 PM, Jeff Johnson wrote: > So I suspect this last line should be replaced with two: >> + * Copyright (c) 2012-2018 The Linux Foundation. All rights reserved. >> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved. I agree. -- ---Trilok Soni
Re: [PATCH V2 2/4] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On 8/29/2024 6:40 AM, Gokul Sriram Palanisamy wrote: ... > +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c > @@ -0,0 +1,354 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2016-2018 Linaro Ltd. > + * Copyright (C) 2014 Sony Mobile Communications AB > + * Copyright (c) 2012-2018, 2024 The Linux Foundation. All rights reserved. I'm currently vetting a proposed ath driver change that is dependent upon two of yours, and my vetting tools ran on your patchsets as well. This is one of several issues that my vetting tools flagged. This last copyright doesn't look correct. As of December 2021 Qualcomm stopped assigning copyright to the Linux Foundation via the Code Aurora Forum. Any Qualcomm contributions after December 2021 should be Copyright Qualcomm Innovation Center, Inc. So I suspect this last line should be replaced with two: > + * Copyright (c) 2012-2018 The Linux Foundation. All rights reserved. > + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
Re: [PATCH V2 2/4] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On 8/29/2024 7:10 PM, Gokul Sriram Palanisamy wrote:
From: Vignesh Viswanathan
Add support to bring up hexagon based WCSS secure PIL remoteproc.
IPQ5332, IPQ9574 supports secure PIL remoteproc.
Signed-off-by: Vignesh Viswanathan
Signed-off-by: Manikanta Mylavarapu
Signed-off-by: Gokul Sriram Palanisamy
---
changes since v1: Addressed comments by Krzysztof
- moved of_node_put( ) before return value check to avoid
leaking refcount.
- simplified if/else for error handling.
- implemented 'devm_clk_get_enabled' instead of using
'devm_clk_get' and 'clk_prepare_enable' conscutively.
- implemented 'dev_err_probe' for error handling.
drivers/remoteproc/Kconfig | 22 ++
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5_wcss_sec.c | 354
3 files changed, 377 insertions(+)
create mode 100644 drivers/remoteproc/qcom_q6v5_wcss_sec.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 0f0862e20a93..3e7c6fc62ca1 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -255,6 +255,28 @@ config QCOM_Q6V5_WCSS
Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
a non-TrustZone wireless subsystem.
+config QCOM_Q6V5_WCSS_SEC
+ tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
+ depends on OF && ARCH_QCOM
+ depends on QCOM_SMEM
+ depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n
+ depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+ depends on QCOM_SYSMON || QCOM_SYSMON=n
+ depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
+ depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
+ select QCOM_MDT_LOADER
+ select QCOM_PIL_INFO
+ select QCOM_Q6V5_COMMON
+ select QCOM_RPROC_COMMON
+ select QCOM_SCM
+ help
+ Say y here to support the Qualcomm Secure Peripheral Image Loader
+ for the Hexagon based remote processors on e.g. IPQ5332.
+
+ This is TrustZone wireless subsystem. The firmware is
+ verified and booted with the help of the Peripheral Authentication
+ System (PAS) in TrustZone.
+
config QCOM_SYSMON
tristate "Qualcomm sysmon driver"
depends on RPMSG
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 5ff4e2fee4ab..d4971b672812 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP) += qcom_q6v5_adsp.o
obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
+obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
qcom_wcnss_pil-y += qcom_wcnss.o
diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c
b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
new file mode 100644
index ..3c8bb2639567
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2018 Linaro Ltd.
+ * Copyright (C) 2014 Sony Mobile Communications AB
+ * Copyright (c) 2012-2018, 2024 The Linux Foundation. All rights reserved.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "qcom_common.h"
+#include "qcom_q6v5.h"
+
+#include "remoteproc_internal.h"
+
+#define WCSS_CRASH_REASON 421
+
+#define WCSS_PAS_ID0x6
+#define MPD_WCSS_PAS_ID0xD
+
+struct wcss_sec {
+ struct device *dev;
+ struct qcom_rproc_glink glink_subdev;
+ struct qcom_rproc_ssr ssr_subdev;
+ struct qcom_q6v5 q6;
+ phys_addr_t mem_phys;
+ phys_addr_t mem_reloc;
+ void *mem_region;
+ size_t mem_size;
+ const struct wcss_data *desc;
+ const char *fw_name;
+
+ struct clk *sleep_clk;
+};
+
+struct wcss_data {
+ u32 pasid;
+ const struct rproc_ops *ops;
+ bool need_auto_boot;
+ u8 bootargs_version;
+ int (*init_clk)(struct wcss_sec *wcss);
+};
+
+static int wcss_sec_start(struct rproc *rproc)
+{
+ struct wcss_sec *wcss = rproc->priv;
+ struct device *dev = wcss->dev;
+ const struct wcss_data *desc = of_device_get_match_data(dev);
+ int ret;
+
+ qcom_q6v5_prepare(&wcss->q6);
+
+ ret = qcom_scm_pas_auth_and_reset(desc->pasid);
+ if (ret) {
+ dev_err(dev, "wcss_reset failed\n");
+ return ret;
+ }
+
+ ret = qcom_q6v5_wait_for_start(&wcss->q6, 5 * HZ);
+ if (ret == -ETIMEDOUT)
+ dev_err(dev, "start timed out\
Re: [PATCH V2 2/4] remoteproc: qcom: add hexagon based WCSS secure PIL driver
On Thu, Aug 29, 2024 at 07:10:19PM GMT, Gokul Sriram Palanisamy wrote:
> From: Vignesh Viswanathan
>
> Add support to bring up hexagon based WCSS secure PIL remoteproc.
> IPQ5332, IPQ9574 supports secure PIL remoteproc.
Could you please clarify, why this can't be handled by the qcom_q6v5_pas
driver.
>
> Signed-off-by: Vignesh Viswanathan
> Signed-off-by: Manikanta Mylavarapu
> Signed-off-by: Gokul Sriram Palanisamy
> ---
> changes since v1: Addressed comments by Krzysztof
> - moved of_node_put( ) before return value check to avoid
> leaking refcount.
> - simplified if/else for error handling.
> - implemented 'devm_clk_get_enabled' instead of using
> 'devm_clk_get' and 'clk_prepare_enable' conscutively.
> - implemented 'dev_err_probe' for error handling.
>
> drivers/remoteproc/Kconfig | 22 ++
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/qcom_q6v5_wcss_sec.c | 354
> 3 files changed, 377 insertions(+)
> create mode 100644 drivers/remoteproc/qcom_q6v5_wcss_sec.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 0f0862e20a93..3e7c6fc62ca1 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -255,6 +255,28 @@ config QCOM_Q6V5_WCSS
> Hexagon V5 based WCSS remote processors on e.g. IPQ8074. This is
> a non-TrustZone wireless subsystem.
>
> +config QCOM_Q6V5_WCSS_SEC
> + tristate "Qualcomm Hexagon based WCSS Secure Peripheral Image Loader"
> + depends on OF && ARCH_QCOM
> + depends on QCOM_SMEM
> + depends on RPMSG_QCOM_SMD || RPMSG_QCOM_SMD=n
> + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> + depends on QCOM_SYSMON || QCOM_SYSMON=n
> + depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
> + depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
> + select QCOM_MDT_LOADER
> + select QCOM_PIL_INFO
> + select QCOM_Q6V5_COMMON
> + select QCOM_RPROC_COMMON
> + select QCOM_SCM
> + help
> + Say y here to support the Qualcomm Secure Peripheral Image Loader
> + for the Hexagon based remote processors on e.g. IPQ5332.
> +
> + This is TrustZone wireless subsystem. The firmware is
> + verified and booted with the help of the Peripheral Authentication
> + System (PAS) in TrustZone.
> +
> config QCOM_SYSMON
> tristate "Qualcomm sysmon driver"
> depends on RPMSG
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 5ff4e2fee4ab..d4971b672812 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_QCOM_Q6V5_ADSP)+=
> qcom_q6v5_adsp.o
> obj-$(CONFIG_QCOM_Q6V5_MSS) += qcom_q6v5_mss.o
> obj-$(CONFIG_QCOM_Q6V5_PAS) += qcom_q6v5_pas.o
> obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
> +obj-$(CONFIG_QCOM_Q6V5_WCSS_SEC) += qcom_q6v5_wcss_sec.o
> obj-$(CONFIG_QCOM_SYSMON)+= qcom_sysmon.o
> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
> qcom_wcnss_pil-y += qcom_wcnss.o
> diff --git a/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> new file mode 100644
> index ..3c8bb2639567
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
> @@ -0,0 +1,354 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016-2018 Linaro Ltd.
> + * Copyright (C) 2014 Sony Mobile Communications AB
> + * Copyright (c) 2012-2018, 2024 The Linux Foundation. All rights reserved.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "qcom_common.h"
> +#include "qcom_q6v5.h"
> +
> +#include "remoteproc_internal.h"
> +
> +#define WCSS_CRASH_REASON421
> +
> +#define WCSS_PAS_ID 0x6
> +#define MPD_WCSS_PAS_ID 0xD
> +
> +struct wcss_sec {
> + struct device *dev;
> + struct qcom_rproc_glink glink_subdev;
> + struct qcom_rproc_ssr ssr_subdev;
> + struct qcom_q6v5 q6;
> + phys_addr_t mem_phys;
> + phys_addr_t mem_reloc;
> + void *mem_region;
> + size_t mem_size;
> + const struct wcss_data *desc;
> + const char *fw_name;
> +
> + struct clk *sleep_clk;
> +};
> +
> +struct wcss_data {
> + u32 pasid;
> + const struct rproc_ops *ops;
> + bool need_auto_boot;
This isn't set anywere, drop it
> + u8 bootargs_version;
Not used, drop it
> + int (*init_clk)(struct wcss_sec *wcss);
> +};
> +
> +static int wcss_sec_start(struct rproc *rproc)
> +{
> + struct wcss_sec *wcss = rproc->priv;
> + struct device *dev = wcss->dev;
> + const struct wcss_data *desc = of_device_get_match_data(dev);
> + int ret;
> +
> + qcom_q6v5_prepare(&wcss->q6);
