> -----Original Message----- > From: iommu-bounces@xxxxxxxxxxxxxxxxxxxxxxxxxx [mailto:iommu- > bounces@xxxxxxxxxxxxxxxxxxxxxxxxxx] On Behalf Of Alex Williamson > Sent: Friday, May 31, 2013 12:09 AM > To: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; dwmw2@xxxxxxxxxxxxx; > joro@xxxxxxxxxx > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: [PATCH] iommu: amd/intel: Remove multifunction assumption around > grouping > > If a device is multifunction and does not have ACS enabled then we assume > that the entire package lacks ACS and use function 0 as the base of the > group. The PCIe spec however states that components are permitted to > implement ACS on some, none, or all of their applicable functions. It's > therefore conceivable that function 0 may be fully independent and > support ACS while other functions do not. Instead use the lowest > function of the slot that does not have ACS enabled as the base of the > group. This may be the current device, which is intentional. So long as > we use a consistent algorithm, all the non-ACS functions will be grouped > together and ACS functions will get separate groups. > > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx> > --- > drivers/iommu/amd_iommu.c | 25 +++++++++++++++++++------ > drivers/iommu/intel-iommu.c | 25 +++++++++++++++++++------ > 2 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index > 1d84be1..565c745 100644 > --- a/drivers/iommu/amd_iommu.c > +++ b/drivers/iommu/amd_iommu.c > @@ -287,14 +287,27 @@ static struct pci_dev *get_isolation_root(struct > pci_dev *pdev) > > /* > * If it's a multifunction device that does not support our > - * required ACS flags, add to the same group as function 0. > + * required ACS flags, add to the same group as lowest numbered > + * function that also does not suport the required ACS flags. > */ > if (dma_pdev->multifunction && > - !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) > - swap_pci_ref(&dma_pdev, > - pci_get_slot(dma_pdev->bus, > - PCI_DEVFN(PCI_SLOT(dma_pdev->devfn), > - 0))); > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { > + u8 i, slot = PCI_SLOT(dma_pdev->devfn); > + > + for (i = 0; i < 8; i++) { > + struct pci_dev *tmp; > + > + tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i)); > + if (!tmp) > + continue; > + > + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { > + swap_pci_ref(&dma_pdev, tmp); > + break; > + } > + pci_dev_put(tmp); > + } > + } > > /* > * Devices on the root bus go through the iommu. If that's not us, > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index b4f0e28..eec0d3e 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -4182,14 +4182,27 @@ static int intel_iommu_add_device(struct device > *dev) > > /* > * If it's a multifunction device that does not support our > - * required ACS flags, add to the same group as function 0. > + * required ACS flags, add to the same group as lowest numbered > + * function that also does not suport the required ACS flags. > */ > if (dma_pdev->multifunction && > - !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) > - swap_pci_ref(&dma_pdev, > - pci_get_slot(dma_pdev->bus, > - PCI_DEVFN(PCI_SLOT(dma_pdev->devfn), > - 0))); > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) { > + u8 i, slot = PCI_SLOT(dma_pdev->devfn); > + > + for (i = 0; i < 8; i++) { [Sethi Varun-B16395] A macro like PCI_MAX_FUNCTIONS would improve code readability. > + struct pci_dev *tmp; > + > + tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i)); > + if (!tmp) > + continue; > + > + if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) { > + swap_pci_ref(&dma_pdev, tmp); > + break; > + } > + pci_dev_put(tmp); > + } > + } It would be nice if this code could be represented as a function in a common file like iommu/pci.c. -Varun -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
