Re: [PATCH] soc: loongson: add Loongson Security Module driver

From: Christophe JAILLET
Date: Sat Nov 16 2024 - 04:31:25 EST


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@xxxxxxxxxxx>
---
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@xxxxxxxxxxx>
+L: loongarch@xxxxxxxxxxxxxxx
+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@xxxxxxxxxxx>
L: linux-pm@xxxxxxxxxxxxxxx
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