Re: [PATCH v10 04/12] iommu: Add attach/detach_dev_pasid iommu interface

[Baolu Lu]

On 2022/7/29 12:22, Tian, Kevin wrote:
> > From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> > Sent: Friday, July 29, 2022 11:21 AM
> >
> > On 2022/7/29 10:56, Tian, Kevin wrote:
> > > > +static bool iommu_group_device_pasid_viable(struct iommu_group
> > *group,
> > > > +					    struct device *dev)
> > > > +{
> > > > +	int count;
> > > > +
> > > > +	count = iommu_group_device_count(group);
> > > > +	if (count != 1)
> > > > +		return false;
> > > > +
> > > > +	/*
> > > > +	 * Block PASID attachment in cases where the PCI fabric is
> > > > +	 * routing based on address. PCI/ACS disables that.
> > > > +	 */
> > > > +	if (dev_is_pci(dev))
> > > > +		return pci_acs_path_enabled(to_pci_dev(dev), NULL,
> > > > +					    REQ_ACS_FLAGS);
> > > I think we are leaning toward doing above check in pci_enable_pasid().
> > > Then no singleton check inside iommu core.
> >
> > The iommu grouping also considers other things, like PCI alias. There
> > are many calls of pci_add_dma_alias() in drivers/pci/quirks.c.
> > Therefore, I believe that pci_acs_path_enabled() returning true doesn't
> > guarantees a singleton group.
>
> Is there an actual problem of sharing PASID table between aliasing RIDs?
> As long as ACS is enabled the device is isolated from other devices
> in the fabric. DMA aliases don't change that fact and there is no p2p
> between aliasing RIDs.

Yes. Agreed.

At present, the visible PASID use cases only occur on the singleton
groups, so we can start to support it from this simple situation. In the
future, if the multi-device groups need to support pasid, we can simply
apply the domain to the PASIDs of all device of the group.

> > > Presumably similar check can be done in DT/ACPI path of enabling pasid?
> >
> > I can't find the pasid (or anything similar) enabling interfaces for
> > DT or ACPI. They are device specific?
>
> Looks only PCI PASID is supported so far. both in Intel/ARM/AMD
> drivers. If other buses will support PASID one day, then ACS-equivalent
> can be also checked in their PASID enabling APIs.

Yes. Fair enough.

Best regards,
baolu