Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Hi Yi,
On 7/30/20 5:36 PM, Liu, Yi L wrote:
From: Lu Baolu
Sent: Tuesday, July 14, 2020 1:57 PM
Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group
data structure so that it could be reused in other places.
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio_iommu_type1.c | 44 ++---
1 file changed, 7 insertions(+), 37 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac9102a..f8812e68de77 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,6 +100,7 @@ struct vfio_dma {
struct vfio_group {
struct iommu_group *iommu_group;
struct list_headnext;
+ struct device *iommu_device;
I know mdev group has only one device, so such a group has a single
iommu_device. But I guess may be helpful to add a comment here or in
commit message. Otherwise, it looks weird that a group structure
contains a single iommu_device field instead of a list of iommu_device.
Right! I will add some comments if this is still needed in the next
version.
Best regards,
baolu
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Hi Alex,
On 7/31/20 5:17 AM, Alex Williamson wrote:
On Thu, 30 Jul 2020 10:41:32 +0800
Lu Baolu wrote:
Hi Alex,
On 7/30/20 4:32 AM, Alex Williamson wrote:
On Tue, 14 Jul 2020 13:57:03 +0800
Lu Baolu wrote:
Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the
vfio_group data structure so that it could be reused in other places.
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio_iommu_type1.c | 44 ++---
1 file changed, 7 insertions(+), 37 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac9102a..f8812e68de77 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,6 +100,7 @@ struct vfio_dma {
struct vfio_group {
struct iommu_group *iommu_group;
struct list_headnext;
+ struct device *iommu_device;
boolmdev_group; /* An mdev group */
boolpinned_page_dirty_scope;
};
@@ -1627,45 +1628,13 @@ static struct device *vfio_mdev_get_iommu_device(struct
device *dev)
return NULL;
}
-static int vfio_mdev_attach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- return iommu_aux_attach_device(domain, iommu_device);
- else
- return iommu_attach_device(domain, iommu_device);
- }
-
- return -EINVAL;
-}
-
-static int vfio_mdev_detach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- iommu_aux_detach_device(domain, iommu_device);
- else
- iommu_detach_device(domain, iommu_device);
- }
-
- return 0;
-}
-
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
- return iommu_group_for_each_dev(group->iommu_group,
- domain->domain,
- vfio_mdev_attach_domain);
+ return iommu_aux_attach_group(domain->domain,
+ group->iommu_group,
+ group->iommu_device);
No, we previously iterated all devices in the group and used the aux
interface only when we have an iommu_device supporting aux. If we
simply assume an mdev group only uses an aux domain we break existing
users, ex. SR-IOV VF backed mdevs. Thanks,
Oh, yes. Sorry! I didn't consider the physical device backed mdevs
cases.
Looked into this part of code, it seems that there's a lock issue here.
The group->mutex is held in iommu_group_for_each_dev() and will be
acquired again in iommu_attach_device().
These are two different groups. We walk the devices in the mdev's
group with iommu_group_for_each_dev(), holding the mdev's group lock,
but we call iommu_attach_device() with iommu_device, which results in
acquiring the lock for the iommu_device's group.
You are right. Sorry for the noise. Please ignore it.
How about making it like:
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group) {
struct device *iommu_device = group->iommu_device;
if (WARN_ON(!iommu_device))
return -EINVAL;
if (iommu_dev_feature_enabled(iommu_device,
IOMMU_DEV_FEAT_AUX))
return iommu_aux_attach_device(domain->domain,
iommu_device);
else
return iommu_attach_device(domain->domain,
iommu_device);
} else {
return iommu_attach_group(domain->domain,
group->iommu_group);
}
}
The caller (vfio_iommu_type1_attach_group) has guaranteed that all mdevs
in an iommu group should be derived from a same physical device.
Have we?
We have done this with below.
static int vfio_mdev_iommu_device(struct device *dev, void *data)
{
struct device **old = data, *new;
new = vfio_mdev_get_iommu_device(dev);
if (!new || (*old && *old != new))
return -EINVAL;
*old = new;
return 0;
}
But I agree that as a generic iommu aux-domain api, we shouldn't put
this limited assumption in it.
iommu_attach_device() will
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Thu, 30 Jul 2020 10:41:32 +0800
Lu Baolu wrote:
> Hi Alex,
>
> On 7/30/20 4:32 AM, Alex Williamson wrote:
> > On Tue, 14 Jul 2020 13:57:03 +0800
> > Lu Baolu wrote:
> >
> >> Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
> >> It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the
> >> vfio_group data structure so that it could be reused in other places.
> >>
> >> Signed-off-by: Lu Baolu
> >> ---
> >> drivers/vfio/vfio_iommu_type1.c | 44 ++---
> >> 1 file changed, 7 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/drivers/vfio/vfio_iommu_type1.c
> >> b/drivers/vfio/vfio_iommu_type1.c
> >> index 5e556ac9102a..f8812e68de77 100644
> >> --- a/drivers/vfio/vfio_iommu_type1.c
> >> +++ b/drivers/vfio/vfio_iommu_type1.c
> >> @@ -100,6 +100,7 @@ struct vfio_dma {
> >> struct vfio_group {
> >>struct iommu_group *iommu_group;
> >>struct list_headnext;
> >> + struct device *iommu_device;
> >>boolmdev_group; /* An mdev group */
> >>boolpinned_page_dirty_scope;
> >> };
> >> @@ -1627,45 +1628,13 @@ static struct device
> >> *vfio_mdev_get_iommu_device(struct device *dev)
> >>return NULL;
> >> }
> >>
> >> -static int vfio_mdev_attach_domain(struct device *dev, void *data)
> >> -{
> >> - struct iommu_domain *domain = data;
> >> - struct device *iommu_device;
> >> -
> >> - iommu_device = vfio_mdev_get_iommu_device(dev);
> >> - if (iommu_device) {
> >> - if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> >> - return iommu_aux_attach_device(domain, iommu_device);
> >> - else
> >> - return iommu_attach_device(domain, iommu_device);
> >> - }
> >> -
> >> - return -EINVAL;
> >> -}
> >> -
> >> -static int vfio_mdev_detach_domain(struct device *dev, void *data)
> >> -{
> >> - struct iommu_domain *domain = data;
> >> - struct device *iommu_device;
> >> -
> >> - iommu_device = vfio_mdev_get_iommu_device(dev);
> >> - if (iommu_device) {
> >> - if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> >> - iommu_aux_detach_device(domain, iommu_device);
> >> - else
> >> - iommu_detach_device(domain, iommu_device);
> >> - }
> >> -
> >> - return 0;
> >> -}
> >> -
> >> static int vfio_iommu_attach_group(struct vfio_domain *domain,
> >> struct vfio_group *group)
> >> {
> >>if (group->mdev_group)
> >> - return iommu_group_for_each_dev(group->iommu_group,
> >> - domain->domain,
> >> - vfio_mdev_attach_domain);
> >> + return iommu_aux_attach_group(domain->domain,
> >> +group->iommu_group,
> >> +group->iommu_device);
> >
> > No, we previously iterated all devices in the group and used the aux
> > interface only when we have an iommu_device supporting aux. If we
> > simply assume an mdev group only uses an aux domain we break existing
> > users, ex. SR-IOV VF backed mdevs. Thanks,
>
> Oh, yes. Sorry! I didn't consider the physical device backed mdevs
> cases.
>
> Looked into this part of code, it seems that there's a lock issue here.
> The group->mutex is held in iommu_group_for_each_dev() and will be
> acquired again in iommu_attach_device().
These are two different groups. We walk the devices in the mdev's
group with iommu_group_for_each_dev(), holding the mdev's group lock,
but we call iommu_attach_device() with iommu_device, which results in
acquiring the lock for the iommu_device's group.
> How about making it like:
>
> static int vfio_iommu_attach_group(struct vfio_domain *domain,
> struct vfio_group *group)
> {
> if (group->mdev_group) {
> struct device *iommu_device = group->iommu_device;
>
> if (WARN_ON(!iommu_device))
> return -EINVAL;
>
> if (iommu_dev_feature_enabled(iommu_device,
> IOMMU_DEV_FEAT_AUX))
> return iommu_aux_attach_device(domain->domain,
> iommu_device);
> else
> return iommu_attach_device(domain->domain,
> iommu_device);
> } else {
> return iommu_attach_group(domain->domain,
> group->iommu_group);
> }
> }
>
> The caller (vfio_iommu_type1_attach_group) has guaranteed that all mdevs
> in an iommu group should be derived from a same physical device.
Have we? iommu_attach_device() will fail if the group is not
singleton, but that's just encouraging us to use the _attach_group()
interface where the _attach_device() interface is relegated to special
cases. Ideally we'd get out of those special cases and create an
_attach_group() for aux that doesn't
RE: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
> From: Lu Baolu
> Sent: Tuesday, July 14, 2020 1:57 PM
>
> Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
> It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group
> data structure so that it could be reused in other places.
>
> Signed-off-by: Lu Baolu
> ---
> drivers/vfio/vfio_iommu_type1.c | 44 ++---
> 1 file changed, 7 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5e556ac9102a..f8812e68de77 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -100,6 +100,7 @@ struct vfio_dma {
> struct vfio_group {
> struct iommu_group *iommu_group;
> struct list_headnext;
> + struct device *iommu_device;
I know mdev group has only one device, so such a group has a single
iommu_device. But I guess may be helpful to add a comment here or in
commit message. Otherwise, it looks weird that a group structure
contains a single iommu_device field instead of a list of iommu_device.
Regards,
Yi Liu
> boolmdev_group; /* An mdev group */
> boolpinned_page_dirty_scope;
> };
> @@ -1627,45 +1628,13 @@ static struct device
> *vfio_mdev_get_iommu_device(struct device *dev)
> return NULL;
> }
>
> -static int vfio_mdev_attach_domain(struct device *dev, void *data) -{
> - struct iommu_domain *domain = data;
> - struct device *iommu_device;
> -
> - iommu_device = vfio_mdev_get_iommu_device(dev);
> - if (iommu_device) {
> - if (iommu_dev_feature_enabled(iommu_device,
> IOMMU_DEV_FEAT_AUX))
> - return iommu_aux_attach_device(domain, iommu_device);
> - else
> - return iommu_attach_device(domain, iommu_device);
> - }
> -
> - return -EINVAL;
> -}
> -
> -static int vfio_mdev_detach_domain(struct device *dev, void *data) -{
> - struct iommu_domain *domain = data;
> - struct device *iommu_device;
> -
> - iommu_device = vfio_mdev_get_iommu_device(dev);
> - if (iommu_device) {
> - if (iommu_dev_feature_enabled(iommu_device,
> IOMMU_DEV_FEAT_AUX))
> - iommu_aux_detach_device(domain, iommu_device);
> - else
> - iommu_detach_device(domain, iommu_device);
> - }
> -
> - return 0;
> -}
> -
> static int vfio_iommu_attach_group(struct vfio_domain *domain,
> struct vfio_group *group)
> {
> if (group->mdev_group)
> - return iommu_group_for_each_dev(group->iommu_group,
> - domain->domain,
> - vfio_mdev_attach_domain);
> + return iommu_aux_attach_group(domain->domain,
> + group->iommu_group,
> + group->iommu_device);
> else
> return iommu_attach_group(domain->domain, group-
> >iommu_group); } @@ -1674,8 +1643,8 @@ static void
> vfio_iommu_detach_group(struct vfio_domain *domain,
> struct vfio_group *group)
> {
> if (group->mdev_group)
> - iommu_group_for_each_dev(group->iommu_group, domain-
> >domain,
> - vfio_mdev_detach_domain);
> + iommu_aux_detach_group(domain->domain, group->iommu_group,
> +group->iommu_device);
> else
> iommu_detach_group(domain->domain, group->iommu_group); }
> @@ -2007,6 +1976,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> return 0;
> }
>
> + group->iommu_device = iommu_device;
> bus = iommu_device->bus;
> }
>
> --
> 2.17.1
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Hi Alex,
On 7/30/20 4:32 AM, Alex Williamson wrote:
On Tue, 14 Jul 2020 13:57:03 +0800
Lu Baolu wrote:
Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the
vfio_group data structure so that it could be reused in other places.
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio_iommu_type1.c | 44 ++---
1 file changed, 7 insertions(+), 37 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac9102a..f8812e68de77 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,6 +100,7 @@ struct vfio_dma {
struct vfio_group {
struct iommu_group *iommu_group;
struct list_headnext;
+ struct device *iommu_device;
boolmdev_group; /* An mdev group */
boolpinned_page_dirty_scope;
};
@@ -1627,45 +1628,13 @@ static struct device *vfio_mdev_get_iommu_device(struct
device *dev)
return NULL;
}
-static int vfio_mdev_attach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- return iommu_aux_attach_device(domain, iommu_device);
- else
- return iommu_attach_device(domain, iommu_device);
- }
-
- return -EINVAL;
-}
-
-static int vfio_mdev_detach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- iommu_aux_detach_device(domain, iommu_device);
- else
- iommu_detach_device(domain, iommu_device);
- }
-
- return 0;
-}
-
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
- return iommu_group_for_each_dev(group->iommu_group,
- domain->domain,
- vfio_mdev_attach_domain);
+ return iommu_aux_attach_group(domain->domain,
+ group->iommu_group,
+ group->iommu_device);
No, we previously iterated all devices in the group and used the aux
interface only when we have an iommu_device supporting aux. If we
simply assume an mdev group only uses an aux domain we break existing
users, ex. SR-IOV VF backed mdevs. Thanks,
Oh, yes. Sorry! I didn't consider the physical device backed mdevs
cases.
Looked into this part of code, it seems that there's a lock issue here.
The group->mutex is held in iommu_group_for_each_dev() and will be
acquired again in iommu_attach_device().
How about making it like:
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group) {
struct device *iommu_device = group->iommu_device;
if (WARN_ON(!iommu_device))
return -EINVAL;
if (iommu_dev_feature_enabled(iommu_device,
IOMMU_DEV_FEAT_AUX))
return iommu_aux_attach_device(domain->domain,
iommu_device);
else
return iommu_attach_device(domain->domain,
iommu_device);
} else {
return iommu_attach_group(domain->domain,
group->iommu_group);
}
}
The caller (vfio_iommu_type1_attach_group) has guaranteed that all mdevs
in an iommu group should be derived from a same physical device.
Any thoughts?
Alex
Best regards,
baolu
else
return iommu_attach_group(domain->domain, group->iommu_group);
}
@@ -1674,8 +1643,8 @@ static void vfio_iommu_detach_group(struct vfio_domain
*domain,
struct vfio_group *group)
{
if (group->mdev_group)
- iommu_group_for_each_dev(group->iommu_group, domain->domain,
-vfio_mdev_detach_domain);
+ iommu_aux_detach_group(domain->domain, group->iommu_group,
+ group->iommu_device);
else
iommu_detach_group(domain->domain, group->iommu_group);
}
@@ -2007,6 +1976,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
return 0;
}
+ group->iommu_device = iommu_device;
bus =
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Tue, 14 Jul 2020 13:57:03 +0800
Lu Baolu wrote:
> Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
> It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the
> vfio_group data structure so that it could be reused in other places.
>
> Signed-off-by: Lu Baolu
> ---
> drivers/vfio/vfio_iommu_type1.c | 44 ++---
> 1 file changed, 7 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5e556ac9102a..f8812e68de77 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -100,6 +100,7 @@ struct vfio_dma {
> struct vfio_group {
> struct iommu_group *iommu_group;
> struct list_headnext;
> + struct device *iommu_device;
> boolmdev_group; /* An mdev group */
> boolpinned_page_dirty_scope;
> };
> @@ -1627,45 +1628,13 @@ static struct device
> *vfio_mdev_get_iommu_device(struct device *dev)
> return NULL;
> }
>
> -static int vfio_mdev_attach_domain(struct device *dev, void *data)
> -{
> - struct iommu_domain *domain = data;
> - struct device *iommu_device;
> -
> - iommu_device = vfio_mdev_get_iommu_device(dev);
> - if (iommu_device) {
> - if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> - return iommu_aux_attach_device(domain, iommu_device);
> - else
> - return iommu_attach_device(domain, iommu_device);
> - }
> -
> - return -EINVAL;
> -}
> -
> -static int vfio_mdev_detach_domain(struct device *dev, void *data)
> -{
> - struct iommu_domain *domain = data;
> - struct device *iommu_device;
> -
> - iommu_device = vfio_mdev_get_iommu_device(dev);
> - if (iommu_device) {
> - if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> - iommu_aux_detach_device(domain, iommu_device);
> - else
> - iommu_detach_device(domain, iommu_device);
> - }
> -
> - return 0;
> -}
> -
> static int vfio_iommu_attach_group(struct vfio_domain *domain,
> struct vfio_group *group)
> {
> if (group->mdev_group)
> - return iommu_group_for_each_dev(group->iommu_group,
> - domain->domain,
> - vfio_mdev_attach_domain);
> + return iommu_aux_attach_group(domain->domain,
> + group->iommu_group,
> + group->iommu_device);
No, we previously iterated all devices in the group and used the aux
interface only when we have an iommu_device supporting aux. If we
simply assume an mdev group only uses an aux domain we break existing
users, ex. SR-IOV VF backed mdevs. Thanks,
Alex
> else
> return iommu_attach_group(domain->domain, group->iommu_group);
> }
> @@ -1674,8 +1643,8 @@ static void vfio_iommu_detach_group(struct vfio_domain
> *domain,
> struct vfio_group *group)
> {
> if (group->mdev_group)
> - iommu_group_for_each_dev(group->iommu_group, domain->domain,
> - vfio_mdev_detach_domain);
> + iommu_aux_detach_group(domain->domain, group->iommu_group,
> +group->iommu_device);
> else
> iommu_detach_group(domain->domain, group->iommu_group);
> }
> @@ -2007,6 +1976,7 @@ static int vfio_iommu_type1_attach_group(void
> *iommu_data,
> return 0;
> }
>
> + group->iommu_device = iommu_device;
> bus = iommu_device->bus;
> }
>
RE: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
> From: Lu Baolu > Sent: Wednesday, July 15, 2020 9:00 AM > > Hi Christoph and Jacob, > > On 7/15/20 12:29 AM, Jacob Pan wrote: > > On Tue, 14 Jul 2020 09:25:14 +0100 > > Christoph Hellwig wrote: > > > >> On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > >>> Replace iommu_aux_at(de)tach_device() with > >>> iommu_aux_at(de)tach_group(). It also saves the > >>> IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data > >>> structure so that it could be reused in other places. > >> This removes the last user of iommu_aux_attach_device and > >> iommu_aux_detach_device, which can be removed now. > > it is still used in patch 2/4 inside iommu_aux_attach_group(), right? > > > > There is a need to use this interface. For example, an aux-domain is > attached to a subset of a physical device and used in the kernel. In > this usage scenario, there's no need to use vfio/mdev. The device driver > could just allocate an aux-domain and call iommu_aux_attach_device() to > setup the iommu. > and here is one example usage for adding per-instance pagetables for drm/msm: https://lore.kernel.org/lkml/20200626200414.14382-5-jcro...@codeaurora.org/ Thanks Kevin
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Hi Christoph and Jacob, On 7/15/20 12:29 AM, Jacob Pan wrote: On Tue, 14 Jul 2020 09:25:14 +0100 Christoph Hellwig wrote: On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group(). It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data structure so that it could be reused in other places. This removes the last user of iommu_aux_attach_device and iommu_aux_detach_device, which can be removed now. it is still used in patch 2/4 inside iommu_aux_attach_group(), right? There is a need to use this interface. For example, an aux-domain is attached to a subset of a physical device and used in the kernel. In this usage scenario, there's no need to use vfio/mdev. The device driver could just allocate an aux-domain and call iommu_aux_attach_device() to setup the iommu. Best regards, baolu
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Tue, 14 Jul 2020 09:25:14 +0100 Christoph Hellwig wrote: > On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > > Replace iommu_aux_at(de)tach_device() with > > iommu_aux_at(de)tach_group(). It also saves the > > IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data > > structure so that it could be reused in other places. > > This removes the last user of iommu_aux_attach_device and > iommu_aux_detach_device, which can be removed now. it is still used in patch 2/4 inside iommu_aux_attach_group(), right? > ___ > iommu mailing list > io...@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu [Jacob Pan]
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group(). > It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the > vfio_group data structure so that it could be reused in other places. This removes the last user of iommu_aux_attach_device and iommu_aux_detach_device, which can be removed now.
