Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure Application client

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux