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

From: Dmitry Baryshkov
Date: Thu Mar 09 2023 - 03:07:25 EST


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@xxxxxxxxx>
---

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@xxxxxxxxx>
+L:    linux-arm-msm@xxxxxxxxxxxxxxx
+S:    Maintained
+F:    drivers/firmware/qcom_qseecom.c
+F:    include/linux/firmware/qcom/qcom_qseecom.h
+
  QUALCOMM TSENS THERMAL DRIVER
  M:    Amit Kucheria <amitk@xxxxxxxxxx>
  M:    Thara Gopinath <thara.gopinath@xxxxxxxxx>



+
+
+/* -- 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