Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d1bc99bb-82ce-aa6e-7fad-e9309fa1c19b@gmail.com>
Date:   Tue, 26 Jul 2022 19:01:28 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 4/4] dt-bindings: firmware: Add Qualcomm UEFI Secure
 Application client
Content-Language: en-US
To:     Sudeep Holla <sudeep.holla@arm.com>
Cc:     Andy Gross <agross@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Konrad Dybcio <konrad.dybcio@somainline.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Steev Klimaszewski <steev@kali.org>,
        Shawn Guo <shawn.guo@linaro.org>,
        Cristian Marussi <cristian.marussi@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-arm-msm@vger.kernel.org, linux-efi@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220723224949.1089973-1-luzmaximilian@gmail.com>
 <20220723224949.1089973-5-luzmaximilian@gmail.com>
 <20220726143005.wt4be7yo7sbd3xut@bogus>
 <829c8fee-cae5-597d-933d-784b4b57bd73@gmail.com>
 <20220726154138.74avqs6iqlzqpzjk@bogus>
From:   Maximilian Luz <luzmaximilian@gmail.com>
In-Reply-To: <20220726154138.74avqs6iqlzqpzjk@bogus>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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

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

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.

Windows uses a hard-coded list of supported apps in the driver (to
specify which apps the driver supports) and in addition to that uses
Registry entries (added via .inf files) to specify which app is supposed
to be present on the device and, for apps that need to be loaded, where
the firmware to be loaded is stored. It only ever attempts to connect to
those apps that are marked present in the registry. Which, again, may be
model/vendor specific.

And this is another reason I'm hesitant to use that function: Windows
doesn't use it in this way and that I don't know whether there can be
any subtle breakage or unexpected behavior if we use the function like
that (i.e., who's to say some broken firmware returns "app is present"
but the app is broken?).

Regards,
Max