Re: [PATCH] firmware: qcom_scm: Introduce batching of hyp assign calls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12-Feb-24 4:39 PM, Konrad Dybcio wrote:
On 9.02.2024 19:06, Elliot Berman wrote:
On Fri, Feb 09, 2024 at 04:55:36PM +0530, Dibakar Singh wrote:
Expose an API qcom_scm_assign_table to allow client drivers to batch
multiple memory regions in a single hyp assign call.

In the existing situation, if our goal is to process an sg_table and
transfer its ownership from the current VM to a different VM, we have a
couple of strategies. The first strategy involves processing the entire
sg_table at once and then transferring the ownership. However, this
method may have an adverse impact on the system because during an SMC
call, the NS interrupts are disabled, and this delay could be
significant when dealing with large sg_tables. To address this issue, we
can adopt a second strategy, which involves processing each sg_list in
the sg_table individually and reassigning memory ownership. Although
this method is slower and potentially impacts performance, it will not
keep the NS interrupts disabled for an extended period.

A more efficient strategy is to process the sg_table in batches. This
approach addresses both scenarios by involving memory processing in
batches, thus avoiding prolonged NS interrupt disablement for longer
duration when dealing with large sg_tables. Moreover, since we process
in batches, this method is faster compared to processing each item
individually. The observations on testing both the approaches for
performance is as follows:

Allocation Size/            256MB            512MB            1024MB
Algorithm Used           ===========      ===========      ============

Processing each sg_list   73708(us)        149289(us)       266964(us)
in sg_table one by one

Processing sg_table in    46925(us)         92691(us)       176893(us)
batches

This implementation serves as a wrapper around the helper function
__qcom_scm_assign_mem, which takes an sg_list and processes it in
batches. We’ve set the limit to a minimum of 32 sg_list in a batch or a
total batch size of 512 pages. The selection of these numbers is
heuristic, based on the test runs conducted. Opting for a smaller number
would compromise performance, while a larger number would result in
non-secure interrupts being disabled for an extended duration.

Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@xxxxxxxxxxx>
Signed-off-by: Dibakar Singh <quic_dibasing@xxxxxxxxxxx>
---
  drivers/firmware/qcom/qcom_scm.c       | 211 +++++++++++++++++++++++++
  include/linux/firmware/qcom/qcom_scm.h |   7 +
  2 files changed, 218 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 520de9b5633a..038b96503d65 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -21,6 +21,8 @@
  #include <linux/platform_device.h>
  #include <linux/reset-controller.h>
  #include <linux/types.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
#include "qcom_scm.h" @@ -1048,6 +1050,215 @@ int qcom_scm_assign_mem(phys_addr_t mem_addr, size_t mem_sz,
  }
  EXPORT_SYMBOL_GPL(qcom_scm_assign_mem);
+/**
+ * qcom_scm_assign_mem_batch() - Make a secure call to reassign memory
+ *				   ownership of several memory regions
+ * @mem_regions:    A buffer describing the set of memory regions that need to
+ *		    be reassigned
+ * @nr_mem_regions: The number of memory regions that need to be reassigned
+ * @srcvms:	    A buffer populated with he vmid(s) for the current set of
+ *		    owners
+ * @src_sz:	    The size of the srcvms buffer (in bytes)
+ * @destvms:	    A buffer populated with the new owners and corresponding
+ *		    permission flags.
+ * @dest_sz:	    The size of the destvms buffer (in bytes)
+ *
+ * Return negative errno on failure, 0 on success.
+ */
+static int qcom_scm_assign_mem_batch(struct qcom_scm_mem_map_info *mem_regions,
+				     size_t nr_mem_regions, u32 *srcvms,
+				     size_t src_sz,
+				     struct qcom_scm_current_perm_info *destvms,
+				     size_t dest_sz)
+{
+	dma_addr_t mem_dma_addr;
+	size_t mem_regions_sz;
+	int ret = 0, i;
+
+	for (i = 0; i < nr_mem_regions; i++) {
+		mem_regions[i].mem_addr = cpu_to_le64(mem_regions[i].mem_addr);
+		mem_regions[i].mem_size = cpu_to_le64(mem_regions[i].mem_size);
+	}
+
+	mem_regions_sz = nr_mem_regions * sizeof(*mem_regions);
+	mem_dma_addr = dma_map_single(__scm->dev, mem_regions, mem_regions_sz,
+				      DMA_TO_DEVICE);
+	if (dma_mapping_error(__scm->dev, mem_dma_addr)) {
+		dev_err(__scm->dev, "mem_dma_addr mapping failed\n");
+		return -ENOMEM;
+	}
+
+	ret = __qcom_scm_assign_mem(__scm->dev, virt_to_phys(mem_regions),
+				    mem_regions_sz, virt_to_phys(srcvms), src_sz,
+				    virt_to_phys(destvms), dest_sz);
+
+	dma_unmap_single(__scm->dev, mem_dma_addr, mem_regions_sz, DMA_TO_DEVICE);
+	return ret;
+}
+
+/**
+ * qcom_scm_prepare_mem_batch() - Prepare batches of memory regions
+ * @sg_table:       A scatter list whose memory needs to be reassigned
+ * @srcvms:	    A buffer populated with he vmid(s) for the current set of
+ *		    owners
+ * @nr_src:	    The number of the src_vms buffer
+ * @destvms:	    A buffer populated with he vmid(s) for the new owners
+ * @destvms_perms:  A buffer populated with the permission flags of new owners
+ * @nr_dest:	    The number of the destvms
+ * @last_sgl:	    Denotes to the last scatter list element. Used in case of rollback
+ * @roll_back:	    Identifies whether we are executing rollback in case of failure
+ *
+ * Return negative errno on failure, 0 on success.
+ */
+static int qcom_scm_prepare_mem_batch(struct sg_table *table,
+				      u32 *srcvms, int nr_src,
+				      int *destvms, int *destvms_perms,
+				      int nr_dest,
+				      struct scatterlist *last_sgl, bool roll_back)
+{
+	struct qcom_scm_current_perm_info *destvms_cp;
+	struct qcom_scm_mem_map_info *mem_regions_buf;
+	struct scatterlist *curr_sgl = table->sgl;
+	dma_addr_t source_dma_addr, dest_dma_addr;
+	size_t batch_iterator;
+	size_t batch_start = 0;
+	size_t destvms_cp_sz;
+	size_t srcvms_cp_sz;
+	size_t batch_size;
+	u32 *srcvms_cp;
+	int ret = 0;
+	int i;
+
+	if (!table || !table->sgl || !srcvms || !nr_src ||
+	    !destvms || !destvms_perms || !nr_dest || !table->nents)
+		return -EINVAL;
+
+	srcvms_cp_sz = sizeof(*srcvms_cp) * nr_src;
+	srcvms_cp = kmemdup(srcvms, srcvms_cp_sz, GFP_KERNEL);
+	if (!srcvms_cp)
+		return -ENOMEM;
+
+	for (i = 0; i < nr_src; i++)
+		srcvms_cp[i] = cpu_to_le32(srcvms_cp[i]);
+
+	source_dma_addr = dma_map_single(__scm->dev, srcvms_cp,
+					 srcvms_cp_sz, DMA_TO_DEVICE);

Please use the new tzmem allocator:

https://lore.kernel.org/all/20240205182810.58382-1-brgl@xxxxxxxx/

And the new __free annotations, please


Noted, I will incorporate these changes in the V2 patch.

Konrad

Thanks,
Dibakar




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux