[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] x86/SVM-IOMMU: Don't opencode memcpy() in queue_iommu_command()
> -----Original Message----- > From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx] > Sent: 24 September 2018 11:55 > To: Xen-devel <xen-devel@xxxxxxxxxxxxx> > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich > <JBeulich@xxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger Pau Monne > <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Suravee > Suthikulpanit <suravee.suthikulpanit@xxxxxxx>; Brian Woods > <brian.woods@xxxxxxx> > 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 <andrew.cooper3@xxxxxxxxxx> > --- > CC: Jan Beulich <JBeulich@xxxxxxxx> > CC: Wei Liu <wei.liu2@xxxxxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > CC: Paul Durrant <paul.durrant@xxxxxxxxxx> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx> > CC: Brian Woods <brian.woods@xxxxxxx> > --- > 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 <paul.durrant@xxxxxxxxxx> > > 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 Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |