* [PATCH v10 01/15] dt-bindings: firmware: qcom,scm: add memory-region for sa8775p
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 18:41 ` Krzysztof Kozlowski
2024-05-27 12:54 ` [PATCH v10 02/15] firmware: qcom: add a dedicated TrustZone buffer allocator Bartosz Golaszewski
` (15 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Document a new property (currently only for sa8775p) that describes the
memory region reserved for communicating with the TrustZone.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index 47d3d2d52acd..2cc83771d8e7 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -93,6 +93,11 @@ properties:
protocol to handle sleeping SCM calls.
maxItems: 1
+ memory-region:
+ description:
+ Phandle to the memory region reserved for the shared memory bridge to TZ.
+ maxItems: 1
+
qcom,sdi-enabled:
description:
Indicates that the SDI (Secure Debug Image) has been enabled by TZ
@@ -193,6 +198,16 @@ allOf:
then:
properties:
interrupts: false
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,scm-sa8775p
+ then:
+ properties:
+ memory-region: false
required:
- compatible
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v10 01/15] dt-bindings: firmware: qcom,scm: add memory-region for sa8775p
2024-05-27 12:54 ` [PATCH v10 01/15] dt-bindings: firmware: qcom,scm: add memory-region for sa8775p Bartosz Golaszewski
@ 2024-05-27 18:41 ` Krzysztof Kozlowski
0 siblings, 0 replies; 27+ messages in thread
From: Krzysztof Kozlowski @ 2024-05-27 18:41 UTC (permalink / raw)
To: Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko, Das Srinagesh,
Bartosz Golaszewski, Maximilian Luz, Catalin Marinas, Will Deacon,
Srini Kandagatla, Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
On 27/05/2024 14:54, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Document a new property (currently only for sa8775p) that describes the
> memory region reserved for communicating with the TrustZone.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v10 02/15] firmware: qcom: add a dedicated TrustZone buffer allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 01/15] dt-bindings: firmware: qcom,scm: add memory-region for sa8775p Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 03/15] firmware: qcom: scm: enable the TZ mem allocator Bartosz Golaszewski
` (14 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We have several SCM calls that require passing buffers to the TrustZone
on top of the SMC core which allocates memory for calls that require
more than 4 arguments.
Currently every user does their own thing which leads to code
duplication. Many users call dma_alloc_coherent() for every call which
is terribly unperformant (speed- and size-wise).
Provide a set of library functions for creating and managing pools of
memory which is suitable for sharing with the TrustZone, that is:
page-aligned, contiguous and non-cachable as well as provides a way of
mapping of kernel virtual addresses to physical space.
Make the allocator ready for extending with additional modes of operation
which will allow us to support the SHM bridge safety mechanism once all
users convert.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
MAINTAINERS | 8 +
drivers/firmware/qcom/Kconfig | 20 ++
drivers/firmware/qcom/Makefile | 1 +
drivers/firmware/qcom/qcom_tzmem.c | 389 +++++++++++++++++++++++++++++++
drivers/firmware/qcom/qcom_tzmem.h | 13 ++
include/linux/firmware/qcom/qcom_tzmem.h | 56 +++++
6 files changed, 487 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index dbc5d9ec3d20..305a1ea0f6be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18606,6 +18606,14 @@ F: Documentation/networking/device_drivers/cellular/qualcomm/rmnet.rst
F: drivers/net/ethernet/qualcomm/rmnet/
F: include/linux/if_rmnet.h
+QUALCOMM TRUST ZONE MEMORY ALLOCATOR
+M: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
+L: linux-arm-msm@vger.kernel.org
+S: Maintained
+F: drivers/firmware/qcom/qcom_tzmem.c
+F: drivers/firmware/qcom/qcom_tzmem.h
+F: include/linux/firmware/qcom/qcom_tzmem.h
+
QUALCOMM TSENS THERMAL DRIVER
M: Amit Kucheria <amitk@kernel.org>
M: Thara Gopinath <thara.gopinath@gmail.com>
diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
index 3f05d9854ddf..3c495f8698e1 100644
--- a/drivers/firmware/qcom/Kconfig
+++ b/drivers/firmware/qcom/Kconfig
@@ -9,6 +9,26 @@ menu "Qualcomm firmware drivers"
config QCOM_SCM
tristate
+config QCOM_TZMEM
+ tristate
+ select GENERIC_ALLOCATOR
+
+choice
+ prompt "TrustZone interface memory allocator mode"
+ default QCOM_TZMEM_MODE_GENERIC
+ help
+ Selects the mode of the memory allocator providing memory buffers of
+ suitable format for sharing with the TrustZone. If in doubt, select
+ 'Generic'.
+
+config QCOM_TZMEM_MODE_GENERIC
+ bool "Generic"
+ help
+ Use the generic allocator mode. The memory is page-aligned, non-cachable
+ and physically contiguous.
+
+endchoice
+
config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
bool "Qualcomm download mode enabled by default"
depends on QCOM_SCM
diff --git a/drivers/firmware/qcom/Makefile b/drivers/firmware/qcom/Makefile
index c9f12ee8224a..0be40a1abc13 100644
--- a/drivers/firmware/qcom/Makefile
+++ b/drivers/firmware/qcom/Makefile
@@ -5,5 +5,6 @@
obj-$(CONFIG_QCOM_SCM) += qcom-scm.o
qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_QCOM_TZMEM) += qcom_tzmem.o
obj-$(CONFIG_QCOM_QSEECOM) += qcom_qseecom.o
obj-$(CONFIG_QCOM_QSEECOM_UEFISECAPP) += qcom_qseecom_uefisecapp.o
diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
new file mode 100644
index 000000000000..3853385bf215
--- /dev/null
+++ b/drivers/firmware/qcom/qcom_tzmem.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Memory allocator for buffers shared with the TrustZone.
+ *
+ * Copyright (C) 2023-2024 Linaro Ltd.
+ */
+
+#include <linux/bug.h>
+#include <linux/cleanup.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/firmware/qcom/qcom_tzmem.h>
+#include <linux/genalloc.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/radix-tree.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include "qcom_tzmem.h"
+
+struct qcom_tzmem_area {
+ struct list_head list;
+ void *vaddr;
+ dma_addr_t paddr;
+ size_t size;
+ void *priv;
+};
+
+struct qcom_tzmem_pool {
+ struct gen_pool *genpool;
+ struct list_head areas;
+ enum qcom_tzmem_policy policy;
+ size_t increment;
+ size_t max_size;
+ spinlock_t lock;
+};
+
+struct qcom_tzmem_chunk {
+ phys_addr_t paddr;
+ size_t size;
+ struct qcom_tzmem_pool *owner;
+};
+
+static struct device *qcom_tzmem_dev;
+static RADIX_TREE(qcom_tzmem_chunks, GFP_ATOMIC);
+static DEFINE_SPINLOCK(qcom_tzmem_chunks_lock);
+
+#if IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_GENERIC)
+
+static int qcom_tzmem_init(void)
+{
+ return 0;
+}
+
+static int qcom_tzmem_init_area(struct qcom_tzmem_area *area)
+{
+ return 0;
+}
+
+static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area)
+{
+
+}
+
+#endif /* CONFIG_QCOM_TZMEM_MODE_GENERIC */
+
+static int qcom_tzmem_pool_add_memory(struct qcom_tzmem_pool *pool,
+ size_t size, gfp_t gfp)
+{
+ int ret;
+
+ struct qcom_tzmem_area *area __free(kfree) = kzalloc(sizeof(*area),
+ gfp);
+ if (!area)
+ return -ENOMEM;
+
+ area->size = PAGE_ALIGN(size);
+
+ area->vaddr = dma_alloc_coherent(qcom_tzmem_dev, area->size,
+ &area->paddr, gfp);
+ if (!area->vaddr)
+ return -ENOMEM;
+
+ ret = qcom_tzmem_init_area(area);
+ if (ret) {
+ dma_free_coherent(qcom_tzmem_dev, area->size,
+ area->vaddr, area->paddr);
+ return ret;
+ }
+
+ ret = gen_pool_add_virt(pool->genpool, (unsigned long)area->vaddr,
+ (phys_addr_t)area->paddr, size, -1);
+ if (ret) {
+ dma_free_coherent(qcom_tzmem_dev, area->size,
+ area->vaddr, area->paddr);
+ return ret;
+ }
+
+ scoped_guard(spinlock_irqsave, &pool->lock)
+ list_add_tail(&area->list, &pool->areas);
+
+ area = NULL;
+ return 0;
+}
+
+/**
+ * qcom_tzmem_pool_new() - Create a new TZ memory pool.
+ * @config: Pool configuration.
+ *
+ * Create a new pool of memory suitable for sharing with the TrustZone.
+ *
+ * Must not be used in atomic context.
+ *
+ * Return: New memory pool address or ERR_PTR() on error.
+ */
+struct qcom_tzmem_pool *
+qcom_tzmem_pool_new(const struct qcom_tzmem_pool_config *config)
+{
+ int ret = -ENOMEM;
+
+ might_sleep();
+
+ switch (config->policy) {
+ case QCOM_TZMEM_POLICY_STATIC:
+ if (!config->initial_size)
+ return ERR_PTR(-EINVAL);
+ break;
+ case QCOM_TZMEM_POLICY_MULTIPLIER:
+ if (!config->increment)
+ return ERR_PTR(-EINVAL);
+ break;
+ case QCOM_TZMEM_POLICY_ON_DEMAND:
+ break;
+ default:
+ return ERR_PTR(-EINVAL);
+ }
+
+ struct qcom_tzmem_pool *pool __free(kfree) = kzalloc(sizeof(*pool),
+ GFP_KERNEL);
+ if (!pool)
+ return ERR_PTR(-ENOMEM);
+
+ pool->genpool = gen_pool_create(PAGE_SHIFT, -1);
+ if (!pool->genpool)
+ return ERR_PTR(-ENOMEM);
+
+ gen_pool_set_algo(pool->genpool, gen_pool_best_fit, NULL);
+
+ pool->policy = config->policy;
+ pool->increment = config->increment;
+ pool->max_size = config->max_size;
+ INIT_LIST_HEAD(&pool->areas);
+ spin_lock_init(&pool->lock);
+
+ if (config->initial_size) {
+ ret = qcom_tzmem_pool_add_memory(pool, config->initial_size,
+ GFP_KERNEL);
+ if (ret) {
+ gen_pool_destroy(pool->genpool);
+ return ERR_PTR(ret);
+ }
+ }
+
+ return no_free_ptr(pool);
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_pool_new);
+
+/**
+ * qcom_tzmem_pool_free() - Destroy a TZ memory pool and free all resources.
+ * @pool: Memory pool to free.
+ *
+ * Must not be called if any of the allocated chunks has not been freed.
+ * Must not be used in atomic context.
+ */
+void qcom_tzmem_pool_free(struct qcom_tzmem_pool *pool)
+{
+ struct qcom_tzmem_area *area, *next;
+ struct qcom_tzmem_chunk *chunk;
+ struct radix_tree_iter iter;
+ bool non_empty = false;
+ void __rcu **slot;
+
+ might_sleep();
+
+ if (!pool)
+ return;
+
+ scoped_guard(spinlock_irqsave, &qcom_tzmem_chunks_lock) {
+ radix_tree_for_each_slot(slot, &qcom_tzmem_chunks, &iter, 0) {
+ chunk = radix_tree_deref_slot_protected(slot,
+ &qcom_tzmem_chunks_lock);
+
+ if (chunk->owner == pool)
+ non_empty = true;
+ }
+ }
+
+ WARN(non_empty, "Freeing TZ memory pool with memory still allocated");
+
+ list_for_each_entry_safe(area, next, &pool->areas, list) {
+ list_del(&area->list);
+ qcom_tzmem_cleanup_area(area);
+ dma_free_coherent(qcom_tzmem_dev, area->size,
+ area->vaddr, area->paddr);
+ kfree(area);
+ }
+
+ gen_pool_destroy(pool->genpool);
+ kfree(pool);
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_pool_free);
+
+static void devm_qcom_tzmem_pool_free(void *data)
+{
+ struct qcom_tzmem_pool *pool = data;
+
+ qcom_tzmem_pool_free(pool);
+}
+
+/**
+ * devm_qcom_tzmem_pool_new() - Managed variant of qcom_tzmem_pool_new().
+ * @dev: Device managing this resource.
+ * @config: Pool configuration.
+ *
+ * Must not be used in atomic context.
+ *
+ * Return: Address of the managed pool or ERR_PTR() on failure.
+ */
+struct qcom_tzmem_pool *
+devm_qcom_tzmem_pool_new(struct device *dev,
+ const struct qcom_tzmem_pool_config *config)
+{
+ struct qcom_tzmem_pool *pool;
+ int ret;
+
+ pool = qcom_tzmem_pool_new(config);
+ if (IS_ERR(pool))
+ return pool;
+
+ ret = devm_add_action_or_reset(dev, devm_qcom_tzmem_pool_free, pool);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return pool;
+}
+
+static bool qcom_tzmem_try_grow_pool(struct qcom_tzmem_pool *pool,
+ size_t requested, gfp_t gfp)
+{
+ size_t current_size = gen_pool_size(pool->genpool);
+
+ if (pool->max_size && (current_size + requested) > pool->max_size)
+ return false;
+
+ switch (pool->policy) {
+ case QCOM_TZMEM_POLICY_STATIC:
+ return false;
+ case QCOM_TZMEM_POLICY_MULTIPLIER:
+ requested = current_size * pool->increment;
+ break;
+ case QCOM_TZMEM_POLICY_ON_DEMAND:
+ break;
+ }
+
+ return !qcom_tzmem_pool_add_memory(pool, requested, gfp);
+}
+
+/**
+ * qcom_tzmem_alloc() - Allocate a memory chunk suitable for sharing with TZ.
+ * @pool: TZ memory pool from which to allocate memory.
+ * @size: Number of bytes to allocate.
+ * @gfp: GFP flags.
+ *
+ * Can be used in any context.
+ *
+ * Return:
+ * Address of the allocated buffer or NULL if no more memory can be allocated.
+ * The buffer must be released using qcom_tzmem_free().
+ */
+void *qcom_tzmem_alloc(struct qcom_tzmem_pool *pool, size_t size, gfp_t gfp)
+{
+ unsigned long vaddr;
+ int ret;
+
+ if (!size)
+ return NULL;
+
+ size = PAGE_ALIGN(size);
+
+ struct qcom_tzmem_chunk *chunk __free(kfree) = kzalloc(sizeof(*chunk),
+ gfp);
+ if (!chunk)
+ return NULL;
+
+again:
+ vaddr = gen_pool_alloc(pool->genpool, size);
+ if (!vaddr) {
+ if (qcom_tzmem_try_grow_pool(pool, size, gfp))
+ goto again;
+
+ return NULL;
+ }
+
+ chunk->paddr = gen_pool_virt_to_phys(pool->genpool, vaddr);
+ chunk->size = size;
+ chunk->owner = pool;
+
+ scoped_guard(spinlock_irqsave, &qcom_tzmem_chunks_lock) {
+ ret = radix_tree_insert(&qcom_tzmem_chunks, vaddr, chunk);
+ if (ret) {
+ gen_pool_free(pool->genpool, vaddr, size);
+ return NULL;
+ }
+
+ chunk = NULL;
+ }
+
+ return (void *)vaddr;
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_alloc);
+
+/**
+ * qcom_tzmem_free() - Release a buffer allocated from a TZ memory pool.
+ * @vaddr: Virtual address of the buffer.
+ *
+ * Can be used in any context.
+ */
+void qcom_tzmem_free(void *vaddr)
+{
+ struct qcom_tzmem_chunk *chunk;
+
+ scoped_guard(spinlock_irqsave, &qcom_tzmem_chunks_lock)
+ chunk = radix_tree_delete_item(&qcom_tzmem_chunks,
+ (unsigned long)vaddr, NULL);
+
+ if (!chunk) {
+ WARN(1, "Virtual address %p not owned by TZ memory allocator",
+ vaddr);
+ return;
+ }
+
+ scoped_guard(spinlock_irqsave, &chunk->owner->lock)
+ gen_pool_free(chunk->owner->genpool, (unsigned long)vaddr,
+ chunk->size);
+ kfree(chunk);
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_free);
+
+/**
+ * qcom_tzmem_to_phys() - Map the virtual address of a TZ buffer to physical.
+ * @vaddr: Virtual address of the buffer allocated from a TZ memory pool.
+ *
+ * Can be used in any context. The address must have been returned by a call
+ * to qcom_tzmem_alloc().
+ *
+ * Returns: Physical address of the buffer.
+ */
+phys_addr_t qcom_tzmem_to_phys(void *vaddr)
+{
+ struct qcom_tzmem_chunk *chunk;
+
+ guard(spinlock_irqsave)(&qcom_tzmem_chunks_lock);
+
+ chunk = radix_tree_lookup(&qcom_tzmem_chunks, (unsigned long)vaddr);
+ if (!chunk)
+ return 0;
+
+ return chunk->paddr;
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_to_phys);
+
+int qcom_tzmem_enable(struct device *dev)
+{
+ if (qcom_tzmem_dev)
+ return -EBUSY;
+
+ qcom_tzmem_dev = dev;
+
+ return qcom_tzmem_init();
+}
+EXPORT_SYMBOL_GPL(qcom_tzmem_enable);
+
+MODULE_DESCRIPTION("TrustZone memory allocator for Qualcomm firmware drivers");
+MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@linaro.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/qcom/qcom_tzmem.h b/drivers/firmware/qcom/qcom_tzmem.h
new file mode 100644
index 000000000000..8fa8a3eb940e
--- /dev/null
+++ b/drivers/firmware/qcom/qcom_tzmem.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023-2024 Linaro Ltd.
+ */
+
+#ifndef __QCOM_TZMEM_PRIV_H
+#define __QCOM_TZMEM_PRIV_H
+
+struct device;
+
+int qcom_tzmem_enable(struct device *dev);
+
+#endif /* __QCOM_TZMEM_PRIV_H */
diff --git a/include/linux/firmware/qcom/qcom_tzmem.h b/include/linux/firmware/qcom/qcom_tzmem.h
new file mode 100644
index 000000000000..b83b63a0c049
--- /dev/null
+++ b/include/linux/firmware/qcom/qcom_tzmem.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023-2024 Linaro Ltd.
+ */
+
+#ifndef __QCOM_TZMEM_H
+#define __QCOM_TZMEM_H
+
+#include <linux/cleanup.h>
+#include <linux/gfp.h>
+#include <linux/types.h>
+
+struct device;
+struct qcom_tzmem_pool;
+
+/**
+ * enum qcom_tzmem_policy - Policy for pool growth.
+ */
+enum qcom_tzmem_policy {
+ /**< Static pool, never grow above initial size. */
+ QCOM_TZMEM_POLICY_STATIC = 1,
+ /**< When out of memory, add increment * current size of memory. */
+ QCOM_TZMEM_POLICY_MULTIPLIER,
+ /**< When out of memory add as much as is needed until max_size. */
+ QCOM_TZMEM_POLICY_ON_DEMAND,
+};
+
+/**
+ * struct qcom_tzmem_pool_config - TZ memory pool configuration.
+ * @initial_size: Number of bytes to allocate for the pool during its creation.
+ * @policy: Pool size growth policy.
+ * @increment: Used with policies that allow pool growth.
+ * @max_size: Size above which the pool will never grow.
+ */
+struct qcom_tzmem_pool_config {
+ size_t initial_size;
+ enum qcom_tzmem_policy policy;
+ size_t increment;
+ size_t max_size;
+};
+
+struct qcom_tzmem_pool *
+qcom_tzmem_pool_new(const struct qcom_tzmem_pool_config *config);
+void qcom_tzmem_pool_free(struct qcom_tzmem_pool *pool);
+struct qcom_tzmem_pool *
+devm_qcom_tzmem_pool_new(struct device *dev,
+ const struct qcom_tzmem_pool_config *config);
+
+void *qcom_tzmem_alloc(struct qcom_tzmem_pool *pool, size_t size, gfp_t gfp);
+void qcom_tzmem_free(void *ptr);
+
+DEFINE_FREE(qcom_tzmem, void *, if (_T) qcom_tzmem_free(_T))
+
+phys_addr_t qcom_tzmem_to_phys(void *ptr);
+
+#endif /* __QCOM_TZMEM */
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 03/15] firmware: qcom: scm: enable the TZ mem allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 01/15] dt-bindings: firmware: qcom,scm: add memory-region for sa8775p Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 02/15] firmware: qcom: add a dedicated TrustZone buffer allocator Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 04/15] firmware: qcom: scm: smc: switch to using the SCM allocator Bartosz Golaszewski
` (13 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Select the TrustZone memory allocator in Kconfig and create a pool of
memory shareable with the TrustZone when probing the SCM driver.
This will allow a gradual conversion of all relevant SCM calls to using
the dedicated allocator.
The policy used for the pool is "on-demand" and the initial size is 0
as - depending on the config - it's possible that no SCM calls needing
to allocate memory will be called. The sizes of possible allocations also
vary substiantially further warranting the "on-demand" approach.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/Kconfig | 1 +
drivers/firmware/qcom/qcom_scm.c | 22 ++++++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
index 3c495f8698e1..4634f8cecc7b 100644
--- a/drivers/firmware/qcom/Kconfig
+++ b/drivers/firmware/qcom/Kconfig
@@ -7,6 +7,7 @@
menu "Qualcomm firmware drivers"
config QCOM_SCM
+ select QCOM_TZMEM
tristate
config QCOM_TZMEM
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 68f4df7e6c3c..0e7b1813e4a7 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -10,8 +10,10 @@
#include <linux/completion.h>
#include <linux/cpumask.h>
#include <linux/dma-mapping.h>
+#include <linux/err.h>
#include <linux/export.h>
#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/firmware/qcom/qcom_tzmem.h>
#include <linux/init.h>
#include <linux/interconnect.h>
#include <linux/interrupt.h>
@@ -22,9 +24,11 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/reset-controller.h>
+#include <linux/sizes.h>
#include <linux/types.h>
#include "qcom_scm.h"
+#include "qcom_tzmem.h"
static bool download_mode = IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
module_param(download_mode, bool, 0);
@@ -43,6 +47,8 @@ struct qcom_scm {
int scm_vote_count;
u64 dload_mode_addr;
+
+ struct qcom_tzmem_pool *mempool;
};
struct qcom_scm_current_perm_info {
@@ -1810,6 +1816,7 @@ static irqreturn_t qcom_scm_irq_handler(int irq, void *data)
static int qcom_scm_probe(struct platform_device *pdev)
{
+ struct qcom_tzmem_pool_config pool_config;
struct qcom_scm *scm;
int irq, ret;
@@ -1885,6 +1892,21 @@ static int qcom_scm_probe(struct platform_device *pdev)
if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
qcom_scm_disable_sdi();
+ ret = qcom_tzmem_enable(__scm->dev);
+ if (ret)
+ return dev_err_probe(__scm->dev, ret,
+ "Failed to enable the TrustZone memory allocator\n");
+
+ memset(&pool_config, 0, sizeof(pool_config));
+ pool_config.initial_size = 0;
+ pool_config.policy = QCOM_TZMEM_POLICY_ON_DEMAND;
+ pool_config.max_size = SZ_256K;
+
+ __scm->mempool = devm_qcom_tzmem_pool_new(__scm->dev, &pool_config);
+ if (IS_ERR(__scm->mempool))
+ return dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
+ "Failed to create the SCM memory pool\n");
+
/*
* Initialize the QSEECOM interface.
*
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 04/15] firmware: qcom: scm: smc: switch to using the SCM allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (2 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 03/15] firmware: qcom: scm: enable the TZ mem allocator Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 05/15] firmware: qcom: scm: make qcom_scm_assign_mem() use the TZ allocator Bartosz Golaszewski
` (12 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We need to allocate, map and pass a buffer to the trustzone if we have
more than 4 arguments for a given SCM call. Let's use the new TrustZone
allocator for that memory and shrink the code in process.
As this code lives in a different compilation unit than the rest of the
SCM code, we need to provide a helper in the form of
qcom_scm_get_tzmem_pool() that allows the SMC low-level routines to
access the SCM memory pool.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm-smc.c | 30 ++++++++----------------------
drivers/firmware/qcom/qcom_scm.c | 5 +++++
drivers/firmware/qcom/qcom_scm.h | 3 +++
3 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm-smc.c b/drivers/firmware/qcom/qcom_scm-smc.c
index 16cf88acfa8e..dca5f3f1883b 100644
--- a/drivers/firmware/qcom/qcom_scm-smc.c
+++ b/drivers/firmware/qcom/qcom_scm-smc.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2015,2019 The Linux Foundation. All rights reserved.
*/
+#include <linux/cleanup.h>
#include <linux/io.h>
#include <linux/errno.h>
#include <linux/delay.h>
@@ -9,6 +10,7 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/firmware/qcom/qcom_tzmem.h>
#include <linux/arm-smccc.h>
#include <linux/dma-mapping.h>
@@ -150,11 +152,10 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
enum qcom_scm_convention qcom_convention,
struct qcom_scm_res *res, bool atomic)
{
+ struct qcom_tzmem_pool *mempool = qcom_scm_get_tzmem_pool();
int arglen = desc->arginfo & 0xf;
int i, ret;
- dma_addr_t args_phys = 0;
- void *args_virt = NULL;
- size_t alloc_len;
+ void *args_virt __free(qcom_tzmem) = NULL;
gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL;
u32 smccc_call_type = atomic ? ARM_SMCCC_FAST_CALL : ARM_SMCCC_STD_CALL;
u32 qcom_smccc_convention = (qcom_convention == SMC_CONVENTION_ARM_32) ?
@@ -172,9 +173,9 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
smc.args[i + SCM_SMC_FIRST_REG_IDX] = desc->args[i];
if (unlikely(arglen > SCM_SMC_N_REG_ARGS)) {
- alloc_len = SCM_SMC_N_EXT_ARGS * sizeof(u64);
- args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag);
-
+ args_virt = qcom_tzmem_alloc(mempool,
+ SCM_SMC_N_EXT_ARGS * sizeof(u64),
+ flag);
if (!args_virt)
return -ENOMEM;
@@ -192,25 +193,10 @@ int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
SCM_SMC_FIRST_EXT_IDX]);
}
- args_phys = dma_map_single(dev, args_virt, alloc_len,
- DMA_TO_DEVICE);
-
- if (dma_mapping_error(dev, args_phys)) {
- kfree(args_virt);
- return -ENOMEM;
- }
-
- smc.args[SCM_SMC_LAST_REG_IDX] = args_phys;
+ smc.args[SCM_SMC_LAST_REG_IDX] = qcom_tzmem_to_phys(args_virt);
}
- /* ret error check follows after args_virt cleanup*/
ret = __scm_smc_do(dev, &smc, &smc_res, atomic);
-
- if (args_virt) {
- dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE);
- kfree(args_virt);
- }
-
if (ret)
return ret;
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 0e7b1813e4a7..1d207c14afc8 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -204,6 +204,11 @@ static void qcom_scm_bw_disable(void)
enum qcom_scm_convention qcom_scm_convention = SMC_CONVENTION_UNKNOWN;
static DEFINE_SPINLOCK(scm_query_lock);
+struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void)
+{
+ return __scm->mempool;
+}
+
static enum qcom_scm_convention __get_convention(void)
{
unsigned long flags;
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index 4532907e8489..aa7d06939f8e 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -5,6 +5,7 @@
#define __QCOM_SCM_INT_H
struct device;
+struct qcom_tzmem_pool;
enum qcom_scm_convention {
SMC_CONVENTION_UNKNOWN,
@@ -78,6 +79,8 @@ int scm_legacy_call_atomic(struct device *dev, const struct qcom_scm_desc *desc,
int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
struct qcom_scm_res *res);
+struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void);
+
#define QCOM_SCM_SVC_BOOT 0x01
#define QCOM_SCM_BOOT_SET_ADDR 0x01
#define QCOM_SCM_BOOT_TERMINATE_PC 0x02
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 05/15] firmware: qcom: scm: make qcom_scm_assign_mem() use the TZ allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (3 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 04/15] firmware: qcom: scm: smc: switch to using the SCM allocator Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 06/15] firmware: qcom: scm: make qcom_scm_ice_set_key() " Bartosz Golaszewski
` (11 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Let's use the new TZ memory allocator to obtain a buffer for this call
instead of using dma_alloc_coherent().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 1d207c14afc8..ea4d71bb0ad3 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -6,6 +6,7 @@
#include <linux/arm-smccc.h>
#include <linux/bitfield.h>
#include <linux/bits.h>
+#include <linux/cleanup.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/cpumask.h>
@@ -1019,14 +1020,13 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
struct qcom_scm_mem_map_info *mem_to_map;
phys_addr_t mem_to_map_phys;
phys_addr_t dest_phys;
- dma_addr_t ptr_phys;
+ phys_addr_t ptr_phys;
size_t mem_to_map_sz;
size_t dest_sz;
size_t src_sz;
size_t ptr_sz;
int next_vm;
__le32 *src;
- void *ptr;
int ret, i, b;
u64 srcvm_bits = *srcvm;
@@ -1036,10 +1036,13 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
ptr_sz = ALIGN(src_sz, SZ_64) + ALIGN(mem_to_map_sz, SZ_64) +
ALIGN(dest_sz, SZ_64);
- ptr = dma_alloc_coherent(__scm->dev, ptr_sz, &ptr_phys, GFP_KERNEL);
+ void *ptr __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
+ ptr_sz, GFP_KERNEL);
if (!ptr)
return -ENOMEM;
+ ptr_phys = qcom_tzmem_to_phys(ptr);
+
/* Fill source vmid detail */
src = ptr;
i = 0;
@@ -1068,7 +1071,6 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
ret = __qcom_scm_assign_mem(__scm->dev, mem_to_map_phys, mem_to_map_sz,
ptr_phys, src_sz, dest_phys, dest_sz);
- dma_free_coherent(__scm->dev, ptr_sz, ptr, ptr_phys);
if (ret) {
dev_err(__scm->dev,
"Assign memory protection call failed %d\n", ret);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 06/15] firmware: qcom: scm: make qcom_scm_ice_set_key() use the TZ allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (4 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 05/15] firmware: qcom: scm: make qcom_scm_assign_mem() use the TZ allocator Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 07/15] firmware: qcom: scm: make qcom_scm_lmh_dcvsh() " Bartosz Golaszewski
` (10 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Let's use the new TZ memory allocator to obtain a buffer for this call
instead of using dma_alloc_coherent().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index ea4d71bb0ad3..2c350de1df07 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1218,32 +1218,21 @@ int qcom_scm_ice_set_key(u32 index, const u8 *key, u32 key_size,
.args[4] = data_unit_size,
.owner = ARM_SMCCC_OWNER_SIP,
};
- void *keybuf;
- dma_addr_t key_phys;
+
int ret;
- /*
- * 'key' may point to vmalloc()'ed memory, but we need to pass a
- * physical address that's been properly flushed. The sanctioned way to
- * do this is by using the DMA API. But as is best practice for crypto
- * keys, we also must wipe the key after use. This makes kmemdup() +
- * dma_map_single() not clearly correct, since the DMA API can use
- * bounce buffers. Instead, just use dma_alloc_coherent(). Programming
- * keys is normally rare and thus not performance-critical.
- */
-
- keybuf = dma_alloc_coherent(__scm->dev, key_size, &key_phys,
- GFP_KERNEL);
+ void *keybuf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
+ key_size,
+ GFP_KERNEL);
if (!keybuf)
return -ENOMEM;
memcpy(keybuf, key, key_size);
- desc.args[1] = key_phys;
+ desc.args[1] = qcom_tzmem_to_phys(keybuf);
ret = qcom_scm_call(__scm->dev, &desc, NULL);
memzero_explicit(keybuf, key_size);
- dma_free_coherent(__scm->dev, key_size, keybuf, key_phys);
return ret;
}
EXPORT_SYMBOL_GPL(qcom_scm_ice_set_key);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 07/15] firmware: qcom: scm: make qcom_scm_lmh_dcvsh() use the TZ allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (5 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 06/15] firmware: qcom: scm: make qcom_scm_ice_set_key() " Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 08/15] firmware: qcom: scm: make qcom_scm_qseecom_app_get_id() " Bartosz Golaszewski
` (9 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Let's use the new TZ memory allocator to obtain a buffer for this call
instead of using dma_alloc_coherent().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 2c350de1df07..431c279df12b 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1361,8 +1361,6 @@ EXPORT_SYMBOL_GPL(qcom_scm_lmh_profile_change);
int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
u64 limit_node, u32 node_id, u64 version)
{
- dma_addr_t payload_phys;
- u32 *payload_buf;
int ret, payload_size = 5 * sizeof(u32);
struct qcom_scm_desc desc = {
@@ -1377,7 +1375,9 @@ int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
.owner = ARM_SMCCC_OWNER_SIP,
};
- payload_buf = dma_alloc_coherent(__scm->dev, payload_size, &payload_phys, GFP_KERNEL);
+ u32 *payload_buf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
+ payload_size,
+ GFP_KERNEL);
if (!payload_buf)
return -ENOMEM;
@@ -1387,11 +1387,10 @@ int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
payload_buf[3] = 1;
payload_buf[4] = payload_val;
- desc.args[0] = payload_phys;
+ desc.args[0] = qcom_tzmem_to_phys(payload_buf);
ret = qcom_scm_call(__scm->dev, &desc, NULL);
- dma_free_coherent(__scm->dev, payload_size, payload_buf, payload_phys);
return ret;
}
EXPORT_SYMBOL_GPL(qcom_scm_lmh_dcvsh);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 08/15] firmware: qcom: scm: make qcom_scm_qseecom_app_get_id() use the TZ allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (6 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 07/15] firmware: qcom: scm: make qcom_scm_lmh_dcvsh() " Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-05-27 12:54 ` [PATCH v10 09/15] firmware: qcom: qseecom: convert to using " Bartosz Golaszewski
` (8 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Let's use the new TZ memory allocator to obtain a buffer for this call
instead of manually kmalloc()ing it and then mapping to physical space.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Maximilian Luz <luzmaximilian@gmail.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 431c279df12b..1b64a4f4d07d 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1546,37 +1546,27 @@ int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id)
unsigned long app_name_len = strlen(app_name);
struct qcom_scm_desc desc = {};
struct qcom_scm_qseecom_resp res = {};
- dma_addr_t name_buf_phys;
- char *name_buf;
int status;
if (app_name_len >= name_buf_size)
return -EINVAL;
- name_buf = kzalloc(name_buf_size, GFP_KERNEL);
+ char *name_buf __free(qcom_tzmem) = qcom_tzmem_alloc(__scm->mempool,
+ name_buf_size,
+ GFP_KERNEL);
if (!name_buf)
return -ENOMEM;
memcpy(name_buf, app_name, app_name_len);
- name_buf_phys = dma_map_single(__scm->dev, name_buf, name_buf_size, DMA_TO_DEVICE);
- status = dma_mapping_error(__scm->dev, name_buf_phys);
- if (status) {
- kfree(name_buf);
- dev_err(__scm->dev, "qseecom: failed to map dma address\n");
- return status;
- }
-
desc.owner = QSEECOM_TZ_OWNER_QSEE_OS;
desc.svc = QSEECOM_TZ_SVC_APP_MGR;
desc.cmd = QSEECOM_TZ_CMD_APP_LOOKUP;
desc.arginfo = QCOM_SCM_ARGS(2, QCOM_SCM_RW, QCOM_SCM_VAL);
- desc.args[0] = name_buf_phys;
+ desc.args[0] = qcom_tzmem_to_phys(name_buf);
desc.args[1] = app_name_len;
status = qcom_scm_qseecom_call(&desc, &res);
- dma_unmap_single(__scm->dev, name_buf_phys, name_buf_size, DMA_TO_DEVICE);
- kfree(name_buf);
if (status)
return status;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 09/15] firmware: qcom: qseecom: convert to using the TZ allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (7 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 08/15] firmware: qcom: scm: make qcom_scm_qseecom_app_get_id() " Bartosz Golaszewski
@ 2024-05-27 12:54 ` Bartosz Golaszewski
2024-06-20 23:14 ` Elliot Berman
2024-06-21 1:24 ` Amirreza Zarrabi
2024-05-27 12:55 ` [PATCH v10 10/15] firmware: qcom: scm: add support for SHM bridge operations Bartosz Golaszewski
` (7 subsequent siblings)
16 siblings, 2 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:54 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Drop the DMA mapping operations from qcom_scm_qseecom_app_send() and
convert all users of it in the qseecom module to using the TZ allocator
for creating SCM call buffers. As this is largely a module separate from
the SCM driver, let's use a separate memory pool. Set the initial size to
4K and - if we run out - add twice the current amount to the pool.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/firmware/qcom/qcom_qseecom_uefisecapp.c | 256 +++++++++---------------
drivers/firmware/qcom/qcom_scm.c | 17 +-
include/linux/firmware/qcom/qcom_qseecom.h | 8 +-
include/linux/firmware/qcom/qcom_scm.h | 8 +-
4 files changed, 117 insertions(+), 172 deletions(-)
diff --git a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
index bc550ad0dbe0..6fefa4fe80e8 100644
--- a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
+++ b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
@@ -13,11 +13,14 @@
#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/sizes.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/ucs2_string.h>
#include <linux/firmware/qcom/qcom_qseecom.h>
+#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/firmware/qcom/qcom_tzmem.h>
/* -- Qualcomm "uefisecapp" interface definitions. -------------------------- */
@@ -272,6 +275,7 @@ struct qsee_rsp_uefi_query_variable_info {
struct qcuefi_client {
struct qseecom_client *client;
struct efivars efivars;
+ struct qcom_tzmem_pool *mempool;
};
static struct device *qcuefi_dev(struct qcuefi_client *qcuefi)
@@ -293,12 +297,11 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
{
struct qsee_req_uefi_get_variable *req_data;
struct qsee_rsp_uefi_get_variable *rsp_data;
+ void *cmd_buf __free(qcom_tzmem) = NULL;
unsigned long buffer_size = *data_size;
- efi_status_t efi_status = EFI_SUCCESS;
unsigned long name_length;
- dma_addr_t cmd_buf_dma;
+ efi_status_t efi_status;
size_t cmd_buf_size;
- void *cmd_buf;
size_t guid_offs;
size_t name_offs;
size_t req_size;
@@ -333,11 +336,9 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
__reqdata_offs(rsp_size, &rsp_offs)
);
- cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
- if (!cmd_buf) {
- efi_status = EFI_OUT_OF_RESOURCES;
- goto out;
- }
+ cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
+ if (!cmd_buf)
+ return EFI_OUT_OF_RESOURCES;
req_data = cmd_buf + req_offs;
rsp_data = cmd_buf + rsp_offs;
@@ -351,30 +352,22 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
req_data->length = req_size;
status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
- if (status < 0) {
- efi_status = EFI_INVALID_PARAMETER;
- goto out_free;
- }
+ if (status < 0)
+ return EFI_INVALID_PARAMETER;
memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
status = qcom_qseecom_app_send(qcuefi->client,
- cmd_buf_dma + req_offs, req_size,
- cmd_buf_dma + rsp_offs, rsp_size);
- if (status) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ cmd_buf + req_offs, req_size,
+ cmd_buf + rsp_offs, rsp_size);
+ if (status)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->length < sizeof(*rsp_data)) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length < sizeof(*rsp_data))
+ return EFI_DEVICE_ERROR;
if (rsp_data->status) {
dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
@@ -388,18 +381,14 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
*attributes = rsp_data->attributes;
}
- goto out_free;
+ return qsee_uefi_status_to_efi(rsp_data->status);
}
- if (rsp_data->length > rsp_size) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length > rsp_size)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length)
+ return EFI_DEVICE_ERROR;
/*
* Note: We need to set attributes and data size even if the buffer is
@@ -422,22 +411,15 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
if (attributes)
*attributes = rsp_data->attributes;
- if (buffer_size == 0 && !data) {
- efi_status = EFI_SUCCESS;
- goto out_free;
- }
+ if (buffer_size == 0 && !data)
+ return EFI_SUCCESS;
- if (buffer_size < rsp_data->data_size) {
- efi_status = EFI_BUFFER_TOO_SMALL;
- goto out_free;
- }
+ if (buffer_size < rsp_data->data_size)
+ return EFI_BUFFER_TOO_SMALL;
memcpy(data, ((void *)rsp_data) + rsp_data->data_offset, rsp_data->data_size);
-out_free:
- qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
-out:
- return efi_status;
+ return EFI_SUCCESS;
}
static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const efi_char16_t *name,
@@ -446,11 +428,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
{
struct qsee_req_uefi_set_variable *req_data;
struct qsee_rsp_uefi_set_variable *rsp_data;
- efi_status_t efi_status = EFI_SUCCESS;
+ void *cmd_buf __free(qcom_tzmem) = NULL;
unsigned long name_length;
- dma_addr_t cmd_buf_dma;
size_t cmd_buf_size;
- void *cmd_buf;
size_t name_offs;
size_t guid_offs;
size_t data_offs;
@@ -486,11 +466,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
__reqdata_offs(sizeof(*rsp_data), &rsp_offs)
);
- cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
- if (!cmd_buf) {
- efi_status = EFI_OUT_OF_RESOURCES;
- goto out;
- }
+ cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
+ if (!cmd_buf)
+ return EFI_OUT_OF_RESOURCES;
req_data = cmd_buf + req_offs;
rsp_data = cmd_buf + rsp_offs;
@@ -506,10 +484,8 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
req_data->length = req_size;
status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
- if (status < 0) {
- efi_status = EFI_INVALID_PARAMETER;
- goto out_free;
- }
+ if (status < 0)
+ return EFI_INVALID_PARAMETER;
memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
@@ -517,33 +493,24 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
memcpy(((void *)req_data) + req_data->data_offset, data, req_data->data_size);
status = qcom_qseecom_app_send(qcuefi->client,
- cmd_buf_dma + req_offs, req_size,
- cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
- if (status) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ cmd_buf + req_offs, req_size,
+ cmd_buf + rsp_offs, sizeof(*rsp_data));
+ if (status)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->length != sizeof(*rsp_data)) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length != sizeof(*rsp_data))
+ return EFI_DEVICE_ERROR;
if (rsp_data->status) {
dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
__func__, rsp_data->status);
- efi_status = qsee_uefi_status_to_efi(rsp_data->status);
+ return qsee_uefi_status_to_efi(rsp_data->status);
}
-out_free:
- qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
-out:
- return efi_status;
+ return EFI_SUCCESS;
}
static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
@@ -552,10 +519,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
{
struct qsee_req_uefi_get_next_variable *req_data;
struct qsee_rsp_uefi_get_next_variable *rsp_data;
- efi_status_t efi_status = EFI_SUCCESS;
- dma_addr_t cmd_buf_dma;
+ void *cmd_buf __free(qcom_tzmem) = NULL;
+ efi_status_t efi_status;
size_t cmd_buf_size;
- void *cmd_buf;
size_t guid_offs;
size_t name_offs;
size_t req_size;
@@ -587,11 +553,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
__reqdata_offs(rsp_size, &rsp_offs)
);
- cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
- if (!cmd_buf) {
- efi_status = EFI_OUT_OF_RESOURCES;
- goto out;
- }
+ cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
+ if (!cmd_buf)
+ return EFI_OUT_OF_RESOURCES;
req_data = cmd_buf + req_offs;
rsp_data = cmd_buf + rsp_offs;
@@ -606,28 +570,20 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name,
*name_size / sizeof(*name));
- if (status < 0) {
- efi_status = EFI_INVALID_PARAMETER;
- goto out_free;
- }
+ if (status < 0)
+ return EFI_INVALID_PARAMETER;
status = qcom_qseecom_app_send(qcuefi->client,
- cmd_buf_dma + req_offs, req_size,
- cmd_buf_dma + rsp_offs, rsp_size);
- if (status) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ cmd_buf + req_offs, req_size,
+ cmd_buf + rsp_offs, rsp_size);
+ if (status)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->length < sizeof(*rsp_data)) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length < sizeof(*rsp_data))
+ return EFI_DEVICE_ERROR;
if (rsp_data->status) {
dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
@@ -642,53 +598,40 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
if (efi_status == EFI_BUFFER_TOO_SMALL)
*name_size = rsp_data->name_size;
- goto out_free;
+ return efi_status;
}
- if (rsp_data->length > rsp_size) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length > rsp_size)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length)
+ return EFI_DEVICE_ERROR;
if (rsp_data->name_size > *name_size) {
*name_size = rsp_data->name_size;
- efi_status = EFI_BUFFER_TOO_SMALL;
- goto out_free;
+ return EFI_BUFFER_TOO_SMALL;
}
- if (rsp_data->guid_size != sizeof(*guid)) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->guid_size != sizeof(*guid))
+ return EFI_DEVICE_ERROR;
memcpy(guid, ((void *)rsp_data) + rsp_data->guid_offset, rsp_data->guid_size);
status = ucs2_strscpy(name, ((void *)rsp_data) + rsp_data->name_offset,
rsp_data->name_size / sizeof(*name));
*name_size = rsp_data->name_size;
- if (status < 0) {
+ if (status < 0)
/*
* Return EFI_DEVICE_ERROR here because the buffer size should
* have already been validated above, causing this function to
* bail with EFI_BUFFER_TOO_SMALL.
*/
- efi_status = EFI_DEVICE_ERROR;
- }
+ return EFI_DEVICE_ERROR;
-out_free:
- qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
-out:
- return efi_status;
+ return EFI_SUCCESS;
}
static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi, u32 attr,
@@ -697,10 +640,8 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
{
struct qsee_req_uefi_query_variable_info *req_data;
struct qsee_rsp_uefi_query_variable_info *rsp_data;
- efi_status_t efi_status = EFI_SUCCESS;
- dma_addr_t cmd_buf_dma;
+ void *cmd_buf __free(qcom_tzmem) = NULL;
size_t cmd_buf_size;
- void *cmd_buf;
size_t req_offs;
size_t rsp_offs;
int status;
@@ -710,11 +651,9 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
__reqdata_offs(sizeof(*rsp_data), &rsp_offs)
);
- cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
- if (!cmd_buf) {
- efi_status = EFI_OUT_OF_RESOURCES;
- goto out;
- }
+ cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
+ if (!cmd_buf)
+ return EFI_OUT_OF_RESOURCES;
req_data = cmd_buf + req_offs;
rsp_data = cmd_buf + rsp_offs;
@@ -724,28 +663,21 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
req_data->length = sizeof(*req_data);
status = qcom_qseecom_app_send(qcuefi->client,
- cmd_buf_dma + req_offs, sizeof(*req_data),
- cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
- if (status) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ cmd_buf + req_offs, sizeof(*req_data),
+ cmd_buf + rsp_offs, sizeof(*rsp_data));
+ if (status)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO)
+ return EFI_DEVICE_ERROR;
- if (rsp_data->length != sizeof(*rsp_data)) {
- efi_status = EFI_DEVICE_ERROR;
- goto out_free;
- }
+ if (rsp_data->length != sizeof(*rsp_data))
+ return EFI_DEVICE_ERROR;
if (rsp_data->status) {
dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
__func__, rsp_data->status);
- efi_status = qsee_uefi_status_to_efi(rsp_data->status);
- goto out_free;
+ return qsee_uefi_status_to_efi(rsp_data->status);
}
if (storage_space)
@@ -757,10 +689,7 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
if (max_variable_size)
*max_variable_size = rsp_data->max_variable_size;
-out_free:
- qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
-out:
- return efi_status;
+ return EFI_SUCCESS;
}
/* -- Global efivar interface. ---------------------------------------------- */
@@ -871,6 +800,7 @@ static const struct efivar_operations qcom_efivar_ops = {
static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
const struct auxiliary_device_id *aux_dev_id)
{
+ struct qcom_tzmem_pool_config pool_config;
struct qcuefi_client *qcuefi;
int status;
@@ -889,6 +819,16 @@ static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
if (status)
qcuefi_set_reference(NULL);
+ memset(&pool_config, 0, sizeof(pool_config));
+ pool_config.initial_size = SZ_4K;
+ pool_config.policy = QCOM_TZMEM_POLICY_MULTIPLIER;
+ pool_config.increment = 2;
+ pool_config.max_size = SZ_256K;
+
+ qcuefi->mempool = devm_qcom_tzmem_pool_new(&aux_dev->dev, &pool_config);
+ if (IS_ERR(qcuefi->mempool))
+ return PTR_ERR(qcuefi->mempool);
+
return status;
}
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 1b64a4f4d07d..04131e60b63a 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1588,9 +1588,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
/**
* qcom_scm_qseecom_app_send() - Send to and receive data from a given QSEE app.
* @app_id: The ID of the target app.
- * @req: DMA address of the request buffer sent to the app.
+ * @req: Request buffer sent to the app (must be TZ memory)
* @req_size: Size of the request buffer.
- * @rsp: DMA address of the response buffer, written to by the app.
+ * @rsp: Response buffer, written to by the app (must be TZ memory)
* @rsp_size: Size of the response buffer.
*
* Sends a request to the QSEE app associated with the given ID and read back
@@ -1601,13 +1601,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
*
* Return: Zero on success, nonzero on failure.
*/
-int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
- dma_addr_t rsp, size_t rsp_size)
+int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
+ void *rsp, size_t rsp_size)
{
struct qcom_scm_qseecom_resp res = {};
struct qcom_scm_desc desc = {};
+ phys_addr_t req_phys;
+ phys_addr_t rsp_phys;
int status;
+ req_phys = qcom_tzmem_to_phys(req);
+ rsp_phys = qcom_tzmem_to_phys(rsp);
+
desc.owner = QSEECOM_TZ_OWNER_TZ_APPS;
desc.svc = QSEECOM_TZ_SVC_APP_ID_PLACEHOLDER;
desc.cmd = QSEECOM_TZ_CMD_APP_SEND;
@@ -1615,9 +1620,9 @@ int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
QCOM_SCM_RW, QCOM_SCM_VAL,
QCOM_SCM_RW, QCOM_SCM_VAL);
desc.args[0] = app_id;
- desc.args[1] = req;
+ desc.args[1] = req_phys;
desc.args[2] = req_size;
- desc.args[3] = rsp;
+ desc.args[3] = rsp_phys;
desc.args[4] = rsp_size;
status = qcom_scm_qseecom_call(&desc, &res);
diff --git a/include/linux/firmware/qcom/qcom_qseecom.h b/include/linux/firmware/qcom/qcom_qseecom.h
index 366243ee9609..1dc5b3b50aa9 100644
--- a/include/linux/firmware/qcom/qcom_qseecom.h
+++ b/include/linux/firmware/qcom/qcom_qseecom.h
@@ -73,9 +73,9 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
/**
* qcom_qseecom_app_send() - Send to and receive data from a given QSEE app.
* @client: The QSEECOM client associated with the target app.
- * @req: DMA address of the request buffer sent to the app.
+ * @req: Request buffer sent to the app (must be TZ memory).
* @req_size: Size of the request buffer.
- * @rsp: DMA address of the response buffer, written to by the app.
+ * @rsp: Response buffer, written to by the app (must be TZ memory).
* @rsp_size: Size of the response buffer.
*
* Sends a request to the QSEE app associated with the given client and read
@@ -90,8 +90,8 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
* Return: Zero on success, nonzero on failure.
*/
static inline int qcom_qseecom_app_send(struct qseecom_client *client,
- dma_addr_t req, size_t req_size,
- dma_addr_t rsp, size_t rsp_size)
+ void *req, size_t req_size,
+ void *rsp, size_t rsp_size)
{
return qcom_scm_qseecom_app_send(client->app_id, req, req_size, rsp, rsp_size);
}
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index aaa19f93ac43..2c51987fb70a 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -118,8 +118,8 @@ bool qcom_scm_lmh_dcvsh_available(void);
#ifdef CONFIG_QCOM_QSEECOM
int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id);
-int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
- dma_addr_t rsp, size_t rsp_size);
+int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
+ void *rsp, size_t rsp_size);
#else /* CONFIG_QCOM_QSEECOM */
@@ -129,8 +129,8 @@ static inline int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id)
}
static inline int qcom_scm_qseecom_app_send(u32 app_id,
- dma_addr_t req, size_t req_size,
- dma_addr_t rsp, size_t rsp_size)
+ void *req, size_t req_size,
+ void *rsp, size_t rsp_size)
{
return -EINVAL;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v10 09/15] firmware: qcom: qseecom: convert to using the TZ allocator
2024-05-27 12:54 ` [PATCH v10 09/15] firmware: qcom: qseecom: convert to using " Bartosz Golaszewski
@ 2024-06-20 23:14 ` Elliot Berman
2024-06-21 1:24 ` Amirreza Zarrabi
1 sibling, 0 replies; 27+ messages in thread
From: Elliot Berman @ 2024-06-20 23:14 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel
On Mon, May 27, 2024 at 02:54:59PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Drop the DMA mapping operations from qcom_scm_qseecom_app_send() and
> convert all users of it in the qseecom module to using the TZ allocator
> for creating SCM call buffers. As this is largely a module separate from
> the SCM driver, let's use a separate memory pool. Set the initial size to
> 4K and - if we run out - add twice the current amount to the pool.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
From the perspective of "does the conversion look good",
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
I don't have enough expertise in qseecom to say if anything else needed,
though.
Thanks,
Elliot
> ---
> drivers/firmware/qcom/qcom_qseecom_uefisecapp.c | 256 +++++++++---------------
> drivers/firmware/qcom/qcom_scm.c | 17 +-
> include/linux/firmware/qcom/qcom_qseecom.h | 8 +-
> include/linux/firmware/qcom/qcom_scm.h | 8 +-
> 4 files changed, 117 insertions(+), 172 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> index bc550ad0dbe0..6fefa4fe80e8 100644
> --- a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> +++ b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> @@ -13,11 +13,14 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/ucs2_string.h>
>
> #include <linux/firmware/qcom/qcom_qseecom.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_tzmem.h>
>
> /* -- Qualcomm "uefisecapp" interface definitions. -------------------------- */
>
> @@ -272,6 +275,7 @@ struct qsee_rsp_uefi_query_variable_info {
> struct qcuefi_client {
> struct qseecom_client *client;
> struct efivars efivars;
> + struct qcom_tzmem_pool *mempool;
> };
>
> static struct device *qcuefi_dev(struct qcuefi_client *qcuefi)
> @@ -293,12 +297,11 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> {
> struct qsee_req_uefi_get_variable *req_data;
> struct qsee_rsp_uefi_get_variable *rsp_data;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> unsigned long buffer_size = *data_size;
> - efi_status_t efi_status = EFI_SUCCESS;
> unsigned long name_length;
> - dma_addr_t cmd_buf_dma;
> + efi_status_t efi_status;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t guid_offs;
> size_t name_offs;
> size_t req_size;
> @@ -333,11 +336,9 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> __reqdata_offs(rsp_size, &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -351,30 +352,22 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> req_data->length = req_size;
>
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, rsp_size);
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, rsp_size);
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length < sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length < sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> @@ -388,18 +381,14 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> *attributes = rsp_data->attributes;
> }
>
> - goto out_free;
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> - if (rsp_data->length > rsp_size) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length > rsp_size)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> /*
> * Note: We need to set attributes and data size even if the buffer is
> @@ -422,22 +411,15 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> if (attributes)
> *attributes = rsp_data->attributes;
>
> - if (buffer_size == 0 && !data) {
> - efi_status = EFI_SUCCESS;
> - goto out_free;
> - }
> + if (buffer_size == 0 && !data)
> + return EFI_SUCCESS;
>
> - if (buffer_size < rsp_data->data_size) {
> - efi_status = EFI_BUFFER_TOO_SMALL;
> - goto out_free;
> - }
> + if (buffer_size < rsp_data->data_size)
> + return EFI_BUFFER_TOO_SMALL;
>
> memcpy(data, ((void *)rsp_data) + rsp_data->data_offset, rsp_data->data_size);
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const efi_char16_t *name,
> @@ -446,11 +428,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> {
> struct qsee_req_uefi_set_variable *req_data;
> struct qsee_rsp_uefi_set_variable *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> unsigned long name_length;
> - dma_addr_t cmd_buf_dma;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t name_offs;
> size_t guid_offs;
> size_t data_offs;
> @@ -486,11 +466,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> __reqdata_offs(sizeof(*rsp_data), &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -506,10 +484,8 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> req_data->length = req_size;
>
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
>
> @@ -517,33 +493,24 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> memcpy(((void *)req_data) + req_data->data_offset, data, req_data->data_size);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, sizeof(*rsp_data));
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length != sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length != sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> __func__, rsp_data->status);
> - efi_status = qsee_uefi_status_to_efi(rsp_data->status);
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> @@ -552,10 +519,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> {
> struct qsee_req_uefi_get_next_variable *req_data;
> struct qsee_rsp_uefi_get_next_variable *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> - dma_addr_t cmd_buf_dma;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> + efi_status_t efi_status;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t guid_offs;
> size_t name_offs;
> size_t req_size;
> @@ -587,11 +553,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> __reqdata_offs(rsp_size, &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -606,28 +570,20 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name,
> *name_size / sizeof(*name));
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, rsp_size);
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, rsp_size);
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length < sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length < sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> @@ -642,53 +598,40 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> if (efi_status == EFI_BUFFER_TOO_SMALL)
> *name_size = rsp_data->name_size;
>
> - goto out_free;
> + return efi_status;
> }
>
> - if (rsp_data->length > rsp_size) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length > rsp_size)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->name_size > *name_size) {
> *name_size = rsp_data->name_size;
> - efi_status = EFI_BUFFER_TOO_SMALL;
> - goto out_free;
> + return EFI_BUFFER_TOO_SMALL;
> }
>
> - if (rsp_data->guid_size != sizeof(*guid)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->guid_size != sizeof(*guid))
> + return EFI_DEVICE_ERROR;
>
> memcpy(guid, ((void *)rsp_data) + rsp_data->guid_offset, rsp_data->guid_size);
> status = ucs2_strscpy(name, ((void *)rsp_data) + rsp_data->name_offset,
> rsp_data->name_size / sizeof(*name));
> *name_size = rsp_data->name_size;
>
> - if (status < 0) {
> + if (status < 0)
> /*
> * Return EFI_DEVICE_ERROR here because the buffer size should
> * have already been validated above, causing this function to
> * bail with EFI_BUFFER_TOO_SMALL.
> */
> - efi_status = EFI_DEVICE_ERROR;
> - }
> + return EFI_DEVICE_ERROR;
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi, u32 attr,
> @@ -697,10 +640,8 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> {
> struct qsee_req_uefi_query_variable_info *req_data;
> struct qsee_rsp_uefi_query_variable_info *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> - dma_addr_t cmd_buf_dma;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t req_offs;
> size_t rsp_offs;
> int status;
> @@ -710,11 +651,9 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> __reqdata_offs(sizeof(*rsp_data), &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -724,28 +663,21 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> req_data->length = sizeof(*req_data);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, sizeof(*req_data),
> - cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, sizeof(*req_data),
> + cmd_buf + rsp_offs, sizeof(*rsp_data));
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length != sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length != sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> __func__, rsp_data->status);
> - efi_status = qsee_uefi_status_to_efi(rsp_data->status);
> - goto out_free;
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> if (storage_space)
> @@ -757,10 +689,7 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> if (max_variable_size)
> *max_variable_size = rsp_data->max_variable_size;
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> /* -- Global efivar interface. ---------------------------------------------- */
> @@ -871,6 +800,7 @@ static const struct efivar_operations qcom_efivar_ops = {
> static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
> const struct auxiliary_device_id *aux_dev_id)
> {
> + struct qcom_tzmem_pool_config pool_config;
> struct qcuefi_client *qcuefi;
> int status;
>
> @@ -889,6 +819,16 @@ static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
> if (status)
> qcuefi_set_reference(NULL);
>
> + memset(&pool_config, 0, sizeof(pool_config));
> + pool_config.initial_size = SZ_4K;
> + pool_config.policy = QCOM_TZMEM_POLICY_MULTIPLIER;
> + pool_config.increment = 2;
> + pool_config.max_size = SZ_256K;
> +
> + qcuefi->mempool = devm_qcom_tzmem_pool_new(&aux_dev->dev, &pool_config);
> + if (IS_ERR(qcuefi->mempool))
> + return PTR_ERR(qcuefi->mempool);
> +
> return status;
> }
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 1b64a4f4d07d..04131e60b63a 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1588,9 +1588,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
> /**
> * qcom_scm_qseecom_app_send() - Send to and receive data from a given QSEE app.
> * @app_id: The ID of the target app.
> - * @req: DMA address of the request buffer sent to the app.
> + * @req: Request buffer sent to the app (must be TZ memory)
> * @req_size: Size of the request buffer.
> - * @rsp: DMA address of the response buffer, written to by the app.
> + * @rsp: Response buffer, written to by the app (must be TZ memory)
> * @rsp_size: Size of the response buffer.
> *
> * Sends a request to the QSEE app associated with the given ID and read back
> @@ -1601,13 +1601,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
> *
> * Return: Zero on success, nonzero on failure.
> */
> -int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> +int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> struct qcom_scm_qseecom_resp res = {};
> struct qcom_scm_desc desc = {};
> + phys_addr_t req_phys;
> + phys_addr_t rsp_phys;
> int status;
>
> + req_phys = qcom_tzmem_to_phys(req);
> + rsp_phys = qcom_tzmem_to_phys(rsp);
> +
> desc.owner = QSEECOM_TZ_OWNER_TZ_APPS;
> desc.svc = QSEECOM_TZ_SVC_APP_ID_PLACEHOLDER;
> desc.cmd = QSEECOM_TZ_CMD_APP_SEND;
> @@ -1615,9 +1620,9 @@ int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> QCOM_SCM_RW, QCOM_SCM_VAL,
> QCOM_SCM_RW, QCOM_SCM_VAL);
> desc.args[0] = app_id;
> - desc.args[1] = req;
> + desc.args[1] = req_phys;
> desc.args[2] = req_size;
> - desc.args[3] = rsp;
> + desc.args[3] = rsp_phys;
> desc.args[4] = rsp_size;
>
> status = qcom_scm_qseecom_call(&desc, &res);
> diff --git a/include/linux/firmware/qcom/qcom_qseecom.h b/include/linux/firmware/qcom/qcom_qseecom.h
> index 366243ee9609..1dc5b3b50aa9 100644
> --- a/include/linux/firmware/qcom/qcom_qseecom.h
> +++ b/include/linux/firmware/qcom/qcom_qseecom.h
> @@ -73,9 +73,9 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
> /**
> * qcom_qseecom_app_send() - Send to and receive data from a given QSEE app.
> * @client: The QSEECOM client associated with the target app.
> - * @req: DMA address of the request buffer sent to the app.
> + * @req: Request buffer sent to the app (must be TZ memory).
> * @req_size: Size of the request buffer.
> - * @rsp: DMA address of the response buffer, written to by the app.
> + * @rsp: Response buffer, written to by the app (must be TZ memory).
> * @rsp_size: Size of the response buffer.
> *
> * Sends a request to the QSEE app associated with the given client and read
> @@ -90,8 +90,8 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
> * Return: Zero on success, nonzero on failure.
> */
> static inline int qcom_qseecom_app_send(struct qseecom_client *client,
> - dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> + void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> return qcom_scm_qseecom_app_send(client->app_id, req, req_size, rsp, rsp_size);
> }
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index aaa19f93ac43..2c51987fb70a 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -118,8 +118,8 @@ bool qcom_scm_lmh_dcvsh_available(void);
> #ifdef CONFIG_QCOM_QSEECOM
>
> int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id);
> -int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size);
> +int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
> + void *rsp, size_t rsp_size);
>
> #else /* CONFIG_QCOM_QSEECOM */
>
> @@ -129,8 +129,8 @@ static inline int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id)
> }
>
> static inline int qcom_scm_qseecom_app_send(u32 app_id,
> - dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> + void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> return -EINVAL;
> }
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v10 09/15] firmware: qcom: qseecom: convert to using the TZ allocator
2024-05-27 12:54 ` [PATCH v10 09/15] firmware: qcom: qseecom: convert to using " Bartosz Golaszewski
2024-06-20 23:14 ` Elliot Berman
@ 2024-06-21 1:24 ` Amirreza Zarrabi
1 sibling, 0 replies; 27+ messages in thread
From: Amirreza Zarrabi @ 2024-06-21 1:24 UTC (permalink / raw)
To: Bartosz Golaszewski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko, Das Srinagesh,
Bartosz Golaszewski, Maximilian Luz, Catalin Marinas, Will Deacon,
Srini Kandagatla, Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
On 5/27/2024 10:54 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Drop the DMA mapping operations from qcom_scm_qseecom_app_send() and
> convert all users of it in the qseecom module to using the TZ allocator
> for creating SCM call buffers. As this is largely a module separate from
> the SCM driver, let's use a separate memory pool. Set the initial size to
> 4K and - if we run out - add twice the current amount to the pool.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Seems right to me.
Reviewed-by: Amirreza Zarrabi <quic_azarrabi@quicinc.com>
Regards,
Amir
> ---
> drivers/firmware/qcom/qcom_qseecom_uefisecapp.c | 256 +++++++++---------------
> drivers/firmware/qcom/qcom_scm.c | 17 +-
> include/linux/firmware/qcom/qcom_qseecom.h | 8 +-
> include/linux/firmware/qcom/qcom_scm.h | 8 +-
> 4 files changed, 117 insertions(+), 172 deletions(-)
>
> diff --git a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> index bc550ad0dbe0..6fefa4fe80e8 100644
> --- a/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> +++ b/drivers/firmware/qcom/qcom_qseecom_uefisecapp.c
> @@ -13,11 +13,14 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/ucs2_string.h>
>
> #include <linux/firmware/qcom/qcom_qseecom.h>
> +#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_tzmem.h>
>
> /* -- Qualcomm "uefisecapp" interface definitions. -------------------------- */
>
> @@ -272,6 +275,7 @@ struct qsee_rsp_uefi_query_variable_info {
> struct qcuefi_client {
> struct qseecom_client *client;
> struct efivars efivars;
> + struct qcom_tzmem_pool *mempool;
> };
>
> static struct device *qcuefi_dev(struct qcuefi_client *qcuefi)
> @@ -293,12 +297,11 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> {
> struct qsee_req_uefi_get_variable *req_data;
> struct qsee_rsp_uefi_get_variable *rsp_data;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> unsigned long buffer_size = *data_size;
> - efi_status_t efi_status = EFI_SUCCESS;
> unsigned long name_length;
> - dma_addr_t cmd_buf_dma;
> + efi_status_t efi_status;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t guid_offs;
> size_t name_offs;
> size_t req_size;
> @@ -333,11 +336,9 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> __reqdata_offs(rsp_size, &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -351,30 +352,22 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> req_data->length = req_size;
>
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, rsp_size);
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, rsp_size);
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_GET_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length < sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length < sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> @@ -388,18 +381,14 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> *attributes = rsp_data->attributes;
> }
>
> - goto out_free;
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> - if (rsp_data->length > rsp_size) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length > rsp_size)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->data_offset + rsp_data->data_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> /*
> * Note: We need to set attributes and data size even if the buffer is
> @@ -422,22 +411,15 @@ static efi_status_t qsee_uefi_get_variable(struct qcuefi_client *qcuefi, const e
> if (attributes)
> *attributes = rsp_data->attributes;
>
> - if (buffer_size == 0 && !data) {
> - efi_status = EFI_SUCCESS;
> - goto out_free;
> - }
> + if (buffer_size == 0 && !data)
> + return EFI_SUCCESS;
>
> - if (buffer_size < rsp_data->data_size) {
> - efi_status = EFI_BUFFER_TOO_SMALL;
> - goto out_free;
> - }
> + if (buffer_size < rsp_data->data_size)
> + return EFI_BUFFER_TOO_SMALL;
>
> memcpy(data, ((void *)rsp_data) + rsp_data->data_offset, rsp_data->data_size);
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const efi_char16_t *name,
> @@ -446,11 +428,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> {
> struct qsee_req_uefi_set_variable *req_data;
> struct qsee_rsp_uefi_set_variable *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> unsigned long name_length;
> - dma_addr_t cmd_buf_dma;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t name_offs;
> size_t guid_offs;
> size_t data_offs;
> @@ -486,11 +466,9 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> __reqdata_offs(sizeof(*rsp_data), &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -506,10 +484,8 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> req_data->length = req_size;
>
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name, name_length);
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
>
> @@ -517,33 +493,24 @@ static efi_status_t qsee_uefi_set_variable(struct qcuefi_client *qcuefi, const e
> memcpy(((void *)req_data) + req_data->data_offset, data, req_data->data_size);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, sizeof(*rsp_data));
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_SET_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length != sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length != sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> __func__, rsp_data->status);
> - efi_status = qsee_uefi_status_to_efi(rsp_data->status);
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> @@ -552,10 +519,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> {
> struct qsee_req_uefi_get_next_variable *req_data;
> struct qsee_rsp_uefi_get_next_variable *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> - dma_addr_t cmd_buf_dma;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> + efi_status_t efi_status;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t guid_offs;
> size_t name_offs;
> size_t req_size;
> @@ -587,11 +553,9 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> __reqdata_offs(rsp_size, &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -606,28 +570,20 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> memcpy(((void *)req_data) + req_data->guid_offset, guid, req_data->guid_size);
> status = ucs2_strscpy(((void *)req_data) + req_data->name_offset, name,
> *name_size / sizeof(*name));
> - if (status < 0) {
> - efi_status = EFI_INVALID_PARAMETER;
> - goto out_free;
> - }
> + if (status < 0)
> + return EFI_INVALID_PARAMETER;
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, req_size,
> - cmd_buf_dma + rsp_offs, rsp_size);
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, req_size,
> + cmd_buf + rsp_offs, rsp_size);
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_GET_NEXT_VARIABLE)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length < sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length < sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> @@ -642,53 +598,40 @@ static efi_status_t qsee_uefi_get_next_variable(struct qcuefi_client *qcuefi,
> if (efi_status == EFI_BUFFER_TOO_SMALL)
> *name_size = rsp_data->name_size;
>
> - goto out_free;
> + return efi_status;
> }
>
> - if (rsp_data->length > rsp_size) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length > rsp_size)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->name_offset + rsp_data->name_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->guid_offset + rsp_data->guid_size > rsp_data->length)
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->name_size > *name_size) {
> *name_size = rsp_data->name_size;
> - efi_status = EFI_BUFFER_TOO_SMALL;
> - goto out_free;
> + return EFI_BUFFER_TOO_SMALL;
> }
>
> - if (rsp_data->guid_size != sizeof(*guid)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->guid_size != sizeof(*guid))
> + return EFI_DEVICE_ERROR;
>
> memcpy(guid, ((void *)rsp_data) + rsp_data->guid_offset, rsp_data->guid_size);
> status = ucs2_strscpy(name, ((void *)rsp_data) + rsp_data->name_offset,
> rsp_data->name_size / sizeof(*name));
> *name_size = rsp_data->name_size;
>
> - if (status < 0) {
> + if (status < 0)
> /*
> * Return EFI_DEVICE_ERROR here because the buffer size should
> * have already been validated above, causing this function to
> * bail with EFI_BUFFER_TOO_SMALL.
> */
> - efi_status = EFI_DEVICE_ERROR;
> - }
> + return EFI_DEVICE_ERROR;
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi, u32 attr,
> @@ -697,10 +640,8 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> {
> struct qsee_req_uefi_query_variable_info *req_data;
> struct qsee_rsp_uefi_query_variable_info *rsp_data;
> - efi_status_t efi_status = EFI_SUCCESS;
> - dma_addr_t cmd_buf_dma;
> + void *cmd_buf __free(qcom_tzmem) = NULL;
> size_t cmd_buf_size;
> - void *cmd_buf;
> size_t req_offs;
> size_t rsp_offs;
> int status;
> @@ -710,11 +651,9 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> __reqdata_offs(sizeof(*rsp_data), &rsp_offs)
> );
>
> - cmd_buf = qseecom_dma_alloc(qcuefi->client, cmd_buf_size, &cmd_buf_dma, GFP_KERNEL);
> - if (!cmd_buf) {
> - efi_status = EFI_OUT_OF_RESOURCES;
> - goto out;
> - }
> + cmd_buf = qcom_tzmem_alloc(qcuefi->mempool, cmd_buf_size, GFP_KERNEL);
> + if (!cmd_buf)
> + return EFI_OUT_OF_RESOURCES;
>
> req_data = cmd_buf + req_offs;
> rsp_data = cmd_buf + rsp_offs;
> @@ -724,28 +663,21 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> req_data->length = sizeof(*req_data);
>
> status = qcom_qseecom_app_send(qcuefi->client,
> - cmd_buf_dma + req_offs, sizeof(*req_data),
> - cmd_buf_dma + rsp_offs, sizeof(*rsp_data));
> - if (status) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + cmd_buf + req_offs, sizeof(*req_data),
> + cmd_buf + rsp_offs, sizeof(*rsp_data));
> + if (status)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->command_id != QSEE_CMD_UEFI_QUERY_VARIABLE_INFO)
> + return EFI_DEVICE_ERROR;
>
> - if (rsp_data->length != sizeof(*rsp_data)) {
> - efi_status = EFI_DEVICE_ERROR;
> - goto out_free;
> - }
> + if (rsp_data->length != sizeof(*rsp_data))
> + return EFI_DEVICE_ERROR;
>
> if (rsp_data->status) {
> dev_dbg(qcuefi_dev(qcuefi), "%s: uefisecapp error: 0x%x\n",
> __func__, rsp_data->status);
> - efi_status = qsee_uefi_status_to_efi(rsp_data->status);
> - goto out_free;
> + return qsee_uefi_status_to_efi(rsp_data->status);
> }
>
> if (storage_space)
> @@ -757,10 +689,7 @@ static efi_status_t qsee_uefi_query_variable_info(struct qcuefi_client *qcuefi,
> if (max_variable_size)
> *max_variable_size = rsp_data->max_variable_size;
>
> -out_free:
> - qseecom_dma_free(qcuefi->client, cmd_buf_size, cmd_buf, cmd_buf_dma);
> -out:
> - return efi_status;
> + return EFI_SUCCESS;
> }
>
> /* -- Global efivar interface. ---------------------------------------------- */
> @@ -871,6 +800,7 @@ static const struct efivar_operations qcom_efivar_ops = {
> static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
> const struct auxiliary_device_id *aux_dev_id)
> {
> + struct qcom_tzmem_pool_config pool_config;
> struct qcuefi_client *qcuefi;
> int status;
>
> @@ -889,6 +819,16 @@ static int qcom_uefisecapp_probe(struct auxiliary_device *aux_dev,
> if (status)
> qcuefi_set_reference(NULL);
>
> + memset(&pool_config, 0, sizeof(pool_config));
> + pool_config.initial_size = SZ_4K;
> + pool_config.policy = QCOM_TZMEM_POLICY_MULTIPLIER;
> + pool_config.increment = 2;
> + pool_config.max_size = SZ_256K;
> +
> + qcuefi->mempool = devm_qcom_tzmem_pool_new(&aux_dev->dev, &pool_config);
> + if (IS_ERR(qcuefi->mempool))
> + return PTR_ERR(qcuefi->mempool);
> +
> return status;
> }
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 1b64a4f4d07d..04131e60b63a 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -1588,9 +1588,9 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
> /**
> * qcom_scm_qseecom_app_send() - Send to and receive data from a given QSEE app.
> * @app_id: The ID of the target app.
> - * @req: DMA address of the request buffer sent to the app.
> + * @req: Request buffer sent to the app (must be TZ memory)
> * @req_size: Size of the request buffer.
> - * @rsp: DMA address of the response buffer, written to by the app.
> + * @rsp: Response buffer, written to by the app (must be TZ memory)
> * @rsp_size: Size of the response buffer.
> *
> * Sends a request to the QSEE app associated with the given ID and read back
> @@ -1601,13 +1601,18 @@ EXPORT_SYMBOL_GPL(qcom_scm_qseecom_app_get_id);
> *
> * Return: Zero on success, nonzero on failure.
> */
> -int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> +int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> struct qcom_scm_qseecom_resp res = {};
> struct qcom_scm_desc desc = {};
> + phys_addr_t req_phys;
> + phys_addr_t rsp_phys;
> int status;
>
> + req_phys = qcom_tzmem_to_phys(req);
> + rsp_phys = qcom_tzmem_to_phys(rsp);
> +
> desc.owner = QSEECOM_TZ_OWNER_TZ_APPS;
> desc.svc = QSEECOM_TZ_SVC_APP_ID_PLACEHOLDER;
> desc.cmd = QSEECOM_TZ_CMD_APP_SEND;
> @@ -1615,9 +1620,9 @@ int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> QCOM_SCM_RW, QCOM_SCM_VAL,
> QCOM_SCM_RW, QCOM_SCM_VAL);
> desc.args[0] = app_id;
> - desc.args[1] = req;
> + desc.args[1] = req_phys;
> desc.args[2] = req_size;
> - desc.args[3] = rsp;
> + desc.args[3] = rsp_phys;
> desc.args[4] = rsp_size;
>
> status = qcom_scm_qseecom_call(&desc, &res);
> diff --git a/include/linux/firmware/qcom/qcom_qseecom.h b/include/linux/firmware/qcom/qcom_qseecom.h
> index 366243ee9609..1dc5b3b50aa9 100644
> --- a/include/linux/firmware/qcom/qcom_qseecom.h
> +++ b/include/linux/firmware/qcom/qcom_qseecom.h
> @@ -73,9 +73,9 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
> /**
> * qcom_qseecom_app_send() - Send to and receive data from a given QSEE app.
> * @client: The QSEECOM client associated with the target app.
> - * @req: DMA address of the request buffer sent to the app.
> + * @req: Request buffer sent to the app (must be TZ memory).
> * @req_size: Size of the request buffer.
> - * @rsp: DMA address of the response buffer, written to by the app.
> + * @rsp: Response buffer, written to by the app (must be TZ memory).
> * @rsp_size: Size of the response buffer.
> *
> * Sends a request to the QSEE app associated with the given client and read
> @@ -90,8 +90,8 @@ static inline void qseecom_dma_free(struct qseecom_client *client, size_t size,
> * Return: Zero on success, nonzero on failure.
> */
> static inline int qcom_qseecom_app_send(struct qseecom_client *client,
> - dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> + void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> return qcom_scm_qseecom_app_send(client->app_id, req, req_size, rsp, rsp_size);
> }
> diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
> index aaa19f93ac43..2c51987fb70a 100644
> --- a/include/linux/firmware/qcom/qcom_scm.h
> +++ b/include/linux/firmware/qcom/qcom_scm.h
> @@ -118,8 +118,8 @@ bool qcom_scm_lmh_dcvsh_available(void);
> #ifdef CONFIG_QCOM_QSEECOM
>
> int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id);
> -int qcom_scm_qseecom_app_send(u32 app_id, dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size);
> +int qcom_scm_qseecom_app_send(u32 app_id, void *req, size_t req_size,
> + void *rsp, size_t rsp_size);
>
> #else /* CONFIG_QCOM_QSEECOM */
>
> @@ -129,8 +129,8 @@ static inline int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id)
> }
>
> static inline int qcom_scm_qseecom_app_send(u32 app_id,
> - dma_addr_t req, size_t req_size,
> - dma_addr_t rsp, size_t rsp_size)
> + void *req, size_t req_size,
> + void *rsp, size_t rsp_size)
> {
> return -EINVAL;
> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v10 10/15] firmware: qcom: scm: add support for SHM bridge operations
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (8 preceding siblings ...)
2024-05-27 12:54 ` [PATCH v10 09/15] firmware: qcom: qseecom: convert to using " Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-05-27 12:55 ` [PATCH v10 11/15] firmware: qcom: tzmem: enable SHM Bridge support Bartosz Golaszewski
` (6 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
SHM Bridge is a safety mechanism allowing to limit the amount of memory
shared between the kernel and the TrustZone to regions explicitly marked
as such.
Add low-level primitives for enabling SHM bridge support as well as
creating and destroying SHM bridges to qcom-scm.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 60 ++++++++++++++++++++++++++++++++++
drivers/firmware/qcom/qcom_scm.h | 3 ++
include/linux/firmware/qcom/qcom_scm.h | 6 ++++
3 files changed, 69 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 04131e60b63a..94c34cde8179 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -1344,6 +1344,66 @@ bool qcom_scm_lmh_dcvsh_available(void)
}
EXPORT_SYMBOL_GPL(qcom_scm_lmh_dcvsh_available);
+int qcom_scm_shm_bridge_enable(void)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_MP,
+ .cmd = QCOM_SCM_MP_SHM_BRIDGE_ENABLE,
+ .owner = ARM_SMCCC_OWNER_SIP
+ };
+
+ struct qcom_scm_res res;
+
+ if (!__qcom_scm_is_call_available(__scm->dev, QCOM_SCM_SVC_MP,
+ QCOM_SCM_MP_SHM_BRIDGE_ENABLE))
+ return -EOPNOTSUPP;
+
+ return qcom_scm_call(__scm->dev, &desc, &res) ?: res.result[0];
+}
+EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_enable);
+
+int qcom_scm_shm_bridge_create(struct device *dev, u64 pfn_and_ns_perm_flags,
+ u64 ipfn_and_s_perm_flags, u64 size_and_flags,
+ u64 ns_vmids, u64 *handle)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_MP,
+ .cmd = QCOM_SCM_MP_SHM_BRIDGE_CREATE,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ .args[0] = pfn_and_ns_perm_flags,
+ .args[1] = ipfn_and_s_perm_flags,
+ .args[2] = size_and_flags,
+ .args[3] = ns_vmids,
+ .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_VAL, QCOM_SCM_VAL,
+ QCOM_SCM_VAL, QCOM_SCM_VAL),
+ };
+
+ struct qcom_scm_res res;
+ int ret;
+
+ ret = qcom_scm_call(__scm->dev, &desc, &res);
+
+ if (handle && !ret)
+ *handle = res.result[1];
+
+ return ret ?: res.result[0];
+}
+EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_create);
+
+int qcom_scm_shm_bridge_delete(struct device *dev, u64 handle)
+{
+ struct qcom_scm_desc desc = {
+ .svc = QCOM_SCM_SVC_MP,
+ .cmd = QCOM_SCM_MP_SHM_BRIDGE_DELETE,
+ .owner = ARM_SMCCC_OWNER_SIP,
+ .args[0] = handle,
+ .arginfo = QCOM_SCM_ARGS(1, QCOM_SCM_VAL),
+ };
+
+ return qcom_scm_call(__scm->dev, &desc, NULL);
+}
+EXPORT_SYMBOL_GPL(qcom_scm_shm_bridge_delete);
+
int qcom_scm_lmh_profile_change(u32 profile_id)
{
struct qcom_scm_desc desc = {
diff --git a/drivers/firmware/qcom/qcom_scm.h b/drivers/firmware/qcom/qcom_scm.h
index aa7d06939f8e..cb7273aa0a5e 100644
--- a/drivers/firmware/qcom/qcom_scm.h
+++ b/drivers/firmware/qcom/qcom_scm.h
@@ -116,6 +116,9 @@ struct qcom_tzmem_pool *qcom_scm_get_tzmem_pool(void);
#define QCOM_SCM_MP_IOMMU_SET_CP_POOL_SIZE 0x05
#define QCOM_SCM_MP_VIDEO_VAR 0x08
#define QCOM_SCM_MP_ASSIGN 0x16
+#define QCOM_SCM_MP_SHM_BRIDGE_ENABLE 0x1c
+#define QCOM_SCM_MP_SHM_BRIDGE_DELETE 0x1d
+#define QCOM_SCM_MP_SHM_BRIDGE_CREATE 0x1e
#define QCOM_SCM_SVC_OCMEM 0x0f
#define QCOM_SCM_OCMEM_LOCK_CMD 0x01
diff --git a/include/linux/firmware/qcom/qcom_scm.h b/include/linux/firmware/qcom/qcom_scm.h
index 2c51987fb70a..767bffe20766 100644
--- a/include/linux/firmware/qcom/qcom_scm.h
+++ b/include/linux/firmware/qcom/qcom_scm.h
@@ -115,6 +115,12 @@ int qcom_scm_lmh_dcvsh(u32 payload_fn, u32 payload_reg, u32 payload_val,
int qcom_scm_lmh_profile_change(u32 profile_id);
bool qcom_scm_lmh_dcvsh_available(void);
+int qcom_scm_shm_bridge_enable(void);
+int qcom_scm_shm_bridge_create(struct device *dev, u64 pfn_and_ns_perm_flags,
+ u64 ipfn_and_s_perm_flags, u64 size_and_flags,
+ u64 ns_vmids, u64 *handle);
+int qcom_scm_shm_bridge_delete(struct device *dev, u64 handle);
+
#ifdef CONFIG_QCOM_QSEECOM
int qcom_scm_qseecom_app_get_id(const char *app_name, u32 *app_id);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 11/15] firmware: qcom: tzmem: enable SHM Bridge support
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (9 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 10/15] firmware: qcom: scm: add support for SHM bridge operations Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-05-27 12:55 ` [PATCH v10 12/15] firmware: qcom: scm: add support for SHM bridge memory carveout Bartosz Golaszewski
` (5 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
SHM Bridge is a safety mechanism allowing to limit the amount of memory
shared between the kernel and the TrustZone to regions explicitly marked
as such.
Add a variant of the tzmem allocator that configures the memory pools as
SHM bridges. It also enables the SHM bridge globally so non-SHM bridge
memory will no longer work with SCM calls.
If enabled at build-time, it will still be checked for availability at
run-time. If the architecture doesn't support SHM Bridge, the allocator
will fall back to the generic mode.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/Kconfig | 10 +++++
drivers/firmware/qcom/qcom_tzmem.c | 79 +++++++++++++++++++++++++++++++++++++-
2 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/qcom/Kconfig b/drivers/firmware/qcom/Kconfig
index 4634f8cecc7b..7f6eb4174734 100644
--- a/drivers/firmware/qcom/Kconfig
+++ b/drivers/firmware/qcom/Kconfig
@@ -28,6 +28,16 @@ config QCOM_TZMEM_MODE_GENERIC
Use the generic allocator mode. The memory is page-aligned, non-cachable
and physically contiguous.
+config QCOM_TZMEM_MODE_SHMBRIDGE
+ bool "SHM Bridge"
+ help
+ Use Qualcomm Shared Memory Bridge. The memory has the same alignment as
+ in the 'Generic' allocator but is also explicitly marked as an SHM Bridge
+ buffer.
+
+ With this selected, all buffers passed to the TrustZone must be allocated
+ using the TZMem allocator or else the TrustZone will refuse to use them.
+
endchoice
config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
diff --git a/drivers/firmware/qcom/qcom_tzmem.c b/drivers/firmware/qcom/qcom_tzmem.c
index 3853385bf215..aab6376cade5 100644
--- a/drivers/firmware/qcom/qcom_tzmem.c
+++ b/drivers/firmware/qcom/qcom_tzmem.c
@@ -66,7 +66,84 @@ static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area)
}
-#endif /* CONFIG_QCOM_TZMEM_MODE_GENERIC */
+#elif IS_ENABLED(CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE)
+
+#include <linux/firmware/qcom/qcom_scm.h>
+#include <linux/of.h>
+
+#define QCOM_SHM_BRIDGE_NUM_VM_SHIFT 9
+
+static bool qcom_tzmem_using_shm_bridge;
+
+/* List of machines that are known to not support SHM bridge correctly. */
+static const char *const qcom_tzmem_blacklist[] = {
+ "qcom,sc8180x",
+ NULL
+};
+
+static int qcom_tzmem_init(void)
+{
+ const char *const *platform;
+ int ret;
+
+ for (platform = qcom_tzmem_blacklist; *platform; platform++) {
+ if (of_machine_is_compatible(*platform))
+ goto notsupp;
+ }
+
+ ret = qcom_scm_shm_bridge_enable();
+ if (ret == -EOPNOTSUPP)
+ goto notsupp;
+
+ if (!ret)
+ qcom_tzmem_using_shm_bridge = true;
+
+ return ret;
+
+notsupp:
+ dev_info(qcom_tzmem_dev, "SHM Bridge not supported\n");
+ return 0;
+}
+
+static int qcom_tzmem_init_area(struct qcom_tzmem_area *area)
+{
+ u64 pfn_and_ns_perm, ipfn_and_s_perm, size_and_flags;
+ int ret;
+
+ if (!qcom_tzmem_using_shm_bridge)
+ return 0;
+
+ pfn_and_ns_perm = (u64)area->paddr | QCOM_SCM_PERM_RW;
+ ipfn_and_s_perm = (u64)area->paddr | QCOM_SCM_PERM_RW;
+ size_and_flags = area->size | (1 << QCOM_SHM_BRIDGE_NUM_VM_SHIFT);
+
+ u64 *handle __free(kfree) = kzalloc(sizeof(*handle), GFP_KERNEL);
+ if (!handle)
+ return -ENOMEM;
+
+ ret = qcom_scm_shm_bridge_create(qcom_tzmem_dev, pfn_and_ns_perm,
+ ipfn_and_s_perm, size_and_flags,
+ QCOM_SCM_VMID_HLOS, handle);
+ if (ret)
+ return ret;
+
+ area->priv = no_free_ptr(handle);
+
+ return 0;
+}
+
+static void qcom_tzmem_cleanup_area(struct qcom_tzmem_area *area)
+{
+ u64 *handle = area->priv;
+
+ if (!qcom_tzmem_using_shm_bridge)
+ return;
+
+ qcom_scm_shm_bridge_delete(qcom_tzmem_dev, *handle);
+ kfree(handle);
+}
+
+#endif /* CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE */
static int qcom_tzmem_pool_add_memory(struct qcom_tzmem_pool *pool,
size_t size, gfp_t gfp)
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 12/15] firmware: qcom: scm: add support for SHM bridge memory carveout
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (10 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 11/15] firmware: qcom: tzmem: enable SHM Bridge support Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-06-20 23:05 ` Elliot Berman
2024-05-27 12:55 ` [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image() Bartosz Golaszewski
` (4 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Parse the "memory-region" property and - if present - use it to assign
the dedicated reserved memory to the underlying DMA callbacks which will
then allocate memory for the SCM calls from it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/firmware/qcom/qcom_scm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 94c34cde8179..c82957727650 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -23,6 +23,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
#include <linux/platform_device.h>
#include <linux/reset-controller.h>
#include <linux/sizes.h>
@@ -1942,6 +1943,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
qcom_scm_disable_sdi();
+ ret = of_reserved_mem_device_init(__scm->dev);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(__scm->dev, ret,
+ "Failed to setup the reserved memory region for TZ mem\n");
+
ret = qcom_tzmem_enable(__scm->dev);
if (ret)
return dev_err_probe(__scm->dev, ret,
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v10 12/15] firmware: qcom: scm: add support for SHM bridge memory carveout
2024-05-27 12:55 ` [PATCH v10 12/15] firmware: qcom: scm: add support for SHM bridge memory carveout Bartosz Golaszewski
@ 2024-06-20 23:05 ` Elliot Berman
0 siblings, 0 replies; 27+ messages in thread
From: Elliot Berman @ 2024-06-20 23:05 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel
On Mon, May 27, 2024 at 02:55:02PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Parse the "memory-region" property and - if present - use it to assign
> the dedicated reserved memory to the underlying DMA callbacks which will
> then allocate memory for the SCM calls from it.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> drivers/firmware/qcom/qcom_scm.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 94c34cde8179..c82957727650 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -23,6 +23,7 @@
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/platform_device.h>
> #include <linux/reset-controller.h>
> #include <linux/sizes.h>
> @@ -1942,6 +1943,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
> if (of_property_read_bool(pdev->dev.of_node, "qcom,sdi-enabled"))
> qcom_scm_disable_sdi();
>
> + ret = of_reserved_mem_device_init(__scm->dev);
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(__scm->dev, ret,
> + "Failed to setup the reserved memory region for TZ mem\n");
> +
> ret = qcom_tzmem_enable(__scm->dev);
> if (ret)
> return dev_err_probe(__scm->dev, ret,
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image()
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (11 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 12/15] firmware: qcom: scm: add support for SHM bridge memory carveout Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-06-20 22:54 ` Elliot Berman
2024-05-27 12:55 ` [PATCH v10 14/15] arm64: defconfig: enable SHM Bridge support for the TZ memory allocator Bartosz Golaszewski
` (3 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The "memory protection" mechanism mentioned in the comment is the SHM
Bridge. This is also the reason why we do not convert this call to using
the TZ memory allocator.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/qcom/qcom_scm.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index c82957727650..86e26f17ca19 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -583,6 +583,13 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
* During the scm call memory protection will be enabled for the meta
* data blob, so make sure it's physically contiguous, 4K aligned and
* non-cachable to avoid XPU violations.
+ *
+ * For PIL calls the hypervisor creates SHM Bridges for the blob
+ * buffers on behalf of Linus so we must not do it ourselves hence
+ * not using the TZMem allocator here.
+ *
+ * If we pass a buffer that is already part of an SHM Bridge to this
+ * call, it will fail.
*/
mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
GFP_KERNEL);
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image()
2024-05-27 12:55 ` [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image() Bartosz Golaszewski
@ 2024-06-20 22:54 ` Elliot Berman
2024-06-21 13:42 ` Bartosz Golaszewski
0 siblings, 1 reply; 27+ messages in thread
From: Elliot Berman @ 2024-06-20 22:54 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel, Andrew Halaney,
Deepti Jaggi
On Mon, May 27, 2024 at 02:55:03PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> The "memory protection" mechanism mentioned in the comment is the SHM
> Bridge. This is also the reason why we do not convert this call to using
> the TZ memory allocator.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
> Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
> Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> drivers/firmware/qcom/qcom_scm.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index c82957727650..86e26f17ca19 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -583,6 +583,13 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
> * During the scm call memory protection will be enabled for the meta
> * data blob, so make sure it's physically contiguous, 4K aligned and
> * non-cachable to avoid XPU violations.
> + *
> + * For PIL calls the hypervisor creates SHM Bridges for the blob
> + * buffers on behalf of Linus so we must not do it ourselves hence
Linux
> + * not using the TZMem allocator here.
> + *
> + * If we pass a buffer that is already part of an SHM Bridge to this
> + * call, it will fail.
> */
> mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
> GFP_KERNEL);
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image()
2024-06-20 22:54 ` Elliot Berman
@ 2024-06-21 13:42 ` Bartosz Golaszewski
0 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-06-21 13:42 UTC (permalink / raw)
To: Elliot Berman
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel, Andrew Halaney,
Deepti Jaggi
On Fri, Jun 21, 2024 at 12:54 AM Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> On Mon, May 27, 2024 at 02:55:03PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > The "memory protection" mechanism mentioned in the comment is the SHM
> > Bridge. This is also the reason why we do not convert this call to using
> > the TZ memory allocator.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
> > Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
> > Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
> > ---
> > drivers/firmware/qcom/qcom_scm.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index c82957727650..86e26f17ca19 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -583,6 +583,13 @@ int qcom_scm_pas_init_image(u32 peripheral, const void *metadata, size_t size,
> > * During the scm call memory protection will be enabled for the meta
> > * data blob, so make sure it's physically contiguous, 4K aligned and
> > * non-cachable to avoid XPU violations.
> > + *
> > + * For PIL calls the hypervisor creates SHM Bridges for the blob
> > + * buffers on behalf of Linus so we must not do it ourselves hence
> Linux
Can this be fixed when applying? I don't think there's anything else
that warrants a respin.
Bart
> > + * not using the TZMem allocator here.
> > + *
> > + * If we pass a buffer that is already part of an SHM Bridge to this
> > + * call, it will fail.
> > */
> > mdata_buf = dma_alloc_coherent(__scm->dev, size, &mdata_phys,
> > GFP_KERNEL);
> >
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v10 14/15] arm64: defconfig: enable SHM Bridge support for the TZ memory allocator
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (12 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 13/15] firmware: qcom: scm: clarify the comment in qcom_scm_pas_init_image() Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-05-27 12:55 ` [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ Bartosz Golaszewski
` (2 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Enable SHM Bridge support in the Qualcomm TrustZone allocator by default
as even on architectures that don't support it, we automatically fall
back to the generic allocator.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sc8280xp-lenovo-thinkpad-x13s
Tested-by: Deepti Jaggi <quic_djaggi@quicinc.com> #sa8775p-ride
Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 57a9abe78ee4..8ce4beef2ac4 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -256,6 +256,7 @@ CONFIG_GOOGLE_CBMEM=m
CONFIG_GOOGLE_COREBOOT_TABLE=m
CONFIG_EFI_CAPSULE_LOADER=y
CONFIG_IMX_SCU=y
+CONFIG_QCOM_TZMEM_MODE_SHMBRIDGE=y
CONFIG_QCOM_QSEECOM=y
CONFIG_QCOM_QSEECOM_UEFISECAPP=y
CONFIG_GNSS=m
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (13 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 14/15] arm64: defconfig: enable SHM Bridge support for the TZ memory allocator Bartosz Golaszewski
@ 2024-05-27 12:55 ` Bartosz Golaszewski
2024-05-29 14:26 ` Konrad Dybcio
2024-06-17 19:57 ` [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
2024-06-23 22:09 ` (subset) " Bjorn Andersson
16 siblings, 1 reply; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-27 12:55 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a 20MB reserved memory region for use by SCM calls.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
arch/arm64/boot/dts/qcom/sa8775p.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 31de73594839..c183527452d7 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -208,6 +208,7 @@ core3 {
firmware {
scm {
compatible = "qcom,scm-sa8775p", "qcom,scm";
+ memory-region = <&tz_ffi_mem>;
};
};
@@ -418,6 +419,12 @@ cdt_data_backup_mem: cdt-data-backup@91ba0000 {
no-map;
};
+ tz_ffi_mem: tz-ffi@91c00000 {
+ compatible = "shared-dma-pool";
+ reg = <0x0 0x91c00000 0x0 0x1400000>;
+ no-map;
+ };
+
lpass_machine_learning_mem: lpass-machine-learning@93b00000 {
reg = <0x0 0x93b00000 0x0 0xf00000>;
no-map;
--
2.43.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ
2024-05-27 12:55 ` [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ Bartosz Golaszewski
@ 2024-05-29 14:26 ` Konrad Dybcio
2024-05-29 14:39 ` Bartosz Golaszewski
0 siblings, 1 reply; 27+ messages in thread
From: Konrad Dybcio @ 2024-05-29 14:26 UTC (permalink / raw)
To: Bartosz Golaszewski, Bjorn Andersson, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Robert Marko, Das Srinagesh,
Bartosz Golaszewski, Maximilian Luz, Catalin Marinas, Will Deacon,
Srini Kandagatla, Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel
On 27.05.2024 2:55 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Add a 20MB reserved memory region for use by SCM calls.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> index 31de73594839..c183527452d7 100644
> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> @@ -208,6 +208,7 @@ core3 {
> firmware {
> scm {
> compatible = "qcom,scm-sa8775p", "qcom,scm";
> + memory-region = <&tz_ffi_mem>;
> };
> };
>
> @@ -418,6 +419,12 @@ cdt_data_backup_mem: cdt-data-backup@91ba0000 {
> no-map;
> };
>
> + tz_ffi_mem: tz-ffi@91c00000 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x91c00000 0x0 0x1400000>;
How does that size relate to
+ pool_config.max_size = SZ_256K;
in patch 3?
Konrad
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ
2024-05-29 14:26 ` Konrad Dybcio
@ 2024-05-29 14:39 ` Bartosz Golaszewski
2024-05-31 12:40 ` Konrad Dybcio
0 siblings, 1 reply; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-05-29 14:39 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Robert Marko, Das Srinagesh, Bartosz Golaszewski, Maximilian Luz,
Catalin Marinas, Will Deacon, Srini Kandagatla, Arnd Bergmann,
Elliot Berman, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel
On Wed, May 29, 2024 at 4:26 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 27.05.2024 2:55 PM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add a 20MB reserved memory region for use by SCM calls.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > index 31de73594839..c183527452d7 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
> > @@ -208,6 +208,7 @@ core3 {
> > firmware {
> > scm {
> > compatible = "qcom,scm-sa8775p", "qcom,scm";
> > + memory-region = <&tz_ffi_mem>;
> > };
> > };
> >
> > @@ -418,6 +419,12 @@ cdt_data_backup_mem: cdt-data-backup@91ba0000 {
> > no-map;
> > };
> >
> > + tz_ffi_mem: tz-ffi@91c00000 {
> > + compatible = "shared-dma-pool";
> > + reg = <0x0 0x91c00000 0x0 0x1400000>;
>
> How does that size relate to
>
> + pool_config.max_size = SZ_256K;
>
> in patch 3?
>
> Konrad
Short answer: it doesn't.
Slightly less short answer: this is the limit of *this* specific TZMem
pool which in turn gets its memory from the shared DMA pool. We can
have multiple TZMem pools and they all share this DMA pool.
Bartosz
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ
2024-05-29 14:39 ` Bartosz Golaszewski
@ 2024-05-31 12:40 ` Konrad Dybcio
0 siblings, 0 replies; 27+ messages in thread
From: Konrad Dybcio @ 2024-05-31 12:40 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Robert Marko, Das Srinagesh, Bartosz Golaszewski, Maximilian Luz,
Catalin Marinas, Will Deacon, Srini Kandagatla, Arnd Bergmann,
Elliot Berman, Alex Elder, linux-arm-msm, devicetree,
linux-kernel, linux-arm-kernel, kernel
On 29.05.2024 4:39 PM, Bartosz Golaszewski wrote:
> On Wed, May 29, 2024 at 4:26 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 27.05.2024 2:55 PM, Bartosz Golaszewski wrote:
>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> Add a 20MB reserved memory region for use by SCM calls.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>> ---
>>> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> index 31de73594839..c183527452d7 100644
>>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>>> @@ -208,6 +208,7 @@ core3 {
>>> firmware {
>>> scm {
>>> compatible = "qcom,scm-sa8775p", "qcom,scm";
>>> + memory-region = <&tz_ffi_mem>;
>>> };
>>> };
>>>
>>> @@ -418,6 +419,12 @@ cdt_data_backup_mem: cdt-data-backup@91ba0000 {
>>> no-map;
>>> };
>>>
>>> + tz_ffi_mem: tz-ffi@91c00000 {
>>> + compatible = "shared-dma-pool";
>>> + reg = <0x0 0x91c00000 0x0 0x1400000>;
>>
>> How does that size relate to
>>
>> + pool_config.max_size = SZ_256K;
>>
>> in patch 3?
>>
>> Konrad
>
> Short answer: it doesn't.
>
> Slightly less short answer: this is the limit of *this* specific TZMem
> pool which in turn gets its memory from the shared DMA pool. We can
> have multiple TZMem pools and they all share this DMA pool.
OK makes sense, thanks
Konrad
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (14 preceding siblings ...)
2024-05-27 12:55 ` [PATCH v10 15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ Bartosz Golaszewski
@ 2024-06-17 19:57 ` Bartosz Golaszewski
2024-06-23 22:09 ` (subset) " Bjorn Andersson
16 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2024-06-17 19:57 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Robert Marko, Das Srinagesh, Bartosz Golaszewski,
Maximilian Luz, Catalin Marinas, Will Deacon, Srini Kandagatla,
Arnd Bergmann, Elliot Berman, Alex Elder
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
On Mon, May 27, 2024 at 2:56 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> SCM calls that take memory buffers as arguments require that they be
> page-aligned, physically continuous and non-cachable. The same
> requirements apply to the buffer used to pass additional arguments to SCM
> calls that take more than 4.
>
[...]
It's been three weeks. Ping?
Bart
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: (subset) [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge
2024-05-27 12:54 [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
` (15 preceding siblings ...)
2024-06-17 19:57 ` [PATCH v10 00/15] firmware: qcom: implement support for and enable SHM bridge Bartosz Golaszewski
@ 2024-06-23 22:09 ` Bjorn Andersson
16 siblings, 0 replies; 27+ messages in thread
From: Bjorn Andersson @ 2024-06-23 22:09 UTC (permalink / raw)
To: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Robert Marko, Das Srinagesh, Bartosz Golaszewski, Maximilian Luz,
Catalin Marinas, Will Deacon, Srini Kandagatla, Arnd Bergmann,
Elliot Berman, Alex Elder, Bartosz Golaszewski
Cc: linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel, kernel,
Andrew Halaney, Deepti Jaggi
On Mon, 27 May 2024 14:54:50 +0200, Bartosz Golaszewski wrote:
> SCM calls that take memory buffers as arguments require that they be
> page-aligned, physically continuous and non-cachable. The same
> requirements apply to the buffer used to pass additional arguments to SCM
> calls that take more than 4.
>
> To that end drivers typically use dma_alloc_coherent() to allocate memory
> of suitable format which is slow and inefficient space-wise.
>
> [...]
Applied, thanks!
[15/15] arm64: dts: qcom: sa8775p: add a dedicated memory carveout for TZ
commit: d7aeff30093888649789dcad070fe954745adf53
b4 gets confused by me applying patches across multiple branches, the other 14
patches has obviously also been applied.
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply [flat|nested] 27+ messages in thread