Messages in this thread | | | Date | Thu, 28 Jul 2022 12:05:15 +0200 | Subject | Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure Application client | From | Maximilian Luz <> |
| |
On 7/28/22 10:23, Sudeep Holla wrote: > On Tue, Jul 26, 2022 at 07:01:28PM +0200, Maximilian Luz wrote: >> On 7/26/22 17:41, Sudeep Holla wrote: >>> On Tue, Jul 26, 2022 at 05:15:41PM +0200, Maximilian Luz wrote: >>>> >>>> So ultimately I think it's better to add a DT entry for it. >>> >>> I disagree for the reason that once you discover more apps running on the >>> secure side, you want to add more entries and update DT on the platform >>> every time you discover some new firmware entity and you wish to interact >>> with it from the non-secure side. >> >> Just as you'll have to add a driver to the kernel and update whatever is >> probing the TrEE interface and add those strings to that interface. If >> you then start doing SoC-specific lists, I think you'd be pretty much >> re-implementing a DT in the kernel driver... >> > > Yes at the cost of DT being dumping ground for all the SoC specific firmware > crap. Firmware can be and must be discoverable, no point in dumping it in > DT as it forces DT upgrade every time something changes in the firmware i.e. > it can go out of sync quite quickly.
I fully agree with you here on the design level. Firmware _should_ be discoverable. Unfortunately, in this case it really isn't.
Again, in Windows, this information is stored via the Registry and set when the driver is installed. An example:
; UEFIVAR SECURE APP SERVICE HKR,%EFIVarService.RegKey%,Enabled,%REG_DWORD%,1 HKR,%EFIVarService.RegKey%,MajorVersion,%REG_DWORD%,1 HKR,%EFIVarService.RegKey%,MinorVersion,%REG_DWORD%,0
; WINSECAPP SECURE APP SERVICE HKR,%WinSecAppService.RegKey%,Enabled,%REG_DWORD%,1 HKR,%WinSecAppService.RegKey%,SecureApp,%REG_DWORD%,1 HKR,%WinSecAppService.RegKey%,LoadApp,%REG_DWORD%,0 HKR,%WinSecAppService.RegKey%,AppName,,"qcom.tz.winsecapp" HKR,%WinSecAppService.RegKey%,MajorVersion,%REG_DWORD%,1 HKR,%WinSecAppService.RegKey%,MinorVersion,%REG_DWORD%,0 HKR,%WinSecAppService.RegKey%,OSDependencies,%REG_MULTI_SZ%,%RpmbOsService%
; HDCP v2.2 SECURE APP SERVICE HKR,%Hdcp2p2Service.RegKey%,Enabled,%REG_DWORD%,1 HKR,%Hdcp2p2Service.RegKey%,SecureApp,%REG_DWORD%,1 HKR,%Hdcp2p2Service.RegKey%,LoadApp,%REG_DWORD%,1 HKR,%Hdcp2p2Service.RegKey%,AppName,,"qcom.tz.hdcp2p2" HKR,%Hdcp2p2Service.RegKey%,FileName,,"hdcp2p2.mbn" HKR,%Hdcp2p2Service.RegKey%,MajorVersion,%REG_DWORD%,1 HKR,%Hdcp2p2Service.RegKey%,MinorVersion,%REG_DWORD%,0 HKR,%Hdcp2p2Service.RegKey%,OSDependencies,%REG_MULTI_SZ%,%RpmbOsService%,%TzAppsOsService%
The '.RegKey' contains a GUID that specifies the _driver_ interface that is registered by the driver to the kernel (i.e. is not related to the specific firmware and firmware version), e.g. [1]. For uefisecapp, the driver also maps this GUID to the name-string.
[1]: https://github.com/tpn/winsdk-10/blob/9b69fd26ac0c7d0b83d378dba01080e93349c2ed/Include/10.0.16299.0/km/treevariableservice.h#L35
>> I don't quite understand why this is a problem. I think per device, >> there's a reasonably limited set of apps that we would want to interact >> with from the kernel. And for one single device, that set doesn't change >> over time. So what's the difference to, say, an I2C device? >> > > As I said we don't want DT to be dumping ground for all the not well designed > firmware interface. The whole point of firmware being another piece of > software that can be change unlike hardware makes it fragile to present any > more that what you need in the DT. I see this as one of the example.
I can see your point. But this interface has apparently been around since at least sdm850 (e.g. Lenovo C630) and hasn't changed. As I've argued elsewhere: All parties involved have a vested interest that this interface doesn't change in a breaking way. The interface is modeled similar to syscalls, so I very much expect them to extend it if needed, instead of changing/breaking it.
Sure, it _could_ be changed in a breaking way. But again, I believe that to be _very_ unlikely.
> Anyways I don't have the final say, I leave it to the DT maintainers. > >>> As along as get this application ID can handle any random name, I prefer >>> to use that as the discover mechanism and not have this DT. >> >> Apart from the above, some apps must also be loaded from the system. And >> those you can't detect: If an app isn't running, it doesn't have an ID >> (uefisecapp and the tpm app are loaded by the firmware at boot). Those >> are mostly vendor-specific things as far as I can tell, or HDCP stuff. >> So you'd need to specify those as firmware somehow, and since (as far as >> I can tell) those are signed specifically by/for that vendor and >> potentially device (similar to the GPU zap shader or remoteproc >> firmware), you'll need to use per-device paths. >> > > Sounds to me like more can be pushed to user space as it gets loaded at > runtime.
If we have user-space available at the time when these things should be loaded or if they are more or less optional, sure.
>> That means you either hard-code them in the driver and have a compatible >> per model, do DMI matching, or something similar (again, essentially >> baking DTs into the kernel driver...), or just store them in the DT >> (like we already do for GPU/remoteprocs). While you could hard-code some >> known loaded-by-firmware apps and use the DT for others, I think we >> should keep everything in the same place. >> > > Worst case I am fine with that as this needs to be one of and future > platforms must get their act right in designing their f/w interface.
Again, I fully agree with you that this situation shouldn't exist. But reality is sadly different.
Regards, Max
| |