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