Re: [PATCH v6 3/3] firmware: Add support for Qualcomm UEFI Secure Application

From: Trilok Soni <quic_tsoni@quicinc.com>
To: Maximilian Luz <luzmaximilian@gmail.com>,
	Bjorn Andersson <andersson@kernel.org>
Cc: Andy Gross <agross@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Johan Hovold <johan@kernel.org>,
	Steev Klimaszewski <steev@kali.org>,
	<linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Johan Hovold <johan+linaro@kernel.org>
Subject: Re: [PATCH v6 3/3] firmware: Add support for Qualcomm UEFI Secure Application
Date: Thu, 7 Sep 2023 15:05:01 -0700	[thread overview]
Message-ID: <a1d0599c-b1b3-e57e-a857-112030a83872@quicinc.com> (raw)
In-Reply-To: <5487e816-f30c-468f-b860-a4cac30fba36@gmail.com>

On 9/7/2023 1:52 PM, Maximilian Luz wrote:
> On 8/27/23 23:59, Trilok Soni wrote:
>> On 8/27/2023 2:53 PM, Maximilian Luz wrote:
>>> On 8/27/23 23:26, Trilok Soni wrote:
>>>> On 8/27/2023 2:14 PM, Maximilian Luz wrote:
>>>>>    +config QCOM_QSEECOM_UEFISECAPP
>>>>> +    bool "Qualcomm SEE UEFI Secure App client driver"
>>>>
>>>> Why not "tristate"? This driver can be a loadable module, right?
>>>
>>> As I understand, modular efivars have still not been fully sorted out in
>>> the kernel. For example, userspace could try and mount efivarfs before
>>> the module has been loaded and by that erroneously determine that the
>>> system doesn't support efivars. So requiring it to be built in for now
>>> is more of a workaround (which has been suggested by Johan Hovold).
>>>
>>> There is no technical limitation in this part of the code itself, so
>>> enabling it (and QCOM_QSEECOM for that matter) to be built as module
>>> should be fairly straightforward once that's been sorted out.
>>
>> If not this application I would atleast like the QSEECOM driver to be a loadable module due to GKI (Generic Kernel Image) needs. Can we mark QSEECOM as "tristate" please? If not then there is a problem which we are not solving right now as you are documenting above and just moving it it for future and downstream vendors will keep having their additional changes to make it fit for loadable module needs.
> 
> Could you elaborate a bit on why/how switching to a tristate would help
> here? I'm afraid I don't quite follow. Do you mean that this would make
> it easier for downstream vendors to patch the module as opposed to
> create their own new thing? IMHO if they already need to patch it they
> can just as well modify it to be buildable as a module.
> 
> Generally I'm not opposed to have both loadable as modules, but I don't
> quite see the point as it would not be usable as such in upstream at
> the moment (at least not reliably, so to avoid those headaches I think
> it's better to just stick to bool for now).

I just want to be clear that it is not about downstream patches. That is not the intention. Not having the tristate the shows the real problem somewhere in the framework. It means such drivers won't be much usable without hacks on the configuration like GKI - so problem needs to be solved at upstream someday. 

> 
> Regards,
> Max
-- 
---Trilok Soni