| Message ID | 170618452753.3805.4425669653666211728.stgit@ltcd48-lp2.aus.stglab.ibm.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | powerpc: iommu: Fix the vfio-pci bind and unbind issues | expand |
On Thu, Jan 25, 2024 at 06:08:52AM -0600, Shivaprasad G Bhat wrote: > On PPC64, the iommu_ops.def_domain_type() is not defined and > CONFIG_IOMMU_DMA not enabled. With commit 0f6a90436a57 ("iommu: Do not > use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled"), the > iommu_get_default_domain_type() return IOMMU_DOMAIN_IDENTITY. With > commit 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove > set_platform_dma_ops"), the defaule_domain is set wih the type being > PLATFORM. With these two changes, iommu_group_alloc_default_domain() > ends up returning the NULL(with recent changes, ERR_PTR(-EINVAL)) > leading to iommu_probe_device() failure and the device has no > iommu_group set in effect. Subsequently, the bind to vfio(VFIO_IOMMU) > fail as the iommu_group is not set for the device. > > Make the iommu_get_default_domain_type() to take default_domain->type > into consideration along with default_domain_type() and fix > iommu_group_alloc_default_domain() to not error out if the requested > type is same as default domain type. > > Fixes: 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove set_platform_dma_ops") > Fixes: 0f6a90436a57 ("iommu: Do not use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled") > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> > --- > drivers/iommu/iommu.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) Are you OK with this version? https://lore.kernel.org/linux-iommu/20240123174905.GS50608@ziepe.ca/ ? I think it does the same thing Jason
On 1/25/24 21:22, Jason Gunthorpe wrote: > On Thu, Jan 25, 2024 at 06:08:52AM -0600, Shivaprasad G Bhat wrote: >> On PPC64, the iommu_ops.def_domain_type() is not defined and >> CONFIG_IOMMU_DMA not enabled. With commit 0f6a90436a57 ("iommu: Do not >> use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled"), the >> iommu_get_default_domain_type() return IOMMU_DOMAIN_IDENTITY. With >> commit 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove >> set_platform_dma_ops"), the defaule_domain is set wih the type being >> PLATFORM. With these two changes, iommu_group_alloc_default_domain() >> ends up returning the NULL(with recent changes, ERR_PTR(-EINVAL)) >> leading to iommu_probe_device() failure and the device has no >> iommu_group set in effect. Subsequently, the bind to vfio(VFIO_IOMMU) >> fail as the iommu_group is not set for the device. >> >> Make the iommu_get_default_domain_type() to take default_domain->type >> into consideration along with default_domain_type() and fix >> iommu_group_alloc_default_domain() to not error out if the requested >> type is same as default domain type. >> >> Fixes: 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove set_platform_dma_ops") >> Fixes: 0f6a90436a57 ("iommu: Do not use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled") >> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> >> --- >> drivers/iommu/iommu.c | 14 ++++++++++---- >> 1 file changed, 10 insertions(+), 4 deletions(-) > Are you OK with this version? > > https://lore.kernel.org/linux-iommu/20240123174905.GS50608@ziepe.ca/ > > ? > > I think it does the same thing Yes, This works. I see a very minor difference of default_domain->type is given precedence over def_domain_type(). Please keep me CC when you post this fix, I'll test it with any(?) other changes if coming along with it. Thanks, Shivaprasad > Jason
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 68e648b55767..04083a1d9f7e 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -135,6 +135,8 @@ static struct group_device *iommu_group_alloc_device(struct iommu_group *group, struct device *dev); static void __iommu_group_free_device(struct iommu_group *group, struct group_device *grp_dev); +static int iommu_get_def_domain_type(struct iommu_group *group, + struct device *dev, int cur_type); #define IOMMU_GROUP_ATTR(_name, _mode, _show, _store) \ struct iommu_group_attribute iommu_group_attr_##_name = \ @@ -1788,7 +1790,8 @@ __iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) static struct iommu_domain * iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) { - const struct iommu_ops *ops = dev_iommu_ops(iommu_group_first_dev(group)); + struct device *dev = iommu_group_first_dev(group); + const struct iommu_ops *ops = dev_iommu_ops(dev); struct iommu_domain *dom; lockdep_assert_held(&group->mutex); @@ -1799,7 +1802,7 @@ iommu_group_alloc_default_domain(struct iommu_group *group, int req_type) * domain. Do not use in new drivers. */ if (ops->default_domain) { - if (req_type) + if (iommu_get_def_domain_type(group, dev, req_type) != req_type) return ERR_PTR(-EINVAL); return ops->default_domain; } @@ -1871,10 +1874,13 @@ static int iommu_get_def_domain_type(struct iommu_group *group, const struct iommu_ops *ops = dev_iommu_ops(dev); int type; - if (!ops->def_domain_type) + if (ops->def_domain_type) + type = ops->def_domain_type(dev); + else if (group->default_domain) + type = group->default_domain->type; + else return cur_type; - type = ops->def_domain_type(dev); if (!type || cur_type == type) return cur_type; if (!cur_type)
On PPC64, the iommu_ops.def_domain_type() is not defined and CONFIG_IOMMU_DMA not enabled. With commit 0f6a90436a57 ("iommu: Do not use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled"), the iommu_get_default_domain_type() return IOMMU_DOMAIN_IDENTITY. With commit 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove set_platform_dma_ops"), the defaule_domain is set wih the type being PLATFORM. With these two changes, iommu_group_alloc_default_domain() ends up returning the NULL(with recent changes, ERR_PTR(-EINVAL)) leading to iommu_probe_device() failure and the device has no iommu_group set in effect. Subsequently, the bind to vfio(VFIO_IOMMU) fail as the iommu_group is not set for the device. Make the iommu_get_default_domain_type() to take default_domain->type into consideration along with default_domain_type() and fix iommu_group_alloc_default_domain() to not error out if the requested type is same as default domain type. Fixes: 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove set_platform_dma_ops") Fixes: 0f6a90436a57 ("iommu: Do not use IOMMU_DOMAIN_DMA if CONFIG_IOMMU_DMA is not enabled") Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> --- drivers/iommu/iommu.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)