Re: [PATCH] intel-iommu: don't disable ATS for device without page aligned request

["Raj, Ashok" <ashok.raj@intel.com>]

On Wed, Sep 09, 2020 at 10:17:35PM -0400, Jason Wang wrote:
> 
> 
> ----- Original Message -----
> > Hi Jason
> > 
> > On Wed, Sep 09, 2020 at 04:34:32PM +0800, Jason Wang wrote:
> > > Commit 61363c1474b1 ("iommu/vt-d: Enable ATS only if the device uses
> > > page aligned address.") disables ATS for device that can do unaligned
> > > page request.
> > 
> > Did you take a look at the PCI specification?
> > Page Aligned Request is in the ATS capability Register.
> > 
> > ATS Capability Register (Offset 0x04h)
> > 
> > bit (5):
> > Page Aligned Request - If Set, indicates the Untranslated address is always
> > aligned to 4096 byte boundary. Setting this field is recommended. This
> > field permits software to distinguish between implemntations compatible
> > with this specification and those compatible with an earlier version of
> > this specification in which a Requester was permitted to supply anything in
> > bits [11:2].
> 
> Yes, my understanding is that this is optional not mandatory.

Correct, but optional on the device side. An IOMMU might *require* this for
proper normal operation. Our IOMMU's do not get the low 12 bits. Which is
why the spec gives SW a way to detect if the device is compatible for this
IOMMU implementation.

> 
> > 
> > > 
> > > This looks wrong, since the commit log said it's because the page
> > > request descriptor doesn't support reporting unaligned request.
> > 
> > I don't think you can change the definition from ATS to PRI. Both are
> > orthogonal feature.
> 
> I may miss something, here's my understanding is that:
> 
> - page request descriptor will only be used when PRS is enabled
> - ATS spec allows unaligned request
> 
> So any reason for disabling ATS for unaligned request even if PRS is
> not enabled?

I think you are getting confused between the 2 different PCIe features.

ATS - Address Translation Services. Used by device to simply request the
Host Physical Address for some DMA operation. 

When ATS response indicates failed, then the device can request a
page-request (PRS this is like a device page-fault), and then IOMMU driver
would work with the kernel to fault a page then respond with
(Page-response) success/failure. Then the device will send a new ATS 
to get the new translation. 


> 
> > 
> > > 
> > > A victim is Qemu's virtio-pci which doesn't advertise the page aligned
> > > address. Fixing by disable PRI instead of ATS if device doesn't have
> > > page aligned request.
> > 
> > This is a requirement for the Intel IOMMU's.
> > 
> > You say virtio, so is it all emulated device or you talking about some
> > hardware that implemented virtio-pci compliant hw? If you are sure the
> > device actually does comply with the requirement, but just not enumerating
> > the capability, you can maybe work a quirk to overcome that?
> 
> So far only emulated devices. But we are helping some vendor to
> implement virtio hardware so  we need to understand the connection
> between ATS alignment and page request descriptor.

ATS and PRS are 2 separate orthogonal features. 
PRS requires ATS, but not the other way around. 

> 
> > 
> > Now PRI also has an alignment requirement, and Intel IOMMU's requires that
> > as well. If your device supports SRIOV as well, PASID and PRI are
> > enumerated just on the PF and not the VF. You might want to pay attension
> > to that. We are still working on a solution for that problem.
> 
> Thanks for the reminding, but it looks to me according to the ATS
> spec, all PRI message is 4096 byte aligned? E.g lower bites were used
> for group index etc.

Right, I should have been clear. The issue with PRI is we require responses
to have PASID field set. There is another capability on the device that
exposes that. pci_prg_resp_pasid_required(). This is required to enable PRI
for a device.

Cheers,
Ashok
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu