Re: [PATCH rc] iommu: Skip PASID validation for devices without PASID capability

[Tushar Dave <tdave@nvidia.com>]

On 4/24/25 05:31, Jason Gunthorpe wrote:
> On Thu, Apr 24, 2025 at 12:08:56PM +0530, Vasant Hegde wrote:
> 
>>> What the iommu driver should do when set_dev_pasid is called for a non-
>>> PASID device?
> 
> That's a good point, maybe the core code should filter that out based
> on max_pasids? I think we do run into trouble here because the drivers
> are allocating PASID table space based on max_pasids so the non-pasid
> device should fail to add the pasid. Tushar, you should have hit this
> in your testing???

When we have multi-device group with PASID device and non-PASID devices,
set_dev_pasid doesn't fail in my testing for non-PASID devices.

Here is the example topology and bit more detail:

0008:00:00.0 root_port
  └─0008:01:00.0 upstream_port
     ├─0008:02:00.0 downstream_port
     │  └─0008:03:00.0 endpoint (NIC DMA-PF)
     └─0008:02:03.0 downstream_port
        └─0008:04:00.0 upstream_port
           └─0008:05:00.0 downstream_port
              └─0008:06:00.0 endpoint (GPU)


In the above topology, we setup ACS flags on DSP 0008:02:03.0 and 0008:02:00.0 
to achieve desired p2p configuration for GPU and DMA-PF.
Apparently, this creates multi-device group with GPU being only device with 
PASID support in that group. In this case, set_dev_pasid() ops invoked for each 
device within the group with pasid=1 and doesn't fail.

e.g.

  ...
  ..
  .
  pcieport 0008:02:03.0: debug: __iommu_set_group_pasid():  pasid=1 
dev->iommu->max_pasids=0 iommu_group 30
  pcieport 0008:02:03.0: debug: __iommu_set_group_pasid():  ret 0
  pcieport 0008:04:00.0: debug: __iommu_set_group_pasid():  pasid=1 
dev->iommu->max_pasids=0 iommu_group 30
  pcieport 0008:04:00.0: debug: __iommu_set_group_pasid():  ret 0
  pcieport 0008:05:00.0: debug: __iommu_set_group_pasid():  pasid=1 
dev->iommu->max_pasids=0 iommu_group 30
  pcieport 0008:05:00.0: debug: __iommu_set_group_pasid():  ret 0
  nvidia 0008:06:00.0: debug: __iommu_set_group_pasid():  pasid=1 
dev->iommu->max_pasids=1048576 iommu_group 30
  nvidia 0008:06:00.0: debug: __iommu_set_group_pasid():  ret 0


IMO this outcome is expected. Quoting a text from commit
https://github.com/torvalds/linux/commit/16603704559c7a68718059c4f75287886c01b20f


"If multiple devices share a single group, it's fine as long the fabric
always routes every TLP marked with a PASID to the host bridge and only
the host bridge. For example, ACS achieves this universally and has been
checked when pci_enable_pasid() is called. As we can't reliably tell the
source apart in a group, all the devices in a group have to be considered
as the same source, and mapped to the same PASID table."

-Tushar


> 
> We also have a problem setting up the default domain - it won't
> compute IOMMU_HWPT_ALLOC_PASID properly across the group. If the
> no-pasid device probes first then PASID will be broken on the group.
> 
> Tushar isn't hitting this because ARM always uses a PASID compatible
> domain today, but it will not work on AMD.
> 
> That's a huge pain to deal with :\
> 
>> Per device max_pasids check should cover that right?
> 
> The driver shouldn't be doing this though, if the driver is told to
> make a pasid then it should make a pasid.. The driver can fail
> attaching a pasid to a device that is over the device's max_pasid.
> 
>> FYI. One example of such device is some of the AMD GPUs which has
>> both VGA and audio in same group. while VGA supports PASID, audio is
>> not. This used to work fine when we had AMD IOMMU PASID specific
>> driver. GPUs stopped using PASIDs in upstream kernel. So I didn't
>> look into this part in details.
> 
> Uhhh.. That sounds like a worse problem, the only way you should end
> up with same group is if the ACS flags are missing on the GPU so Linux
> assumes the VGA and audio can loopback to each other internally.
> 
> That should completely block PASID support on the GPU side due the
> wrong routing. We can't have a hole in the PASID address space where
> the audio BAR is.
> 
> I suppose the HW doesn't actually behave this way but since it doesn't
> have the right ACS flags the SW doesn't know? Guessing..
> 
> Jason