> -----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. As long as that change poses no danger (and seeing as the code contains no barrer-ing I'd assume that to be the case) then this change looks fine to me (with the 'no functional change' comment removed). Reviewed-by: Paul Durrant <[email protected]> > > iommu->cmd_buffer.tail = tail; > return 1; > diff --git a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > index c479f0b..1f19cd3 100644 > --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > @@ -187,7 +187,6 @@ > > #define IOMMU_CMD_BUFFER_ENTRY_SIZE 16 > #define IOMMU_CMD_BUFFER_POWER_OF2_ENTRIES_PER_PAGE 8 > -#define IOMMU_CMD_BUFFER_U32_PER_ENTRY (IOMMU_CMD_BUFFER_ENTRY_SIZE / > 4) > > #define IOMMU_CMD_OPCODE_MASK 0xF0000000 > #define IOMMU_CMD_OPCODE_SHIFT 28 > -- > 2.1.4 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
