[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8efcd579-15d5-4ea2-bbb9-f9b4969f031f@wanadoo.fr>
Date: Sat, 16 Nov 2024 10:31:07 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Qunqin Zhao <zhaoqunqin@...ngson.cn>, arnd@...db.de, olof@...om.net
Cc: soc@...ts.linux.dev, linux-kernel@...r.kernel.org,
loongarch@...ts.linux.dev
Subject: Re: [PATCH] soc: loongson: add Loongson Security Module driver
Le 16/11/2024 à 09:57, Qunqin Zhao a écrit :
> This driver supports Loongson Security Module, which
> provides the control for it's hardware encryption
> acceleration child devices.
>
> Only ACPI firmware is supported.
>
> Signed-off-by: Qunqin Zhao <zhaoqunqin@...ngson.cn>
> ---
> MAINTAINERS | 7 +
> drivers/soc/loongson/Kconfig | 9 +
> drivers/soc/loongson/Makefile | 1 +
> drivers/soc/loongson/loongson_se.c | 542 +++++++++++++++++++++++++++++
> include/soc/loongson/se.h | 135 +++++++
> 5 files changed, 694 insertions(+)
> create mode 100644 drivers/soc/loongson/loongson_se.c
> create mode 100644 include/soc/loongson/se.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fdeb3d12c..85fff2eb7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13379,6 +13379,13 @@ S: Maintained
> F: Documentation/devicetree/bindings/hwinfo/loongson,ls2k-chipid.yaml
> F: drivers/soc/loongson/loongson2_guts.c
>
> +LOONGSON SECURITY MODULE DRIVER
> +M: Qunqin Zhao <zhaoqunqin@...ngson.cn>
> +L: loongarch@...ts.linux.dev
> +S: Maintained
> +F: drivers/soc/loongson/loongson_se.c
> +F: include/soc/loongson/se.h
> +
> LOONGSON-2 SOC SERIES PM DRIVER
> M: Yinbo Zhu <zhuyinbo@...ngson.cn>
> L: linux-pm@...r.kernel.org
> diff --git a/drivers/soc/loongson/Kconfig b/drivers/soc/loongson/Kconfig
> index 368344943..93ef1d205 100644
> --- a/drivers/soc/loongson/Kconfig
> +++ b/drivers/soc/loongson/Kconfig
> @@ -27,3 +27,12 @@ config LOONGSON2_PM
> Disk), ACPI S5 (Soft Shutdown) and supports multiple wake-up methods
> (USB, GMAC, PWRBTN, etc.). This driver was to add power management
> controller support that base on dts for Loongson-2 series SoCs.
> +
> +config LOONGSON_SE
> + tristate "LOONGSON SECURITY MODULE Interface"
> + depends on LOONGARCH && ACPI
> + help
> + The Loongson security module provides the control for hardware
> + encryption acceleration devices. Each device uses at least one
> + channel to interacts with security module, and each channel may
s/interacts/interact/
> + has its own buffer provided by security module.
s/has/have/
> diff --git a/drivers/soc/loongson/Makefile b/drivers/soc/loongson/Makefile
> index 4118f50f5..503075042 100644
> --- a/drivers/soc/loongson/Makefile
> +++ b/drivers/soc/loongson/Makefile
> @@ -5,3 +5,4 @@
>
> obj-$(CONFIG_LOONGSON2_GUTS) += loongson2_guts.o
> obj-$(CONFIG_LOONGSON2_PM) += loongson2_pm.o
> +obj-$(CONFIG_LOONGSON_SE) += loongson_se.o
> diff --git a/drivers/soc/loongson/loongson_se.c b/drivers/soc/loongson/loongson_se.c
> new file mode 100644
> index 000000000..d85db8423
> --- /dev/null
> +++ b/drivers/soc/loongson/loongson_se.c
...
> +static int loongson_se_get_res(struct loongson_se *se, u32 int_bit, u32 cmd,
> + struct se_data *res)
> +{
> + int err = 0;
> +
> + res->int_bit = int_bit;
> +
> + if (se_get_response(se, res)) {
> + dev_err(se->dev, "Int 0x%x get response fail.\n", int_bit);
> + return -EFAULT;
> + }
> +
> + /* Check response */
> + if (res->u.res.cmd == cmd)
> + err = 0;
Not needed, err is already 0.
> + else {
> + dev_err(se->dev, "Response cmd is 0x%x, not expect cmd 0x%x.\n",
> + res->u.res.cmd, cmd);
> + err = -EFAULT;
> + }
> +
> + return err;
> +}
...
> +static int loongson_se_set_msg(struct lsse_ch *ch)
> +{
> + struct loongson_se *se = ch->se;
> + struct se_data req = {0};
> + struct se_data res = {0};
> + int err;
> +
> + req.int_bit = SE_INT_SETUP;
> + req.u.gcmd.cmd = SE_CMD_SETMSG;
> + /* MSG off */
> + req.u.gcmd.info[0] = ch->id;
> + req.u.gcmd.info[1] = ch->smsg - se->mem_base;
> + req.u.gcmd.info[2] = ch->msg_size;
> +
> + dev_dbg(se->dev, "Set Channel %d msg off 0x%x, msg size %d\n",
> + ch->id, req.u.gcmd.info[1], req.u.gcmd.info[2]);
> +
> + err = se_send_genl_cmd(se, &req, &res, 5);
> + if (res.u.res.cmd_ret)
In the fnction above, we test (err || ...)
Is it needed here as well (or can it be removed in se_send_genl_cmd())
Also, below se_send_genl_cmd() errors are checked only with err.
Should it be consistent?
> + return res.u.res.cmd_ret;
> +
> + return err;
> +}
...
> +void se_deinit_ch(struct lsse_ch *ch)
> +{
> + struct loongson_se *se = ch->se;
> + unsigned long flag;
> + int first, nr;
> + int id = ch->id;
> +
> + if (!se) {
> + pr_err("SE has bot been initialized\n");
> + return;
> + }
> +
> + if (id == 0 || id > SE_CH_MAX) {
> + dev_err(se->dev, "Channel number %d is invalid\n", id);
> + return;
> + }
> +
> + if (!se_ch_status(se, BIT(id))) {
> + dev_err(se->dev, "Channel number %d has not been initialized\n", id);
> + return;
> + }
> +
> + spin_lock_irqsave(&se->dev_lock, flag);
> +
> + se->ch_status &= ~BIT(ch->id);
> +
> + first = (ch->data_buffer - se->mem_base) / PAGE_SIZE;
> + nr = round_up(ch->data_size, PAGE_SIZE) / PAGE_SIZE;
> + bitmap_clear(se->mem_map, first, nr);
> +
> + first = (ch->smsg - se->mem_base) / PAGE_SIZE;
> + nr = round_up(ch->msg_size, PAGE_SIZE) / PAGE_SIZE;
> + bitmap_clear(se->mem_map, first, nr);
> +
> + se_disable_int(se, ch->int_bit);
> +
> + spin_unlock_irqrestore(&se->dev_lock, flag);
> +
Uneeded empty line
> +}
> +EXPORT_SYMBOL_GPL(se_deinit_ch);
> +
> +static int loongson_se_probe(struct platform_device *pdev)
> +{
> + struct loongson_se *se;
> + struct device *dev = &pdev->dev;
> + int nr_irq, irq, err, size;
> +
> + se = devm_kmalloc(dev, sizeof(*se), GFP_KERNEL);
> + if (!se)
> + return -ENOMEM;
> + se->dev = dev;
> + dev_set_drvdata(dev, se);
> + init_completion(&se->cmd_completion);
> + spin_lock_init(&se->cmd_lock);
> + spin_lock_init(&se->dev_lock);
> + /* Setup DMA buffer */
> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> + if (device_property_read_u32(dev, "dmam_size", &size))
> + return -ENODEV;
> + size = roundup_pow_of_two(size);
> + se->mem_base = dmam_alloc_coherent(dev, size, &se->mem_addr, GFP_KERNEL);
> + if (!se->mem_base)
> + return -ENOMEM;
> + memset(se->mem_base, 0, size);
I don't think that it is needed. dmam_alloc_coherent() should return
zeroed memory.
> + se->mem_map_pages = size / PAGE_SIZE;
> + se->mem_map = devm_bitmap_zalloc(dev, se->mem_map_pages, GFP_KERNEL);
> + if (!se->mem_map)
> + return -ENOMEM;
> +
> + se->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(se->base))
> + return PTR_ERR(se->base);
> +
> + nr_irq = platform_irq_count(pdev);
> + if (nr_irq <= 0)
> + return -ENODEV;
> + while (nr_irq) {
> + irq = platform_get_irq(pdev, --nr_irq);
> + if (irq < 0)
> + return -ENODEV;
> + /* Use the same interrupt handler address.
> + * Determine which irq it is accroding
> + * SE_S2LINT_STAT register.
> + */
> + err = devm_request_irq(dev, irq, se_irq, 0,
> + "loongson-se", se);
> + if (err)
> + dev_err(dev, "failed to request irq: %d\n", err);
> + }
> +
> + err = se_init_hw(se, se->mem_addr, size);
> + if (err)
> + se_disable_hw(se);
> +
> + return err;
> +}
> +
> +static struct acpi_device_id loongson_se_acpi_match[] = {
const?
> + {"LOON0011", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, loongson_se_acpi_match);
...
CJ
Powered by blists - more mailing lists