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..).