Message-ID: <7adebeff-a335-8331-bb22-32229f96281e@gmail.com>
Date:   Wed, 27 Jul 2022 16:49:03 +0200
From:   Maximilian Luz <luzmaximilian@...il.com>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Steev Klimaszewski <steev@...i.org>,
        Shawn Guo <shawn.guo@...aro.org>,
        Cristian Marussi <cristian.marussi@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-arm-msm@...r.kernel.org, linux-efi@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure
 Application client

On 7/27/22 15:24, Sudeep Holla wrote:
> On Wed, Jul 27, 2022 at 03:03:49PM +0200, Maximilian Luz wrote:
>>
>> Is there really a good way around it?
> 
> Yes rely on the firmware preferably auto discover, if that is not an option,
> how about query. It seem to be working in your case.
> 
>> As far as I can see the alternative (especially for the apps that
>> need to be loaded manually) is hard-coding everything in the driver.
>> Which IMHO just spreads device specific information everywhere.
>>
> 
> It may not be too bad compared to putting loads of firmware details
> in the DT. What happens if you get a firmware upgrade with changed
> number of firmware entities or even if the names are changed.
> 
> Are these name user ABI in a way that they won't be changed ? Generally
> these entities tend to use UUID and the name you have might get changed.

I am pretty certain that these names do not change for a device once it's
been released. The full ID of the uefisecapp is "qcom.tz.uefisecapp". The
built-in firmware parts here are core components. So I really do not expect
them to just remove or rename things. If they would do that, that would
mean that, on Windows, access to things like the TPM or UEFI variables
would be broken if both the driver and Registry are not updated in parallel
with the firmware. So while I can't myself guarantee that this is a stable
name and interface, it's very much in MS/Qualcomm's interest to keep it
stable.

Also, I'm not advocating on putting loads of details in the DT. I'm (in
this series) advocating for a DT compatible that says "this device stores
EFI variables via that firmware interface". I'd be very surprised if
MS/Qualcomm suddenly decided to change that out for another interface,
potentially breaking their own software and devices.

> I would ideally prefer even the name to be supplied from the userspace.
> In this particular case, make this a driver and have the name as the
> parameter. If the secure side services are used by some non-secure
> applications, then you will need to have a user-interface which means
> you can get the named from the userspace. No need to change the driver
> in either case. Please let me know if I am missing anything to consider
> here.

 From userspace? For access to EFI variables and (hopefully in the future
if I've managed to reverse-engineer that) the TPM? Those are things that
should work out-of-the-box and not require the user to first have to
configure something... Also, those are things that the kernel might want
to use (e.g. EFI variables as pstore for crashdumps) before the user is
even able to configure something (unless we now want to specify things
on the kernel command line...).

If this were something that only userspace would use then sure, let
userspace load it and do all the work. But it isn't.

> 
>> Also: Let's use the TPM app as example. If that would be a SPI or I2C
>> device, you'd model it in the DT. Just because it's a hardware device
>> that's accessible via SCM/firmware you now don't?
>>
> 
> Not sure if I understand the comparison here. But if there is some device
> that is access restricted but needs to be accessed and has mechanism to
> access, then you would model it as device in DT.
> 
> But the one $subject is addressing looks pure software and doesn't make
> sense to model in DT IMO.

So as soon as access runs via some firmware mechanism, it should not be
in the DT? The TPM in the example above would also be accessed via some
firmware API. EFI variables are stored on some SPI flash that is managed
by the TrustZone. So in both cases kernel calls to firmware calls to
device. Where do you draw the line?

>> If I were absolutely certain that there is a reliable mechanism to
>> detect these apps, I'd agree with having a driver to instantiate those
>> devices. But I am not.
>>
> 
> You did say you use some query API to check this. I haven't seen the driver,
> so relying on what you said earlier.

I did say that there is an API that turns a unique identifying string ID
of a secure application into a runtime-dependent integer ID of the
running application, returning an error if the application is not
running. I very much doubt that is supposed to be used for checking
support of certain applications. It could _maybe_ be used that way, but
the Windows driver doesn't, which makes me not very comfortable doing
that either.

Further: As far as I can tell, there is also no way of checking whether
that lookup failure is due to the application not being present or whether
something internal to the firmware failed. the respective results that the
call can (as far as I can tell) return are:

	QCTEE_OS_RESULT_SUCCESS			= 0,
	QCTEE_OS_RESULT_INCOMPLETE		= 1,
	QCTEE_OS_RESULT_BLOCKED_ON_LISTENER	= 2,
	QCTEE_OS_RESULT_FAILURE			= 0xFFFFFFFF,

And it will return QCTEE_OS_RESULT_FAILURE when the app name is wrong.

Again, while it _might_ be possible to use that, I don't think it makes a
very sound approach and I would really prefer not using it in that way.

Regards,
Max

Powered by blists - more mailing lists