On 24/09/18 12:59, Paul Durrant wrote: >> -----Original Message----- >> From: Andrew Cooper [mailto:[email protected]] >> Sent: 24 September 2018 11:55 >> To: Xen-devel <[email protected]> >> Cc: Andrew Cooper <[email protected]>; Jan Beulich >> <[email protected]>; Wei Liu <[email protected]>; Roger Pau Monne >> <[email protected]>; Paul Durrant <[email protected]>; Suravee >> Suthikulpanit <[email protected]>; Brian Woods >> <[email protected]> >> Subject: [PATCH 1/2] x86/SVM-IOMMU: Don't opencode memcpy() in >> queue_iommu_command() >> >> In practice, this allows the compiler to replace the loop with a pair of >> movs. >> >> No functional change. > Well there is a potential functional change... > >> Signed-off-by: Andrew Cooper <[email protected]> >> --- >> CC: Jan Beulich <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: Paul Durrant <[email protected]> >> CC: Suravee Suthikulpanit <[email protected]> >> CC: Brian Woods <[email protected]> >> --- >> xen/drivers/passthrough/amd/iommu_cmd.c | 12 ++++-------- >> xen/include/asm-x86/hvm/svm/amd-iommu-defs.h | 1 - >> 2 files changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c >> b/xen/drivers/passthrough/amd/iommu_cmd.c >> index 08247fa..c6c0b4f 100644 >> --- a/xen/drivers/passthrough/amd/iommu_cmd.c >> +++ b/xen/drivers/passthrough/amd/iommu_cmd.c >> @@ -24,8 +24,7 @@ >> >> static int queue_iommu_command(struct amd_iommu *iommu, u32 cmd[]) >> { >> - u32 tail, head, *cmd_buffer; >> - int i; >> + uint32_t tail, head; >> >> tail = iommu->cmd_buffer.tail; >> if ( ++tail == iommu->cmd_buffer.entries ) >> @@ -35,12 +34,9 @@ static int queue_iommu_command(struct amd_iommu *iommu, >> u32 cmd[]) >> IOMMU_CMD_BUFFER_HEAD_OFFSET)); >> if ( head != tail ) >> { >> - cmd_buffer = (u32 *)(iommu->cmd_buffer.buffer + >> - (iommu->cmd_buffer.tail * >> - IOMMU_CMD_BUFFER_ENTRY_SIZE)); >> - >> - for ( i = 0; i < IOMMU_CMD_BUFFER_U32_PER_ENTRY; i++ ) >> - cmd_buffer[i] = cmd[i]; >> + memcpy(iommu->cmd_buffer.buffer + >> + (iommu->cmd_buffer.tail * IOMMU_CMD_BUFFER_ENTRY_SIZE), >> + cmd, IOMMU_CMD_BUFFER_ENTRY_SIZE); > ...since the built-in memcpy may not guarantee to the copy in 4 byte chunks > in ascending order.
"No functional change" != "The binary is identical". The functionality of queue_iommu_command() does not change, even if it's code generation does. It is just copying bytes into a shared ring, bounded later by updating the tail pointer. ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
