This patch series adds support for TEE based BNXT firmware
management module and the driver changes to invoke OP-TEE
APIs to fastboot firmware and to collect crash dump.
changes from v2:
- address review comments from Jakub
Vasundhara Volam (2):
bnxt_en: Add support to invoke OP-TEE API to reset firmware
bnxt_en: Add support to collect crash dump via ethtool
Vikas Gupta (1):
firmware: broadcom: add OP-TEE based BNXT f/w manager
drivers/firmware/broadcom/Kconfig | 8 +
drivers/firmware/broadcom/Makefile | 1 +
drivers/firmware/broadcom/tee_bnxt_fw.c | 277 ++++++++++++++++++++++
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 6 +
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 37 ++-
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h | 2 +
include/linux/firmware/broadcom/tee_bnxt_fw.h | 14 ++
8 files changed, 354 insertions(+), 4 deletions(-)
create mode 100644 drivers/firmware/broadcom/tee_bnxt_fw.c
create mode 100644 include/linux/firmware/broadcom/tee_bnxt_fw.h
--
1.9.1
From: Vikas Gupta <[email protected]>
This driver registers on TEE bus to interact with OP-TEE based
BNXT firmware management modules
Cc: Jakub Kicinski <[email protected]>
Signed-off-by: Vikas Gupta <[email protected]>
Signed-off-by: Sheetal Tigadoli <[email protected]>
---
drivers/firmware/broadcom/Kconfig | 8 +
drivers/firmware/broadcom/Makefile | 1 +
drivers/firmware/broadcom/tee_bnxt_fw.c | 277 ++++++++++++++++++++++++++
include/linux/firmware/broadcom/tee_bnxt_fw.h | 14 ++
4 files changed, 300 insertions(+)
create mode 100644 drivers/firmware/broadcom/tee_bnxt_fw.c
create mode 100644 include/linux/firmware/broadcom/tee_bnxt_fw.h
diff --git a/drivers/firmware/broadcom/Kconfig b/drivers/firmware/broadcom/Kconfig
index d03ed8e..79505ad 100644
--- a/drivers/firmware/broadcom/Kconfig
+++ b/drivers/firmware/broadcom/Kconfig
@@ -22,3 +22,11 @@ config BCM47XX_SPROM
In case of SoC devices SPROM content is stored on a flash used by
bootloader firmware CFE. This driver provides method to ssb and bcma
drivers to read SPROM on SoC.
+
+config TEE_BNXT_FW
+ bool "Broadcom BNXT firmware manager"
+ depends on (ARCH_BCM_IPROC && OPTEE) || COMPILE_TEST
+ default ARCH_BCM_IPROC
+ help
+ This module help to manage firmware on Broadcom BNXT device. The module
+ registers on tee bus and invoke calls to manage firmware on BNXT device.
diff --git a/drivers/firmware/broadcom/Makefile b/drivers/firmware/broadcom/Makefile
index 72c7fdc..17c5061 100644
--- a/drivers/firmware/broadcom/Makefile
+++ b/drivers/firmware/broadcom/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_BCM47XX_NVRAM) += bcm47xx_nvram.o
obj-$(CONFIG_BCM47XX_SPROM) += bcm47xx_sprom.o
+obj-$(CONFIG_TEE_BNXT_FW) += tee_bnxt_fw.o
diff --git a/drivers/firmware/broadcom/tee_bnxt_fw.c b/drivers/firmware/broadcom/tee_bnxt_fw.c
new file mode 100644
index 0000000..72dcbfa
--- /dev/null
+++ b/drivers/firmware/broadcom/tee_bnxt_fw.c
@@ -0,0 +1,277 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 Broadcom.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+
+#include <linux/firmware/broadcom/tee_bnxt_fw.h>
+
+#define MAX_SHM_MEM_SZ SZ_4M
+
+#define MAX_TEE_PARAM_ARRY_MEMB 4
+
+enum ta_cmd {
+ /*
+ * TA_CMD_BNXT_FASTBOOT - boot bnxt device by copying f/w into sram
+ *
+ * param[0] unused
+ * param[1] unused
+ * param[2] unused
+ * param[3] unused
+ *
+ * Result:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_ITEM_NOT_FOUND - Corrupt f/w image found on memory
+ */
+ TA_CMD_BNXT_FASTBOOT = 0,
+
+ /*
+ * TA_CMD_BNXT_COPY_COREDUMP - copy the core dump into shm
+ *
+ * param[0] (inout memref) - Coredump buffer memory reference
+ * param[1] (in value) - value.a: offset, data to be copied from
+ * value.b: size of data to be copied
+ * param[2] unused
+ * param[3] unused
+ *
+ * Result:
+ * TEE_SUCCESS - Invoke command success
+ * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
+ * TEE_ERROR_ITEM_NOT_FOUND - Corrupt core dump
+ */
+ TA_CMD_BNXT_COPY_COREDUMP = 3,
+};
+
+/**
+ * struct tee_bnxt_fw_private - OP-TEE bnxt private data
+ * @dev: OP-TEE based bnxt device.
+ * @ctx: OP-TEE context handler.
+ * @session_id: TA session identifier.
+ */
+struct tee_bnxt_fw_private {
+ struct device *dev;
+ struct tee_context *ctx;
+ u32 session_id;
+ struct tee_shm *fw_shm_pool;
+};
+
+static struct tee_bnxt_fw_private pvt_data;
+
+static void prepare_args(int cmd,
+ struct tee_ioctl_invoke_arg *arg,
+ struct tee_param *param)
+{
+ memset(arg, 0, sizeof(*arg));
+ memset(param, 0, MAX_TEE_PARAM_ARRY_MEMB * sizeof(*param));
+
+ arg->func = cmd;
+ arg->session = pvt_data.session_id;
+ arg->num_params = MAX_TEE_PARAM_ARRY_MEMB;
+
+ /* Fill invoke cmd params */
+ switch (cmd) {
+ case TA_CMD_BNXT_COPY_COREDUMP:
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ param[0].u.memref.shm = pvt_data.fw_shm_pool;
+ param[0].u.memref.size = MAX_SHM_MEM_SZ;
+ param[0].u.memref.shm_offs = 0;
+ param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
+ break;
+ case TA_CMD_BNXT_FASTBOOT:
+ default:
+ /* Nothing to do */
+ break;
+ }
+}
+
+/**
+ * tee_bnxt_fw_load() - Load the bnxt firmware
+ * Uses an OP-TEE call to start a secure
+ * boot process.
+ * Returns 0 on success, negative errno otherwise.
+ */
+int tee_bnxt_fw_load(void)
+{
+ int ret = 0;
+ struct tee_ioctl_invoke_arg arg;
+ struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
+
+ if (!pvt_data.ctx)
+ return -ENODEV;
+
+ prepare_args(TA_CMD_BNXT_FASTBOOT, &arg, param);
+
+ ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(pvt_data.dev,
+ "TA_CMD_BNXT_FASTBOOT invoke failed TEE err: %x, ret:%x\n",
+ arg.ret, ret);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(tee_bnxt_fw_load);
+
+/**
+ * tee_bnxt_copy_coredump() - Copy coredump from the allocated memory
+ * Uses an OP-TEE call to copy coredump
+ * @buf: desintation buffer where core dump is copied into
+ * @offset: offset from the base address of core dump area
+ * @size: size of the dump
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size)
+{
+ struct tee_ioctl_invoke_arg arg;
+ struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
+ void *core_data;
+ u32 rbytes = size;
+ u32 nbytes = 0;
+ int ret = 0;
+
+ if (!pvt_data.ctx)
+ return -ENODEV;
+
+ prepare_args(TA_CMD_BNXT_COPY_COREDUMP, &arg, param);
+
+ while (rbytes) {
+ nbytes = rbytes;
+
+ nbytes = min_t(u32, rbytes, param[0].u.memref.size);
+
+ /* Fill additional invoke cmd params */
+ param[1].u.value.a = offset;
+ param[1].u.value.b = nbytes;
+
+ ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(pvt_data.dev,
+ "TA_CMD_BNXT_COPY_COREDUMP invoke failed TEE err: %x, ret:%x\n",
+ arg.ret, ret);
+ return -EINVAL;
+ }
+
+ core_data = tee_shm_get_va(pvt_data.fw_shm_pool, 0);
+ if (IS_ERR(core_data)) {
+ dev_err(pvt_data.dev, "tee_shm_get_va failed\n");
+ return PTR_ERR(core_data);
+ }
+
+ memcpy(buf, core_data, nbytes);
+
+ rbytes -= nbytes;
+ buf += nbytes;
+ offset += nbytes;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(tee_bnxt_copy_coredump);
+
+static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ return (ver->impl_id == TEE_IMPL_ID_OPTEE);
+}
+
+static int tee_bnxt_fw_probe(struct device *dev)
+{
+ struct tee_client_device *bnxt_device = to_tee_client_device(dev);
+ int ret, err = -ENODEV;
+ struct tee_ioctl_open_session_arg sess_arg;
+ struct tee_shm *fw_shm_pool;
+
+ memset(&sess_arg, 0, sizeof(sess_arg));
+
+ /* Open context with TEE driver */
+ pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL,
+ NULL);
+ if (IS_ERR(pvt_data.ctx))
+ return -ENODEV;
+
+ /* Open session with Bnxt load Trusted App */
+ memcpy(sess_arg.uuid, bnxt_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ sess_arg.num_params = 0;
+
+ ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL);
+ if (ret < 0 || sess_arg.ret != 0) {
+ dev_err(dev, "tee_client_open_session failed, err: %x\n",
+ sess_arg.ret);
+ err = -EINVAL;
+ goto out_ctx;
+ }
+ pvt_data.session_id = sess_arg.session;
+
+ pvt_data.dev = dev;
+
+ fw_shm_pool = tee_shm_alloc(pvt_data.ctx, MAX_SHM_MEM_SZ,
+ TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
+ if (IS_ERR(fw_shm_pool)) {
+ tee_client_close_context(pvt_data.ctx);
+ dev_err(pvt_data.dev, "tee_shm_alloc failed\n");
+ err = PTR_ERR(fw_shm_pool);
+ goto out_sess;
+ }
+
+ pvt_data.fw_shm_pool = fw_shm_pool;
+
+ return 0;
+
+out_sess:
+ tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+out_ctx:
+ tee_client_close_context(pvt_data.ctx);
+
+ return err;
+}
+
+static int tee_bnxt_fw_remove(struct device *dev)
+{
+ tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+ tee_client_close_context(pvt_data.ctx);
+ pvt_data.ctx = NULL;
+
+ return 0;
+}
+
+static const struct tee_client_device_id tee_bnxt_fw_id_table[] = {
+ {UUID_INIT(0x6272636D, 0x2019, 0x0716,
+ 0x42, 0x43, 0x4D, 0x5F, 0x53, 0x43, 0x48, 0x49)},
+ {}
+};
+
+MODULE_DEVICE_TABLE(tee, tee_bnxt_fw_id_table);
+
+static struct tee_client_driver tee_bnxt_fw_driver = {
+ .id_table = tee_bnxt_fw_id_table,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .bus = &tee_bus_type,
+ .probe = tee_bnxt_fw_probe,
+ .remove = tee_bnxt_fw_remove,
+ },
+};
+
+static int __init tee_bnxt_fw_mod_init(void)
+{
+ return driver_register(&tee_bnxt_fw_driver.driver);
+}
+
+static void __exit tee_bnxt_fw_mod_exit(void)
+{
+ driver_unregister(&tee_bnxt_fw_driver.driver);
+}
+
+module_init(tee_bnxt_fw_mod_init);
+module_exit(tee_bnxt_fw_mod_exit);
+
+MODULE_AUTHOR("Vikas Gupta <[email protected]>");
+MODULE_DESCRIPTION("Broadcom bnxt firmware manager");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/firmware/broadcom/tee_bnxt_fw.h b/include/linux/firmware/broadcom/tee_bnxt_fw.h
new file mode 100644
index 0000000..f24c82d
--- /dev/null
+++ b/include/linux/firmware/broadcom/tee_bnxt_fw.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright 2019 Broadcom.
+ */
+
+#ifndef _BROADCOM_TEE_BNXT_FW_H
+#define _BROADCOM_TEE_BNXT_FW_H
+
+#include <linux/types.h>
+
+int tee_bnxt_fw_load(void);
+int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size);
+
+#endif /* _BROADCOM_TEE_BNXT_FW_H */
--
1.9.1
From: Vasundhara Volam <[email protected]>
In error recovery process when firmware indicates that it is
completely down, initiate a firmware reset by calling OP-TEE API.
Cc: Michael Chan <[email protected]>
Signed-off-by: Vasundhara Volam <[email protected]>
Signed-off-by: Sheetal Tigadoli <[email protected]>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 +++++++++++--
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b4a8cf6..b60b24e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -10581,14 +10581,23 @@ static void bnxt_fw_reset_writel(struct bnxt *bp, int reg_idx)
static void bnxt_reset_all(struct bnxt *bp)
{
struct bnxt_fw_health *fw_health = bp->fw_health;
- int i;
+ int i, rc;
+
+ if (bp->fw_cap & BNXT_FW_CAP_ERR_RECOVER_RELOAD) {
+#ifdef CONFIG_TEE_BNXT_FW
+ rc = tee_bnxt_fw_load();
+ if (rc)
+ netdev_err(bp->dev, "Unable to reset FW rc=%d\n", rc);
+ bp->fw_reset_timestamp = jiffies;
+#endif
+ return;
+ }
if (fw_health->flags & ERROR_RECOVERY_QCFG_RESP_FLAGS_HOST) {
for (i = 0; i < fw_health->fw_reset_seq_cnt; i++)
bnxt_fw_reset_writel(bp, i);
} else if (fw_health->flags & ERROR_RECOVERY_QCFG_RESP_FLAGS_CO_CPU) {
struct hwrm_fw_reset_input req = {0};
- int rc;
bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FW_RESET, -1, -1);
req.resp_addr = cpu_to_le64(bp->hwrm_cmd_kong_resp_dma_addr);
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index d333589..0943715 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -25,6 +25,9 @@
#include <net/dst_metadata.h>
#include <net/xdp.h>
#include <linux/dim.h>
+#ifdef CONFIG_TEE_BNXT_FW
+#include <linux/firmware/broadcom/tee_bnxt_fw.h>
+#endif
struct page_pool;
--
1.9.1
From: Vasundhara Volam <[email protected]>
Driver supports 2 types of core dumps.
1. Live dump - Firmware dump when system is up and running.
2. Crash dump - Dump which is collected during firmware crash
that can be retrieved after recovery.
Crash dump is currently supported only on specific 58800 chips
which can be retrieved using OP-TEE API only, as firmware cannot
access this region directly.
User needs to set the dump flag using following command before
initiating the dump collection:
$ ethtool -W|--set-dump eth0 N
Where N is "0" for live dump and "1" for crash dump
Command to collect the dump after setting the flag:
$ ethtool -w eth0 data Filename
v3: Modify set_dump to support even when CONFIG_TEE_BNXT_FW=n.
Also change log message to netdev_info().
Cc: Jakub Kicinski <[email protected]>
Cc: Michael Chan <[email protected]>
Signed-off-by: Vasundhara Volam <[email protected]>
Signed-off-by: Sheetal Tigadoli <[email protected]>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 37 +++++++++++++++++++++--
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h | 2 ++
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 0943715..3e7d1fb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1807,6 +1807,9 @@ struct bnxt {
u8 num_leds;
struct bnxt_led_info leds[BNXT_MAX_LED];
+ u16 dump_flag;
+#define BNXT_DUMP_LIVE 0
+#define BNXT_DUMP_CRASH 1
struct bpf_prog *xdp_prog;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 51c1404..f2220b8 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -3311,6 +3311,24 @@ static int bnxt_get_coredump(struct bnxt *bp, void *buf, u32 *dump_len)
return rc;
}
+static int bnxt_set_dump(struct net_device *dev, struct ethtool_dump *dump)
+{
+ struct bnxt *bp = netdev_priv(dev);
+
+ if (dump->flag > BNXT_DUMP_CRASH) {
+ netdev_info(dev, "Supports only Live(0) and Crash(1) dumps.\n");
+ return -EINVAL;
+ }
+
+ if (!IS_ENABLED(CONFIG_TEE_BNXT_FW) && dump->flag == BNXT_DUMP_CRASH) {
+ netdev_info(dev, "Cannot collect crash dump as TEE_BNXT_FW config option is not enabled.\n");
+ return -EOPNOTSUPP;
+ }
+
+ bp->dump_flag = dump->flag;
+ return 0;
+}
+
static int bnxt_get_dump_flag(struct net_device *dev, struct ethtool_dump *dump)
{
struct bnxt *bp = netdev_priv(dev);
@@ -3323,7 +3341,12 @@ static int bnxt_get_dump_flag(struct net_device *dev, struct ethtool_dump *dump)
bp->ver_resp.hwrm_fw_bld_8b << 8 |
bp->ver_resp.hwrm_fw_rsvd_8b;
- return bnxt_get_coredump(bp, NULL, &dump->len);
+ dump->flag = bp->dump_flag;
+ if (bp->dump_flag == BNXT_DUMP_CRASH)
+ dump->len = BNXT_CRASH_DUMP_LEN;
+ else
+ bnxt_get_coredump(bp, NULL, &dump->len);
+ return 0;
}
static int bnxt_get_dump_data(struct net_device *dev, struct ethtool_dump *dump,
@@ -3336,7 +3359,16 @@ static int bnxt_get_dump_data(struct net_device *dev, struct ethtool_dump *dump,
memset(buf, 0, dump->len);
- return bnxt_get_coredump(bp, buf, &dump->len);
+ dump->flag = bp->dump_flag;
+ if (dump->flag == BNXT_DUMP_CRASH) {
+#ifdef CONFIG_TEE_BNXT_FW
+ return tee_bnxt_copy_coredump(buf, 0, dump->len);
+#endif
+ } else {
+ return bnxt_get_coredump(bp, buf, &dump->len);
+ }
+
+ return 0;
}
void bnxt_ethtool_init(struct bnxt *bp)
@@ -3446,6 +3478,7 @@ void bnxt_ethtool_free(struct bnxt *bp)
.set_phys_id = bnxt_set_phys_id,
.self_test = bnxt_self_test,
.reset = bnxt_reset,
+ .set_dump = bnxt_set_dump,
.get_dump_flag = bnxt_get_dump_flag,
.get_dump_data = bnxt_get_dump_data,
};
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
index b5b65b3..01de7e7 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
@@ -59,6 +59,8 @@ struct hwrm_dbg_cmn_output {
#define HWRM_DBG_CMN_FLAGS_MORE 1
};
+#define BNXT_CRASH_DUMP_LEN (8 << 20)
+
#define BNXT_LED_DFLT_ENA \
(PORT_LED_CFG_REQ_ENABLES_LED0_ID | \
PORT_LED_CFG_REQ_ENABLES_LED0_STATE | \
--
1.9.1
On 10/23/19 10:32 PM, Sheetal Tigadoli wrote:
> This patch series adds support for TEE based BNXT firmware
> management module and the driver changes to invoke OP-TEE
> APIs to fastboot firmware and to collect crash dump.
Sorry for chiming on this so late, the more I look into this and the
more it seems like you have built a custom TEE firmware loading solution
rather than thinking about extending the firmware API to load a firmware
opaque handle from somewhere other than the filesystem.
The TEE integration appears okay to me in that you leverage the TEE bus
to advertise your driver. What seems to violating layers is that you
have bnxt directly tap into your TEE driver's services and that looks
not ideal to say the least. That approach does not scale well over
multiple drivers (bnxt or otherwise), but also does not really scale
over trusted components providers. TEE is one of them, but conceptually
the same thing could exist with ACPI/UEFI or any platform that has
services that offer some sort of secure/non-secured world differentiation.
The way I would imagine you to integrate this is to basically register a
TEE firmware provider through the firmware API, continue using the
firmware API from within bnxt, possibly with using a specific file
handle/flag that designates whether you want to favor loading from
disk/file system or TEE. It should not matter to bnxt how the firmware
is obtained basically.
>
> changes from v2:
> - address review comments from Jakub
>
> Vasundhara Volam (2):
> bnxt_en: Add support to invoke OP-TEE API to reset firmware
> bnxt_en: Add support to collect crash dump via ethtool
>
> Vikas Gupta (1):
> firmware: broadcom: add OP-TEE based BNXT f/w manager
>
> drivers/firmware/broadcom/Kconfig | 8 +
> drivers/firmware/broadcom/Makefile | 1 +
> drivers/firmware/broadcom/tee_bnxt_fw.c | 277 ++++++++++++++++++++++
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 6 +
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 37 ++-
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h | 2 +
> include/linux/firmware/broadcom/tee_bnxt_fw.h | 14 ++
> 8 files changed, 354 insertions(+), 4 deletions(-)
> create mode 100644 drivers/firmware/broadcom/tee_bnxt_fw.c
> create mode 100644 include/linux/firmware/broadcom/tee_bnxt_fw.h
>
--
Florian
On Thu, 24 Oct 2019 11:02:39 +0530, Sheetal Tigadoli wrote:
> From: Vikas Gupta <[email protected]>
>
> This driver registers on TEE bus to interact with OP-TEE based
> BNXT firmware management modules
>
> Cc: Jakub Kicinski <[email protected]>
> Signed-off-by: Vikas Gupta <[email protected]>
> Signed-off-by: Sheetal Tigadoli <[email protected]>
I don't know anything about TEE, but code looks reasonable.
Acked-by: Jakub Kicinski <[email protected]>
On Thu, 24 Oct 2019 11:02:41 +0530, Sheetal Tigadoli wrote:
> From: Vasundhara Volam <[email protected]>
>
> Driver supports 2 types of core dumps.
>
> 1. Live dump - Firmware dump when system is up and running.
> 2. Crash dump - Dump which is collected during firmware crash
> that can be retrieved after recovery.
> Crash dump is currently supported only on specific 58800 chips
> which can be retrieved using OP-TEE API only, as firmware cannot
> access this region directly.
>
> User needs to set the dump flag using following command before
> initiating the dump collection:
>
> $ ethtool -W|--set-dump eth0 N
>
> Where N is "0" for live dump and "1" for crash dump
>
> Command to collect the dump after setting the flag:
>
> $ ethtool -w eth0 data Filename
>
> v3: Modify set_dump to support even when CONFIG_TEE_BNXT_FW=n.
> Also change log message to netdev_info().
>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Michael Chan <[email protected]>
> Signed-off-by: Vasundhara Volam <[email protected]>
> Signed-off-by: Sheetal Tigadoli <[email protected]>
Acked-by: Jakub Kicinski <[email protected]>
On 10/24/19 11:39 AM, Florian Fainelli wrote:
> On 10/23/19 10:32 PM, Sheetal Tigadoli wrote:
>> This patch series adds support for TEE based BNXT firmware
>> management module and the driver changes to invoke OP-TEE
>> APIs to fastboot firmware and to collect crash dump.
>
> Sorry for chiming on this so late, the more I look into this and the
> more it seems like you have built a custom TEE firmware loading solution
> rather than thinking about extending the firmware API to load a firmware
> opaque handle from somewhere other than the filesystem.
>
> The TEE integration appears okay to me in that you leverage the TEE bus
> to advertise your driver. What seems to violating layers is that you
> have bnxt directly tap into your TEE driver's services and that looks
> not ideal to say the least. That approach does not scale well over
> multiple drivers (bnxt or otherwise), but also does not really scale
> over trusted components providers. TEE is one of them, but conceptually
> the same thing could exist with ACPI/UEFI or any platform that has
> services that offer some sort of secure/non-secured world differentiation.
>
> The way I would imagine you to integrate this is to basically register a
> TEE firmware provider through the firmware API, continue using the
> firmware API from within bnxt, possibly with using a specific file
> handle/flag that designates whether you want to favor loading from
> disk/file system or TEE. It should not matter to bnxt how the firmware
> is obtained basically.
And I should have probably ended this paragraph with saying that the
suggestion does not need to happen right now but would be nice to be
done as a cleanup exercise (of course, by saying that, I am also opening
the door for this to not happen at all..).
--
Florian
Hi Sheetal,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[cannot apply to v5.4-rc4 next-20191025]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sheetal-Tigadoli/Add-OP-TEE-based-bnxt-f-w-manager/20191027-112745
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 503a64635d5ef7351657c78ad77f8b5ff658d5fc
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
drivers/firmware/broadcom/tee_bnxt_fw.c: In function 'prepare_args':
>> drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: error: 'SZ_4M' undeclared (first use in this function)
#define MAX_SHM_MEM_SZ SZ_4M
^
>> drivers/firmware/broadcom/tee_bnxt_fw.c:81:28: note: in expansion of macro 'MAX_SHM_MEM_SZ'
param[0].u.memref.size = MAX_SHM_MEM_SZ;
^~~~~~~~~~~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: note: each undeclared identifier is reported only once for each function it appears in
#define MAX_SHM_MEM_SZ SZ_4M
^
>> drivers/firmware/broadcom/tee_bnxt_fw.c:81:28: note: in expansion of macro 'MAX_SHM_MEM_SZ'
param[0].u.memref.size = MAX_SHM_MEM_SZ;
^~~~~~~~~~~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c: In function 'tee_bnxt_fw_probe':
>> drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: error: 'SZ_4M' undeclared (first use in this function)
#define MAX_SHM_MEM_SZ SZ_4M
^
drivers/firmware/broadcom/tee_bnxt_fw.c:214:44: note: in expansion of macro 'MAX_SHM_MEM_SZ'
fw_shm_pool = tee_shm_alloc(pvt_data.ctx, MAX_SHM_MEM_SZ,
^~~~~~~~~~~~~~
vim +/SZ_4M +14 drivers/firmware/broadcom/tee_bnxt_fw.c
13
> 14 #define MAX_SHM_MEM_SZ SZ_4M
15
16 #define MAX_TEE_PARAM_ARRY_MEMB 4
17
18 enum ta_cmd {
19 /*
20 * TA_CMD_BNXT_FASTBOOT - boot bnxt device by copying f/w into sram
21 *
22 * param[0] unused
23 * param[1] unused
24 * param[2] unused
25 * param[3] unused
26 *
27 * Result:
28 * TEE_SUCCESS - Invoke command success
29 * TEE_ERROR_ITEM_NOT_FOUND - Corrupt f/w image found on memory
30 */
31 TA_CMD_BNXT_FASTBOOT = 0,
32
33 /*
34 * TA_CMD_BNXT_COPY_COREDUMP - copy the core dump into shm
35 *
36 * param[0] (inout memref) - Coredump buffer memory reference
37 * param[1] (in value) - value.a: offset, data to be copied from
38 * value.b: size of data to be copied
39 * param[2] unused
40 * param[3] unused
41 *
42 * Result:
43 * TEE_SUCCESS - Invoke command success
44 * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
45 * TEE_ERROR_ITEM_NOT_FOUND - Corrupt core dump
46 */
47 TA_CMD_BNXT_COPY_COREDUMP = 3,
48 };
49
50 /**
51 * struct tee_bnxt_fw_private - OP-TEE bnxt private data
52 * @dev: OP-TEE based bnxt device.
53 * @ctx: OP-TEE context handler.
54 * @session_id: TA session identifier.
55 */
56 struct tee_bnxt_fw_private {
57 struct device *dev;
58 struct tee_context *ctx;
59 u32 session_id;
60 struct tee_shm *fw_shm_pool;
61 };
62
63 static struct tee_bnxt_fw_private pvt_data;
64
65 static void prepare_args(int cmd,
66 struct tee_ioctl_invoke_arg *arg,
67 struct tee_param *param)
68 {
69 memset(arg, 0, sizeof(*arg));
70 memset(param, 0, MAX_TEE_PARAM_ARRY_MEMB * sizeof(*param));
71
72 arg->func = cmd;
73 arg->session = pvt_data.session_id;
74 arg->num_params = MAX_TEE_PARAM_ARRY_MEMB;
75
76 /* Fill invoke cmd params */
77 switch (cmd) {
78 case TA_CMD_BNXT_COPY_COREDUMP:
79 param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
80 param[0].u.memref.shm = pvt_data.fw_shm_pool;
> 81 param[0].u.memref.size = MAX_SHM_MEM_SZ;
82 param[0].u.memref.shm_offs = 0;
83 param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
84 break;
85 case TA_CMD_BNXT_FASTBOOT:
86 default:
87 /* Nothing to do */
88 break;
89 }
90 }
91
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
From: Sheetal Tigadoli <[email protected]>
Date: Thu, 24 Oct 2019 11:02:38 +0530
> This patch series adds support for TEE based BNXT firmware
> management module and the driver changes to invoke OP-TEE
> APIs to fastboot firmware and to collect crash dump.
>
> changes from v2:
> - address review comments from Jakub
Series applied to net-next.
Please properly annotate your Subject lines in the future to indicate
the exact GIT tree your patches are targetting, ala "[PATCH net-next ...]"
Thank you.
From: David Miller <[email protected]>
Date: Mon, 28 Oct 2019 11:49:15 -0700 (PDT)
> From: Sheetal Tigadoli <[email protected]>
> Date: Thu, 24 Oct 2019 11:02:38 +0530
>
>> This patch series adds support for TEE based BNXT firmware
>> management module and the driver changes to invoke OP-TEE
>> APIs to fastboot firmware and to collect crash dump.
>>
>> changes from v2:
>> - address review comments from Jakub
>
> Series applied to net-next.
>
> Please properly annotate your Subject lines in the future to indicate
> the exact GIT tree your patches are targetting, ala "[PATCH net-next ...]"
Actually, reverted, this doesn't even compile:
drivers/firmware/broadcom/tee_bnxt_fw.c: In function ?prepare_args?:
drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: error: ?SZ_4M? undeclared (first use in this function)
#define MAX_SHM_MEM_SZ SZ_4M
^~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c:81:28: note: in expansion of macro ?MAX_SHM_MEM_SZ?
param[0].u.memref.size = MAX_SHM_MEM_SZ;
^~~~~~~~~~~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: note: each undeclared identifier is reported only once for each function it appears in
#define MAX_SHM_MEM_SZ SZ_4M
^~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c:81:28: note: in expansion of macro ?MAX_SHM_MEM_SZ?
param[0].u.memref.size = MAX_SHM_MEM_SZ;
^~~~~~~~~~~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c: In function ?tee_bnxt_fw_probe?:
drivers/firmware/broadcom/tee_bnxt_fw.c:14:24: error: ?SZ_4M? undeclared (first use in this function)
#define MAX_SHM_MEM_SZ SZ_4M
^~~~~
drivers/firmware/broadcom/tee_bnxt_fw.c:214:44: note: in expansion of macro ?MAX_SHM_MEM_SZ?
fw_shm_pool = tee_shm_alloc(pvt_data.ctx, MAX_SHM_MEM_SZ,
^~~~~~~~~~~~~~
On Thu, 24 Oct 2019 at 11:09, Sheetal Tigadoli
<[email protected]> wrote:
>
> From: Vikas Gupta <[email protected]>
>
> This driver registers on TEE bus to interact with OP-TEE based
> BNXT firmware management modules
>
> Cc: Jakub Kicinski <[email protected]>
> Signed-off-by: Vikas Gupta <[email protected]>
> Signed-off-by: Sheetal Tigadoli <[email protected]>
> ---
> drivers/firmware/broadcom/Kconfig | 8 +
> drivers/firmware/broadcom/Makefile | 1 +
> drivers/firmware/broadcom/tee_bnxt_fw.c | 277 ++++++++++++++++++++++++++
> include/linux/firmware/broadcom/tee_bnxt_fw.h | 14 ++
> 4 files changed, 300 insertions(+)
> create mode 100644 drivers/firmware/broadcom/tee_bnxt_fw.c
> create mode 100644 include/linux/firmware/broadcom/tee_bnxt_fw.h
>
> diff --git a/drivers/firmware/broadcom/Kconfig b/drivers/firmware/broadcom/Kconfig
> index d03ed8e..79505ad 100644
> --- a/drivers/firmware/broadcom/Kconfig
> +++ b/drivers/firmware/broadcom/Kconfig
> @@ -22,3 +22,11 @@ config BCM47XX_SPROM
> In case of SoC devices SPROM content is stored on a flash used by
> bootloader firmware CFE. This driver provides method to ssb and bcma
> drivers to read SPROM on SoC.
> +
> +config TEE_BNXT_FW
> + bool "Broadcom BNXT firmware manager"
> + depends on (ARCH_BCM_IPROC && OPTEE) || COMPILE_TEST
> + default ARCH_BCM_IPROC
> + help
> + This module help to manage firmware on Broadcom BNXT device. The module
> + registers on tee bus and invoke calls to manage firmware on BNXT device.
> diff --git a/drivers/firmware/broadcom/Makefile b/drivers/firmware/broadcom/Makefile
> index 72c7fdc..17c5061 100644
> --- a/drivers/firmware/broadcom/Makefile
> +++ b/drivers/firmware/broadcom/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-$(CONFIG_BCM47XX_NVRAM) += bcm47xx_nvram.o
> obj-$(CONFIG_BCM47XX_SPROM) += bcm47xx_sprom.o
> +obj-$(CONFIG_TEE_BNXT_FW) += tee_bnxt_fw.o
> diff --git a/drivers/firmware/broadcom/tee_bnxt_fw.c b/drivers/firmware/broadcom/tee_bnxt_fw.c
> new file mode 100644
> index 0000000..72dcbfa
> --- /dev/null
> +++ b/drivers/firmware/broadcom/tee_bnxt_fw.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 Broadcom.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/tee_drv.h>
> +#include <linux/uuid.h>
> +
> +#include <linux/firmware/broadcom/tee_bnxt_fw.h>
> +
> +#define MAX_SHM_MEM_SZ SZ_4M
> +
> +#define MAX_TEE_PARAM_ARRY_MEMB 4
> +
> +enum ta_cmd {
> + /*
> + * TA_CMD_BNXT_FASTBOOT - boot bnxt device by copying f/w into sram
> + *
> + * param[0] unused
> + * param[1] unused
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_ITEM_NOT_FOUND - Corrupt f/w image found on memory
> + */
> + TA_CMD_BNXT_FASTBOOT = 0,
> +
> + /*
> + * TA_CMD_BNXT_COPY_COREDUMP - copy the core dump into shm
> + *
> + * param[0] (inout memref) - Coredump buffer memory reference
> + * param[1] (in value) - value.a: offset, data to be copied from
> + * value.b: size of data to be copied
> + * param[2] unused
> + * param[3] unused
> + *
> + * Result:
> + * TEE_SUCCESS - Invoke command success
> + * TEE_ERROR_BAD_PARAMETERS - Incorrect input param
> + * TEE_ERROR_ITEM_NOT_FOUND - Corrupt core dump
> + */
> + TA_CMD_BNXT_COPY_COREDUMP = 3,
> +};
> +
> +/**
> + * struct tee_bnxt_fw_private - OP-TEE bnxt private data
> + * @dev: OP-TEE based bnxt device.
> + * @ctx: OP-TEE context handler.
> + * @session_id: TA session identifier.
> + */
> +struct tee_bnxt_fw_private {
> + struct device *dev;
> + struct tee_context *ctx;
> + u32 session_id;
> + struct tee_shm *fw_shm_pool;
> +};
> +
> +static struct tee_bnxt_fw_private pvt_data;
> +
> +static void prepare_args(int cmd,
> + struct tee_ioctl_invoke_arg *arg,
> + struct tee_param *param)
> +{
> + memset(arg, 0, sizeof(*arg));
> + memset(param, 0, MAX_TEE_PARAM_ARRY_MEMB * sizeof(*param));
> +
> + arg->func = cmd;
> + arg->session = pvt_data.session_id;
> + arg->num_params = MAX_TEE_PARAM_ARRY_MEMB;
> +
> + /* Fill invoke cmd params */
> + switch (cmd) {
> + case TA_CMD_BNXT_COPY_COREDUMP:
> + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
> + param[0].u.memref.shm = pvt_data.fw_shm_pool;
> + param[0].u.memref.size = MAX_SHM_MEM_SZ;
> + param[0].u.memref.shm_offs = 0;
> + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
> + break;
> + case TA_CMD_BNXT_FASTBOOT:
> + default:
> + /* Nothing to do */
> + break;
> + }
> +}
> +
> +/**
> + * tee_bnxt_fw_load() - Load the bnxt firmware
> + * Uses an OP-TEE call to start a secure
> + * boot process.
> + * Returns 0 on success, negative errno otherwise.
> + */
> +int tee_bnxt_fw_load(void)
> +{
> + int ret = 0;
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
> +
> + if (!pvt_data.ctx)
> + return -ENODEV;
> +
> + prepare_args(TA_CMD_BNXT_FASTBOOT, &arg, param);
> +
> + ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(pvt_data.dev,
> + "TA_CMD_BNXT_FASTBOOT invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tee_bnxt_fw_load);
> +
> +/**
> + * tee_bnxt_copy_coredump() - Copy coredump from the allocated memory
> + * Uses an OP-TEE call to copy coredump
> + * @buf: desintation buffer where core dump is copied into
s/desintation/destination/
> + * @offset: offset from the base address of core dump area
> + * @size: size of the dump
> + *
> + * Returns 0 on success, negative errno otherwise.
> + */
> +int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size)
> +{
> + struct tee_ioctl_invoke_arg arg;
> + struct tee_param param[MAX_TEE_PARAM_ARRY_MEMB];
> + void *core_data;
> + u32 rbytes = size;
> + u32 nbytes = 0;
> + int ret = 0;
> +
> + if (!pvt_data.ctx)
> + return -ENODEV;
> +
> + prepare_args(TA_CMD_BNXT_COPY_COREDUMP, &arg, param);
> +
> + while (rbytes) {
> + nbytes = rbytes;
> +
> + nbytes = min_t(u32, rbytes, param[0].u.memref.size);
> +
> + /* Fill additional invoke cmd params */
> + param[1].u.value.a = offset;
> + param[1].u.value.b = nbytes;
> +
> + ret = tee_client_invoke_func(pvt_data.ctx, &arg, param);
> + if (ret < 0 || arg.ret != 0) {
> + dev_err(pvt_data.dev,
> + "TA_CMD_BNXT_COPY_COREDUMP invoke failed TEE err: %x, ret:%x\n",
> + arg.ret, ret);
> + return -EINVAL;
> + }
> +
> + core_data = tee_shm_get_va(pvt_data.fw_shm_pool, 0);
> + if (IS_ERR(core_data)) {
> + dev_err(pvt_data.dev, "tee_shm_get_va failed\n");
> + return PTR_ERR(core_data);
> + }
> +
> + memcpy(buf, core_data, nbytes);
> +
> + rbytes -= nbytes;
> + buf += nbytes;
> + offset += nbytes;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tee_bnxt_copy_coredump);
> +
> +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
> +{
> + return (ver->impl_id == TEE_IMPL_ID_OPTEE);
> +}
> +
> +static int tee_bnxt_fw_probe(struct device *dev)
> +{
> + struct tee_client_device *bnxt_device = to_tee_client_device(dev);
> + int ret, err = -ENODEV;
> + struct tee_ioctl_open_session_arg sess_arg;
> + struct tee_shm *fw_shm_pool;
> +
> + memset(&sess_arg, 0, sizeof(sess_arg));
> +
> + /* Open context with TEE driver */
> + pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL,
> + NULL);
> + if (IS_ERR(pvt_data.ctx))
> + return -ENODEV;
> +
> + /* Open session with Bnxt load Trusted App */
> + memcpy(sess_arg.uuid, bnxt_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
> + sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
> + sess_arg.num_params = 0;
> +
> + ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL);
> + if (ret < 0 || sess_arg.ret != 0) {
> + dev_err(dev, "tee_client_open_session failed, err: %x\n",
> + sess_arg.ret);
> + err = -EINVAL;
> + goto out_ctx;
> + }
> + pvt_data.session_id = sess_arg.session;
> +
> + pvt_data.dev = dev;
> +
> + fw_shm_pool = tee_shm_alloc(pvt_data.ctx, MAX_SHM_MEM_SZ,
> + TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> + if (IS_ERR(fw_shm_pool)) {
> + tee_client_close_context(pvt_data.ctx);
> + dev_err(pvt_data.dev, "tee_shm_alloc failed\n");
> + err = PTR_ERR(fw_shm_pool);
> + goto out_sess;
> + }
> +
> + pvt_data.fw_shm_pool = fw_shm_pool;
> +
> + return 0;
> +
> +out_sess:
> + tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
> +out_ctx:
> + tee_client_close_context(pvt_data.ctx);
> +
> + return err;
> +}
> +
> +static int tee_bnxt_fw_remove(struct device *dev)
> +{
> + tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
> + tee_client_close_context(pvt_data.ctx);
> + pvt_data.ctx = NULL;
> +
Possible shm leak? "tee_shm_free(pvt_data.fw_shm_pool);" missing here.
> + return 0;
> +}
> +
> +static const struct tee_client_device_id tee_bnxt_fw_id_table[] = {
> + {UUID_INIT(0x6272636D, 0x2019, 0x0716,
> + 0x42, 0x43, 0x4D, 0x5F, 0x53, 0x43, 0x48, 0x49)},
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(tee, tee_bnxt_fw_id_table);
> +
> +static struct tee_client_driver tee_bnxt_fw_driver = {
> + .id_table = tee_bnxt_fw_id_table,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .bus = &tee_bus_type,
> + .probe = tee_bnxt_fw_probe,
> + .remove = tee_bnxt_fw_remove,
> + },
> +};
> +
> +static int __init tee_bnxt_fw_mod_init(void)
> +{
> + return driver_register(&tee_bnxt_fw_driver.driver);
> +}
> +
> +static void __exit tee_bnxt_fw_mod_exit(void)
> +{
> + driver_unregister(&tee_bnxt_fw_driver.driver);
> +}
> +
> +module_init(tee_bnxt_fw_mod_init);
> +module_exit(tee_bnxt_fw_mod_exit);
> +
Apart from minor comments above, this TEE bus driver looks good, so:
Acked-by: Sumit Garg <[email protected]>
-Sumit
> +MODULE_AUTHOR("Vikas Gupta <[email protected]>");
> +MODULE_DESCRIPTION("Broadcom bnxt firmware manager");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/firmware/broadcom/tee_bnxt_fw.h b/include/linux/firmware/broadcom/tee_bnxt_fw.h
> new file mode 100644
> index 0000000..f24c82d
> --- /dev/null
> +++ b/include/linux/firmware/broadcom/tee_bnxt_fw.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright 2019 Broadcom.
> + */
> +
> +#ifndef _BROADCOM_TEE_BNXT_FW_H
> +#define _BROADCOM_TEE_BNXT_FW_H
> +
> +#include <linux/types.h>
> +
> +int tee_bnxt_fw_load(void);
> +int tee_bnxt_copy_coredump(void *buf, u32 offset, u32 size);
> +
> +#endif /* _BROADCOM_TEE_BNXT_FW_H */
> --
> 1.9.1
>
> _______________________________________________
> Tee-dev mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/tee-dev