| Message ID | 20230827211408.689076-1-luzmaximilian@gmail.com |
|---|---|
| Headers | show |
| Series | firmware: Add support for Qualcomm UEFI Secure Application | expand |
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. >> + depends on QCOM_QSEECOM >> + depends on EFI >> + help > >
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.
On Sun, Aug 27, 2023 at 11:53:08PM +0200, 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. > Afaict without resolving the efivars issue this is boolean in practice anyways. As such, I intend to pick this for v6.7, and we can transition to modular support incrementally from here. Many thanks for sticking to the effort, Maximilian. Regards, Bjorn
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). Regards, Max
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
On Sun, 27 Aug 2023 23:14:03 +0200, Maximilian Luz wrote: > This series adds basic support for the QSEECOM interface used to > communicate with secure applications running in the TrustZone on certain > Qualcomm devices. In addition to that, it also provides a driver for > "uefisecapp", the secure application managing access to UEFI variables > on such platforms. > > For a more detailed description, see the blurb of v1. > > [...] Applied, thanks! [1/3] lib/ucs2_string: Add UCS-2 strscpy function commit: e4c89f9380017b6b2e63836e2de1af8eb4535384 [2/3] firmware: qcom_scm: Add support for Qualcomm Secure Execution Environment SCM interface commit: 00b1248606ba3979ccae30ed11df8cdc1a84245a [3/3] firmware: Add support for Qualcomm UEFI Secure Application commit: 759e7a2b62eb3ef3c93ffeb5cca788a09627d7d9 Best regards,
This series adds basic support for the QSEECOM interface used to communicate with secure applications running in the TrustZone on certain Qualcomm devices. In addition to that, it also provides a driver for "uefisecapp", the secure application managing access to UEFI variables on such platforms. For a more detailed description, see the blurb of v1. Previous versions: - V5: https://lore.kernel.org/lkml/20230730161906.606163-1-luzmaximilian@gmail.com/t/ - V4: https://lore.kernel.org/lkml/72c0359a-eda6-30ea-0ec0-b7e9b804b87b@gmail.com/t/ - V3: https://lore.kernel.org/lkml/20230305022119.1331495-4-luzmaximilian@gmail.com/t/ - V2: https://lore.kernel.org/lkml/20230127184650.756795-1-luzmaximilian@gmail.com/ - V1: https://lore.kernel.org/lkml/20220723224949.1089973-1-luzmaximilian@gmail.com/ Changes in v5: - Small code fixes (e.g., missing 'static inline', missing includes, removal of unnecessary functions, improvements and fixes for documentation and comments). No larger or structural changes. Changes in v5: - Re-introduce a dedicated platform device for managing QSEECOM client devices. The device is now added via qcom_scm.c instead of the device tree (as has been done in v3). - Replace ucs2_strlcpy() with ucs2_strscpy() - Drop "firmware: qcom_scm: Clear scm pointer on probe failure" and sort out probe-related issue. - Clean up comments in qcom_qseecom_uefisecapp.c Changes in v4: - Integrate the QSEECOM interface into qcom_scm.c instead of instantiating a custom device and requiring device-tree bindings for it. With that, drop the respective patches exporting SCM call functions from qcom_scm.c and the DT bindings. - Restructure management of DMA memory and move DMA mapping entirely into the app_send() command, removing the need for DMA handling in app client drivers. - Add support for EFI's query_variable_info() call. - Move UCS-2 string helpers to lib/ucs2_string.c (introduces patch 1). - Add fix for related cleanup-issue in qcom_scm.c (introduces patch 2). (Refer to individual patches for more details.) Changes in v3: - Fix doc comment in qcom_scm.c - Rebase on top of latest changes to qcom_scm. Changes in v2: - Bind the qseecom interface to a device. - Establish a device link between the new qseecom device and the SCM device to ensure proper PM and remove ordering. - Remove the compatible for uefisecapp. Instead, introduce a compatible for the qseecom device. This directly reflects ACPI tables and the QCOM0476 device described therein, which is responsible for the secure app / qseecom interface (i.e., the same purpose). Client devices representing apps handled by the kernel (such as uefisecapp) are now directly instantiated by the qseecom driver, based on the respective platform-specific compatible. - Rename the base name (qctree -> qseecom) to allow differentiation between old (qseecom) and new (smcinvoke) interfaces to the trusted execution environment. This directly reflects downstream naming by Qualcomm. Maximilian Luz (3): lib/ucs2_string: Add UCS-2 strscpy function firmware: qcom_scm: Add support for Qualcomm Secure Execution Environment SCM interface firmware: Add support for Qualcomm UEFI Secure Application MAINTAINERS | 12 + drivers/firmware/Kconfig | 32 + drivers/firmware/Makefile | 2 + drivers/firmware/qcom_qseecom.c | 120 +++ drivers/firmware/qcom_qseecom_uefisecapp.c | 871 +++++++++++++++++++++ drivers/firmware/qcom_scm.c | 394 ++++++++++ include/linux/firmware/qcom/qcom_qseecom.h | 46 ++ include/linux/firmware/qcom/qcom_scm.h | 22 + include/linux/ucs2_string.h | 1 + lib/ucs2_string.c | 52 ++ 10 files changed, 1552 insertions(+) create mode 100644 drivers/firmware/qcom_qseecom.c create mode 100644 drivers/firmware/qcom_qseecom_uefisecapp.c create mode 100644 include/linux/firmware/qcom/qcom_qseecom.h