Re: [Xen-devel] [PATCH 4/4] AMD/IOMMU: Treat head/tail pointers as byte offsets

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.02.2020 15:55, Andrew Cooper wrote:
> On 05/02/2020 10:36, Jan Beulich wrote:
>> On 03.02.2020 15:43, Andrew Cooper wrote:
>>> --- a/xen/drivers/passthrough/amd/iommu_cmd.c
>>> +++ b/xen/drivers/passthrough/amd/iommu_cmd.c
>>> @@ -24,16 +24,14 @@ static int queue_iommu_command(struct amd_iommu *iommu, 
>>> u32 cmd[])
>>>  {
>>>      uint32_t tail, head;
>>>  
>>> -    tail = iommu->cmd_buffer.tail;
>>> -    if ( ++tail == iommu->cmd_buffer.entries )
>>> +    tail = iommu->cmd_buffer.tail + IOMMU_CMD_BUFFER_ENTRY_SIZE;
>>> +    if ( tail == iommu->cmd_buffer.size )
>>>          tail = 0;
>>>  
>>> -    head = iommu_get_rb_pointer(readl(iommu->mmio_base +
>>> -                                      IOMMU_CMD_BUFFER_HEAD_OFFSET));
>>> +    head = readl(iommu->mmio_base + IOMMU_CMD_BUFFER_HEAD_OFFSET);
>>>      if ( head != tail )
>> Surely you want to mask off reserved (or more generally
>> unrelated) bits, before consuming the value for the purpose
>> here (and elsewhere below)?
> 
> Reserved bits are defined in the IOMMU spec to be read-only zero.
> 
> It is also undefined behaviour for this value to ever be outside of the
> size configured for command buffer, so using the value like this is spec
> compliant.
> 
> As for actually masking the values, that breaks the optimisers ability
> to construct commands in the command ring.  This aspect can be worked
> around with other code changes, but I also think it is implausible that
> the remaining reserved bits here are going to sprout incompatible future
> uses.

Implausible - perhaps. But impossible - no. There could be a simple
flag bit appearing somewhere in these registers. I simply don't it
is a good idea to set a precedent of not honoring reserved bit as
being reserved. The spec saying "read-only zero" can only be viewed
as correct for the current version of the spec, or else why would
the bits be called "reserved" rather than e.g. "read-as-zero"?

Jan

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