* [PATCHv6 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
2019-09-17 9:45 [PATCHv6 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
@ 2019-09-17 9:45 ` Sai Prakash Ranjan
2019-09-19 0:50 ` Stephen Boyd
2019-09-17 9:45 ` [PATCHv6 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
2019-09-17 9:45 ` [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
2 siblings, 1 reply; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-17 9:45 UTC (permalink / raw)
To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
Vivek Gautam, bjorn.andersson, Andy Gross
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
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>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
---
drivers/firmware/qcom_scm-64.c | 139 ++++++++++++++++++++++-----------
1 file changed, 95 insertions(+), 44 deletions(-)
diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-64.c
index 91d5ad7cf58b..6697bb741b83 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,43 @@ 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 should be called in atomic context only.
+ */
+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)
+{
+ cant_sleep();
+ 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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCHv6 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call
2019-09-17 9:45 ` [PATCHv6 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
@ 2019-09-19 0:50 ` Stephen Boyd
0 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2019-09-19 0:50 UTC (permalink / raw)
To: Andy Gross, Joerg Roedel, Robin Murphy, Sai Prakash Ranjan,
Vivek Gautam, Will Deacon, bjorn.andersson, iommu
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
Quoting Sai Prakash Ranjan (2019-09-17 02:45:02)
> 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>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv6 2/3] firmware/qcom_scm: Add scm call to handle smmu errata
2019-09-17 9:45 [PATCHv6 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
2019-09-17 9:45 ` [PATCHv6 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
@ 2019-09-17 9:45 ` Sai Prakash Ranjan
2019-09-19 0:53 ` Stephen Boyd
2019-09-17 9:45 ` [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
2 siblings, 1 reply; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-17 9:45 UTC (permalink / raw)
To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
Vivek Gautam, bjorn.andersson, Andy Gross
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
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>
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 6697bb741b83..2f1b7d1660ff 100644
--- a/drivers/firmware/qcom_scm-64.c
+++ b/drivers/firmware/qcom_scm-64.c
@@ -553,3 +553,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 2ddc118dba1b..2b3b7a8c4270 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -344,6 +344,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 3f12cc77fb58..aee3d8580d89 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -57,6 +57,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
@@ -96,6 +97,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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCHv6 2/3] firmware/qcom_scm: Add scm call to handle smmu errata
2019-09-17 9:45 ` [PATCHv6 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
@ 2019-09-19 0:53 ` Stephen Boyd
0 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2019-09-19 0:53 UTC (permalink / raw)
To: Andy Gross, Joerg Roedel, Robin Murphy, Sai Prakash Ranjan,
Vivek Gautam, Will Deacon, bjorn.andersson, iommu
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
Quoting Sai Prakash Ranjan (2019-09-17 02:45:03)
> 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>
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> ---
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-17 9:45 [PATCHv6 0/3] QCOM smmu-500 wait-for-safe handling for sdm845 Sai Prakash Ranjan
2019-09-17 9:45 ` [PATCHv6 1/3] firmware: qcom_scm-64: Add atomic version of qcom_scm_call Sai Prakash Ranjan
2019-09-17 9:45 ` [PATCHv6 2/3] firmware/qcom_scm: Add scm call to handle smmu errata Sai Prakash Ranjan
@ 2019-09-17 9:45 ` Sai Prakash Ranjan
2019-09-19 0:25 ` Bjorn Andersson
2019-09-19 0:57 ` Stephen Boyd
2 siblings, 2 replies; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-17 9:45 UTC (permalink / raw)
To: Robin Murphy, Will Deacon, Joerg Roedel, iommu, Stephen Boyd,
Vivek Gautam, bjorn.andersson, Andy Gross
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
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 | 6 +++--
drivers/iommu/arm-smmu-qcom.c | 51 +++++++++++++++++++++++++++++++++++
drivers/iommu/arm-smmu.h | 3 +++
4 files changed, 59 insertions(+), 3 deletions(-)
create mode 100644 drivers/iommu/arm-smmu-qcom.c
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index a2729aadd300..2816e49a8c46 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
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 3f88cd078dd5..d62da270f430 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -9,7 +9,6 @@
#include "arm-smmu.h"
-
static int arm_smmu_gr0_ns(int offset)
{
switch(offset) {
@@ -109,7 +108,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 +169,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 ac9eac966cf5..4bc9e853c95d 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -391,5 +391,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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-17 9:45 ` [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
@ 2019-09-19 0:25 ` Bjorn Andersson
2019-09-19 0:52 ` Stephen Boyd
2019-09-19 3:18 ` Sai Prakash Ranjan
2019-09-19 0:57 ` Stephen Boyd
1 sibling, 2 replies; 14+ messages in thread
From: Bjorn Andersson @ 2019-09-19 0:25 UTC (permalink / raw)
To: Sai Prakash Ranjan
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, Stephen Boyd, iommu,
Andy Gross, linux-arm-msm, Will Deacon
On Tue 17 Sep 02:45 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>
> ---
> drivers/iommu/Makefile | 2 +-
> drivers/iommu/arm-smmu-impl.c | 6 +++--
> drivers/iommu/arm-smmu-qcom.c | 51 +++++++++++++++++++++++++++++++++++
> drivers/iommu/arm-smmu.h | 3 +++
> 4 files changed, 59 insertions(+), 3 deletions(-)
> create mode 100644 drivers/iommu/arm-smmu-qcom.c
>
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index a2729aadd300..2816e49a8c46 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
> 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 3f88cd078dd5..d62da270f430 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -9,7 +9,6 @@
>
> #include "arm-smmu.h"
>
> -
> static int arm_smmu_gr0_ns(int offset)
> {
> switch(offset) {
> @@ -109,7 +108,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 +169,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);
In the transition to this new design we lost the ability to
enable/disable the safe toggle per board, which according to Vivek
would result in some issue with Cheza.
Can you confirm that this is okay? (Or introduce the DT property for
enabling the safe_toggle logic?)
Regards,
Bjorn
> + 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 ac9eac966cf5..4bc9e853c95d 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -391,5 +391,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
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 0:25 ` Bjorn Andersson
@ 2019-09-19 0:52 ` Stephen Boyd
2019-09-19 3:18 ` Sai Prakash Ranjan
1 sibling, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2019-09-19 0:52 UTC (permalink / raw)
To: Bjorn Andersson, Sai Prakash Ranjan
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, iommu, Andy Gross,
linux-arm-msm, Will Deacon
Quoting Bjorn Andersson (2019-09-18 17:25:01)
> On Tue 17 Sep 02:45 PDT 2019, Sai Prakash Ranjan wrote:
>
> > From: Vivek Gautam <vivek.gautam@codeaurora.org>
> >
> > 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.
> >
[...]
> > 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);
>
> In the transition to this new design we lost the ability to
> enable/disable the safe toggle per board, which according to Vivek
> would result in some issue with Cheza.
>
> Can you confirm that this is okay? (Or introduce the DT property for
> enabling the safe_toggle logic?)
>
I can test this on Cheza. Not sure if anything will happen but it's
worth a shot.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 0:25 ` Bjorn Andersson
2019-09-19 0:52 ` Stephen Boyd
@ 2019-09-19 3:18 ` Sai Prakash Ranjan
2019-09-19 18:54 ` Sai Prakash Ranjan
1 sibling, 1 reply; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-19 3:18 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, Stephen Boyd, iommu,
Andy Gross, linux-arm-msm, Will Deacon
On 2019-09-19 05:55, Bjorn Andersson wrote:
> In the transition to this new design we lost the ability to
> enable/disable the safe toggle per board, which according to Vivek
> would result in some issue with Cheza.
>
> Can you confirm that this is okay? (Or introduce the DT property for
> enabling the safe_toggle logic?)
>
Hmm, I don't remember Vivek telling about any issue on Cheza because of
this logic.
But I will test this on Cheza and let you know.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 3:18 ` Sai Prakash Ranjan
@ 2019-09-19 18:54 ` Sai Prakash Ranjan
2019-09-19 20:00 ` Stephen Boyd
0 siblings, 1 reply; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-19 18:54 UTC (permalink / raw)
To: Bjorn Andersson, Stephen Boyd
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, iommu, Andy Gross,
linux-arm-msm, Will Deacon, linux-arm-msm-owner
On 2019-09-19 08:48, Sai Prakash Ranjan wrote:
> On 2019-09-19 05:55, Bjorn Andersson wrote:
>> In the transition to this new design we lost the ability to
>> enable/disable the safe toggle per board, which according to Vivek
>> would result in some issue with Cheza.
>>
>> Can you confirm that this is okay? (Or introduce the DT property for
>> enabling the safe_toggle logic?)
>>
>
> Hmm, I don't remember Vivek telling about any issue on Cheza because
> of this logic.
> But I will test this on Cheza and let you know.
>
I tested this on Cheza and no perf degradation nor any other issue is
seen
atleast openly, although I see this below stack dump always with
cant_sleep change added.
[ 5.048860] BUG: assuming atomic context at
/mnt/host/source/src/third_party/kernel/v5.3/drivers/firmware/qcom_scm-64.c:206
[ 5.060303] in_atomic(): 0, irqs_disabled(): 0, pid: 1, name:
swapper/0
[ 5.067118] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.3.0 #102
[ 5.073299] Hardware name: Google Cheza (rev3+) (DT)
[ 5.078416] Call trace:
[ 5.080953] dump_backtrace+0x0/0x16c
[ 5.084727] show_stack+0x20/0x2c
[ 5.088156] dump_stack+0x90/0xcc
[ 5.091585] __cant_sleep+0xb4/0xc4
[ 5.095192] __qcom_scm_qsmmu500_wait_safe_toggle+0x5c/0xa0
[ 5.100929] qcom_scm_qsmmu500_wait_safe_toggle+0x28/0x34
[ 5.106491] qcom_sdm845_smmu500_reset+0x24/0x50
[ 5.111249] arm_smmu_device_reset+0x1a4/0x25c
[ 5.115827] arm_smmu_device_probe+0x418/0x50c
[ 5.120406] platform_drv_probe+0x90/0xb0
[ 5.124542] really_probe+0x14c/0x3b8
[ 5.128327] driver_probe_device+0x70/0x140
[ 5.132643] device_driver_attach+0x4c/0x6c
[ 5.136960] __driver_attach+0xc8/0x150
[ 5.140917] bus_for_each_dev+0x84/0xcc
[ 5.144873] driver_attach+0x2c/0x38
[ 5.148555] bus_add_driver+0x108/0x1fc
[ 5.152512] driver_register+0x64/0xf8
[ 5.156375] __platform_driver_register+0x4c/0x58
[ 5.161226] arm_smmu_driver_init+0x1c/0x24
[ 5.165545] do_one_initcall+0x11c/0x2dc
[ 5.169595] do_initcall_level+0x14c/0x174
[ 5.173822] do_basic_setup+0x30/0x48
[ 5.177595] kernel_init_freeable+0xc4/0x144
[ 5.181990] kernel_init+0x14/0x100
[ 5.185584] ret_from_fork+0x10/0x18
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 18:54 ` Sai Prakash Ranjan
@ 2019-09-19 20:00 ` Stephen Boyd
2019-09-20 7:54 ` Sai Prakash Ranjan
0 siblings, 1 reply; 14+ messages in thread
From: Stephen Boyd @ 2019-09-19 20:00 UTC (permalink / raw)
To: Bjorn Andersson, Sai Prakash Ranjan
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, iommu, Andy Gross,
linux-arm-msm, Will Deacon, linux-arm-msm-owner
Quoting Sai Prakash Ranjan (2019-09-19 11:54:27)
> On 2019-09-19 08:48, Sai Prakash Ranjan wrote:
> > On 2019-09-19 05:55, Bjorn Andersson wrote:
> >> In the transition to this new design we lost the ability to
> >> enable/disable the safe toggle per board, which according to Vivek
> >> would result in some issue with Cheza.
> >>
> >> Can you confirm that this is okay? (Or introduce the DT property for
> >> enabling the safe_toggle logic?)
> >>
> >
> > Hmm, I don't remember Vivek telling about any issue on Cheza because
> > of this logic.
> > But I will test this on Cheza and let you know.
> >
>
> I tested this on Cheza and no perf degradation nor any other issue is
> seen
> atleast openly, although I see this below stack dump always with
> cant_sleep change added.
The usage of cant_sleep() here is wrong then, and the comment should be
removed from the scm API as well because it looks like it's perfectly OK
to call the function from a context that can sleep.
>
> [ 5.048860] BUG: assuming atomic context at
> /mnt/host/source/src/third_party/kernel/v5.3/drivers/firmware/qcom_scm-64.c:206
> [ 5.060303] in_atomic(): 0, irqs_disabled(): 0, pid: 1, name:
> swapper/0
> [ 5.067118] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.3.0 #102
> [ 5.073299] Hardware name: Google Cheza (rev3+) (DT)
> [ 5.078416] Call trace:
> [ 5.080953] dump_backtrace+0x0/0x16c
> [ 5.084727] show_stack+0x20/0x2c
> [ 5.088156] dump_stack+0x90/0xcc
> [ 5.091585] __cant_sleep+0xb4/0xc4
> [ 5.095192] __qcom_scm_qsmmu500_wait_safe_toggle+0x5c/0xa0
> [ 5.100929] qcom_scm_qsmmu500_wait_safe_toggle+0x28/0x34
> [ 5.106491] qcom_sdm845_smmu500_reset+0x24/0x50
> [ 5.111249] arm_smmu_device_reset+0x1a4/0x25c
> [ 5.115827] arm_smmu_device_probe+0x418/0x50c
> [ 5.120406] platform_drv_probe+0x90/0xb0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 20:00 ` Stephen Boyd
@ 2019-09-20 7:54 ` Sai Prakash Ranjan
0 siblings, 0 replies; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-20 7:54 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, Bjorn Andersson,
iommu, Andy Gross, linux-arm-msm, Will Deacon,
linux-arm-msm-owner
On 2019-09-20 01:30, Stephen Boyd wrote:
> Quoting Sai Prakash Ranjan (2019-09-19 11:54:27)
>> On 2019-09-19 08:48, Sai Prakash Ranjan wrote:
>> > On 2019-09-19 05:55, Bjorn Andersson wrote:
>> >> In the transition to this new design we lost the ability to
>> >> enable/disable the safe toggle per board, which according to Vivek
>> >> would result in some issue with Cheza.
>> >>
>> >> Can you confirm that this is okay? (Or introduce the DT property for
>> >> enabling the safe_toggle logic?)
>> >>
>> >
>> > Hmm, I don't remember Vivek telling about any issue on Cheza because
>> > of this logic.
>> > But I will test this on Cheza and let you know.
>> >
>>
>> I tested this on Cheza and no perf degradation nor any other issue is
>> seen
>> atleast openly, although I see this below stack dump always with
>> cant_sleep change added.
>
> The usage of cant_sleep() here is wrong then, and the comment should be
> removed from the scm API as well because it looks like it's perfectly
> OK
> to call the function from a context that can sleep.
>
Looking more into this downstream, it says this *can be called in atomic
context*
and not *should be called only in atomic context*. So will remove
cant_sleep and
modify the comment accordingly.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-17 9:45 ` [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook Sai Prakash Ranjan
2019-09-19 0:25 ` Bjorn Andersson
@ 2019-09-19 0:57 ` Stephen Boyd
2019-09-19 3:13 ` Sai Prakash Ranjan
1 sibling, 1 reply; 14+ messages in thread
From: Stephen Boyd @ 2019-09-19 0:57 UTC (permalink / raw)
To: Andy Gross, Joerg Roedel, Robin Murphy, Sai Prakash Ranjan,
Vivek Gautam, Will Deacon, bjorn.andersson, iommu
Cc: linux-arm-msm, Rajendra Nayak, linux-kernel
Quoting Sai Prakash Ranjan (2019-09-17 02:45:04)
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index 3f88cd078dd5..d62da270f430 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -9,7 +9,6 @@
>
> #include "arm-smmu.h"
>
> -
> static int arm_smmu_gr0_ns(int offset)
> {
> switch(offset) {
Why is this hunk still around?
> 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 @@
[...]
> +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);
This copy is interesting but OK I guess cavium does it.
> +
> + return &qsmmu->smmu;
> +}
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCHv6 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
2019-09-19 0:57 ` Stephen Boyd
@ 2019-09-19 3:13 ` Sai Prakash Ranjan
0 siblings, 0 replies; 14+ messages in thread
From: Sai Prakash Ranjan @ 2019-09-19 3:13 UTC (permalink / raw)
To: Stephen Boyd
Cc: Rajendra Nayak, Robin Murphy, linux-kernel, bjorn.andersson,
iommu, Andy Gross, linux-arm-msm, Will Deacon,
linux-arm-msm-owner
On 2019-09-19 06:27, Stephen Boyd wrote:
> Quoting Sai Prakash Ranjan (2019-09-17 02:45:04)
>> diff --git a/drivers/iommu/arm-smmu-impl.c
>> b/drivers/iommu/arm-smmu-impl.c
>> index 3f88cd078dd5..d62da270f430 100644
>> --- a/drivers/iommu/arm-smmu-impl.c
>> +++ b/drivers/iommu/arm-smmu-impl.c
>> @@ -9,7 +9,6 @@
>>
>> #include "arm-smmu.h"
>>
>> -
>> static int arm_smmu_gr0_ns(int offset)
>> {
>> switch(offset) {
>
> Why is this hunk still around?
I remember correcting this in previous version but somehow slipped in
this version. Will correct it.
>
>> 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 @@
> [...]
>> +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);
>
> This copy is interesting but OK I guess cavium does it.
>
This is from nvidia impl since Robin pointed me at its implementation.
>> +
>> + return &qsmmu->smmu;
>> +}
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 14+ messages in thread