Re: [PATCH 1/2] x86/SVM-IOMMU: Don't opencode memcpy() in queue_iommu_command()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/09/18 13:09, Paul Durrant wrote:
>>
>>>> @@ -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.
> Yes, my point is that the ring is shared and so DMA by the h/w may race. This is clearly not a good situation but the fact that the code generation may change may have side effects.

This is writing past the tail pointer, and then updating the tail
pointer to match.

Hardware conforming to the spec won't read any of the data until it is
all ready.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel