linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
@ 2019-09-20  8:04 Sai Prakash Ranjan
  2019-09-20  8:04 ` [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-20  8:04 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, Andy Gross
  Cc: linux-arm-msm, linux-kernel, Rajendra Nayak, Sai Prakash Ranjan

Previous version of the patches are at [1]:

QCOM's implementation of smmu-500 on sdm845 adds a hardware logic called
wait-for-safe. This logic helps in meeting the invalidation requirements
from 'real-time clients', such as display and camera. This wait-for-safe
logic ensures that the invalidations happen after getting an ack from these
devices.
In this patch-series we are disabling this wait-for-safe logic from the
arm-smmu driver's probe as with this enabled the hardware tries to
throttle invalidations from 'non-real-time clients', such as USB and UFS.

For detailed information please refer to patch [3/4] in this series.
I have included the device tree patch too in this series for someone who
would like to test out this. Here's a branch [2] that gets display on MTP
SDM845 device.

This patch series is inspired from downstream work to handle under-performance
issues on real-time clients on sdm845. In downstream we add separate page table
ops to handle TLB maintenance and toggle wait-for-safe in tlb_sync call so that
achieve required performance for display and camera [3, 4].

Changes since v6:
 * Removed cant_sleep from qcom_scm_call_atomic since it can be called from both contexts. 

Changes since v5:
 * Addressed Robin's comments and removed unwanted header file.

Changes since v4:
 * Addressed Stephen's comments.
 * Moved QCOM specific implementation to arm-smmu-qcom.c as per Robin's suggestion.

Changes since v3:
 * Based on arm-smmu implementation cleanup series [5] by Robin Murphy which is
   already merged in Will's tree [6].
 * Implemented the sdm845 specific reset hook which does arm_smmu_device_reset()
   followed by making SCM call to disable the wait-for-safe logic.
 * Removed depedency for SCM call on any dt flag. We invariably try to disable
   the wait-for-safe logic on sdm845. The platforms such as mtp845, and db845
   that implement handlers for this particular SCM call should be able disable
   wait-for-safe logic.
   Other platforms such as cheza don't enable the wait-for-safe logic at all
   from their bootloaders. So there's no need to disable the same.
 * No change in SCM call patches 1 & 2.

Changes since v2:
 * Dropped the patch to add atomic io_read/write scm API.
 * Removed support for any separate page table ops to handle wait-for-safe.
   Currently just disabling this wait-for-safe logic from arm_smmu_device_probe()
   to achieve performance on USB/UFS on sdm845.
 * Added a device tree patch to add smmu option for fw-implemented support
   for SCM call to take care of SAFE toggling.

Changes since v1:
 * Addressed Will and Robin's comments:
    - Dropped the patch[4] that forked out __arm_smmu_tlb_inv_range_nosync(),
      and __arm_smmu_tlb_sync().
    - Cleaned up the errata patch further to use downstream polling mechanism
      for tlb sync.
 * No change in SCM call patches - patches 1 to 3.

[1] https://lore.kernel.org/patchwork/cover/1128328/ 
[2] https://github.com/vivekgautam1/linux/tree/v5.2-rc4/sdm845-display-working
[3] https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/drivers/iommu/arm-smmu.c?h=CogSystems-msm-49/msm-4.9&id=da765c6c75266b38191b38ef086274943f353ea7
[4] https://source.codeaurora.org/quic/la/kernel/msm-4.9/commit/drivers/iommu/arm-smmu.c?h=CogSystems-msm-49/msm-4.9&id=8696005aaaf745de68f57793c1a534a34345c30a
[5] https://patchwork.kernel.org/patch/11096265/
[6] https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/

Vivek Gautam (3):
  firmware: qcom_scm-64: Add atomic version of qcom_scm_call
  firmware/qcom_scm: Add scm call to handle smmu errata
  iommu: arm-smmu-impl: Add sdm845 implementation hook

 drivers/firmware/qcom_scm-32.c |   5 ++
 drivers/firmware/qcom_scm-64.c | 151 +++++++++++++++++++++++----------
 drivers/firmware/qcom_scm.c    |   6 ++
 drivers/firmware/qcom_scm.h    |   5 ++
 drivers/iommu/Makefile         |   2 +-
 drivers/iommu/arm-smmu-impl.c  |   5 +-
 drivers/iommu/arm-smmu-qcom.c  |  51 +++++++++++
 drivers/iommu/arm-smmu.h       |   3 +
 include/linux/qcom_scm.h       |   2 +
 9 files changed, 184 insertions(+), 46 deletions(-)
 create mode 100644 drivers/iommu/arm-smmu-qcom.c

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
  2019-09-20  8:04 [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
@ 2019-09-20  8:04 ` Sai Prakash Ranjan
  2019-11-04  5:16   ` Andy Gross
  2019-09-20  8:04 ` [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-20  8:04 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, Andy Gross
  Cc: linux-arm-msm, linux-kernel, Rajendra Nayak, Sai Prakash Ranjan

From: Vivek Gautam <vivek.gautam@codeaurora.org>

There are scnenarios where drivers are required to make a
scm call in atomic context, such as in one of the qcom's
arm-smmu-500 errata [1].

[1] ("https://source.codeaurora.org/quic/la/kernel/msm-4.9/
      tree/drivers/iommu/arm-smmu.c?h=msm-4.9#n4842")

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/firmware/qcom_scm-64.c | 138 ++++++++++++++++++++++-----------
 1 file changed, 94 insertions(+), 44 deletions(-)

diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 91d5ad7cf58b..65d772b7fd1e 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -62,32 +62,72 @@ static DEFINE_MUTEX(qcom_scm_lock);
 #define FIRST_EXT_ARG_IDX 3
 #define N_REGISTER_ARGS (MAX_QCOM_SCM_ARGS - N_EXT_QCOM_SCM_ARGS + 1)
 
-/**
- * qcom_scm_call() - Invoke a syscall in the secure world
- * @dev:	device
- * @svc_id:	service identifier
- * @cmd_id:	command identifier
- * @desc:	Descriptor structure containing arguments and return values
- *
- * Sends a command to the SCM and waits for the command to finish processing.
- * This should *only* be called in pre-emptible context.
-*/
-static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
-			 const struct qcom_scm_desc *desc,
-			 struct arm_smccc_res *res)
+static void __qcom_scm_call_do(const struct qcom_scm_desc *desc,
+			       struct arm_smccc_res *res, u32 fn_id,
+			       u64 x5, u32 type)
+{
+	u64 cmd;
+	struct arm_smccc_quirk quirk = { .id = ARM_SMCCC_QUIRK_QCOM_A6 };
+
+	cmd = ARM_SMCCC_CALL_VAL(type, qcom_smccc_convention,
+				 ARM_SMCCC_OWNER_SIP, fn_id);
+
+	quirk.state.a6 = 0;
+
+	do {
+		arm_smccc_smc_quirk(cmd, desc->arginfo, desc->args[0],
+				    desc->args[1], desc->args[2], x5,
+				    quirk.state.a6, 0, res, &quirk);
+
+		if (res->a0 == QCOM_SCM_INTERRUPTED)
+			cmd = res->a0;
+
+	} while (res->a0 == QCOM_SCM_INTERRUPTED);
+}
+
+static void qcom_scm_call_do(const struct qcom_scm_desc *desc,
+			     struct arm_smccc_res *res, u32 fn_id,
+			     u64 x5, bool atomic)
+{
+	int retry_count = 0;
+
+	if (atomic) {
+		__qcom_scm_call_do(desc, res, fn_id, x5, ARM_SMCCC_FAST_CALL);
+		return;
+	}
+
+	do {
+		mutex_lock(&qcom_scm_lock);
+
+		__qcom_scm_call_do(desc, res, fn_id, x5,
+				   ARM_SMCCC_STD_CALL);
+
+		mutex_unlock(&qcom_scm_lock);
+
+		if (res->a0 == QCOM_SCM_V2_EBUSY) {
+			if (retry_count++ > QCOM_SCM_EBUSY_MAX_RETRY)
+				break;
+			msleep(QCOM_SCM_EBUSY_WAIT_MS);
+		}
+	}  while (res->a0 == QCOM_SCM_V2_EBUSY);
+}
+
+static int ___qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
+			    const struct qcom_scm_desc *desc,
+			    struct arm_smccc_res *res, bool atomic)
 {
 	int arglen = desc->arginfo & 0xf;
-	int retry_count = 0, i;
+	int i;
 	u32 fn_id = QCOM_SCM_FNID(svc_id, cmd_id);
-	u64 cmd, x5 = desc->args[FIRST_EXT_ARG_IDX];
+	u64 x5 = desc->args[FIRST_EXT_ARG_IDX];
 	dma_addr_t args_phys = 0;
 	void *args_virt = NULL;
 	size_t alloc_len;
-	struct arm_smccc_quirk quirk = {.id = ARM_SMCCC_QUIRK_QCOM_A6};
+	gfp_t flag = atomic ? GFP_ATOMIC : GFP_KERNEL;
 
 	if (unlikely(arglen > N_REGISTER_ARGS)) {
 		alloc_len = N_EXT_QCOM_SCM_ARGS * sizeof(u64);
-		args_virt = kzalloc(PAGE_ALIGN(alloc_len), GFP_KERNEL);
+		args_virt = kzalloc(PAGE_ALIGN(alloc_len), flag);
 
 		if (!args_virt)
 			return -ENOMEM;
@@ -117,33 +157,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
 		x5 = args_phys;
 	}
 
-	do {
-		mutex_lock(&qcom_scm_lock);
-
-		cmd = ARM_SMCCC_CALL_VAL(ARM_SMCCC_STD_CALL,
-					 qcom_smccc_convention,
-					 ARM_SMCCC_OWNER_SIP, fn_id);
-
-		quirk.state.a6 = 0;
-
-		do {
-			arm_smccc_smc_quirk(cmd, desc->arginfo, desc->args[0],
-				      desc->args[1], desc->args[2], x5,
-				      quirk.state.a6, 0, res, &quirk);
-
-			if (res->a0 == QCOM_SCM_INTERRUPTED)
-				cmd = res->a0;
-
-		} while (res->a0 == QCOM_SCM_INTERRUPTED);
-
-		mutex_unlock(&qcom_scm_lock);
-
-		if (res->a0 == QCOM_SCM_V2_EBUSY) {
-			if (retry_count++ > QCOM_SCM_EBUSY_MAX_RETRY)
-				break;
-			msleep(QCOM_SCM_EBUSY_WAIT_MS);
-		}
-	}  while (res->a0 == QCOM_SCM_V2_EBUSY);
+	qcom_scm_call_do(desc, res, fn_id, x5, atomic);
 
 	if (args_virt) {
 		dma_unmap_single(dev, args_phys, alloc_len, DMA_TO_DEVICE);
@@ -156,6 +170,42 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
 	return 0;
 }
 
+/**
+ * qcom_scm_call() - Invoke a syscall in the secure world
+ * @dev:	device
+ * @svc_id:	service identifier
+ * @cmd_id:	command identifier
+ * @desc:	Descriptor structure containing arguments and return values
+ *
+ * Sends a command to the SCM and waits for the command to finish processing.
+ * This should *only* be called in pre-emptible context.
+ */
+static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
+			 const struct qcom_scm_desc *desc,
+			 struct arm_smccc_res *res)
+{
+	might_sleep();
+	return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, false);
+}
+
+/**
+ * qcom_scm_call_atomic() - atomic variation of qcom_scm_call()
+ * @dev:	device
+ * @svc_id:	service identifier
+ * @cmd_id:	command identifier
+ * @desc:	Descriptor structure containing arguments and return values
+ * @res:	Structure containing results from SMC/HVC call
+ *
+ * Sends a command to the SCM and waits for the command to finish processing.
+ * This can be called in atomic context.
+ */
+static int qcom_scm_call_atomic(struct device *dev, u32 svc_id, u32 cmd_id,
+				const struct qcom_scm_desc *desc,
+				struct arm_smccc_res *res)
+{
+	return ___qcom_scm_call(dev, svc_id, cmd_id, desc, res, true);
+}
+
 /**
  * qcom_scm_set_cold_boot_addr() - Set the cold boot address for cpus
  * @entry: Entry point function for the cpus
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata
  2019-09-20  8:04 [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
  2019-09-20  8:04 ` [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
@ 2019-09-20  8:04 ` Sai Prakash Ranjan
  2019-11-04  5:15   ` Andy Gross
  2019-09-20  8:04 ` [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
  2019-11-01 16:31 ` [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Will Deacon
  3 siblings, 1 reply; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-20  8:04 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, Andy Gross
  Cc: linux-arm-msm, linux-kernel, Rajendra Nayak, Sai Prakash Ranjan

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Qcom's smmu-500 needs to toggle wait-for-safe sequence to
handle TLB invalidation sync's.
Few firmwares allow doing that through SCM interface.
Add API to toggle wait for safe from firmware through a
SCM call.

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/firmware/qcom_scm-32.c |  5 +++++
 drivers/firmware/qcom_scm-64.c | 13 +++++++++++++
 drivers/firmware/qcom_scm.c    |  6 ++++++
 drivers/firmware/qcom_scm.h    |  5 +++++
 include/linux/qcom_scm.h       |  2 ++
 5 files changed, 31 insertions(+)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index 215061c581e1..bee8729525ec 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -614,3 +614,8 @@ int __qcom_scm_io_writel(struct device *dev, phys_addr_t addr, unsigned int val)
 	return qcom_scm_call_atomic2(QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
 				     addr, val);
 }
+
+int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev, bool enable)
+{
+	return -ENODEV;
+}
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 65d772b7fd1e..76867868042a 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -552,3 +552,16 @@ int __qcom_scm_io_writel(struct device *dev, phys_addr_t addr, unsigned int val)
 	return qcom_scm_call(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
 			     &desc, &res);
 }
+
+int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev, bool en)
+{
+	struct qcom_scm_desc desc = {0};
+	struct arm_smccc_res res;
+
+	desc.args[0] = QCOM_SCM_CONFIG_ERRATA1_CLIENT_ALL;
+	desc.args[1] = en;
+	desc.arginfo = QCOM_SCM_ARGS(2);
+
+	return qcom_scm_call_atomic(dev, QCOM_SCM_SVC_SMMU_PROGRAM,
+				    QCOM_SCM_CONFIG_ERRATA1, &desc, &res);
+}
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 4802ab170fe5..a729e05c21b8 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -345,6 +345,12 @@ int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare)
 }
 EXPORT_SYMBOL(qcom_scm_iommu_secure_ptbl_init);
 
+int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
+{
+	return __qcom_scm_qsmmu500_wait_safe_toggle(__scm->dev, en);
+}
+EXPORT_SYMBOL(qcom_scm_qsmmu500_wait_safe_toggle);
+
 int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val)
 {
 	return __qcom_scm_io_readl(__scm->dev, addr, val);
diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
index 99506bd873c0..baee744dbcfe 100644
--- a/drivers/firmware/qcom_scm.h
+++ b/drivers/firmware/qcom_scm.h
@@ -91,10 +91,15 @@ extern int __qcom_scm_restore_sec_cfg(struct device *dev, u32 device_id,
 				      u32 spare);
 #define QCOM_SCM_IOMMU_SECURE_PTBL_SIZE	3
 #define QCOM_SCM_IOMMU_SECURE_PTBL_INIT	4
+#define QCOM_SCM_SVC_SMMU_PROGRAM	0x15
+#define QCOM_SCM_CONFIG_ERRATA1		0x3
+#define QCOM_SCM_CONFIG_ERRATA1_CLIENT_ALL	0x2
 extern int __qcom_scm_iommu_secure_ptbl_size(struct device *dev, u32 spare,
 					     size_t *size);
 extern int __qcom_scm_iommu_secure_ptbl_init(struct device *dev, u64 addr,
 					     u32 size, u32 spare);
+extern int __qcom_scm_qsmmu500_wait_safe_toggle(struct device *dev,
+						bool enable);
 #define QCOM_MEM_PROT_ASSIGN_ID	0x16
 extern int  __qcom_scm_assign_mem(struct device *dev,
 				  phys_addr_t mem_region, size_t mem_sz,
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index 2d5eff506e13..ffd72b3b14ee 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -58,6 +58,7 @@ extern int qcom_scm_set_remote_state(u32 state, u32 id);
 extern int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare);
 extern int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size);
 extern int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare);
+extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
 extern int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val);
 extern int qcom_scm_io_writel(phys_addr_t addr, unsigned int val);
 #else
@@ -97,6 +98,7 @@ qcom_scm_set_remote_state(u32 state,u32 id) { return -ENODEV; }
 static inline int qcom_scm_restore_sec_cfg(u32 device_id, u32 spare) { return -ENODEV; }
 static inline int qcom_scm_iommu_secure_ptbl_size(u32 spare, size_t *size) { return -ENODEV; }
 static inline int qcom_scm_iommu_secure_ptbl_init(u64 addr, u32 size, u32 spare) { return -ENODEV; }
+static inline int qcom_scm_qsmmu500_wait_safe_toggle(bool en) { return -ENODEV; }
 static inline int qcom_scm_io_readl(phys_addr_t addr, unsigned int *val) { return -ENODEV; }
 static inline int qcom_scm_io_writel(phys_addr_t addr, unsigned int val) { return -ENODEV; }
 #endif
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
  2019-09-20  8:04 [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
  2019-09-20  8:04 ` [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
  2019-09-20  8:04 ` [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
@ 2019-09-20  8:04 ` Sai Prakash Ranjan
  2019-09-20 21:07   ` Stephen Boyd
  2019-10-05  5:03   ` Bjorn Andersson
  2019-11-01 16:31 ` [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Will Deacon
  3 siblings, 2 replies; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-20  8:04 UTC (permalink / raw)
  To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, Andy Gross
  Cc: linux-arm-msm, linux-kernel, Rajendra Nayak, Sai Prakash Ranjan

From: Vivek Gautam <vivek.gautam@codeaurora.org>

Add reset hook for sdm845 based platforms to turn off
the wait-for-safe sequence.

Understanding how wait-for-safe logic affects USB and UFS performance
on MTP845 and DB845 boards:

Qcom's implementation of arm,mmu-500 adds a WAIT-FOR-SAFE logic
to address under-performance issues in real-time clients, such as
Display, and Camera.
On receiving an invalidation requests, the SMMU forwards SAFE request
to these clients and waits for SAFE ack signal from real-time clients.
The SAFE signal from such clients is used to qualify the start of
invalidation.
This logic is controlled by chicken bits, one for each - MDP (display),
IFE0, and IFE1 (camera), that can be accessed only from secure software
on sdm845.

This configuration, however, degrades the performance of non-real time
clients, such as USB, and UFS etc. This happens because, with wait-for-safe
logic enabled the hardware tries to throttle non-real time clients while
waiting for SAFE ack signals from real-time clients.

On mtp845 and db845 devices, with wait-for-safe logic enabled by the
bootloaders we see degraded performance of USB and UFS when kernel
enables the smmu stage-1 translations for these clients.
Turn off this wait-for-safe logic from the kernel gets us back the perf
of USB and UFS devices until we re-visit this when we start seeing perf
issues on display/camera on upstream supported SDM845 platforms.
The bootloaders on these boards implement secure monitor callbacks to
handle a specific command - QCOM_SCM_SVC_SMMU_PROGRAM with which the
logic can be toggled.

There are other boards such as cheza whose bootloaders don't enable this
logic. Such boards don't implement callbacks to handle the specific SCM
call so disabling this logic for such boards will be a no-op.

This change is inspired by the downstream change from Patrick Daly
to address performance issues with display and camera by handling
this wait-for-safe within separte io-pagetable ops to do TLB
maintenance. So a big thanks to him for the change and for all the
offline discussions.

Without this change the UFS reads are pretty slow:
$ time dd if=/dev/sda of=/dev/zero bs=1048576 count=10 conv=sync
10+0 records in
10+0 records out
10485760 bytes (10.0MB) copied, 22.394903 seconds, 457.2KB/s
real    0m 22.39s
user    0m 0.00s
sys     0m 0.01s

With this change they are back to rock!
$ time dd if=/dev/sda of=/dev/zero bs=1048576 count=300 conv=sync
300+0 records in
300+0 records out
314572800 bytes (300.0MB) copied, 1.030541 seconds, 291.1MB/s
real    0m 1.03s
user    0m 0.00s
sys     0m 0.54s

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
 drivers/iommu/Makefile        |  2 +-
 drivers/iommu/arm-smmu-impl.c |  5 +++-
 drivers/iommu/arm-smmu-qcom.c | 51 +++++++++++++++++++++++++++++++++++
 drivers/iommu/arm-smmu.h      |  3 +++
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/arm-smmu-qcom.c

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 4f405f926e73..86dadd13b2e6 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
 obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
-obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
+obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o arm-smmu-qcom.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index 5c87a38620c4..b2fe72a8f019 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -109,7 +109,7 @@ static struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smm
 #define ARM_MMU500_ACR_S2CRB_TLBEN	(1 << 10)
 #define ARM_MMU500_ACR_SMTNMB_TLBEN	(1 << 8)
 
-static int arm_mmu500_reset(struct arm_smmu_device *smmu)
+int arm_mmu500_reset(struct arm_smmu_device *smmu)
 {
 	u32 reg, major;
 	int i;
@@ -170,5 +170,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 				  "calxeda,smmu-secure-config-access"))
 		smmu->impl = &calxeda_impl;
 
+	if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
+		return qcom_smmu_impl_init(smmu);
+
 	return smmu;
 }
diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
new file mode 100644
index 000000000000..24c071c1d8b0
--- /dev/null
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/qcom_scm.h>
+
+#include "arm-smmu.h"
+
+struct qcom_smmu {
+	struct arm_smmu_device smmu;
+};
+
+static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
+{
+	int ret;
+
+	arm_mmu500_reset(smmu);
+
+	/*
+	 * To address performance degradation in non-real time clients,
+	 * such as USB and UFS, turn off wait-for-safe on sdm845 based boards,
+	 * such as MTP and db845, whose firmwares implement secure monitor
+	 * call handlers to turn on/off the wait-for-safe logic.
+	 */
+	ret = qcom_scm_qsmmu500_wait_safe_toggle(0);
+	if (ret)
+		dev_warn(smmu->dev, "Failed to turn off SAFE logic\n");
+
+	return ret;
+}
+
+static const struct arm_smmu_impl qcom_smmu_impl = {
+	.reset = qcom_sdm845_smmu500_reset,
+};
+
+struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
+{
+	struct qcom_smmu *qsmmu;
+
+	qsmmu = devm_kzalloc(smmu->dev, sizeof(*qsmmu), GFP_KERNEL);
+	if (!qsmmu)
+		return ERR_PTR(-ENOMEM);
+
+	qsmmu->smmu = *smmu;
+
+	qsmmu->smmu.impl = &qcom_smmu_impl;
+	devm_kfree(smmu->dev, smmu);
+
+	return &qsmmu->smmu;
+}
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index b19b6cae9b5e..de99a85e140a 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -398,5 +398,8 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
 	arm_smmu_writeq((s), ARM_SMMU_CB((s), (n)), (o), (v))
 
 struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
+struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
+
+int arm_mmu500_reset(struct arm_smmu_device *smmu);
 
 #endif /* _ARM_SMMU_H */
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
  2019-09-20  8:04 ` [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
@ 2019-09-20 21:07   ` Stephen Boyd
  2019-10-05  5:03   ` Bjorn Andersson
  1 sibling, 0 replies; 18+ messages in thread
From: Stephen Boyd @ 2019-09-20 21:07 UTC (permalink / raw)
  To: Andy Gross, Joerg Roedel, Robin Murphy, Sai Prakash Ranjan,
	Vivek Gautam, Will Deacon, bjorn.andersson, iommu
  Cc: linux-arm-msm, linux-kernel, Rajendra Nayak, Sai Prakash Ranjan

Quoting Sai Prakash Ranjan (2019-09-20 01:04:29)
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Add reset hook for sdm845 based platforms to turn off
> the wait-for-safe sequence.
> 
> Understanding how wait-for-safe logic affects USB and UFS performance
> on MTP845 and DB845 boards:
> 
> Qcom's implementation of arm,mmu-500 adds a WAIT-FOR-SAFE logic
> to address under-performance issues in real-time clients, such as
> Display, and Camera.
> On receiving an invalidation requests, the SMMU forwards SAFE request
> to these clients and waits for SAFE ack signal from real-time clients.
> The SAFE signal from such clients is used to qualify the start of
> invalidation.
> This logic is controlled by chicken bits, one for each - MDP (display),
> IFE0, and IFE1 (camera), that can be accessed only from secure software
> on sdm845.
> 
> This configuration, however, degrades the performance of non-real time
> clients, such as USB, and UFS etc. This happens because, with wait-for-safe
> logic enabled the hardware tries to throttle non-real time clients while
> waiting for SAFE ack signals from real-time clients.
> 
> On mtp845 and db845 devices, with wait-for-safe logic enabled by the
> bootloaders we see degraded performance of USB and UFS when kernel
> enables the smmu stage-1 translations for these clients.
> Turn off this wait-for-safe logic from the kernel gets us back the perf
> of USB and UFS devices until we re-visit this when we start seeing perf
> issues on display/camera on upstream supported SDM845 platforms.
> The bootloaders on these boards implement secure monitor callbacks to
> handle a specific command - QCOM_SCM_SVC_SMMU_PROGRAM with which the
> logic can be toggled.
> 
> There are other boards such as cheza whose bootloaders don't enable this
> logic. Such boards don't implement callbacks to handle the specific SCM
> call so disabling this logic for such boards will be a no-op.
> 
> This change is inspired by the downstream change from Patrick Daly
> to address performance issues with display and camera by handling
> this wait-for-safe within separte io-pagetable ops to do TLB
> maintenance. So a big thanks to him for the change and for all the
> offline discussions.
> 
> Without this change the UFS reads are pretty slow:
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=10 conv=sync
> 10+0 records in
> 10+0 records out
> 10485760 bytes (10.0MB) copied, 22.394903 seconds, 457.2KB/s
> real    0m 22.39s
> user    0m 0.00s
> sys     0m 0.01s
> 
> With this change they are back to rock!
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=300 conv=sync
> 300+0 records in
> 300+0 records out
> 314572800 bytes (300.0MB) copied, 1.030541 seconds, 291.1MB/s
> real    0m 1.03s
> user    0m 0.00s
> sys     0m 0.54s
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
  2019-09-20  8:04 ` [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
  2019-09-20 21:07   ` Stephen Boyd
@ 2019-10-05  5:03   ` Bjorn Andersson
  1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2019-10-05  5:03 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, Andy Gross, linux-arm-msm, linux-kernel,
	Rajendra Nayak

On Fri 20 Sep 01:04 PDT 2019, Sai Prakash Ranjan wrote:

> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Add reset hook for sdm845 based platforms to turn off
> the wait-for-safe sequence.
> 
> Understanding how wait-for-safe logic affects USB and UFS performance
> on MTP845 and DB845 boards:
> 
> Qcom's implementation of arm,mmu-500 adds a WAIT-FOR-SAFE logic
> to address under-performance issues in real-time clients, such as
> Display, and Camera.
> On receiving an invalidation requests, the SMMU forwards SAFE request
> to these clients and waits for SAFE ack signal from real-time clients.
> The SAFE signal from such clients is used to qualify the start of
> invalidation.
> This logic is controlled by chicken bits, one for each - MDP (display),
> IFE0, and IFE1 (camera), that can be accessed only from secure software
> on sdm845.
> 
> This configuration, however, degrades the performance of non-real time
> clients, such as USB, and UFS etc. This happens because, with wait-for-safe
> logic enabled the hardware tries to throttle non-real time clients while
> waiting for SAFE ack signals from real-time clients.
> 
> On mtp845 and db845 devices, with wait-for-safe logic enabled by the
> bootloaders we see degraded performance of USB and UFS when kernel
> enables the smmu stage-1 translations for these clients.
> Turn off this wait-for-safe logic from the kernel gets us back the perf
> of USB and UFS devices until we re-visit this when we start seeing perf
> issues on display/camera on upstream supported SDM845 platforms.
> The bootloaders on these boards implement secure monitor callbacks to
> handle a specific command - QCOM_SCM_SVC_SMMU_PROGRAM with which the
> logic can be toggled.
> 
> There are other boards such as cheza whose bootloaders don't enable this
> logic. Such boards don't implement callbacks to handle the specific SCM
> call so disabling this logic for such boards will be a no-op.
> 
> This change is inspired by the downstream change from Patrick Daly
> to address performance issues with display and camera by handling
> this wait-for-safe within separte io-pagetable ops to do TLB
> maintenance. So a big thanks to him for the change and for all the
> offline discussions.
> 
> Without this change the UFS reads are pretty slow:
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=10 conv=sync
> 10+0 records in
> 10+0 records out
> 10485760 bytes (10.0MB) copied, 22.394903 seconds, 457.2KB/s
> real    0m 22.39s
> user    0m 0.00s
> sys     0m 0.01s
> 
> With this change they are back to rock!
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=300 conv=sync
> 300+0 records in
> 300+0 records out
> 314572800 bytes (300.0MB) copied, 1.030541 seconds, 291.1MB/s
> real    0m 1.03s
> user    0m 0.00s
> sys     0m 0.54s
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> ---
>  drivers/iommu/Makefile        |  2 +-
>  drivers/iommu/arm-smmu-impl.c |  5 +++-
>  drivers/iommu/arm-smmu-qcom.c | 51 +++++++++++++++++++++++++++++++++++
>  drivers/iommu/arm-smmu.h      |  3 +++
>  4 files changed, 59 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iommu/arm-smmu-qcom.c
> 
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 4f405f926e73..86dadd13b2e6 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -13,7 +13,7 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
>  obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
>  obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
>  obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
> -obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
> +obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o arm-smmu-qcom.o
>  obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
>  obj-$(CONFIG_DMAR_TABLE) += dmar.o
>  obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index 5c87a38620c4..b2fe72a8f019 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -109,7 +109,7 @@ static struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smm
>  #define ARM_MMU500_ACR_S2CRB_TLBEN	(1 << 10)
>  #define ARM_MMU500_ACR_SMTNMB_TLBEN	(1 << 8)
>  
> -static int arm_mmu500_reset(struct arm_smmu_device *smmu)
> +int arm_mmu500_reset(struct arm_smmu_device *smmu)
>  {
>  	u32 reg, major;
>  	int i;
> @@ -170,5 +170,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>  				  "calxeda,smmu-secure-config-access"))
>  		smmu->impl = &calxeda_impl;
>  
> +	if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
> +		return qcom_smmu_impl_init(smmu);
> +
>  	return smmu;
>  }
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> new file mode 100644
> index 000000000000..24c071c1d8b0
> --- /dev/null
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/qcom_scm.h>
> +
> +#include "arm-smmu.h"
> +
> +struct qcom_smmu {
> +	struct arm_smmu_device smmu;
> +};
> +
> +static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +
> +	arm_mmu500_reset(smmu);
> +
> +	/*
> +	 * To address performance degradation in non-real time clients,
> +	 * such as USB and UFS, turn off wait-for-safe on sdm845 based boards,
> +	 * such as MTP and db845, whose firmwares implement secure monitor
> +	 * call handlers to turn on/off the wait-for-safe logic.
> +	 */
> +	ret = qcom_scm_qsmmu500_wait_safe_toggle(0);
> +	if (ret)
> +		dev_warn(smmu->dev, "Failed to turn off SAFE logic\n");
> +
> +	return ret;
> +}
> +
> +static const struct arm_smmu_impl qcom_smmu_impl = {
> +	.reset = qcom_sdm845_smmu500_reset,
> +};
> +
> +struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
> +{
> +	struct qcom_smmu *qsmmu;
> +
> +	qsmmu = devm_kzalloc(smmu->dev, sizeof(*qsmmu), GFP_KERNEL);
> +	if (!qsmmu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	qsmmu->smmu = *smmu;
> +
> +	qsmmu->smmu.impl = &qcom_smmu_impl;
> +	devm_kfree(smmu->dev, smmu);
> +
> +	return &qsmmu->smmu;
> +}
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index b19b6cae9b5e..de99a85e140a 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -398,5 +398,8 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
>  	arm_smmu_writeq((s), ARM_SMMU_CB((s), (n)), (o), (v))
>  
>  struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
> +struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
> +
> +int arm_mmu500_reset(struct arm_smmu_device *smmu);
>  
>  #endif /* _ARM_SMMU_H */
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-09-20  8:04 [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
                   ` (2 preceding siblings ...)
  2019-09-20  8:04 ` [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
@ 2019-11-01 16:31 ` Will Deacon
  2019-11-01 17:19   ` Sai Prakash Ranjan
  3 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-01 16:31 UTC (permalink / raw)
  To: Sai Prakash Ranjan, agross
  Cc: Robin Murphy, Joerg Roedel, iommu, Stephen Boyd, Vivek Gautam,
	bjorn.andersson, linux-arm-msm, linux-kernel, Rajendra Nayak

On Fri, Sep 20, 2019 at 01:34:26PM +0530, Sai Prakash Ranjan wrote:
> Previous version of the patches are at [1]:
> 
> QCOM's implementation of smmu-500 on sdm845 adds a hardware logic called
> wait-for-safe. This logic helps in meeting the invalidation requirements
> from 'real-time clients', such as display and camera. This wait-for-safe
> logic ensures that the invalidations happen after getting an ack from these
> devices.
> In this patch-series we are disabling this wait-for-safe logic from the
> arm-smmu driver's probe as with this enabled the hardware tries to
> throttle invalidations from 'non-real-time clients', such as USB and UFS.
> 
> For detailed information please refer to patch [3/4] in this series.
> I have included the device tree patch too in this series for someone who
> would like to test out this. Here's a branch [2] that gets display on MTP
> SDM845 device.
> 
> This patch series is inspired from downstream work to handle under-performance
> issues on real-time clients on sdm845. In downstream we add separate page table
> ops to handle TLB maintenance and toggle wait-for-safe in tlb_sync call so that
> achieve required performance for display and camera [3, 4].

What's the plan for getting this merged? I'm not happy taking the firmware
bits without Andy's ack, but I also think the SMMU changes should go via
the IOMMU tree to avoid conflicts.

Andy?

Will

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-01 16:31 ` [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Will Deacon
@ 2019-11-01 17:19   ` Sai Prakash Ranjan
  2019-11-01 17:25     ` Will Deacon
  0 siblings, 1 reply; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-11-01 17:19 UTC (permalink / raw)
  To: Will Deacon
  Cc: agross, Robin Murphy, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On 2019-11-01 22:01, Will Deacon wrote:
> On Fri, Sep 20, 2019 at 01:34:26PM +0530, Sai Prakash Ranjan wrote:
>> Previous version of the patches are at [1]:
>> 
>> QCOM's implementation of smmu-500 on sdm845 adds a hardware logic 
>> called
>> wait-for-safe. This logic helps in meeting the invalidation 
>> requirements
>> from 'real-time clients', such as display and camera. This 
>> wait-for-safe
>> logic ensures that the invalidations happen after getting an ack from 
>> these
>> devices.
>> In this patch-series we are disabling this wait-for-safe logic from 
>> the
>> arm-smmu driver's probe as with this enabled the hardware tries to
>> throttle invalidations from 'non-real-time clients', such as USB and 
>> UFS.
>> 
>> For detailed information please refer to patch [3/4] in this series.
>> I have included the device tree patch too in this series for someone 
>> who
>> would like to test out this. Here's a branch [2] that gets display on 
>> MTP
>> SDM845 device.
>> 
>> This patch series is inspired from downstream work to handle 
>> under-performance
>> issues on real-time clients on sdm845. In downstream we add separate 
>> page table
>> ops to handle TLB maintenance and toggle wait-for-safe in tlb_sync 
>> call so that
>> achieve required performance for display and camera [3, 4].
> 
> What's the plan for getting this merged? I'm not happy taking the 
> firmware
> bits without Andy's ack, but I also think the SMMU changes should go 
> via
> the IOMMU tree to avoid conflicts.
> 
> Andy?
> 

Bjorn maintains QCOM stuff now if I am not wrong and he has already 
reviewed the firmware bits. So I'm hoping you could take all these 
through IOMMU tree.

-Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-01 17:19   ` Sai Prakash Ranjan
@ 2019-11-01 17:25     ` Will Deacon
  2019-11-01 17:31       ` Sai Prakash Ranjan
  0 siblings, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-01 17:25 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: agross, Robin Murphy, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On Fri, Nov 01, 2019 at 10:49:00PM +0530, Sai Prakash Ranjan wrote:
> On 2019-11-01 22:01, Will Deacon wrote:
> > On Fri, Sep 20, 2019 at 01:34:26PM +0530, Sai Prakash Ranjan wrote:
> > > Previous version of the patches are at [1]:
> > > 
> > > QCOM's implementation of smmu-500 on sdm845 adds a hardware logic
> > > called
> > > wait-for-safe. This logic helps in meeting the invalidation
> > > requirements
> > > from 'real-time clients', such as display and camera. This
> > > wait-for-safe
> > > logic ensures that the invalidations happen after getting an ack
> > > from these
> > > devices.
> > > In this patch-series we are disabling this wait-for-safe logic from
> > > the
> > > arm-smmu driver's probe as with this enabled the hardware tries to
> > > throttle invalidations from 'non-real-time clients', such as USB and
> > > UFS.
> > > 
> > > For detailed information please refer to patch [3/4] in this series.
> > > I have included the device tree patch too in this series for someone
> > > who
> > > would like to test out this. Here's a branch [2] that gets display
> > > on MTP
> > > SDM845 device.
> > > 
> > > This patch series is inspired from downstream work to handle
> > > under-performance
> > > issues on real-time clients on sdm845. In downstream we add separate
> > > page table
> > > ops to handle TLB maintenance and toggle wait-for-safe in tlb_sync
> > > call so that
> > > achieve required performance for display and camera [3, 4].
> > 
> > What's the plan for getting this merged? I'm not happy taking the
> > firmware
> > bits without Andy's ack, but I also think the SMMU changes should go via
> > the IOMMU tree to avoid conflicts.
> > 
> > Andy?
> > 
> 
> Bjorn maintains QCOM stuff now if I am not wrong and he has already reviewed
> the firmware bits. So I'm hoping you could take all these through IOMMU
> tree.

Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
run:

$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c

in linux-next, I get:

Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
linux-kernel@vger.kernel.org (open list)

Will

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-01 17:25     ` Will Deacon
@ 2019-11-01 17:31       ` Sai Prakash Ranjan
  2019-11-04  5:19         ` Andy Gross
  0 siblings, 1 reply; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-11-01 17:31 UTC (permalink / raw)
  To: Will Deacon, bjorn.andersson
  Cc: agross, Robin Murphy, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On 2019-11-01 22:55, Will Deacon wrote:
> On Fri, Nov 01, 2019 at 10:49:00PM +0530, Sai Prakash Ranjan wrote:
>> On 2019-11-01 22:01, Will Deacon wrote:
>> > On Fri, Sep 20, 2019 at 01:34:26PM +0530, Sai Prakash Ranjan wrote:
>> > > Previous version of the patches are at [1]:
>> > >
>> > > QCOM's implementation of smmu-500 on sdm845 adds a hardware logic
>> > > called
>> > > wait-for-safe. This logic helps in meeting the invalidation
>> > > requirements
>> > > from 'real-time clients', such as display and camera. This
>> > > wait-for-safe
>> > > logic ensures that the invalidations happen after getting an ack
>> > > from these
>> > > devices.
>> > > In this patch-series we are disabling this wait-for-safe logic from
>> > > the
>> > > arm-smmu driver's probe as with this enabled the hardware tries to
>> > > throttle invalidations from 'non-real-time clients', such as USB and
>> > > UFS.
>> > >
>> > > For detailed information please refer to patch [3/4] in this series.
>> > > I have included the device tree patch too in this series for someone
>> > > who
>> > > would like to test out this. Here's a branch [2] that gets display
>> > > on MTP
>> > > SDM845 device.
>> > >
>> > > This patch series is inspired from downstream work to handle
>> > > under-performance
>> > > issues on real-time clients on sdm845. In downstream we add separate
>> > > page table
>> > > ops to handle TLB maintenance and toggle wait-for-safe in tlb_sync
>> > > call so that
>> > > achieve required performance for display and camera [3, 4].
>> >
>> > What's the plan for getting this merged? I'm not happy taking the
>> > firmware
>> > bits without Andy's ack, but I also think the SMMU changes should go via
>> > the IOMMU tree to avoid conflicts.
>> >
>> > Andy?
>> >
>> 
>> Bjorn maintains QCOM stuff now if I am not wrong and he has already 
>> reviewed
>> the firmware bits. So I'm hoping you could take all these through 
>> IOMMU
>> tree.
> 
> Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If 
> I
> run:
> 
> $ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> 
> in linux-next, I get:
> 
> Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> linux-kernel@vger.kernel.org (open list)
> 

It hasn't been updated yet then. I will leave it to Bjorn or Andy to 
comment on this.

-Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata
  2019-09-20  8:04 ` [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
@ 2019-11-04  5:15   ` Andy Gross
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2019-11-04  5:15 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, linux-arm-msm, linux-kernel,
	Rajendra Nayak

On Fri, Sep 20, 2019 at 01:34:28PM +0530, Sai Prakash Ranjan wrote:
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> Qcom's smmu-500 needs to toggle wait-for-safe sequence to
> handle TLB invalidation sync's.
> Few firmwares allow doing that through SCM interface.
> Add API to toggle wait for safe from firmware through a
> SCM call.
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---

Acked-by: Andy Gross <agross@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
  2019-09-20  8:04 ` [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
@ 2019-11-04  5:16   ` Andy Gross
  0 siblings, 0 replies; 18+ messages in thread
From: Andy Gross @ 2019-11-04  5:16 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, bjorn.andersson, linux-arm-msm, linux-kernel,
	Rajendra Nayak

On Fri, Sep 20, 2019 at 01:34:27PM +0530, Sai Prakash Ranjan wrote:
> From: Vivek Gautam <vivek.gautam@codeaurora.org>
> 
> There are scnenarios where drivers are required to make a
> scm call in atomic context, such as in one of the qcom's
> arm-smmu-500 errata [1].
> 
> [1] ("https://source.codeaurora.org/quic/la/kernel/msm-4.9/
>       tree/drivers/iommu/arm-smmu.c?h=msm-4.9#n4842")
> 
> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---

Acked-by: Andy Gross <agross@kernel.org>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-01 17:31       ` Sai Prakash Ranjan
@ 2019-11-04  5:19         ` Andy Gross
  2019-11-04  5:32           ` Sai Prakash Ranjan
  2019-11-04 15:15           ` Will Deacon
  0 siblings, 2 replies; 18+ messages in thread
From: Andy Gross @ 2019-11-04  5:19 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Will Deacon, bjorn.andersson, Robin Murphy, Joerg Roedel, iommu,
	Stephen Boyd, Vivek Gautam, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
> >>> What's the plan for getting this merged? I'm not happy taking the
> >>> firmware
> >>> bits without Andy's ack, but I also think the SMMU changes should go via
> >>> the IOMMU tree to avoid conflicts.
> >>>
> >>> Andy?
> >>>
> >>
> >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
> >>reviewed
> >>the firmware bits. So I'm hoping you could take all these through IOMMU
> >>tree.
> >
> >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
> >run:
> >
> >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> >
> >in linux-next, I get:
> >
> >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> >linux-kernel@vger.kernel.org (open list)
> >
> 
> It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
> on this.

The rumors of my demise have been greatly exaggerated.  All kidding aside, I
ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
He has already done a lot of yeomans work in helping me out the past 3 months.

Andy


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-04  5:19         ` Andy Gross
@ 2019-11-04  5:32           ` Sai Prakash Ranjan
  2019-11-04 15:15           ` Will Deacon
  1 sibling, 0 replies; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-11-04  5:32 UTC (permalink / raw)
  To: Sai Prakash Ranjan
  Cc: Will Deacon, bjorn.andersson, Robin Murphy, Joerg Roedel, iommu,
	Stephen Boyd, Vivek Gautam, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On 2019-11-04 10:49, Andy Gross wrote:
> On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
>> >>> What's the plan for getting this merged? I'm not happy taking the
>> >>> firmware
>> >>> bits without Andy's ack, but I also think the SMMU changes should go via
>> >>> the IOMMU tree to avoid conflicts.
>> >>>
>> >>> Andy?
>> >>>
>> >>
>> >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
>> >>reviewed
>> >>the firmware bits. So I'm hoping you could take all these through IOMMU
>> >>tree.
>> >
>> >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
>> >run:
>> >
>> >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
>> >
>> >in linux-next, I get:
>> >
>> >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
>> >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
>> >linux-kernel@vger.kernel.org (open list)
>> >
>> 
>> It hasn't been updated yet then. I will leave it to Bjorn or Andy to 
>> comment
>> on this.
> 
> The rumors of my demise have been greatly exaggerated.  All kidding 
> aside, I
> ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some 
> point.
> He has already done a lot of yeomans work in helping me out the past 3 
> months.
> 

Thanks for the acks and sorry for that exaggeration :p

-Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-04  5:19         ` Andy Gross
  2019-11-04  5:32           ` Sai Prakash Ranjan
@ 2019-11-04 15:15           ` Will Deacon
  2019-11-04 16:23             ` Will Deacon
  1 sibling, 1 reply; 18+ messages in thread
From: Will Deacon @ 2019-11-04 15:15 UTC (permalink / raw)
  To: Sai Prakash Ranjan, bjorn.andersson, Robin Murphy, Joerg Roedel,
	iommu, Stephen Boyd, Vivek Gautam, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On Sun, Nov 03, 2019 at 11:19:25PM -0600, Andy Gross wrote:
> On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
> > >>> What's the plan for getting this merged? I'm not happy taking the
> > >>> firmware
> > >>> bits without Andy's ack, but I also think the SMMU changes should go via
> > >>> the IOMMU tree to avoid conflicts.
> > >>>
> > >>> Andy?
> > >>>
> > >>
> > >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
> > >>reviewed
> > >>the firmware bits. So I'm hoping you could take all these through IOMMU
> > >>tree.
> > >
> > >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
> > >run:
> > >
> > >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> > >
> > >in linux-next, I get:
> > >
> > >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> > >linux-kernel@vger.kernel.org (open list)
> > >
> > 
> > It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
> > on this.
> 
> The rumors of my demise have been greatly exaggerated.  All kidding aside, I
> ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
> He has already done a lot of yeomans work in helping me out the past 3 months.

Cheers Andy, and I'm pleased to hear that you're still with us! I've queued
this lot for 5.5 and I'll send to Joerg this week.

Will

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-04 15:15           ` Will Deacon
@ 2019-11-04 16:23             ` Will Deacon
  2019-11-04 17:41               ` Bjorn Andersson
  2019-11-05  2:29               ` Sai Prakash Ranjan
  0 siblings, 2 replies; 18+ messages in thread
From: Will Deacon @ 2019-11-04 16:23 UTC (permalink / raw)
  To: Sai Prakash Ranjan, bjorn.andersson, Robin Murphy, Joerg Roedel,
	iommu, Stephen Boyd, Vivek Gautam, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On Mon, Nov 04, 2019 at 03:15:06PM +0000, Will Deacon wrote:
> On Sun, Nov 03, 2019 at 11:19:25PM -0600, Andy Gross wrote:
> > On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
> > > >>> What's the plan for getting this merged? I'm not happy taking the
> > > >>> firmware
> > > >>> bits without Andy's ack, but I also think the SMMU changes should go via
> > > >>> the IOMMU tree to avoid conflicts.
> > > >>>
> > > >>> Andy?
> > > >>>
> > > >>
> > > >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
> > > >>reviewed
> > > >>the firmware bits. So I'm hoping you could take all these through IOMMU
> > > >>tree.
> > > >
> > > >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
> > > >run:
> > > >
> > > >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> > > >
> > > >in linux-next, I get:
> > > >
> > > >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > > >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> > > >linux-kernel@vger.kernel.org (open list)
> > > >
> > > 
> > > It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
> > > on this.
> > 
> > The rumors of my demise have been greatly exaggerated.  All kidding aside, I
> > ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
> > He has already done a lot of yeomans work in helping me out the past 3 months.
> 
> Cheers Andy, and I'm pleased to hear that you're still with us! I've queued
> this lot for 5.5 and I'll send to Joerg this week.

Bah, in doing so I spotted that the existing code doesn't handle error codes
properly because 'a0' is unsigned. I'll queue the patch below at the start
of the series.

Will

--->8

From a9a1047f08de0eff249fb65e2d5d6f6f8b2a87f0 Mon Sep 17 00:00:00 2001
From: Will Deacon <will@kernel.org>
Date: Mon, 4 Nov 2019 15:58:15 +0000
Subject: [PATCH] firmware: qcom: scm: Ensure 'a0' status code is treated as
 signed

The 'a0' member of 'struct arm_smccc_res' is declared as 'unsigned long',
however the Qualcomm SCM firmware interface driver expects to receive
negative error codes via this field, so ensure that it's cast to 'long'
before comparing to see if it is less than 0.

Cc: <stable@vger.kernel.org>
Signed-off-by: Will Deacon <will@kernel.org>
---
 drivers/firmware/qcom_scm-64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 91d5ad7cf58b..25e0f60c759a 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -150,7 +150,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
 		kfree(args_virt);
 	}
 
-	if (res->a0 < 0)
+	if ((long)res->a0 < 0)
 		return qcom_scm_remap_error(res->a0);
 
 	return 0;
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-04 16:23             ` Will Deacon
@ 2019-11-04 17:41               ` Bjorn Andersson
  2019-11-05  2:29               ` Sai Prakash Ranjan
  1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Andersson @ 2019-11-04 17:41 UTC (permalink / raw)
  To: Will Deacon
  Cc: Sai Prakash Ranjan, Robin Murphy, Joerg Roedel, iommu,
	Stephen Boyd, Vivek Gautam, linux-arm-msm, linux-kernel,
	Rajendra Nayak, linux-arm-msm-owner

On Mon 04 Nov 08:23 PST 2019, Will Deacon wrote:

> On Mon, Nov 04, 2019 at 03:15:06PM +0000, Will Deacon wrote:
> > On Sun, Nov 03, 2019 at 11:19:25PM -0600, Andy Gross wrote:
> > > On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
> > > > >>> What's the plan for getting this merged? I'm not happy taking the
> > > > >>> firmware
> > > > >>> bits without Andy's ack, but I also think the SMMU changes should go via
> > > > >>> the IOMMU tree to avoid conflicts.
> > > > >>>
> > > > >>> Andy?
> > > > >>>
> > > > >>
> > > > >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
> > > > >>reviewed
> > > > >>the firmware bits. So I'm hoping you could take all these through IOMMU
> > > > >>tree.
> > > > >
> > > > >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
> > > > >run:
> > > > >
> > > > >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
> > > > >
> > > > >in linux-next, I get:
> > > > >
> > > > >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
> > > > >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
> > > > >linux-kernel@vger.kernel.org (open list)
> > > > >
> > > > 
> > > > It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
> > > > on this.
> > > 
> > > The rumors of my demise have been greatly exaggerated.  All kidding aside, I
> > > ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
> > > He has already done a lot of yeomans work in helping me out the past 3 months.
> > 
> > Cheers Andy, and I'm pleased to hear that you're still with us! I've queued
> > this lot for 5.5 and I'll send to Joerg this week.
> 
> Bah, in doing so I spotted that the existing code doesn't handle error codes
> properly because 'a0' is unsigned. I'll queue the patch below at the start
> of the series.
> 

Thanks, I've hit this a few times but missed this and assumed it was a
firmware issue...

In case you haven't published your branch:
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Will
> 
> --->8
> 
> From a9a1047f08de0eff249fb65e2d5d6f6f8b2a87f0 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Mon, 4 Nov 2019 15:58:15 +0000
> Subject: [PATCH] firmware: qcom: scm: Ensure 'a0' status code is treated as
>  signed
> 
> The 'a0' member of 'struct arm_smccc_res' is declared as 'unsigned long',
> however the Qualcomm SCM firmware interface driver expects to receive
> negative error codes via this field, so ensure that it's cast to 'long'
> before comparing to see if it is less than 0.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  drivers/firmware/qcom_scm-64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
> index 91d5ad7cf58b..25e0f60c759a 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -150,7 +150,7 @@ static int qcom_scm_call(struct device *dev, u32 svc_id, u32 cmd_id,
>  		kfree(args_virt);
>  	}
>  
> -	if (res->a0 < 0)
> +	if ((long)res->a0 < 0)
>  		return qcom_scm_remap_error(res->a0);
>  
>  	return 0;
> -- 
> 2.24.0.rc1.363.gb1bccd3e3d-goog
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845
  2019-11-04 16:23             ` Will Deacon
  2019-11-04 17:41               ` Bjorn Andersson
@ 2019-11-05  2:29               ` Sai Prakash Ranjan
  1 sibling, 0 replies; 18+ messages in thread
From: Sai Prakash Ranjan @ 2019-11-05  2:29 UTC (permalink / raw)
  To: Will Deacon
  Cc: bjorn.andersson, Robin Murphy, Joerg Roedel, iommu, Stephen Boyd,
	Vivek Gautam, linux-arm-msm, linux-kernel, Rajendra Nayak,
	linux-arm-msm-owner, Andy Gross

On 2019-11-04 21:53, Will Deacon wrote:
> On Mon, Nov 04, 2019 at 03:15:06PM +0000, Will Deacon wrote:
>> On Sun, Nov 03, 2019 at 11:19:25PM -0600, Andy Gross wrote:
>> > On Fri, Nov 01, 2019 at 11:01:59PM +0530, Sai Prakash Ranjan wrote:
>> > > >>> What's the plan for getting this merged? I'm not happy taking the
>> > > >>> firmware
>> > > >>> bits without Andy's ack, but I also think the SMMU changes should go via
>> > > >>> the IOMMU tree to avoid conflicts.
>> > > >>>
>> > > >>> Andy?
>> > > >>>
>> > > >>
>> > > >>Bjorn maintains QCOM stuff now if I am not wrong and he has already
>> > > >>reviewed
>> > > >>the firmware bits. So I'm hoping you could take all these through IOMMU
>> > > >>tree.
>> > > >
>> > > >Oh, I didn't realise that. Is there a MAINTAINERS update someplace? If I
>> > > >run:
>> > > >
>> > > >$ ./scripts/get_maintainer.pl -f drivers/firmware/qcom_scm-64.c
>> > > >
>> > > >in linux-next, I get:
>> > > >
>> > > >Andy Gross <agross@kernel.org> (maintainer:ARM/QUALCOMM SUPPORT)
>> > > >linux-arm-msm@vger.kernel.org (open list:ARM/QUALCOMM SUPPORT)
>> > > >linux-kernel@vger.kernel.org (open list)
>> > > >
>> > >
>> > > It hasn't been updated yet then. I will leave it to Bjorn or Andy to comment
>> > > on this.
>> >
>> > The rumors of my demise have been greatly exaggerated.  All kidding aside, I
>> > ack'ed both.  Bjorn will indeed be coming on as a co-maintener at some point.
>> > He has already done a lot of yeomans work in helping me out the past 3 months.
>> 
>> Cheers Andy, and I'm pleased to hear that you're still with us! I've 
>> queued
>> this lot for 5.5 and I'll send to Joerg this week.
> 
> Bah, in doing so I spotted that the existing code doesn't handle error 
> codes
> properly because 'a0' is unsigned. I'll queue the patch below at the 
> start
> of the series.
> 
> Will
> 
> --->8
> 
> From a9a1047f08de0eff249fb65e2d5d6f6f8b2a87f0 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@kernel.org>
> Date: Mon, 4 Nov 2019 15:58:15 +0000
> Subject: [PATCH] firmware: qcom: scm: Ensure 'a0' status code is 
> treated as
>  signed
> 
> The 'a0' member of 'struct arm_smccc_res' is declared as 'unsigned 
> long',
> however the Qualcomm SCM firmware interface driver expects to receive
> negative error codes via this field, so ensure that it's cast to 'long'
> before comparing to see if it is less than 0.
> 
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>  drivers/firmware/qcom_scm-64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/qcom_scm-64.c 
> b/drivers/firmware/qcom_scm-64.c
> index 91d5ad7cf58b..25e0f60c759a 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -150,7 +150,7 @@ static int qcom_scm_call(struct device *dev, u32
> svc_id, u32 cmd_id,
>  		kfree(args_virt);
>  	}
> 
> -	if (res->a0 < 0)
> +	if ((long)res->a0 < 0)
>  		return qcom_scm_remap_error(res->a0);
> 
>  	return 0;

Fixes: 6b1751a86ce2 ("firmware: qcom: scm: Add support for ARM64 SoCs") 
?

FWIW, Reviewed-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

-Sai

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-11-05  2:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20  8:04 [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
2019-09-20  8:04 ` [PATCHv7 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
2019-11-04  5:16   ` Andy Gross
2019-09-20  8:04 ` [PATCHv7 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
2019-11-04  5:15   ` Andy Gross
2019-09-20  8:04 ` [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
2019-09-20 21:07   ` Stephen Boyd
2019-10-05  5:03   ` Bjorn Andersson
2019-11-01 16:31 ` [PATCHv7 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Will Deacon
2019-11-01 17:19   ` Sai Prakash Ranjan
2019-11-01 17:25     ` Will Deacon
2019-11-01 17:31       ` Sai Prakash Ranjan
2019-11-04  5:19         ` Andy Gross
2019-11-04  5:32           ` Sai Prakash Ranjan
2019-11-04 15:15           ` Will Deacon
2019-11-04 16:23             ` Will Deacon
2019-11-04 17:41               ` Bjorn Andersson
2019-11-05  2:29               ` Sai Prakash Ranjan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).