Re: [PATCH v14 3/9] remoteproc: qcom: add hexagon based WCSS secure PIL driver

On 8/3/2026 6:14 PM, Mukesh Ojha wrote:
> On Mon, Aug 03, 2026 at 04:16:09PM +0530, Varadarajan Narayanan wrote:
>> From: Vignesh Viswanathan <[email protected]>
>>
>> Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
>> SoCs support secure Peripheral Image Loading (PIL).
>>
>> Secure PIL image is signed firmware image which only trusted software such
>> as TrustZone (TZ) can authenticate and load. Linux kernel will send a
>> Peripheral Authentication Service (PAS) request to TZ to authenticate and
>> load the PIL images.
>>
>> In order to avoid overloading the existing WCSS driver or PAS driver, we
>> came up with this new PAS based IPQ WCSS driver.
> 
> Can you tell why you need another driver if you are using the same PAS
> method and it is using more of everything what PAS already
> supports ?
> 

Hi Mukesh,

We just use pas_auth_and_reset and pas_shutdown APIs and do not need
the entire framework provided by the PAS driver for IPQ WCSS.

Also, feedback from Bjorn in [1] was to have separate driver
instead of overloading existing WCSS driver.

[1] https://lore.kernel.org/all/[email protected]/

>>
>> Signed-off-by: Vignesh Viswanathan <[email protected]>
>> Signed-off-by: Manikanta Mylavarapu <[email protected]>
>> Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
>> Signed-off-by: George Moussalem <[email protected]>
>> [ Dropped ipq5424 support ]
>> Reviewed-by: Dmitry Baryshkov <[email protected]>
>> Tested-by: Vignesh Viswanathan <[email protected]>
> 
> 
> Author and tested looks weird..here..

Will drop Tested-by here.

Thanks,
Vignesh
> 
> 
>> Signed-off-by: Varadarajan Narayanan <[email protected]>
>> ---
>>  drivers/remoteproc/Kconfig              |  20 ++
>>  drivers/remoteproc/Makefile             |   1 +
>>  drivers/remoteproc/qcom_q6v5_wcss_sec.c | 337 ++++++++++++++++++++++++++++++++
>>  include/linux/remoteproc.h              |   2 +
>>  4 files changed, 360 insertions(+)
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 5b56b2dcc725..3feea5bda129 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -267,6 +267,26 @@ 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_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=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 689686de0d41..f74519ea6502 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -29,6 +29,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 000000000000..4837825d1717
>> --- /dev/null
>> +++ b/drivers/remoteproc/qcom_q6v5_wcss_sec.c
>> @@ -0,0 +1,337 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +#include <linux/clk.h>
>> +#include <linux/firmware/qcom/qcom_pas.h>
>> +#include <linux/io.h>
>> +#include <linux/mailbox_client.h>
>> +#include <linux/of_reserved_mem.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/soc/qcom/mdt_loader.h>
>> +
>> +#include "qcom_common.h"
>> +#include "qcom_q6v5.h"
>> +#include "qcom_pil_info.h"
>> +
>> +#define WCSS_CRASH_REASON		421
>> +
>> +#define WCSS_PAS_ID			0x6
>> +#define MPD_WCSS_PAS_ID			0xd
>> +
>> +#define Q6_WAIT_TIMEOUT			(5 * HZ)
>> +
>> +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;
>> +};
>> +
>> +struct wcss_data {
>> +	u32 pasid;
>> +	const char *ss_name;
>> +	bool auto_boot;
>> +};
>> +
>> +static int wcss_sec_start(struct rproc *rproc)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +	struct device *dev = wcss->dev;
>> +	int ret;
>> +
>> +	ret = qcom_q6v5_prepare(&wcss->q6);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = qcom_pas_auth_and_reset(wcss->desc->pasid);
>> +	if (ret) {
>> +		dev_err(dev, "wcss_reset failed\n");
>> +		goto unprepare;
>> +	}
>> +
>> +	ret = qcom_q6v5_wait_for_start(&wcss->q6, msecs_to_jiffies(Q6_WAIT_TIMEOUT));
>> +
>> +	if (ret == -ETIMEDOUT) {
>> +		dev_err(dev, "start timed out\n");
>> +		qcom_pas_shutdown(wcss->desc->pasid);
>> +		goto unprepare;
>> +	}
>> +
>> +	return 0;
>> +
>> +unprepare:
>> +	qcom_q6v5_unprepare(&wcss->q6);
>> +
>> +	return ret;
>> +}
>> +
>> +static int wcss_sec_stop(struct rproc *rproc)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +	struct device *dev = wcss->dev;
>> +	int ret;
>> +
>> +	ret = qcom_q6v5_request_stop(&wcss->q6, NULL);
>> +	if (ret == -ETIMEDOUT)
>> +		dev_err(dev, "timed out on wait\n");
>> +
>> +	ret = qcom_pas_shutdown(wcss->desc->pasid);
>> +	if (ret)
>> +		dev_err(dev, "Failed to shutdown %d\n", ret);
>> +
>> +	qcom_q6v5_unprepare(&wcss->q6);
>> +
>> +	return ret;
>> +}
>> +
>> +static void *wcss_sec_da_to_va(struct rproc *rproc, u64 da, size_t len,
>> +			       bool *is_iomem)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +	int offset;
>> +
>> +	offset = da - wcss->mem_reloc;
>> +	if (offset < 0 || offset + len > wcss->mem_size)
>> +		return NULL;
>> +
>> +	if (is_iomem)
>> +		*is_iomem = true;
>> +
>> +	return wcss->mem_region + offset;
>> +}
>> +
>> +static int wcss_sec_load(struct rproc *rproc, const struct firmware *fw)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +	struct device *dev = wcss->dev;
>> +	int ret;
>> +
>> +	ret = qcom_mdt_load(dev, fw, rproc->firmware, wcss->desc->pasid, wcss->mem_region,
>> +			    wcss->mem_phys, wcss->mem_size, &wcss->mem_reloc);
>> +	if (ret)
>> +		return ret;
>> +
>> +	qcom_pil_info_store("wcss", wcss->mem_phys, wcss->mem_size);
>> +
>> +	return 0;
>> +}
>> +
>> +static unsigned long wcss_sec_panic(struct rproc *rproc)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +
>> +	return qcom_q6v5_panic(&wcss->q6);
>> +}
>> +
>> +static void wcss_sec_copy_segment(struct rproc *rproc,
>> +				  struct rproc_dump_segment *segment,
>> +				  void *dest, size_t offset, size_t size)
>> +{
>> +	struct wcss_sec *wcss = rproc->priv;
>> +	struct device *dev = wcss->dev;
>> +
>> +	if (!segment->io_ptr)
>> +		segment->io_ptr = ioremap_wc(segment->da, segment->size);
>> +
>> +	if (!segment->io_ptr) {
>> +		dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
>> +			&segment->da, segment->size);
>> +		return;
>> +	}
>> +
>> +	if (offset + size <= segment->size) {
>> +		memcpy_fromio(dest, segment->io_ptr + offset, size);
>> +	} else {
>> +		iounmap(segment->io_ptr);
>> +		segment->io_ptr = NULL;
>> +	}
>> +}
>> +
>> +static int wcss_sec_dump_segments(struct rproc *rproc,
>> +				  const struct firmware *fw)
>> +{
>> +	struct device *dev = rproc->dev.parent;
>> +	struct reserved_mem *rmem = NULL;
>> +	struct device_node *node;
>> +	int num_segs, index;
>> +	int ret;
>> +
>> +	/*
>> +	 * Parse through additional reserved memory regions for the rproc
>> +	 * and add them to the coredump segments
>> +	 */
>> +	num_segs = of_count_phandle_with_args(dev->of_node,
>> +					      "memory-region", NULL);
>> +	for (index = 0; index < num_segs; index++) {
>> +		node = of_parse_phandle(dev->of_node,
>> +					"memory-region", index);
>> +		if (!node)
>> +			return -EINVAL;
>> +
>> +		rmem = of_reserved_mem_lookup(node);
>> +		of_node_put(node);
>> +		if (!rmem) {
>> +			dev_err(dev, "unable to acquire memory-region index %d num_segs %d\n",
>> +				index, num_segs);
>> +			return -EINVAL;
>> +		}
>> +
>> +		dev_dbg(dev, "Adding segment 0x%pa size 0x%pa",
>> +			&rmem->base, &rmem->size);
>> +		ret = rproc_coredump_add_custom_segment(rproc,
>> +							rmem->base,
>> +							rmem->size,
>> +							wcss_sec_copy_segment,
>> +							NULL);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct rproc_ops wcss_sec_ops = {
>> +	.start = wcss_sec_start,
>> +	.stop = wcss_sec_stop,
>> +	.da_to_va = wcss_sec_da_to_va,
>> +	.load = wcss_sec_load,
>> +	.get_boot_addr = rproc_elf_get_boot_addr,
>> +	.panic = wcss_sec_panic,
>> +	.parse_fw = wcss_sec_dump_segments,
>> +};
>> +
>> +static int wcss_sec_alloc_memory_region(struct wcss_sec *wcss)
>> +{
>> +	struct device *dev = wcss->dev;
>> +	struct resource res;
>> +	int ret;
>> +
>> +	ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
>> +	if (ret) {
>> +		dev_err(dev, "unable to acquire memory-region resource\n");
>> +		return ret;
>> +	}
>> +
>> +	wcss->mem_phys = res.start;
>> +	wcss->mem_reloc = res.start;
>> +	wcss->mem_size = resource_size(&res);
>> +	wcss->mem_region = devm_ioremap_resource_wc(dev, &res);
>> +	if (IS_ERR(wcss->mem_region)) {
>> +		dev_err(dev, "unable to map memory region: %pR\n", &res);
>> +		return PTR_ERR(wcss->mem_region);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int wcss_sec_probe(struct platform_device *pdev)
>> +{
>> +	const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
>> +	const char *fw_name = NULL;
>> +	struct wcss_sec *wcss;
>> +	struct clk *sleep_clk;
>> +	struct clk *int_clk;
>> +	struct rproc *rproc;
>> +	int ret;
>> +
>> +	ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
>> +				      &fw_name);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	rproc = devm_rproc_alloc(&pdev->dev, desc->ss_name, &wcss_sec_ops,
>> +				 fw_name, sizeof(*wcss));
>> +	if (!rproc) {
>> +		dev_err(&pdev->dev, "failed to allocate rproc\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	wcss = rproc->priv;
>> +	wcss->dev = &pdev->dev;
>> +	wcss->desc = desc;
>> +
>> +	ret = wcss_sec_alloc_memory_region(wcss);
>> +	if (ret)
>> +		return ret;
>> +
>> +	sleep_clk = devm_clk_get_optional_enabled(&pdev->dev, "sleep");
>> +	if (IS_ERR(sleep_clk))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(sleep_clk),
>> +				     "Failed to get sleep clock\n");
>> +
>> +	int_clk = devm_clk_get_optional_enabled(&pdev->dev, "interconnect");
>> +	if (IS_ERR(int_clk))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(int_clk),
>> +				     "Failed to get interconnect clock\n");
>> +
>> +	ret = qcom_q6v5_init(&wcss->q6, pdev, rproc,
>> +			     WCSS_CRASH_REASON, NULL, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	qcom_add_glink_subdev(rproc, &wcss->glink_subdev, desc->ss_name);
>> +	qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, desc->ss_name);
>> +
>> +	rproc->auto_boot = desc->auto_boot;
>> +	rproc->dump_conf = RPROC_COREDUMP_INLINE;
>> +	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
>> +
>> +	ret = devm_rproc_add(&pdev->dev, rproc);
>> +	if (ret) {
>> +		qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
>> +		qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
>> +		qcom_q6v5_deinit(&wcss->q6);
>> +		return ret;
>> +	}
>> +
>> +	platform_set_drvdata(pdev, rproc);
>> +
>> +	return 0;
>> +}
>> +
>> +static void wcss_sec_remove(struct platform_device *pdev)
>> +{
>> +	struct rproc *rproc = platform_get_drvdata(pdev);
>> +	struct wcss_sec *wcss = rproc->priv;
>> +
>> +	qcom_remove_ssr_subdev(rproc, &wcss->ssr_subdev);
>> +	qcom_remove_glink_subdev(rproc, &wcss->glink_subdev);
>> +	qcom_q6v5_deinit(&wcss->q6);
>> +}
>> +
>> +static const struct wcss_data wcss_sec_ipq5332_res_init = {
>> +	.pasid = MPD_WCSS_PAS_ID,
>> +	.ss_name = "q6wcss",
>> +};
>> +
>> +static const struct wcss_data wcss_sec_ipq9574_res_init = {
>> +	.pasid = WCSS_PAS_ID,
>> +	.ss_name = "q6wcss",
>> +};
>> +
>> +static const struct of_device_id wcss_sec_of_match[] = {
>> +	{ .compatible = "qcom,ipq5018-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
>> +	{ .compatible = "qcom,ipq5332-wcss-sec-pil", .data = &wcss_sec_ipq5332_res_init },
>> +	{ .compatible = "qcom,ipq9574-wcss-sec-pil", .data = &wcss_sec_ipq9574_res_init },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, wcss_sec_of_match);
>> +
>> +static struct platform_driver wcss_sec_driver = {
>> +	.probe = wcss_sec_probe,
>> +	.remove = wcss_sec_remove,
>> +	.driver = {
>> +		.name = "qcom-wcss-secure-pil",
>> +		.of_match_table = wcss_sec_of_match,
>> +	},
>> +};
>> +module_platform_driver(wcss_sec_driver);
>> +
>> +MODULE_DESCRIPTION("Hexagon WCSS Secure Peripheral Image Loader");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
>> index ef711a5b1a7f..cf06b9846309 100644
>> --- a/include/linux/remoteproc.h
>> +++ b/include/linux/remoteproc.h
>> @@ -204,6 +204,7 @@ enum rproc_dump_mechanism {
>>   * @node:	list node related to the rproc segment list
>>   * @da:		device address of the segment
>>   * @size:	size of the segment
>> + * @io_ptr:	ptr to store the ioremapped dump segment
>>   * @priv:	private data associated with the dump_segment
>>   * @dump:	custom dump function to fill device memory segment associated
>>   *		with coredump
>> @@ -215,6 +216,7 @@ struct rproc_dump_segment {
>>  	dma_addr_t da;
>>  	size_t size;
>>  
>> +	void __iomem *io_ptr;
>>  	void *priv;
>>  	void (*dump)(struct rproc *rproc, struct rproc_dump_segment *segment,
>>  		     void *dest, size_t offset, size_t size);
>>
>> -- 
>> 2.34.1
>>
>