> From: Lu Baolu <[email protected]> > Sent: Sunday, April 10, 2022 6:25 PM > > Some features require that a single device must be immutably isolated, > even when hot plug is supported.
This reads confusing, as hotplug cannot be allowed in a singleton group. What you actually meant suppose to be: "Enabling certain device features require the device in a singleton iommu group which is immutable in fabric i.e. not affected by hotplug" > For example, the SVA bind()/unbind() > interfaces require that the device exists in a singleton group. If we > have a singleton group that doesn't have ACS (or similar technologies) > and someone hotplugs in another device on a bridge, then our SVA is > completely broken and we get data corruption. this needs the background that PASID doesn't join PCI packet routing thus the dma address (CPU VA) may hit a p2p range. > > This adds a flag in the iommu_group struct to indicate an immutable > singleton group, and uses standard PCI bus topology, isolation features, > and DMA alias quirks to set the flag. If the device came from DT, assume > it is static and then the singleton attribute can know from the device > count in the group. where does the assumption come from? > > Suggested-by: Jason Gunthorpe <[email protected]> > Suggested-by: Kevin Tian <[email protected]> > Signed-off-by: Lu Baolu <[email protected]> > --- > drivers/iommu/iommu.c | 67 ++++++++++++++++++++++++++++++++++++---- > --- > 1 file changed, 57 insertions(+), 10 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 0c42ece25854..56ffbf5fdc18 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -48,6 +48,7 @@ struct iommu_group { > struct list_head entry; > unsigned int owner_cnt; > void *owner; > + bool immutable_singleton; Just call it 'singleton' with a comment to explain it must be immutable? > }; > > struct group_device { > @@ -74,6 +75,16 @@ static const char * const > iommu_group_resv_type_string[] = { > #define IOMMU_CMD_LINE_DMA_API BIT(0) > #define IOMMU_CMD_LINE_STRICT BIT(1) > > +/* > + * To consider a PCI device isolated, we require ACS to support Source > + * Validation, Request Redirection, Completer Redirection, and Upstream > + * Forwarding. This effectively means that devices cannot spoof their > + * requester ID, requests and completions cannot be redirected, and all > + * transactions are forwarded upstream, even as it passes through a > + * bridge where the target device is downstream. > + */ > +#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | > PCI_ACS_UF) > + > static int iommu_alloc_default_domain(struct iommu_group *group, > struct device *dev); > static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus, > @@ -89,6 +100,7 @@ static int > iommu_create_device_direct_mappings(struct iommu_group *group, > static struct iommu_group *iommu_group_get_for_dev(struct device *dev); > static ssize_t iommu_group_store_type(struct iommu_group *group, > const char *buf, size_t count); > +static int iommu_group_device_count(struct iommu_group *group); > > #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) > \ > struct iommu_group_attribute iommu_group_attr_##_name = \ > @@ -844,6 +856,37 @@ static bool iommu_is_attach_deferred(struct device > *dev) > return false; > } > > +static int has_pci_alias(struct pci_dev *pdev, u16 alias, void *opaque) > +{ > + return -EEXIST; > +} > + > +static bool pci_immutably_isolated(struct pci_dev *pdev) > +{ > + /* Skip the bridges. */ > + if (pci_is_bridge(pdev)) > + return false; > + > + /* > + * The device could be considered to be fully isolated if > + * all devices on the path from the parent to the host-PCI > + * bridge are protected from peer-to-peer DMA by ACS. > + */ > + if (!pci_is_root_bus(pdev->bus) && > + !pci_acs_path_enabled(pdev->bus->self, NULL, REQ_ACS_FLAGS)) > + return false; > + > + /* Multi-function devices should have ACS enabled. */ > + if (pdev->multifunction && !pci_acs_enabled(pdev, REQ_ACS_FLAGS)) > + return false; Looks my earlier comment was lost, i.e. you can just use pci_acs_path_enabled(pdev) to cover above two checks. > + > + /* Filter out devices which has any alias device. */ > + if (pci_for_each_dma_alias(pdev, has_pci_alias, NULL)) > + return false; > + > + return true; > +} > + > /** > * iommu_group_add_device - add a device to an iommu group > * @group: the group into which to add the device (reference should be held) > @@ -898,6 +941,20 @@ int iommu_group_add_device(struct iommu_group > *group, struct device *dev) > list_add_tail(&device->list, &group->devices); > if (group->domain && !iommu_is_attach_deferred(dev)) > ret = __iommu_attach_device(group->domain, dev); > + > + /* > + * Use standard PCI bus topology, isolation features, and DMA > + * alias quirks to set the immutable singleton attribute. If > + * the device came from DT, assume it is static and then > + * singleton can know from the device count in the group. > + */ > + if (dev_is_pci(dev)) > + group->immutable_singleton = > + pci_immutably_isolated(to_pci_dev(dev)); > + else if (is_of_node(dev_fwnode(dev))) > + group->immutable_singleton = > + (iommu_group_device_count(group) == 1); > + > mutex_unlock(&group->mutex); > if (ret) > goto err_put_group; > @@ -1290,16 +1347,6 @@ EXPORT_SYMBOL_GPL(iommu_group_id); > static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev, > unsigned long *devfns); > > -/* > - * To consider a PCI device isolated, we require ACS to support Source > - * Validation, Request Redirection, Completer Redirection, and Upstream > - * Forwarding. This effectively means that devices cannot spoof their > - * requester ID, requests and completions cannot be redirected, and all > - * transactions are forwarded upstream, even as it passes through a > - * bridge where the target device is downstream. > - */ > -#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | > PCI_ACS_UF) > - > /* > * For multifunction devices which are not isolated from each other, find > * all the other non-isolated functions and look for existing groups. For > -- > 2.25.1 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
