* [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping
@ 2013-05-30 18:39 Alex Williamson
2013-06-03 7:28 ` Sethi Varun-B16395
2013-06-20 15:22 ` Joerg Roedel
0 siblings, 2 replies; 4+ messages in thread
From: Alex Williamson @ 2013-05-30 18:39 UTC (permalink / raw)
To: iommu, dwmw2, joro; +Cc: linux-pci, ddutile, linux-kernel
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@redhat.com>
---
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++) {
+ 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,
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping
2013-05-30 18:39 [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping Alex Williamson
@ 2013-06-03 7:28 ` Sethi Varun-B16395
[not found] ` <C5ECD7A89D1DC44195F34B25E172658D56A9F4-RL0Hj/+nBVDYdknt8GnhQq4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org>
2013-06-20 15:22 ` Joerg Roedel
1 sibling, 1 reply; 4+ messages in thread
From: Sethi Varun-B16395 @ 2013-06-03 7:28 UTC (permalink / raw)
To: Alex Williamson, iommu@lists.linux-foundation.org,
dwmw2@infradead.org, joro@8bytes.org
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
> -----Original Message-----
> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> Sent: Friday, May 31, 2013 12:09 AM
> To: iommu@lists.linux-foundation.org; dwmw2@infradead.org;
> joro@8bytes.org
> Cc: linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org
> 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@redhat.com>
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping
2013-05-30 18:39 [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping Alex Williamson
2013-06-03 7:28 ` Sethi Varun-B16395
@ 2013-06-20 15:22 ` Joerg Roedel
1 sibling, 0 replies; 4+ messages in thread
From: Joerg Roedel @ 2013-06-20 15:22 UTC (permalink / raw)
To: Alex Williamson; +Cc: iommu, dwmw2, linux-pci, ddutile, linux-kernel
On Thu, May 30, 2013 at 12:39:18PM -0600, Alex Williamson wrote:
> 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@redhat.com>
> ---
> drivers/iommu/amd_iommu.c | 25 +++++++++++++++++++------
> drivers/iommu/intel-iommu.c | 25 +++++++++++++++++++------
Didn't really fit into any of my existing branches. Applied it to
x86/vt-d because it is probably not worth creating a new x86-specific
branch just for this patch.
Thanks,
Joerg
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-20 15:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-30 18:39 [PATCH] iommu: amd/intel: Remove multifunction assumption around grouping Alex Williamson
2013-06-03 7:28 ` Sethi Varun-B16395
[not found] ` <C5ECD7A89D1DC44195F34B25E172658D56A9F4-RL0Hj/+nBVDYdknt8GnhQq4g8xLGJsHaLnY5E4hWTkheoWH0uzbU5w@public.gmane.org>
2013-06-03 15:10 ` Alex Williamson
2013-06-20 15:22 ` Joerg Roedel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).