> -----Original Message-----
> From: [email protected] [mailto:iommu-
> [email protected]] On Behalf Of Alex Williamson
> Sent: Friday, May 31, 2013 12:09 AM
> To: [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected]
> 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 <[email protected]>
> ---
> 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-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
