linux-kernel - Re: [PATCH v3 2/4] firmware: Add support for Qualcomm Secure Execution Environment SCM interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ddda680-b169-5105-a3cb-20790f0868ee@linaro.org>
Date:   Thu, 9 Mar 2023 10:07:13 +0200
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Maximilian Luz <luzmaximilian@...il.com>,
        Bjorn Andersson <andersson@...nel.org>
Cc:     Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Johan Hovold <johan@...nel.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Sumit Garg <sumit.garg@...aro.org>,
        Steev Klimaszewski <steev@...i.org>,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/4] firmware: Add support for Qualcomm Secure
 Execution Environment SCM interface

On 08/03/2023 16:06, Maximilian Luz wrote:
> On 3/7/23 16:36, Dmitry Baryshkov wrote:
>> On 05/03/2023 04:21, Maximilian Luz wrote:
>>> Add support for SCM calls to Secure OS and the Secure Execution
>>> Environment (SEE) residing in the TrustZone (TZ) via the QSEECOM
>>> interface. This allows communication with Secure/TZ applications, for
>>> example 'uefisecapp' managing access to UEFI variables.
>>>
>>> The interface is managed by a platform device to ensure correct lifetime
>>> and establish a device link to the Qualcomm SCM device.
>>>
>>> While this patch introduces only a very basic interface without the more
>>> advanced features (such as re-entrant and blocking SCM calls and
>>> listeners/callbacks), this is enough to talk to the aforementioned
>>> 'uefisecapp'.
>>>
>>> Signed-off-by: Maximilian Luz <luzmaximilian@...il.com>
>>> ---
>>>
>>> Changes in v3:
>>>   - Rebase ontop of latest qcom_scm changes (qcom_scm.h moved).
>>>   - Move qcom_qseecom.h in accordance with qcom_scm.
>>>
>>> Changes in v2:
>>>   - Bind the interface to a device.
>>>   - Establish a device link to the SCM device to ensure proper ordering.
>>>   - Register client apps as child devices instead of requiring them 
>>> to be
>>>     specified in the device tree.
>>>   - Rename (qctree -> qseecom) to allow differentiation between old
>>>     (qseecom) and new (smcinvoke) interfaces to the trusted execution
>>>     environment.
>>>
>>> ---
>>>   MAINTAINERS                                |   7 +
>>>   drivers/firmware/Kconfig                   |  15 +
>>>   drivers/firmware/Makefile                  |   1 +
>>>   drivers/firmware/qcom_qseecom.c            | 314 +++++++++++++++++++++
>>>   include/linux/firmware/qcom/qcom_qseecom.h | 190 +++++++++++++
>>>   5 files changed, 527 insertions(+)
>>>   create mode 100644 drivers/firmware/qcom_qseecom.c
>>>   create mode 100644 include/linux/firmware/qcom/qcom_qseecom.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 9201967d198d..1545914a592c 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -17380,6 +17380,13 @@ F:    
>>> Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
>>>   F:    drivers/net/ethernet/qualcomm/rmnet/
>>>   F:    include/linux/if_rmnet.h
>>> +QUALCOMM SECURE EXECUTION ENVIRONMENT COMMUNICATION DRIVER
>>> +M:    Maximilian Luz <luzmaximilian@...il.com>
>>> +L:    linux-arm-msm@...r.kernel.org
>>> +S:    Maintained
>>> +F:    drivers/firmware/qcom_qseecom.c
>>> +F:    include/linux/firmware/qcom/qcom_qseecom.h
>>> +
>>>   QUALCOMM TSENS THERMAL DRIVER
>>>   M:    Amit Kucheria <amitk@...nel.org>
>>>   M:    Thara Gopinath <thara.gopinath@...il.com>
>>
>>
>>
>>> +
>>> +
>>> +/* -- Platform specific data. 
>>> ----------------------------------------------- */
>>> +
>>> +struct qseecom_data {
>>> +    const struct mfd_cell *cells;
>>
>> The child qseecom devices are not platform devices, so MFD should not 
>> be used here. Please use aux devices instead.
> 
> Okay, makes sense. Would this still work with your suggestion in patch 4
> regarding a custom (?) bus or can the aux bus be used to implement that? 
> From a
> quick look, I believe we could use aux bus for this but I haven't worked 
> with
> that before, so I don't know if I'm missing something.

Initially I thought that a custom bus might be required, to provide 
custom probe function. After giving it a thought, I think you can get 
away with using aux bus. So, embed the struct auxiliary_device into 
qseecom_app_device.

> 
>>> +    int num_cells;
>>> +};
>>> +
>>> +static const struct of_device_id qseecom_dt_match[] = {
>>> +    { .compatible = "qcom,qseecom-sc8280xp", },
>>
>> Forgot to mention, while doign review. There is no need for this 
>> compat until you provide the actual data. Please move it to the patch 4.
> 
> Sure, will do that.
> 
>>> +    { .compatible = "qcom,qseecom", },
>>> +    { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, qseecom_dt_match);
>>
>>
> 
> Regards,
> Max

-- 
With best wishes
Dmitry