On Thu, Aug 29, 2024 at 07:10:19PM GMT, Gokul Sriram Palanisamy wrote:
> From: Vignesh Viswanathan <quic_viswanat@xxxxxxxxxxx>
>
> 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 <quic_viswanat@xxxxxxxxxxx>
> Signed-off-by: Manikanta Mylavarapu <quic_mmanikan@xxxxxxxxxxx>
> Signed-off-by: Gokul Sriram Palanisamy <quic_gokulsri@xxxxxxxxxxx>
> ---
> 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 000000000000..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 <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/soc/qcom/mdt_loader.h>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/smem_state.h>
> +#include "qcom_common.h"
> +#include "qcom_q6v5.h"
> +
> +#include "remoteproc_internal.h"
> +
> +#define WCSS_CRASH_REASON 421
> +
> +#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);
> +
> + 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\n");
> +
> + return ret;
> +}
> +
> +static int wcss_sec_stop(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;
> +
> + ret = qcom_scm_pas_shutdown(desc->pasid);
> + if (ret) {
> + dev_err(dev, "not able to shutdown\n");
> + return ret;
> + }
> + qcom_q6v5_unprepare(&wcss->q6);
> +
> + return 0;
> +}
> +
> +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;
> +
> + 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;
> + const struct wcss_data *desc = of_device_get_match_data(dev);
> +
> + return qcom_mdt_load(dev, fw, rproc->firmware, desc->pasid,
> + wcss->mem_region, wcss->mem_phys, wcss->mem_size,
> + &wcss->mem_reloc);
No qcom_pil_info_store() ?
> +}
> +
> +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;
> + void *ptr;
> +
> + ptr = devm_ioremap_wc(dev, segment->da, segment->size);
Please don't use devm_ioremap if you are going to unmap it several lines
below. There is no need to burden devres with it.
> + if (!ptr) {
> + dev_err(dev, "Failed to ioremap segment %pad size %zx\n",
> + &segment->da, segment->size);
> + return;
> + }
> +
> + if (size <= segment->size - offset)
> + memcpy(dest, ptr + offset, size);
> + else
> + dev_err(dev, "Copy size greater than segment size. Skipping\n");
> + devm_iounmap(dev, ptr);
> +}
> +
> +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 = 0;
> + 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);
> + while (index < num_segs) {
> + 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;
> +
> + index++;
> + }
> +
> + 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 reserved_mem *rmem = NULL;
> + struct device_node *node;
> + struct device *dev = wcss->dev;
> +
> + node = of_parse_phandle(dev->of_node, "memory-region", 0);
> + if (!node) {
> + dev_err(dev, "can't find phandle memory-region\n");
> + return -EINVAL;
> + }
> +
> + rmem = of_reserved_mem_lookup(node);
> + of_node_put(node);
> +
> + if (!rmem) {
> + dev_err(dev, "unable to acquire memory-region\n");
> + return -EINVAL;
> + }
> +
> + wcss->mem_phys = rmem->base;
> + wcss->mem_reloc = rmem->base;
> + wcss->mem_size = rmem->size;
> + wcss->mem_region = devm_ioremap_wc(dev, wcss->mem_phys, wcss->mem_size);
> + if (!wcss->mem_region) {
> + dev_err(dev, "unable to map memory region: %pa+%pa\n",
> + &rmem->base, &rmem->size);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int wcss_sec_probe(struct platform_device *pdev)
> +{
> + struct wcss_sec *wcss;
> + struct rproc *rproc;
> + const char *fw_name = NULL;
> + const struct wcss_data *desc = of_device_get_match_data(&pdev->dev);
> + int ret;
> +
> + if (!desc)
> + return -EINVAL;
> +
> + ret = of_property_read_string(pdev->dev.of_node, "firmware-name",
> + &fw_name);
> + if (ret < 0)
> + return ret;
> +
> + rproc = rproc_alloc(&pdev->dev, pdev->name, desc->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;
> + wcss->fw_name = fw_name;
> +
> + ret = wcss_sec_alloc_memory_region(wcss);
> + if (ret)
> + goto free_rproc;
> +
> + if (desc->init_clk) {
Just use devm_clk_get_optional_enabled() instead of an optional
callback.
> + ret = desc->init_clk(wcss);
> + if (ret)
> + goto free_rproc;
> + }
> +
> + ret = qcom_q6v5_init(&wcss->q6, pdev, rproc,
> + WCSS_CRASH_REASON, NULL, NULL);
> + if (ret)
> + goto free_rproc;
> +
> + qcom_add_glink_subdev(rproc, &wcss->glink_subdev, "q6wcss");
> + qcom_add_ssr_subdev(rproc, &wcss->ssr_subdev, pdev->name);
> +
> + rproc->auto_boot = desc->need_auto_boot;
> + rproc->dump_conf = RPROC_COREDUMP_INLINE;
> + rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> +
> + ret = rproc_add(rproc);
> + if (ret)
> + goto free_rproc;
> +
> + platform_set_drvdata(pdev, rproc);
> +
> + return 0;
> +
> +free_rproc:
> + rproc_free(rproc);
> +
> + return ret;
> +}
> +
> +static void wcss_sec_remove(struct platform_device *pdev)
> +{
> + struct rproc *rproc = platform_get_drvdata(pdev);
> + struct wcss_sec *wcss = rproc->priv;
> +
> + qcom_q6v5_deinit(&wcss->q6);
> +
> + rproc_del(rproc);
> + rproc_free(rproc);
> +}
> +
> +static int wcss_sec_ipq5332_init_clk(struct wcss_sec *wcss)
> +{
> + int ret;
> + struct device *dev = wcss->dev;
> +
> + wcss->sleep_clk = devm_clk_get_enabled(dev, "sleep");
> + if (IS_ERR(wcss->sleep_clk)) {
> + return dev_err_probe(dev, PTR_ERR(wcss->sleep_clk),
> + "Could not get sleep clock\n");
> + }
> +
> + return 0;
> +}
> +
> +static const struct wcss_data wcss_sec_ipq5332_res_init = {
> + .pasid = MPD_WCSS_PAS_ID,
> + .ops = &wcss_sec_ops,
> + .bootargs_version = 2,
> + .init_clk = wcss_sec_ipq5332_init_clk,
> +};
> +
> +static const struct wcss_data wcss_sec_ipq9574_res_init = {
> + .pasid = WCSS_PAS_ID,
> + .ops = &wcss_sec_ops,
> +};
> +
> +static const struct of_device_id wcss_sec_of_match[] = {
> + { .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");
> --
> 2.34.1
>
--
With best wishes
Dmitry
