2019-10-25 13:46:24

by Denys Vlasenko

Subject: [PATCH] iommu/amd: Do not re-fetch iommu->cmd_buf_tail

The compiler is not smart enough to realize that iommu->cmd_buf_tail
can't be modified across memcpy:

41 8b 45 74 mov 0x74(%r13),%eax # iommu->cmd_buf_tail
44 8d 78 10 lea 0x10(%rax),%r15d # += sizeof(*cmd)
41 81 e7 ff 1f 00 00 and $0x1fff,%r15d # %= CMD_BUFFER_SIZE
49 03 45 68 add 0x68(%r13),%rax # target = iommu->cmd_buf + iommu->cmd_buf_tail
45 89 7d 74 mov %r15d,0x74(%r13) # store to iommu->cmd_buf_tail
49 8b 34 24 mov (%r12),%rsi # memcpy
49 8b 7c 24 08 mov 0x8(%r12),%rdi # memcpy
48 89 30 mov %rsi,(%rax) # memcpy
48 89 78 08 mov %rdi,0x8(%rax) # memcpy
49 8b 55 38 mov 0x38(%r13),%rdx # iommu->mmio_base
41 8b 45 74 mov 0x74(%r13),%eax # redundant load of iommu->cmd_buf_tail
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
89 82 08 20 00 00 mov %eax,0x2008(%rdx) # writel

CC: Tom Lendacky <[email protected]>
CC: Joerg Roedel <[email protected]>
CC: [email protected]
Signed-off-by: Denys Vlasenko <[email protected]>
---
drivers/iommu/amd_iommu.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index dd555078258c..34c497c4b0a7 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -876,17 +876,18 @@ static void copy_cmd_to_buffer(struct amd_iommu *iommu,
struct iommu_cmd *cmd)
{
u8 *target;
-
- target = iommu->cmd_buf + iommu->cmd_buf_tail;
-
- iommu->cmd_buf_tail += sizeof(*cmd);
- iommu->cmd_buf_tail %= CMD_BUFFER_SIZE;
+ u32 tail;

/* Copy command to buffer */
+ tail = iommu->cmd_buf_tail;
+ target = iommu->cmd_buf + tail;
memcpy(target, cmd, sizeof(*cmd));

+ tail = (tail + sizeof(*cmd)) % CMD_BUFFER_SIZE;
+ iommu->cmd_buf_tail = tail;
+
/* Tell the IOMMU about it */
- writel(iommu->cmd_buf_tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
+ writel(tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
}

static void build_completion_wait(struct iommu_cmd *cmd, u64 address)
--
2.21.0

2019-10-30 09:21:46

by Jörg Rödel

Subject: Re: [PATCH] iommu/amd: Do not re-fetch iommu->cmd_buf_tail

On Thu, Oct 24, 2019 at 02:54:10PM +0200, Denys Vlasenko wrote:
> The compiler is not smart enough to realize that iommu->cmd_buf_tail
> can't be modified across memcpy:
>
> 41 8b 45 74 mov 0x74(%r13),%eax # iommu->cmd_buf_tail
> 44 8d 78 10 lea 0x10(%rax),%r15d # += sizeof(*cmd)
> 41 81 e7 ff 1f 00 00 and $0x1fff,%r15d # %= CMD_BUFFER_SIZE
> 49 03 45 68 add 0x68(%r13),%rax # target = iommu->cmd_buf + iommu->cmd_buf_tail
> 45 89 7d 74 mov %r15d,0x74(%r13) # store to iommu->cmd_buf_tail
> 49 8b 34 24 mov (%r12),%rsi # memcpy
> 49 8b 7c 24 08 mov 0x8(%r12),%rdi # memcpy
> 48 89 30 mov %rsi,(%rax) # memcpy
> 48 89 78 08 mov %rdi,0x8(%rax) # memcpy
> 49 8b 55 38 mov 0x38(%r13),%rdx # iommu->mmio_base
> 41 8b 45 74 mov 0x74(%r13),%eax # redundant load of iommu->cmd_buf_tail
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 89 82 08 20 00 00 mov %eax,0x2008(%rdx) # writel
>
> CC: Tom Lendacky <[email protected]>
> CC: Joerg Roedel <[email protected]>
> CC: [email protected]
> Signed-off-by: Denys Vlasenko <[email protected]>
> ---
> drivers/iommu/amd_iommu.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)

Applied, thanks.