RFC --> v1
1. Resend the patches for ECMDQ preparation and remove the patches for ECMDQ implementation.
2. Patch 2 is modified. Other patches remain unchanged.
1) Add static helper __arm_smmu_cmdq_issue_cmd(), and make arm_smmu_cmdq_issue_cmd()
and arm_smmu_cmdq_issue_cmd_with_sync() implement based on it.
2) Remove unused arm_smmu_cmdq_issue_sync().
RFC:
https://www.spinics.net/lists/arm-kernel/msg904879.html
Zhen Lei (4):
iommu/arm-smmu-v3: Use command queue batching helpers to improve
performance
iommu/arm-smmu-v3: Add and use static helper function
arm_smmu_cmdq_issue_cmd_with_sync()
iommu/arm-smmu-v3: Add and use static helper function
arm_smmu_get_cmdq()
iommu/arm-smmu-v3: Extract reusable function
__arm_smmu_cmdq_skip_err()
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 71 ++++++++++++---------
1 file changed, 42 insertions(+), 29 deletions(-)
--
2.26.0.106.g9fadedd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, function arm_smmu_cmdq_issue_cmd_with_sync() is added to insert
the 'cmd+sync' commands at a time.
Signed-off-by: Zhen Lei <redacted>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 35 +++++++++++----------
1 file changed, 19 insertions(+), 16 deletions(-)
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, use command queue batching helpers to insert multiple commands
at a time.
Signed-off-by: Zhen Lei <redacted>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
When SMMU_GERROR.CMDQP_ERR is different to SMMU_GERRORN.CMDQP_ERR, it
indicates that one or more errors have been encountered on a command queue
control page interface. We need to traverse all ECMDQs in that control
page to find all errors. For each ECMDQ error handling, it is much the
same as the CMDQ error handling. This common processing part is extracted
as a new function __arm_smmu_cmdq_skip_err().
Signed-off-by: Zhen Lei <redacted>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
One SMMU has only one normal CMDQ. Therefore, this CMDQ is used regardless
of the core on which the command is inserted. It can be referenced
directly through "smmu->cmdq". However, one SMMU has multiple ECMDQs, and
the ECMDQ used by the core on which the command insertion is executed may
be different. So the helper function arm_smmu_get_cmdq() is added, which
returns the CMDQ/ECMDQ that the current core should use. Currently, the
code that supports ECMDQ is not added. just simply returns "&smmu->cmdq".
Many subfunctions of arm_smmu_cmdq_issue_cmdlist() use "&smmu->cmdq" or
"&smmu->cmdq.q" directly. To support ECMDQ, they need to call the newly
added function arm_smmu_get_cmdq() instead.
Note that normal CMDQ is still required until ECMDQ is available.
Signed-off-by: Zhen Lei <redacted>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 ++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-08-13 14:34:03
On Wed, 11 Aug 2021 19:48:48 +0800, Zhen Lei wrote:
RFC --> v1
1. Resend the patches for ECMDQ preparation and remove the patches for ECMDQ implementation.
2. Patch 2 is modified. Other patches remain unchanged.
1) Add static helper __arm_smmu_cmdq_issue_cmd(), and make arm_smmu_cmdq_issue_cmd()
and arm_smmu_cmdq_issue_cmd_with_sync() implement based on it.
2) Remove unused arm_smmu_cmdq_issue_sync().
[...]
From: Robin Murphy <robin.murphy@arm.com> Date: 2021-08-13 16:01:46
On 2021-08-11 12:48, Zhen Lei wrote:
quoted hunk
The obvious key to the performance optimization of commit 587e6c10a7ce
("iommu/arm-smmu-v3: Reduce contention during command-queue insertion") is
to allow multiple cores to insert commands in parallel after a brief mutex
contention.
Obviously, inserting as many commands at a time as possible can reduce the
number of times the mutex contention participates, thereby improving the
overall performance. At least it reduces the number of calls to function
arm_smmu_cmdq_issue_cmdlist().
Therefore, use command queue batching helpers to insert multiple commands
at a time.
Signed-off-by: Zhen Lei <redacted>
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
BTW, it looks like this has crossed over with John's patch removing these.
It is only called from arm_smmu_disable_ats(), so not hot-path by the
look for it. Or who even has ats HW ...?
But it should be at least cleaned-up for consistency. Leizhen?
Thanks,
John
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
BTW, it looks like this has crossed over with John's patch removing these.
It is only called from arm_smmu_disable_ats(), so not hot-path by the look for it. Or who even has ats HW ...?
But it should be at least cleaned-up for consistency. Leizhen?
Okay, I'll revise it. But Will already took it. So I'm not sure whether to send v2 or a separate patch.
BTW, it looks like this has crossed over with John's patch removing these.
It is only called from arm_smmu_disable_ats(), so not hot-path by the look for it. Or who even has ats HW ...?
But it should be at least cleaned-up for consistency. Leizhen?
Okay, I'll revise it. But Will already took it. So I'm not sure whether to send v2 or a separate patch.
I think I'd better post v2, otherwise I should write the same description.
In addition, I find that function arm_smmu_cmdq_build_cmd() can also be optimized
slightly, three useless instructions can be reduced.
Case 1):
void arm_smmu_cmdq_build_cmd_tst1(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
cmd[0] |= FIELD_PREP(CMDQ_0_OP, ent->opcode);
}
0000000000004608 <arm_smmu_cmdq_build_cmd_tst1>:
4608: a9007c1f stp xzr, xzr, [x0]
460c: 39400022 ldrb w2, [x1]
4610: f9400001 ldr x1, [x0]
4614: aa020021 orr x1, x1, x2
4618: f9000001 str x1, [x0]
461c: d65f03c0 ret
Case 2):
void arm_smmu_cmdq_build_cmd_tst2(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
int i;
cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
for (i = 1; i < CMDQ_ENT_DWORDS; i++)
cmd[i] = 0;
}
0000000000004620 <arm_smmu_cmdq_build_cmd_tst2>:
4620: 39400021 ldrb w1, [x1]
4624: a9007c01 stp x1, xzr, [x0]
4628: d65f03c0 ret
462c: d503201f nop
Case 3):
void arm_smmu_cmdq_build_cmd_tst3(u64 *cmd, struct arm_smmu_cmdq_ent *ent)
{
memset(cmd, 0, 1 << CMDQ_ENT_SZ_SHIFT);
cmd[0] = FIELD_PREP(CMDQ_0_OP, ent->opcode);
}
0000000000004630 <arm_smmu_cmdq_build_cmd_tst3>:
4630: a9007c1f stp xzr, xzr, [x0]
4634: 39400021 ldrb w1, [x1]
4638: f9000001 str x1, [x0]
463c: d65f03c0 ret
From: John Garry <hidden> Date: 2021-08-16 07:25:26
In addition, I find that function arm_smmu_cmdq_build_cmd() can also be optimized
slightly, three useless instructions can be reduced.
I think that you could optimise further by pre-building commonly used
commands.
For example, CMD_SYNC without MSI polling is always the same. And then
only different in 1 field for MSI polling.
But you need to check if the performance gain is worth the change.
In addition, I find that function arm_smmu_cmdq_build_cmd() can also be optimized
slightly, three useless instructions can be reduced.
I think that you could optimise further by pre-building commonly used commands.
For example, CMD_SYNC without MSI polling is always the same. And then only different in 1 field for MSI polling.
But you need to check if the performance gain is worth the change.
From: Will Deacon <will@kernel.org> Date: 2021-08-16 08:21:38
On Mon, Aug 16, 2021 at 03:47:58PM +0800, Leizhen (ThunderTown) wrote:
On 2021/8/16 15:24, John Garry wrote:
quoted
quoted
In addition, I find that function arm_smmu_cmdq_build_cmd() can also be optimized
slightly, three useless instructions can be reduced.
I think that you could optimise further by pre-building commonly used commands.
For example, CMD_SYNC without MSI polling is always the same. And then only different in 1 field for MSI polling.
But you need to check if the performance gain is worth the change.
Good advice. I can give it a try.
Please send it as a new patch on top. I've queued the old one and sent
it to Joerg. Since this is just further cleanup, it can be done separately.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 16, 2021 at 03:47:58PM +0800, Leizhen (ThunderTown) wrote:
quoted
On 2021/8/16 15:24, John Garry wrote:
quoted
quoted
In addition, I find that function arm_smmu_cmdq_build_cmd() can also be optimized
slightly, three useless instructions can be reduced.
I think that you could optimise further by pre-building commonly used commands.
For example, CMD_SYNC without MSI polling is always the same. And then only different in 1 field for MSI polling.
But you need to check if the performance gain is worth the change.
Good advice. I can give it a try.
Please send it as a new patch on top. I've queued the old one and sent
it to Joerg. Since this is just further cleanup, it can be done separately.