This series is meant to refactor IOMMU group support in amd_iommu
to properly support virtual aliases. If multiple devices alias to
the same virtual alias, they should be grouped together. This code
also verifies whether the alias should be the root of the group vs
devices above the alias.
This seems to do the right thing on my system, but that's not saying
a lot since it doesn't do anything interesting with aliases. I'd
appreciate if Joerg and Florian could test this on their systems.
Thanks,
Alex
---
Alex Williamson (5):
amd_iommu: Properly account for virtual aliases in IOMMU groups
amd_iommu: Split IOMMU group allocation and attach
amd_iommu: Split upstream bus device lookup
amd_iommu: Split IOMMU Group topology walk
amd_iommu: Split IOMMU group initialization
drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
drivers/iommu/amd_iommu_types.h | 1
2 files changed, 142 insertions(+), 43 deletions(-)
This needs to be broken apart, start with pulling all the IOMMU
group init code into a new function.
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/iommu/amd_iommu.c | 61 ++++++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 55074cb..b65b377 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -276,39 +276,32 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
-static int iommu_init_device(struct device *dev)
+static int init_iommu_group(struct device *dev)
{
- struct pci_dev *dma_pdev = NULL, *pdev = to_pci_dev(dev);
struct iommu_dev_data *dev_data;
struct iommu_group *group;
- u16 alias;
+ struct pci_dev *dma_pdev = NULL;
int ret;
- if (dev->archdata.iommu)
+ group = iommu_group_get(dev);
+ if (group) {
+ iommu_group_put(group);
return 0;
+ }
dev_data = find_dev_data(get_device_id(dev));
if (!dev_data)
return -ENOMEM;
- alias = amd_iommu_alias_table[dev_data->devid];
- if (alias != dev_data->devid) {
- struct iommu_dev_data *alias_data;
-
- alias_data = find_dev_data(alias);
- if (alias_data == NULL) {
- pr_err("AMD-Vi: Warning: Unhandled device %s\n",
- dev_name(dev));
- free_dev_data(dev_data);
- return -ENOTSUPP;
- }
- dev_data->alias_data = alias_data;
+ if (dev_data->alias_data) {
+ u16 alias;
+ alias = amd_iommu_alias_table[dev_data->devid];
dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
}
- if (dma_pdev == NULL)
- dma_pdev = pci_dev_get(pdev);
+ if (!dma_pdev)
+ dma_pdev = pci_dev_get(to_pci_dev(dev));
/* Account for quirked devices */
swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
@@ -358,6 +351,38 @@ root_bus:
iommu_group_put(group);
+ return ret;
+}
+
+static int iommu_init_device(struct device *dev)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct iommu_dev_data *dev_data;
+ u16 alias;
+ int ret;
+
+ if (dev->archdata.iommu)
+ return 0;
+
+ dev_data = find_dev_data(get_device_id(dev));
+ if (!dev_data)
+ return -ENOMEM;
+
+ alias = amd_iommu_alias_table[dev_data->devid];
+ if (alias != dev_data->devid) {
+ struct iommu_dev_data *alias_data;
+
+ alias_data = find_dev_data(alias);
+ if (alias_data == NULL) {
+ pr_err("AMD-Vi: Warning: Unhandled device %s\n",
+ dev_name(dev));
+ free_dev_data(dev_data);
+ return -ENOTSUPP;
+ }
+ dev_data->alias_data = alias_data;
+ }
+
+ ret = init_iommu_group(dev);
if (ret)
return ret;
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/iommu/amd_iommu.c | 58 ++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b65b377..6edbd0e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -276,32 +276,9 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
-static int init_iommu_group(struct device *dev)
+static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
{
- struct iommu_dev_data *dev_data;
- struct iommu_group *group;
- struct pci_dev *dma_pdev = NULL;
- int ret;
-
- group = iommu_group_get(dev);
- if (group) {
- iommu_group_put(group);
- return 0;
- }
-
- dev_data = find_dev_data(get_device_id(dev));
- if (!dev_data)
- return -ENOMEM;
-
- if (dev_data->alias_data) {
- u16 alias;
-
- alias = amd_iommu_alias_table[dev_data->devid];
- dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
- }
-
- if (!dma_pdev)
- dma_pdev = pci_dev_get(to_pci_dev(dev));
+ struct pci_dev *dma_pdev = pdev;
/* Account for quirked devices */
swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
@@ -339,6 +316,37 @@ static int init_iommu_group(struct device *dev)
}
root_bus:
+ return dma_pdev;
+}
+
+static int init_iommu_group(struct device *dev)
+{
+ struct iommu_dev_data *dev_data;
+ struct iommu_group *group;
+ struct pci_dev *dma_pdev = NULL;
+ int ret;
+
+ group = iommu_group_get(dev);
+ if (group) {
+ iommu_group_put(group);
+ return 0;
+ }
+
+ dev_data = find_dev_data(get_device_id(dev));
+ if (!dev_data)
+ return -ENOMEM;
+
+ if (dev_data->alias_data) {
+ u16 alias;
+
+ alias = amd_iommu_alias_table[dev_data->devid];
+ dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
+ }
+
+ if (!dma_pdev)
+ dma_pdev = pci_dev_get(to_pci_dev(dev));
+
+ dma_pdev = get_isolation_root(dma_pdev);
group = iommu_group_get(&dma_pdev->dev);
pci_dev_put(dma_pdev);
if (!group) {
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/iommu/amd_iommu.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6edbd0e..3a00b5ce 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -274,6 +274,18 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
*from = to;
}
+static struct pci_bus *find_hosted_bus(struct pci_bus *bus)
+{
+ while (!bus->self) {
+ if (!pci_is_root_bus(bus))
+ bus = bus->parent;
+ else
+ return ERR_PTR(-ENODEV);
+ }
+
+ return bus;
+}
+
#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
@@ -300,14 +312,9 @@ static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
* Finding the next device may require skipping virtual buses.
*/
while (!pci_is_root_bus(dma_pdev->bus)) {
- struct pci_bus *bus = dma_pdev->bus;
-
- while (!bus->self) {
- if (!pci_is_root_bus(bus))
- bus = bus->parent;
- else
- goto root_bus;
- }
+ struct pci_bus *bus = find_hosted_bus(dma_pdev->bus);
+ if (IS_ERR(bus))
+ break;
if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
break;
@@ -315,7 +322,6 @@ static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
}
-root_bus:
return dma_pdev;
}
Add a WARN_ON to make it clear why we don't add dma_pdev->dev to the
group we're allocating.
Signed-off-by: Alex Williamson <[email protected]>
---
drivers/iommu/amd_iommu.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 3a00b5ce..7fa97a5 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -325,6 +325,24 @@ static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
return dma_pdev;
}
+static int use_pdev_iommu_group(struct pci_dev *pdev, struct device *dev)
+{
+ struct iommu_group *group = iommu_group_get(&pdev->dev);
+ int ret;
+
+ if (!group) {
+ group = iommu_group_alloc();
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ WARN_ON(&pdev->dev != dev);
+ }
+
+ ret = iommu_group_add_device(group, dev);
+ iommu_group_put(group);
+ return ret;
+}
+
static int init_iommu_group(struct device *dev)
{
struct iommu_dev_data *dev_data;
@@ -353,18 +371,8 @@ static int init_iommu_group(struct device *dev)
dma_pdev = pci_dev_get(to_pci_dev(dev));
dma_pdev = get_isolation_root(dma_pdev);
- group = iommu_group_get(&dma_pdev->dev);
+ ret = use_pdev_iommu_group(dma_pdev, dev);
pci_dev_put(dma_pdev);
- if (!group) {
- group = iommu_group_alloc();
- if (IS_ERR(group))
- return PTR_ERR(group);
- }
-
- ret = iommu_group_add_device(group, dev);
-
- iommu_group_put(group);
-
return ret;
}
An alias doesn't always point to a physical device. When this
happens we must first verify that the IOMMU group isn't rooted in
a device above the alias. In this case the alias is effectively
just another quirk for the devices aliased to it. Alternatively,
the virtual alias itself may be the root of the IOMMU group. To
support this, allow a group to be hosted on the alias dev_data
for use by anything that might have the same alias.
Signed-off-by: Alex williamson <[email protected]>
---
drivers/iommu/amd_iommu.c | 61 ++++++++++++++++++++++++++++++++++++---
drivers/iommu/amd_iommu_types.h | 1 +
2 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 7fa97a5..cb63cc5 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -140,6 +140,9 @@ static void free_dev_data(struct iommu_dev_data *dev_data)
list_del(&dev_data->dev_data_list);
spin_unlock_irqrestore(&dev_data_list_lock, flags);
+ if (dev_data->group)
+ iommu_group_put(dev_data->group);
+
kfree(dev_data);
}
@@ -343,11 +346,25 @@ static int use_pdev_iommu_group(struct pci_dev *pdev, struct device *dev)
return ret;
}
+static int use_dev_data_iommu_group(struct iommu_dev_data *dev_data,
+ struct device *dev)
+{
+ if (!dev_data->group) {
+ struct iommu_group *group = iommu_group_alloc();
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ dev_data->group = group;
+ }
+
+ return iommu_group_add_device(dev_data->group, dev);
+}
+
static int init_iommu_group(struct device *dev)
{
struct iommu_dev_data *dev_data;
struct iommu_group *group;
- struct pci_dev *dma_pdev = NULL;
+ struct pci_dev *dma_pdev;
int ret;
group = iommu_group_get(dev);
@@ -362,18 +379,52 @@ static int init_iommu_group(struct device *dev)
if (dev_data->alias_data) {
u16 alias;
+ struct pci_bus *bus;
+
+ if (dev_data->alias_data->group)
+ goto use_group;
+ /*
+ * If the alias device exists, it's effectively just a first
+ * level quirk for finding the DMA source.
+ */
alias = amd_iommu_alias_table[dev_data->devid];
dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
- }
+ if (dma_pdev) {
+ dma_pdev = get_isolation_root(dma_pdev);
+ goto use_pdev;
+ }
+
+ /*
+ * If the alias is virtual, try to find a parent device
+ * and test whether the IOMMU group is actualy rooted above
+ * the alias. Be careful to also test the parent device if
+ * we think the alias is the root of the group.
+ */
+ bus = pci_find_bus(0, alias >> 8);
+ if (!bus)
+ goto use_group;
+
+ bus = find_hosted_bus(bus);
+ if (IS_ERR(bus) || !bus->self)
+ goto use_group;
- if (!dma_pdev)
- dma_pdev = pci_dev_get(to_pci_dev(dev));
+ dma_pdev = get_isolation_root(pci_dev_get(bus->self));
+ if (dma_pdev != bus->self || (dma_pdev->multifunction &&
+ !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)))
+ goto use_pdev;
+
+ pci_dev_put(dma_pdev);
+ goto use_group;
+ }
- dma_pdev = get_isolation_root(dma_pdev);
+ dma_pdev = get_isolation_root(pci_dev_get(to_pci_dev(dev)));
+use_pdev:
ret = use_pdev_iommu_group(dma_pdev, dev);
pci_dev_put(dma_pdev);
return ret;
+use_group:
+ return use_dev_data_iommu_group(dev_data->alias_data, dev);
}
static int iommu_init_device(struct device *dev)
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index c9aa3d0..e38ab43 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -426,6 +426,7 @@ struct iommu_dev_data {
struct iommu_dev_data *alias_data;/* The alias dev_data */
struct protection_domain *domain; /* Domain the device is bound to */
atomic_t bind; /* Domain attach reference count */
+ struct iommu_group *group; /* IOMMU group for virtual aliases */
u16 devid; /* PCI Device ID */
bool iommu_v2; /* Device can make use of IOMMUv2 */
bool passthrough; /* Default for device is pt_domain */
Alex,
couple of comments in-lined:
On Mon, Oct 8, 2012 at 10:49 PM, Alex Williamson
<[email protected]> wrote:
> This needs to be broken apart, start with pulling all the IOMMU
> group init code into a new function.
>
> Signed-off-by: Alex Williamson <[email protected]>
> ---
>
> drivers/iommu/amd_iommu.c | 61 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 55074cb..b65b377 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -276,39 +276,32 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
>
> #define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
>
> -static int iommu_init_device(struct device *dev)
> +static int init_iommu_group(struct device *dev)
> {
> - struct pci_dev *dma_pdev = NULL, *pdev = to_pci_dev(dev);
> struct iommu_dev_data *dev_data;
> struct iommu_group *group;
> - u16 alias;
> + struct pci_dev *dma_pdev = NULL;
> int ret;
I don't see ret get initialized
>
> - if (dev->archdata.iommu)
> + group = iommu_group_get(dev);
> + if (group) {
> + iommu_group_put(group);
> return 0;
> + }
>
> dev_data = find_dev_data(get_device_id(dev));
> if (!dev_data)
> return -ENOMEM;
>
> - alias = amd_iommu_alias_table[dev_data->devid];
> - if (alias != dev_data->devid) {
> - struct iommu_dev_data *alias_data;
> -
> - alias_data = find_dev_data(alias);
> - if (alias_data == NULL) {
> - pr_err("AMD-Vi: Warning: Unhandled device %s\n",
> - dev_name(dev));
> - free_dev_data(dev_data);
> - return -ENOTSUPP;
> - }
> - dev_data->alias_data = alias_data;
> + if (dev_data->alias_data) {
> + u16 alias;
>
> + alias = amd_iommu_alias_table[dev_data->devid];
> dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
> }
>
> - if (dma_pdev == NULL)
> - dma_pdev = pci_dev_get(pdev);
> + if (!dma_pdev)
> + dma_pdev = pci_dev_get(to_pci_dev(dev));
>
> /* Account for quirked devices */
> swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
> @@ -358,6 +351,38 @@ root_bus:
>
> iommu_group_put(group);
>
> + return ret;
Do you even need ret - can this be simply return 0;
> +}
> +
> +static int iommu_init_device(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct iommu_dev_data *dev_data;
> + u16 alias;
> + int ret;
> +
> + if (dev->archdata.iommu)
> + return 0;
> +
> + dev_data = find_dev_data(get_device_id(dev));
> + if (!dev_data)
> + return -ENOMEM;
> +
> + alias = amd_iommu_alias_table[dev_data->devid];
> + if (alias != dev_data->devid) {
> + struct iommu_dev_data *alias_data;
> +
> + alias_data = find_dev_data(alias);
> + if (alias_data == NULL) {
> + pr_err("AMD-Vi: Warning: Unhandled device %s\n",
> + dev_name(dev));
> + free_dev_data(dev_data);
> + return -ENOTSUPP;
> + }
> + dev_data->alias_data = alias_data;
> + }
> +
> + ret = init_iommu_group(dev);
> if (ret)
> return ret;
>
>
> --
> 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/
On Tue, 2012-10-09 at 08:33 -0600, Shuah Khan wrote:
> Alex,
>
> couple of comments in-lined:
>
> On Mon, Oct 8, 2012 at 10:49 PM, Alex Williamson
> <[email protected]> wrote:
> > This needs to be broken apart, start with pulling all the IOMMU
> > group init code into a new function.
> >
> > Signed-off-by: Alex Williamson <[email protected]>
> > ---
> >
> > drivers/iommu/amd_iommu.c | 61 ++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 43 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> > index 55074cb..b65b377 100644
> > --- a/drivers/iommu/amd_iommu.c
> > +++ b/drivers/iommu/amd_iommu.c
> > @@ -276,39 +276,32 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
> >
> > #define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
> >
> > -static int iommu_init_device(struct device *dev)
> > +static int init_iommu_group(struct device *dev)
> > {
> > - struct pci_dev *dma_pdev = NULL, *pdev = to_pci_dev(dev);
> > struct iommu_dev_data *dev_data;
> > struct iommu_group *group;
> > - u16 alias;
> > + struct pci_dev *dma_pdev = NULL;
> > int ret;
>
> I don't see ret get initialized
It looks that way, but...
> >
> > - if (dev->archdata.iommu)
> > + group = iommu_group_get(dev);
> > + if (group) {
> > + iommu_group_put(group);
> > return 0;
> > + }
> >
> > dev_data = find_dev_data(get_device_id(dev));
> > if (!dev_data)
> > return -ENOMEM;
> >
> > - alias = amd_iommu_alias_table[dev_data->devid];
> > - if (alias != dev_data->devid) {
> > - struct iommu_dev_data *alias_data;
> > -
> > - alias_data = find_dev_data(alias);
> > - if (alias_data == NULL) {
> > - pr_err("AMD-Vi: Warning: Unhandled device %s\n",
> > - dev_name(dev));
> > - free_dev_data(dev_data);
> > - return -ENOTSUPP;
> > - }
> > - dev_data->alias_data = alias_data;
> > + if (dev_data->alias_data) {
> > + u16 alias;
> >
> > + alias = amd_iommu_alias_table[dev_data->devid];
> > dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
> > }
> >
> > - if (dma_pdev == NULL)
> > - dma_pdev = pci_dev_get(pdev);
> > + if (!dma_pdev)
> > + dma_pdev = pci_dev_get(to_pci_dev(dev));
> >
> > /* Account for quirked devices */
> > swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
> > @@ -358,6 +351,38 @@ root_bus:
> >
> > iommu_group_put(group);
> >
> > + return ret;
>
> Do you even need ret - can this be simply return 0;
If you apply the patch, you'll see the end of this function is:
ret = iommu_group_add_device(group, dev);
iommu_group_put(group);
return ret;
}
It's unfortunate that diff didn't make this easier to review. Thanks,
Alex
> > +}
> > +
> > +static int iommu_init_device(struct device *dev)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + struct iommu_dev_data *dev_data;
> > + u16 alias;
> > + int ret;
> > +
> > + if (dev->archdata.iommu)
> > + return 0;
> > +
> > + dev_data = find_dev_data(get_device_id(dev));
> > + if (!dev_data)
> > + return -ENOMEM;
> > +
> > + alias = amd_iommu_alias_table[dev_data->devid];
> > + if (alias != dev_data->devid) {
> > + struct iommu_dev_data *alias_data;
> > +
> > + alias_data = find_dev_data(alias);
> > + if (alias_data == NULL) {
> > + pr_err("AMD-Vi: Warning: Unhandled device %s\n",
> > + dev_name(dev));
> > + free_dev_data(dev_data);
> > + return -ENOTSUPP;
> > + }
> > + dev_data->alias_data = alias_data;
> > + }
> > +
> > + ret = init_iommu_group(dev);
> > if (ret)
> > return ret;
> >
> >
> > --
> > 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/
Am Mon, 08 Oct 2012 22:49:28 -0600
schrieb Alex Williamson <[email protected]>:
> This series is meant to refactor IOMMU group support in amd_iommu
> to properly support virtual aliases. If multiple devices alias to
> the same virtual alias, they should be grouped together. This code
> also verifies whether the alias should be the root of the group vs
> devices above the alias.
>
> This seems to do the right thing on my system, but that's not saying
> a lot since it doesn't do anything interesting with aliases. I'd
> appreciate if Joerg and Florian could test this on their systems.
> Thanks,
>
> Alex
>
> ---
>
> Alex Williamson (5):
> amd_iommu: Properly account for virtual aliases in IOMMU groups
> amd_iommu: Split IOMMU group allocation and attach
> amd_iommu: Split upstream bus device lookup
> amd_iommu: Split IOMMU Group topology walk
> amd_iommu: Split IOMMU group initialization
>
>
> drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
> drivers/iommu/amd_iommu_types.h | 1
> 2 files changed, 142 insertions(+), 43 deletions(-)
>
patch #5 does not apply cleanly on top of 3.6, I had to edit amd_iommu_types.h manually (easy enough), apart from that, it is working fine!
thanks, Florian
dmesg:
[ 1.475665] pci 0000:01:00.0: Boot video device
[ 1.475762] PCI: CLS 64 bytes, default 64
[ 1.478371] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: 3e info 1300
[ 1.478409] AMD-Vi: mmio-addr: 00000000feb20000
[ 1.478487] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:00.0 flags: 00
[ 1.478521] AMD-Vi: DEV_RANGE_END devid: 00:00.2
[ 1.478555] AMD-Vi: DEV_SELECT devid: 00:02.0 flags: 00
[ 1.478588] AMD-Vi: DEV_SELECT_RANGE_START devid: 01:00.0 flags: 00
[ 1.478622] AMD-Vi: DEV_RANGE_END devid: 01:00.1
[ 1.478655] AMD-Vi: DEV_SELECT devid: 00:04.0 flags: 00
[ 1.478688] AMD-Vi: DEV_SELECT devid: 02:00.0 flags: 00
[ 1.478722] AMD-Vi: DEV_SELECT devid: 00:05.0 flags: 00
[ 1.478755] AMD-Vi: DEV_SELECT devid: 03:00.0 flags: 00
[ 1.478788] AMD-Vi: DEV_SELECT devid: 00:06.0 flags: 00
[ 1.478822] AMD-Vi: DEV_SELECT devid: 04:00.0 flags: 00
[ 1.478855] AMD-Vi: DEV_SELECT devid: 00:07.0 flags: 00
[ 1.478888] AMD-Vi: DEV_SELECT devid: 05:00.0 flags: 00
[ 1.478921] AMD-Vi: DEV_SELECT devid: 00:09.0 flags: 00
[ 1.478955] AMD-Vi: DEV_SELECT devid: 06:00.0 flags: 00
[ 1.478988] AMD-Vi: DEV_SELECT devid: 00:0d.0 flags: 00
[ 1.479021] AMD-Vi: DEV_SELECT devid: 07:00.0 flags: 00
[ 1.479055] AMD-Vi: DEV_ALIAS_RANGE devid: 08:01.0 flags: 00 devid_to: 08:00.0
[ 1.479091] AMD-Vi: DEV_RANGE_END devid: 08:1f.7
[ 1.479128] AMD-Vi: DEV_SELECT devid: 00:11.0 flags: 00
[ 1.479162] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:12.0 flags: 00
[ 1.479195] AMD-Vi: DEV_RANGE_END devid: 00:12.2
[ 1.479229] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:13.0 flags: 00
[ 1.479262] AMD-Vi: DEV_RANGE_END devid: 00:13.2
[ 1.479295] AMD-Vi: DEV_SELECT devid: 00:14.0 flags: d7
[ 1.479329] AMD-Vi: DEV_SELECT devid: 00:14.3 flags: 00
[ 1.479362] AMD-Vi: DEV_SELECT devid: 00:14.4 flags: 00
[ 1.479396] AMD-Vi: DEV_ALIAS_RANGE devid: 09:00.0 flags: 00 devid_to: 00:14.4
[ 1.479432] AMD-Vi: DEV_RANGE_END devid: 09:1f.7
[ 1.479478] AMD-Vi: DEV_SELECT devid: 00:14.5 flags: 00
[ 1.479513] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:16.0 flags: 00
[ 1.479547] AMD-Vi: DEV_RANGE_END devid: 00:16.2
[ 1.533745] AMD-Vi: Found IOMMU at 0000:00:00.2 cap 0x40
[ 1.533781]
[ 1.533830] pci 0000:00:00.2: irq 72 for MSI/MSI-X
[ 1.542957] AMD-Vi: Lazy IO/TLB flushing enabled
lspci:
00:00.0 Host bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (external gfx0 port B) (rev 02)
00:00.2 IOMMU: Advanced Micro Devices [AMD] nee ATI RD990 I/O Memory Management Unit (IOMMU)
00:02.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port B)
00:04.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port D)
00:05.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port E)
00:06.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port F)
00:07.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port G)
00:09.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port H)
00:0d.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (external gfx1 port B)
00:11.0 SATA controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 SATA Controller [AHCI mode] (rev 40)
00:12.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
00:12.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
00:13.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
00:13.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
00:14.3 ISA bridge: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 LPC host controller (rev 40)
00:14.4 PCI bridge: Advanced Micro Devices [AMD] nee ATI SBx00 PCI to PCI Bridge (rev 40)
00:14.5 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI2 Controller
00:16.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
00:16.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
00:18.0 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor HyperTransport Configuration
00:18.1 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Address Map
00:18.2 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor DRAM Controller
00:18.3 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Miscellaneous Control
00:18.4 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Link Control
01:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RV730XT [Radeon HD 4670]
01:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI RV710/730 HDMI Audio [Radeon HD 4000 series]
02:00.0 SATA controller: ASMedia Technology Inc. ASM1062 Serial ATA Controller (rev 01)
03:00.0 Ethernet controller: Intel Corporation 82583V Gigabit Network Connection
04:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
05:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
06:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
07:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa)
08:04.0 Multimedia audio controller: C-Media Electronics Inc CMI8788 [Oxygen HD Audio]
On Tue, 2012-10-09 at 20:27 +0200, Florian Dazinger wrote:
> Am Mon, 08 Oct 2012 22:49:28 -0600
> schrieb Alex Williamson <[email protected]>:
>
> > This series is meant to refactor IOMMU group support in amd_iommu
> > to properly support virtual aliases. If multiple devices alias to
> > the same virtual alias, they should be grouped together. This code
> > also verifies whether the alias should be the root of the group vs
> > devices above the alias.
> >
> > This seems to do the right thing on my system, but that's not saying
> > a lot since it doesn't do anything interesting with aliases. I'd
> > appreciate if Joerg and Florian could test this on their systems.
> > Thanks,
> >
> > Alex
> >
> > ---
> >
> > Alex Williamson (5):
> > amd_iommu: Properly account for virtual aliases in IOMMU groups
> > amd_iommu: Split IOMMU group allocation and attach
> > amd_iommu: Split upstream bus device lookup
> > amd_iommu: Split IOMMU Group topology walk
> > amd_iommu: Split IOMMU group initialization
> >
> >
> > drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
> > drivers/iommu/amd_iommu_types.h | 1
> > 2 files changed, 142 insertions(+), 43 deletions(-)
> >
>
> patch #5 does not apply cleanly on top of 3.6, I had to edit
> amd_iommu_types.h manually (easy enough), apart from that, it is
> working fine!
Thanks Florian. Would you mind also reporting
'find /sys/kernel/iommu_groups' for both stock 3.6 and with this series
applied? I think we want to find that devices 07:00.0 and 08:04.0 are
grouped together and I'd like to verify whether that actually works.
Thanks,
Alex
> dmesg:
> [ 1.475665] pci 0000:01:00.0: Boot video device
> [ 1.475762] PCI: CLS 64 bytes, default 64
> [ 1.478371] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: 3e info 1300
> [ 1.478409] AMD-Vi: mmio-addr: 00000000feb20000
> [ 1.478487] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:00.0 flags: 00
> [ 1.478521] AMD-Vi: DEV_RANGE_END devid: 00:00.2
> [ 1.478555] AMD-Vi: DEV_SELECT devid: 00:02.0 flags: 00
> [ 1.478588] AMD-Vi: DEV_SELECT_RANGE_START devid: 01:00.0 flags: 00
> [ 1.478622] AMD-Vi: DEV_RANGE_END devid: 01:00.1
> [ 1.478655] AMD-Vi: DEV_SELECT devid: 00:04.0 flags: 00
> [ 1.478688] AMD-Vi: DEV_SELECT devid: 02:00.0 flags: 00
> [ 1.478722] AMD-Vi: DEV_SELECT devid: 00:05.0 flags: 00
> [ 1.478755] AMD-Vi: DEV_SELECT devid: 03:00.0 flags: 00
> [ 1.478788] AMD-Vi: DEV_SELECT devid: 00:06.0 flags: 00
> [ 1.478822] AMD-Vi: DEV_SELECT devid: 04:00.0 flags: 00
> [ 1.478855] AMD-Vi: DEV_SELECT devid: 00:07.0 flags: 00
> [ 1.478888] AMD-Vi: DEV_SELECT devid: 05:00.0 flags: 00
> [ 1.478921] AMD-Vi: DEV_SELECT devid: 00:09.0 flags: 00
> [ 1.478955] AMD-Vi: DEV_SELECT devid: 06:00.0 flags: 00
> [ 1.478988] AMD-Vi: DEV_SELECT devid: 00:0d.0 flags: 00
> [ 1.479021] AMD-Vi: DEV_SELECT devid: 07:00.0 flags: 00
> [ 1.479055] AMD-Vi: DEV_ALIAS_RANGE devid: 08:01.0 flags: 00 devid_to: 08:00.0
> [ 1.479091] AMD-Vi: DEV_RANGE_END devid: 08:1f.7
> [ 1.479128] AMD-Vi: DEV_SELECT devid: 00:11.0 flags: 00
> [ 1.479162] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:12.0 flags: 00
> [ 1.479195] AMD-Vi: DEV_RANGE_END devid: 00:12.2
> [ 1.479229] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:13.0 flags: 00
> [ 1.479262] AMD-Vi: DEV_RANGE_END devid: 00:13.2
> [ 1.479295] AMD-Vi: DEV_SELECT devid: 00:14.0 flags: d7
> [ 1.479329] AMD-Vi: DEV_SELECT devid: 00:14.3 flags: 00
> [ 1.479362] AMD-Vi: DEV_SELECT devid: 00:14.4 flags: 00
> [ 1.479396] AMD-Vi: DEV_ALIAS_RANGE devid: 09:00.0 flags: 00 devid_to: 00:14.4
> [ 1.479432] AMD-Vi: DEV_RANGE_END devid: 09:1f.7
> [ 1.479478] AMD-Vi: DEV_SELECT devid: 00:14.5 flags: 00
> [ 1.479513] AMD-Vi: DEV_SELECT_RANGE_START devid: 00:16.0 flags: 00
> [ 1.479547] AMD-Vi: DEV_RANGE_END devid: 00:16.2
> [ 1.533745] AMD-Vi: Found IOMMU at 0000:00:00.2 cap 0x40
> [ 1.533781]
> [ 1.533830] pci 0000:00:00.2: irq 72 for MSI/MSI-X
> [ 1.542957] AMD-Vi: Lazy IO/TLB flushing enabled
>
> lspci:
> 00:00.0 Host bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (external gfx0 port B) (rev 02)
> 00:00.2 IOMMU: Advanced Micro Devices [AMD] nee ATI RD990 I/O Memory Management Unit (IOMMU)
> 00:02.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port B)
> 00:04.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port D)
> 00:05.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port E)
> 00:06.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port F)
> 00:07.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port G)
> 00:09.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (PCI express gpp port H)
> 00:0d.0 PCI bridge: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (external gfx1 port B)
> 00:11.0 SATA controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 SATA Controller [AHCI mode] (rev 40)
> 00:12.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
> 00:12.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
> 00:13.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
> 00:13.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
> 00:14.0 SMBus: Advanced Micro Devices [AMD] nee ATI SBx00 SMBus Controller (rev 42)
> 00:14.3 ISA bridge: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 LPC host controller (rev 40)
> 00:14.4 PCI bridge: Advanced Micro Devices [AMD] nee ATI SBx00 PCI to PCI Bridge (rev 40)
> 00:14.5 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI2 Controller
> 00:16.0 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB OHCI0 Controller
> 00:16.2 USB controller: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 USB EHCI Controller
> 00:18.0 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor HyperTransport Configuration
> 00:18.1 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Address Map
> 00:18.2 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor DRAM Controller
> 00:18.3 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Miscellaneous Control
> 00:18.4 Host bridge: Advanced Micro Devices [AMD] Family 10h Processor Link Control
> 01:00.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RV730XT [Radeon HD 4670]
> 01:00.1 Audio device: Advanced Micro Devices [AMD] nee ATI RV710/730 HDMI Audio [Radeon HD 4000 series]
> 02:00.0 SATA controller: ASMedia Technology Inc. ASM1062 Serial ATA Controller (rev 01)
> 03:00.0 Ethernet controller: Intel Corporation 82583V Gigabit Network Connection
> 04:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
> 05:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
> 06:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller
> 07:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa)
> 08:04.0 Multimedia audio controller: C-Media Electronics Inc CMI8788 [Oxygen HD Audio]
Am Tue, 09 Oct 2012 12:35:39 -0600
schrieb Alex Williamson <[email protected]>:
> On Tue, 2012-10-09 at 20:27 +0200, Florian Dazinger wrote:
> > Am Mon, 08 Oct 2012 22:49:28 -0600
> > schrieb Alex Williamson <[email protected]>:
> >
> > > This series is meant to refactor IOMMU group support in amd_iommu
> > > to properly support virtual aliases. If multiple devices alias to
> > > the same virtual alias, they should be grouped together. This code
> > > also verifies whether the alias should be the root of the group vs
> > > devices above the alias.
> > >
> > > This seems to do the right thing on my system, but that's not saying
> > > a lot since it doesn't do anything interesting with aliases. I'd
> > > appreciate if Joerg and Florian could test this on their systems.
> > > Thanks,
> > >
> > > Alex
> > >
> > > ---
> > >
> > > Alex Williamson (5):
> > > amd_iommu: Properly account for virtual aliases in IOMMU groups
> > > amd_iommu: Split IOMMU group allocation and attach
> > > amd_iommu: Split upstream bus device lookup
> > > amd_iommu: Split IOMMU Group topology walk
> > > amd_iommu: Split IOMMU group initialization
> > >
> > >
> > > drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
> > > drivers/iommu/amd_iommu_types.h | 1
> > > 2 files changed, 142 insertions(+), 43 deletions(-)
> > >
> >
> > patch #5 does not apply cleanly on top of 3.6, I had to edit
> > amd_iommu_types.h manually (easy enough), apart from that, it is
> > working fine!
>
> Thanks Florian. Would you mind also reporting
> 'find /sys/kernel/iommu_groups' for both stock 3.6 and with this series
> applied? I think we want to find that devices 07:00.0 and 08:04.0 are
> grouped together and I'd like to verify whether that actually works.
> Thanks,
>
> Alex
shure... there does not seem to be much difference?
with stock 3.6:
/sys/kernel/iommu_groups
/sys/kernel/iommu_groups/0
/sys/kernel/iommu_groups/0/devices
/sys/kernel/iommu_groups/0/devices/0000:00:00.0
/sys/kernel/iommu_groups/0/devices/0000:00:00.2
/sys/kernel/iommu_groups/1
/sys/kernel/iommu_groups/1/devices
/sys/kernel/iommu_groups/1/devices/0000:00:02.0
/sys/kernel/iommu_groups/2
/sys/kernel/iommu_groups/2/devices
/sys/kernel/iommu_groups/2/devices/0000:00:04.0
/sys/kernel/iommu_groups/3
/sys/kernel/iommu_groups/3/devices
/sys/kernel/iommu_groups/3/devices/0000:00:05.0
/sys/kernel/iommu_groups/4
/sys/kernel/iommu_groups/4/devices
/sys/kernel/iommu_groups/4/devices/0000:00:06.0
/sys/kernel/iommu_groups/5
/sys/kernel/iommu_groups/5/devices
/sys/kernel/iommu_groups/5/devices/0000:00:07.0
/sys/kernel/iommu_groups/6
/sys/kernel/iommu_groups/6/devices
/sys/kernel/iommu_groups/6/devices/0000:00:09.0
/sys/kernel/iommu_groups/7
/sys/kernel/iommu_groups/7/devices
/sys/kernel/iommu_groups/7/devices/0000:00:0d.0
/sys/kernel/iommu_groups/8
/sys/kernel/iommu_groups/8/devices
/sys/kernel/iommu_groups/8/devices/0000:00:11.0
/sys/kernel/iommu_groups/9
/sys/kernel/iommu_groups/9/devices
/sys/kernel/iommu_groups/9/devices/0000:00:12.0
/sys/kernel/iommu_groups/9/devices/0000:00:12.2
/sys/kernel/iommu_groups/10
/sys/kernel/iommu_groups/10/devices
/sys/kernel/iommu_groups/10/devices/0000:00:13.0
/sys/kernel/iommu_groups/10/devices/0000:00:13.2
/sys/kernel/iommu_groups/11
/sys/kernel/iommu_groups/11/devices
/sys/kernel/iommu_groups/11/devices/0000:00:14.0
/sys/kernel/iommu_groups/11/devices/0000:00:14.3
/sys/kernel/iommu_groups/11/devices/0000:00:14.4
/sys/kernel/iommu_groups/11/devices/0000:00:14.5
/sys/kernel/iommu_groups/12
/sys/kernel/iommu_groups/12/devices
/sys/kernel/iommu_groups/12/devices/0000:00:16.0
/sys/kernel/iommu_groups/12/devices/0000:00:16.2
/sys/kernel/iommu_groups/13
/sys/kernel/iommu_groups/13/devices
/sys/kernel/iommu_groups/13/devices/0000:01:00.0
/sys/kernel/iommu_groups/13/devices/0000:01:00.1
/sys/kernel/iommu_groups/14
/sys/kernel/iommu_groups/14/devices
/sys/kernel/iommu_groups/14/devices/0000:02:00.0
/sys/kernel/iommu_groups/15
/sys/kernel/iommu_groups/15/devices
/sys/kernel/iommu_groups/15/devices/0000:03:00.0
/sys/kernel/iommu_groups/16
/sys/kernel/iommu_groups/16/devices
/sys/kernel/iommu_groups/16/devices/0000:04:00.0
/sys/kernel/iommu_groups/17
/sys/kernel/iommu_groups/17/devices
/sys/kernel/iommu_groups/17/devices/0000:05:00.0
/sys/kernel/iommu_groups/18
/sys/kernel/iommu_groups/18/devices
/sys/kernel/iommu_groups/18/devices/0000:06:00.0
/sys/kernel/iommu_groups/19
/sys/kernel/iommu_groups/19/devices
/sys/kernel/iommu_groups/19/devices/0000:07:00.0
/sys/kernel/iommu_groups/20
/sys/kernel/iommu_groups/20/devices
/sys/kernel/iommu_groups/20/devices/0000:08:04.0
3.6 + your patch-series:
3.6 with your patch-series:
/sys/kernel/iommu_groups
/sys/kernel/iommu_groups/0
/sys/kernel/iommu_groups/0/devices
/sys/kernel/iommu_groups/0/devices/0000:00:00.0
/sys/kernel/iommu_groups/0/devices/0000:00:00.2
/sys/kernel/iommu_groups/1
/sys/kernel/iommu_groups/1/devices
/sys/kernel/iommu_groups/1/devices/0000:00:02.0
/sys/kernel/iommu_groups/2
/sys/kernel/iommu_groups/2/devices
/sys/kernel/iommu_groups/2/devices/0000:00:04.0
/sys/kernel/iommu_groups/3
/sys/kernel/iommu_groups/3/devices
/sys/kernel/iommu_groups/3/devices/0000:00:05.0
/sys/kernel/iommu_groups/4
/sys/kernel/iommu_groups/4/devices
/sys/kernel/iommu_groups/4/devices/0000:00:06.0
/sys/kernel/iommu_groups/5
/sys/kernel/iommu_groups/5/devices
/sys/kernel/iommu_groups/5/devices/0000:00:07.0
/sys/kernel/iommu_groups/6
/sys/kernel/iommu_groups/6/devices
/sys/kernel/iommu_groups/6/devices/0000:00:09.0
/sys/kernel/iommu_groups/7
/sys/kernel/iommu_groups/7/devices
/sys/kernel/iommu_groups/7/devices/0000:00:0d.0
/sys/kernel/iommu_groups/8
/sys/kernel/iommu_groups/8/devices
/sys/kernel/iommu_groups/8/devices/0000:00:11.0
/sys/kernel/iommu_groups/9
/sys/kernel/iommu_groups/9/devices
/sys/kernel/iommu_groups/9/devices/0000:00:12.0
/sys/kernel/iommu_groups/9/devices/0000:00:12.2
/sys/kernel/iommu_groups/10
/sys/kernel/iommu_groups/10/devices
/sys/kernel/iommu_groups/10/devices/0000:00:13.0
/sys/kernel/iommu_groups/10/devices/0000:00:13.2
/sys/kernel/iommu_groups/11
/sys/kernel/iommu_groups/11/devices
/sys/kernel/iommu_groups/11/devices/0000:00:14.0
/sys/kernel/iommu_groups/11/devices/0000:00:14.3
/sys/kernel/iommu_groups/11/devices/0000:00:14.4
/sys/kernel/iommu_groups/11/devices/0000:00:14.5
/sys/kernel/iommu_groups/12
/sys/kernel/iommu_groups/12/devices
/sys/kernel/iommu_groups/12/devices/0000:00:16.0
/sys/kernel/iommu_groups/12/devices/0000:00:16.2
/sys/kernel/iommu_groups/13
/sys/kernel/iommu_groups/13/devices
/sys/kernel/iommu_groups/13/devices/0000:01:00.0
/sys/kernel/iommu_groups/13/devices/0000:01:00.1
/sys/kernel/iommu_groups/14
/sys/kernel/iommu_groups/14/devices
/sys/kernel/iommu_groups/14/devices/0000:02:00.0
/sys/kernel/iommu_groups/15
/sys/kernel/iommu_groups/15/devices
/sys/kernel/iommu_groups/15/devices/0000:03:00.0
/sys/kernel/iommu_groups/16
/sys/kernel/iommu_groups/16/devices
/sys/kernel/iommu_groups/16/devices/0000:04:00.0
/sys/kernel/iommu_groups/17
/sys/kernel/iommu_groups/17/devices
/sys/kernel/iommu_groups/17/devices/0000:05:00.0
/sys/kernel/iommu_groups/18
/sys/kernel/iommu_groups/18/devices
/sys/kernel/iommu_groups/18/devices/0000:06:00.0
/sys/kernel/iommu_groups/19
/sys/kernel/iommu_groups/19/devices
/sys/kernel/iommu_groups/19/devices/0000:07:00.0
/sys/kernel/iommu_groups/20
/sys/kernel/iommu_groups/20/devices
/sys/kernel/iommu_groups/20/devices/0000:08:04.0
On Tue, 2012-10-09 at 20:57 +0200, Florian Dazinger wrote:
> Am Tue, 09 Oct 2012 12:35:39 -0600
> schrieb Alex Williamson <[email protected]>:
>
> > On Tue, 2012-10-09 at 20:27 +0200, Florian Dazinger wrote:
> > > Am Mon, 08 Oct 2012 22:49:28 -0600
> > > schrieb Alex Williamson <[email protected]>:
> > >
> > > > This series is meant to refactor IOMMU group support in amd_iommu
> > > > to properly support virtual aliases. If multiple devices alias to
> > > > the same virtual alias, they should be grouped together. This code
> > > > also verifies whether the alias should be the root of the group vs
> > > > devices above the alias.
> > > >
> > > > This seems to do the right thing on my system, but that's not saying
> > > > a lot since it doesn't do anything interesting with aliases. I'd
> > > > appreciate if Joerg and Florian could test this on their systems.
> > > > Thanks,
> > > >
> > > > Alex
> > > >
> > > > ---
> > > >
> > > > Alex Williamson (5):
> > > > amd_iommu: Properly account for virtual aliases in IOMMU groups
> > > > amd_iommu: Split IOMMU group allocation and attach
> > > > amd_iommu: Split upstream bus device lookup
> > > > amd_iommu: Split IOMMU Group topology walk
> > > > amd_iommu: Split IOMMU group initialization
> > > >
> > > >
> > > > drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
> > > > drivers/iommu/amd_iommu_types.h | 1
> > > > 2 files changed, 142 insertions(+), 43 deletions(-)
> > > >
> > >
> > > patch #5 does not apply cleanly on top of 3.6, I had to edit
> > > amd_iommu_types.h manually (easy enough), apart from that, it is
> > > working fine!
> >
> > Thanks Florian. Would you mind also reporting
> > 'find /sys/kernel/iommu_groups' for both stock 3.6 and with this series
> > applied? I think we want to find that devices 07:00.0 and 08:04.0 are
> > grouped together and I'd like to verify whether that actually works.
> > Thanks,
> >
> > Alex
>
> shure... there does not seem to be much difference?
>
> with stock 3.6:
> /sys/kernel/iommu_groups/19
> /sys/kernel/iommu_groups/19/devices
> /sys/kernel/iommu_groups/19/devices/0000:07:00.0
> /sys/kernel/iommu_groups/20
> /sys/kernel/iommu_groups/20/devices
> /sys/kernel/iommu_groups/20/devices/0000:08:04.0
>
>
> 3.6 + your patch-series:
> 3.6 with your patch-series:
> /sys/kernel/iommu_groups/19
> /sys/kernel/iommu_groups/19/devices
> /sys/kernel/iommu_groups/19/devices/0000:07:00.0
> /sys/kernel/iommu_groups/20
> /sys/kernel/iommu_groups/20/devices
> /sys/kernel/iommu_groups/20/devices/0000:08:04.0
Ok, I was thinking we'd see these last two devices together, but on
second thought this is exactly what the alias is telling us, ie. it can
distinguish a DMA from a device on bus 08 from a DMA from 07:00.0
itself, but it cannot distinguish between devices on bus 08 (hopefully
that's actually true). If you somehow had a device 08:05.0 you would
see a new group 21 with just that device on stock 3.6 whereas with the
patches you should see 08:05.0 added to group 20. If we plugged this
same card into an Intel VT-d system, the grouping would be done as I was
thinking with both 07:00.0 and 08:04.0 being in group 19. Thanks again
for your testing,
Alex
Joerg,
I think this series is good-to-go, modulo any testing you may be able to
do on it. I'm going to be offline for a couple weeks so feel free to
incorporate this RFC directly or I can re-spin something when I'm back.
Thanks,
Alex
On Mon, 2012-10-08 at 22:49 -0600, Alex Williamson wrote:
> This series is meant to refactor IOMMU group support in amd_iommu
> to properly support virtual aliases. If multiple devices alias to
> the same virtual alias, they should be grouped together. This code
> also verifies whether the alias should be the root of the group vs
> devices above the alias.
>
> This seems to do the right thing on my system, but that's not saying
> a lot since it doesn't do anything interesting with aliases. I'd
> appreciate if Joerg and Florian could test this on their systems.
> Thanks,
>
> Alex
>
> ---
>
> Alex Williamson (5):
> amd_iommu: Properly account for virtual aliases in IOMMU groups
> amd_iommu: Split IOMMU group allocation and attach
> amd_iommu: Split upstream bus device lookup
> amd_iommu: Split IOMMU Group topology walk
> amd_iommu: Split IOMMU group initialization
>
>
> drivers/iommu/amd_iommu.c | 184 ++++++++++++++++++++++++++++++---------
> drivers/iommu/amd_iommu_types.h | 1
> 2 files changed, 142 insertions(+), 43 deletions(-)
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
On Thu, Oct 18, 2012 at 03:29:10PM -0600, Alex Williamson wrote:
> I think this series is good-to-go, modulo any testing you may be able to
> do on it. I'm going to be offline for a couple weeks so feel free to
> incorporate this RFC directly or I can re-spin something when I'm back.
Okay, I applied the patches and will do some testing before pushing them
out. I will fix all objections by follow-on patches.
Joerg
--
AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632