| Message ID | 20181105073408.21815-8-baolu.lu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/mdev: IOMMU aware mediated device | expand |
Hi Lu, On 11/5/18 8:34 AM, Lu Baolu wrote: > This adds helpers to attach or detach a domain to a > group. This will replace iommu_attach_group() which > only works for pci devices. s/pci/non mdev? > > If a domain is attaching to a group which includes the > mediated devices, it should attach to the iommu device > (a pci device which represents the mdev in iommu scope) > instead. The added helper supports attaching domain to > groups for both pci and mdev devices. > > Cc: Ashok Raj <ashok.raj@intel.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Kevin Tian <kevin.tian@intel.com> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > --- > drivers/vfio/vfio_iommu_type1.c | 114 ++++++++++++++++++++++++++++++-- > 1 file changed, 107 insertions(+), 7 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index d9fd3188615d..178264b330e7 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -91,6 +91,7 @@ struct vfio_dma { > struct vfio_group { > struct iommu_group *iommu_group; > struct list_head next; > + bool mdev_group; /* An mdev group */ > }; > > /* > @@ -1327,6 +1328,105 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) > return ret; > } > > +static struct device *vfio_mdev_get_iommu_device(struct device *dev) > +{ > + struct device *(*fn)(struct device *dev); > + struct device *iommu_parent; > + > + fn = symbol_get(mdev_get_iommu_device); > + if (fn) { > + iommu_parent = fn(dev); > + symbol_put(mdev_get_iommu_device); > + > + return iommu_parent; > + } > + > + return NULL; > +} > + > +static int vfio_mdev_set_domain(struct device *dev, struct iommu_domain *domain) > +{ > + int (*fn)(struct device *dev, void *domain); > + int ret; > + > + fn = symbol_get(mdev_set_iommu_domain); > + if (fn) { > + ret = fn(dev, domain); > + symbol_put(mdev_set_iommu_domain); > + > + return ret; > + } > + > + return -EINVAL; > +} > + > +static int vfio_mdev_attach_domain(struct device *dev, void *data) > +{ > + struct iommu_domain *domain = data; > + struct device *iommu_device; > + int ret; > + > + ret = vfio_mdev_set_domain(dev, domain); > + if (ret) > + return ret; > + > + iommu_device = vfio_mdev_get_iommu_device(dev); > + if (iommu_device) { > + bool aux_mode = false; > + > + iommu_get_dev_attr(iommu_device, > + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); Don' you need to test the returned value before using aux_mode? > + if (aux_mode) > + return iommu_attach_device_aux(domain, iommu_device); > + else > + return iommu_attach_device(domain, iommu_device); if for some reason the above ops fail, don't you want to call vfio_mdev_set_domain(dev, NULL) > + } > + > + return -EINVAL; > +} > + > +static int vfio_mdev_detach_domain(struct device *dev, void *data) > +{ > + struct iommu_domain *domain = data; > + struct device *iommu_device; > + > + vfio_mdev_set_domain(dev, NULL); > + iommu_device = vfio_mdev_get_iommu_device(dev); > + if (iommu_device) { > + bool aux_mode = false; > + > + iommu_get_dev_attr(iommu_device, > + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); same here > + if (aux_mode) > + iommu_detach_device_aux(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); > + else > + return iommu_attach_group(domain->domain, group->iommu_group); > +} > + > +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); > + else > + iommu_detach_group(domain->domain, group->iommu_group); > +} > + > static int vfio_iommu_type1_attach_group(void *iommu_data, > struct iommu_group *iommu_group) > { > @@ -1402,7 +1502,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > goto out_domain; > } > > - ret = iommu_attach_group(domain->domain, iommu_group); > + ret = vfio_iommu_attach_group(domain, group); > if (ret) > goto out_domain; > > @@ -1434,8 +1534,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > list_for_each_entry(d, &iommu->domain_list, next) { > if (d->domain->ops == domain->domain->ops && > d->prot == domain->prot) { > - iommu_detach_group(domain->domain, iommu_group); > - if (!iommu_attach_group(d->domain, iommu_group)) { > + vfio_iommu_detach_group(domain, group); > + if (!vfio_iommu_attach_group(d, group)) { > list_add(&group->next, &d->group_list); > iommu_domain_free(domain->domain); > kfree(domain); > @@ -1443,7 +1543,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > return 0; > } > > - ret = iommu_attach_group(domain->domain, iommu_group); > + ret = vfio_iommu_attach_group(domain, group); > if (ret) > goto out_domain; > } > @@ -1469,7 +1569,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > return 0; > > out_detach: > - iommu_detach_group(domain->domain, iommu_group); > + vfio_iommu_detach_group(domain, group); > out_domain: > iommu_domain_free(domain->domain); > out_free: > @@ -1560,7 +1660,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data, > if (!group) > continue; > > - iommu_detach_group(domain->domain, iommu_group); > + vfio_iommu_detach_group(domain, group); > list_del(&group->next); > kfree(group); > /* > @@ -1625,7 +1725,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external) > list_for_each_entry_safe(group, group_tmp, > &domain->group_list, next) { > if (!external) > - iommu_detach_group(domain->domain, group->iommu_group); > + vfio_iommu_detach_group(domain, group); > list_del(&group->next); > kfree(group); > } > Thanks Eric
Hi, On 11/23/18 10:13 PM, Auger Eric wrote: > Hi Lu, > > On 11/5/18 8:34 AM, Lu Baolu wrote: >> This adds helpers to attach or detach a domain to a >> group. This will replace iommu_attach_group() which >> only works for pci devices. > s/pci/non mdev? ... which doesn't work for mdev devices. >> >> If a domain is attaching to a group which includes the >> mediated devices, it should attach to the iommu device >> (a pci device which represents the mdev in iommu scope) >> instead. The added helper supports attaching domain to >> groups for both pci and mdev devices. >> >> Cc: Ashok Raj <ashok.raj@intel.com> >> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> >> Cc: Kevin Tian <kevin.tian@intel.com> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> >> Signed-off-by: Liu Yi L <yi.l.liu@intel.com> >> --- >> drivers/vfio/vfio_iommu_type1.c | 114 ++++++++++++++++++++++++++++++-- >> 1 file changed, 107 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index d9fd3188615d..178264b330e7 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -91,6 +91,7 @@ struct vfio_dma { >> struct vfio_group { >> struct iommu_group *iommu_group; >> struct list_head next; >> + bool mdev_group; /* An mdev group */ >> }; >> >> /* >> @@ -1327,6 +1328,105 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) >> return ret; >> } >> >> +static struct device *vfio_mdev_get_iommu_device(struct device *dev) >> +{ >> + struct device *(*fn)(struct device *dev); >> + struct device *iommu_parent; >> + >> + fn = symbol_get(mdev_get_iommu_device); >> + if (fn) { >> + iommu_parent = fn(dev); >> + symbol_put(mdev_get_iommu_device); >> + >> + return iommu_parent; >> + } >> + >> + return NULL; >> +} >> + >> +static int vfio_mdev_set_domain(struct device *dev, struct iommu_domain *domain) >> +{ >> + int (*fn)(struct device *dev, void *domain); >> + int ret; >> + >> + fn = symbol_get(mdev_set_iommu_domain); >> + if (fn) { >> + ret = fn(dev, domain); >> + symbol_put(mdev_set_iommu_domain); >> + >> + return ret; >> + } >> + >> + return -EINVAL; >> +} >> + >> +static int vfio_mdev_attach_domain(struct device *dev, void *data) >> +{ >> + struct iommu_domain *domain = data; >> + struct device *iommu_device; >> + int ret; >> + >> + ret = vfio_mdev_set_domain(dev, domain); >> + if (ret) >> + return ret; >> + >> + iommu_device = vfio_mdev_get_iommu_device(dev); >> + if (iommu_device) { >> + bool aux_mode = false; >> + >> + iommu_get_dev_attr(iommu_device, >> + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); > Don' you need to test the returned value before using aux_mode? Yes. Good catch. >> + if (aux_mode) >> + return iommu_attach_device_aux(domain, iommu_device); >> + else >> + return iommu_attach_device(domain, iommu_device); > if for some reason the above ops fail, don't you want to call > vfio_mdev_set_domain(dev, NULL) Yes. Good catch. > >> + } >> + >> + return -EINVAL; >> +} >> + >> +static int vfio_mdev_detach_domain(struct device *dev, void *data) >> +{ >> + struct iommu_domain *domain = data; >> + struct device *iommu_device; >> + >> + vfio_mdev_set_domain(dev, NULL); >> + iommu_device = vfio_mdev_get_iommu_device(dev); >> + if (iommu_device) { >> + bool aux_mode = false; >> + >> + iommu_get_dev_attr(iommu_device, >> + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); > same here Will fix it. >> + if (aux_mode) >> + iommu_detach_device_aux(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); >> + else >> + return iommu_attach_group(domain->domain, group->iommu_group); >> +} >> + >> +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); >> + else >> + iommu_detach_group(domain->domain, group->iommu_group); >> +} >> + >> static int vfio_iommu_type1_attach_group(void *iommu_data, >> struct iommu_group *iommu_group) >> { >> @@ -1402,7 +1502,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> goto out_domain; >> } >> >> - ret = iommu_attach_group(domain->domain, iommu_group); >> + ret = vfio_iommu_attach_group(domain, group); >> if (ret) >> goto out_domain; >> >> @@ -1434,8 +1534,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> list_for_each_entry(d, &iommu->domain_list, next) { >> if (d->domain->ops == domain->domain->ops && >> d->prot == domain->prot) { >> - iommu_detach_group(domain->domain, iommu_group); >> - if (!iommu_attach_group(d->domain, iommu_group)) { >> + vfio_iommu_detach_group(domain, group); >> + if (!vfio_iommu_attach_group(d, group)) { >> list_add(&group->next, &d->group_list); >> iommu_domain_free(domain->domain); >> kfree(domain); >> @@ -1443,7 +1543,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> return 0; >> } >> >> - ret = iommu_attach_group(domain->domain, iommu_group); >> + ret = vfio_iommu_attach_group(domain, group); >> if (ret) >> goto out_domain; >> } >> @@ -1469,7 +1569,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> return 0; >> >> out_detach: >> - iommu_detach_group(domain->domain, iommu_group); >> + vfio_iommu_detach_group(domain, group); >> out_domain: >> iommu_domain_free(domain->domain); >> out_free: >> @@ -1560,7 +1660,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data, >> if (!group) >> continue; >> >> - iommu_detach_group(domain->domain, iommu_group); >> + vfio_iommu_detach_group(domain, group); >> list_del(&group->next); >> kfree(group); >> /* >> @@ -1625,7 +1725,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external) >> list_for_each_entry_safe(group, group_tmp, >> &domain->group_list, next) { >> if (!external) >> - iommu_detach_group(domain->domain, group->iommu_group); >> + vfio_iommu_detach_group(domain, group); >> list_del(&group->next); >> kfree(group); >> } >> > Thanks > > Eric > Best regards, Lu Baolu
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index d9fd3188615d..178264b330e7 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -91,6 +91,7 @@ struct vfio_dma { struct vfio_group { struct iommu_group *iommu_group; struct list_head next; + bool mdev_group; /* An mdev group */ }; /* @@ -1327,6 +1328,105 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base) return ret; } +static struct device *vfio_mdev_get_iommu_device(struct device *dev) +{ + struct device *(*fn)(struct device *dev); + struct device *iommu_parent; + + fn = symbol_get(mdev_get_iommu_device); + if (fn) { + iommu_parent = fn(dev); + symbol_put(mdev_get_iommu_device); + + return iommu_parent; + } + + return NULL; +} + +static int vfio_mdev_set_domain(struct device *dev, struct iommu_domain *domain) +{ + int (*fn)(struct device *dev, void *domain); + int ret; + + fn = symbol_get(mdev_set_iommu_domain); + if (fn) { + ret = fn(dev, domain); + symbol_put(mdev_set_iommu_domain); + + return ret; + } + + return -EINVAL; +} + +static int vfio_mdev_attach_domain(struct device *dev, void *data) +{ + struct iommu_domain *domain = data; + struct device *iommu_device; + int ret; + + ret = vfio_mdev_set_domain(dev, domain); + if (ret) + return ret; + + iommu_device = vfio_mdev_get_iommu_device(dev); + if (iommu_device) { + bool aux_mode = false; + + iommu_get_dev_attr(iommu_device, + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); + if (aux_mode) + return iommu_attach_device_aux(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; + + vfio_mdev_set_domain(dev, NULL); + iommu_device = vfio_mdev_get_iommu_device(dev); + if (iommu_device) { + bool aux_mode = false; + + iommu_get_dev_attr(iommu_device, + IOMMU_DEV_ATTR_AUXD_ENABLED, &aux_mode); + if (aux_mode) + iommu_detach_device_aux(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); + else + return iommu_attach_group(domain->domain, group->iommu_group); +} + +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); + else + iommu_detach_group(domain->domain, group->iommu_group); +} + static int vfio_iommu_type1_attach_group(void *iommu_data, struct iommu_group *iommu_group) { @@ -1402,7 +1502,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, goto out_domain; } - ret = iommu_attach_group(domain->domain, iommu_group); + ret = vfio_iommu_attach_group(domain, group); if (ret) goto out_domain; @@ -1434,8 +1534,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, list_for_each_entry(d, &iommu->domain_list, next) { if (d->domain->ops == domain->domain->ops && d->prot == domain->prot) { - iommu_detach_group(domain->domain, iommu_group); - if (!iommu_attach_group(d->domain, iommu_group)) { + vfio_iommu_detach_group(domain, group); + if (!vfio_iommu_attach_group(d, group)) { list_add(&group->next, &d->group_list); iommu_domain_free(domain->domain); kfree(domain); @@ -1443,7 +1543,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, return 0; } - ret = iommu_attach_group(domain->domain, iommu_group); + ret = vfio_iommu_attach_group(domain, group); if (ret) goto out_domain; } @@ -1469,7 +1569,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, return 0; out_detach: - iommu_detach_group(domain->domain, iommu_group); + vfio_iommu_detach_group(domain, group); out_domain: iommu_domain_free(domain->domain); out_free: @@ -1560,7 +1660,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data, if (!group) continue; - iommu_detach_group(domain->domain, iommu_group); + vfio_iommu_detach_group(domain, group); list_del(&group->next); kfree(group); /* @@ -1625,7 +1725,7 @@ static void vfio_release_domain(struct vfio_domain *domain, bool external) list_for_each_entry_safe(group, group_tmp, &domain->group_list, next) { if (!external) - iommu_detach_group(domain->domain, group->iommu_group); + vfio_iommu_detach_group(domain, group); list_del(&group->next); kfree(group); }