Much as I'd like to remove iommu_present(), the final remaining users
are proving stubbornly difficult to clean up, so kick that can down the
road and just rework it to preserve the current behaviour without
depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
to IOMMU device registration"), any registered IOMMU instance is already
considered "present" for every entry in iommu_buses, so it's simply a
case of validating the bus and checking we have at least once IOMMU.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
v3: Tweak to use the ops-based check rather than group-based, to
properly match the existing behaviour
v4: Just look for IOMMU instances instead of managed devices
---
drivers/iommu/iommu.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5a3ce293a5de..7bb92e8b7a49 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2000,9 +2000,28 @@ int bus_iommu_probe(const struct bus_type *bus)
return 0;
}
+/**
+ * iommu_present() - make platform-specific assumptions about an IOMMU
+ * @bus: bus to check
+ *
+ * Do not use this function. You want device_iommu_mapped() instead.
+ *
+ * Return: true if some IOMMU is present and aware of devices on the given bus;
+ * in general it may not be the only IOMMU, and it may not have anything to do
+ * with whatever device you are ultimately interested in.
+ */
bool iommu_present(const struct bus_type *bus)
{
- return bus->iommu_ops != NULL;
+ bool ret = false;
+
+ for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
+ if (iommu_buses[i] == bus) {
+ spin_lock(&iommu_device_lock);
+ ret = !list_empty(&iommu_device_list);
+ spin_unlock(&iommu_device_lock);
+ }
+ }
+ return ret;
}
EXPORT_SYMBOL_GPL(iommu_present);
--
2.39.2.101.g768bb238c484.dirtyOn Wed, Oct 11, 2023 at 07:14:49PM +0100, Robin Murphy wrote:
> Much as I'd like to remove iommu_present(), the final remaining users
> are proving stubbornly difficult to clean up, so kick that can down the
> road and just rework it to preserve the current behaviour without
> depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
> to IOMMU device registration"), any registered IOMMU instance is already
> considered "present" for every entry in iommu_buses, so it's simply a
> case of validating the bus and checking we have at least once IOMMU.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> ---
>
> v3: Tweak to use the ops-based check rather than group-based, to
> properly match the existing behaviour
> v4: Just look for IOMMU instances instead of managed devices
> ---
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
On 2023/10/12 2:14, Robin Murphy wrote:
> Much as I'd like to remove iommu_present(), the final remaining users
> are proving stubbornly difficult to clean up, so kick that can down the
> road and just rework it to preserve the current behaviour without
> depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
> to IOMMU device registration"), any registered IOMMU instance is already
> considered "present" for every entry in iommu_buses, so it's simply a
> case of validating the bus and checking we have at least once IOMMU.
>
> Reviewed-by: Jason Gunthorpe<jgg@nvidia.com>
> Signed-off-by: Robin Murphy<robin.murphy@arm.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Best regards,
baolu
On 10/12/23 2:14 AM, Robin Murphy wrote:
> Much as I'd like to remove iommu_present(), the final remaining users
> are proving stubbornly difficult to clean up, so kick that can down the
> road and just rework it to preserve the current behaviour without
> depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
The iommu_present() is only used in below two drivers.
$ git grep iommu_present
drivers/gpu/drm/mediatek/mtk_drm_drv.c: if
(!iommu_present(&platform_bus_type))
drivers/gpu/drm/tegra/drm.c: if (host1x_drm_wants_iommu(dev) &&
iommu_present(&platform_bus_type)) {
Both are platform drivers and have the device pointer passed in. Just
out of curiosity, why not replacing them with device_iommu_mapped()
instead? Sorry if I overlooked previous discussion.
Best regards,
baolu
> to IOMMU device registration"), any registered IOMMU instance is already
> considered "present" for every entry in iommu_buses, so it's simply a
> case of validating the bus and checking we have at least once IOMMU.
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>
> ---
>
> v3: Tweak to use the ops-based check rather than group-based, to
> properly match the existing behaviour
> v4: Just look for IOMMU instances instead of managed devices
> ---
> drivers/iommu/iommu.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 5a3ce293a5de..7bb92e8b7a49 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2000,9 +2000,28 @@ int bus_iommu_probe(const struct bus_type *bus)
> return 0;
> }
>
> +/**
> + * iommu_present() - make platform-specific assumptions about an IOMMU
> + * @bus: bus to check
> + *
> + * Do not use this function. You want device_iommu_mapped() instead.
> + *
> + * Return: true if some IOMMU is present and aware of devices on the given bus;
> + * in general it may not be the only IOMMU, and it may not have anything to do
> + * with whatever device you are ultimately interested in.
> + */
> bool iommu_present(const struct bus_type *bus)
> {
> - return bus->iommu_ops != NULL;
> + bool ret = false;
> +
> + for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
> + if (iommu_buses[i] == bus) {
> + spin_lock(&iommu_device_lock);
> + ret = !list_empty(&iommu_device_list);
> + spin_unlock(&iommu_device_lock);
> + }
> + }
> + return ret;
> }
> EXPORT_SYMBOL_GPL(iommu_present);
>
On 2023-10-12 07:05, Baolu Lu wrote:
> On 10/12/23 2:14 AM, Robin Murphy wrote:
>> Much as I'd like to remove iommu_present(), the final remaining users
>> are proving stubbornly difficult to clean up, so kick that can down the
>> road and just rework it to preserve the current behaviour without
>> depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
>
> The iommu_present() is only used in below two drivers.
>
> $ git grep iommu_present
> drivers/gpu/drm/mediatek/mtk_drm_drv.c: if
> (!iommu_present(&platform_bus_type))
> drivers/gpu/drm/tegra/drm.c: if (host1x_drm_wants_iommu(dev) &&
> iommu_present(&platform_bus_type)) {
>
> Both are platform drivers and have the device pointer passed in. Just
> out of curiosity, why not replacing them with device_iommu_mapped()
> instead? Sorry if I overlooked previous discussion.
Yes, we've already gone round in circles on this several times, that's
why it's explicitly called out as "stubbornly difficult" in the commit
message. The Mediatek one is entirely redundant, but it seems I have yet
to figure out the right CC list to get anyone to care about that
patch[1]. The Tegra one is making some non-obvious assumptions to
actually check on behalf of some *other* devices, even when the one to
hand may not be using the IOMMU itself[2]. That case is what the new
kerneldoc alludes to.
My hope is to eventually punt this into the Tegra driver itself
(probably at the point when it needs something similar for
iommu_domain_alloc() as well), however previous experience has taught me
that trying to coordinate cross-subsystem work with drm-misc is an
ordeal best avoided until there is no possible alternative.
Thanks,
Robin.
[1] https://patchwork.freedesktop.org/patch/536273/
[2]
https://lore.kernel.org/linux-iommu/a0c7e954-ee3f-74fd-cfea-9b6dbce924dc@collabora.com/
>
> Best regards,
> baolu
>
>> to IOMMU device registration"), any registered IOMMU instance is already
>> considered "present" for every entry in iommu_buses, so it's simply a
>> case of validating the bus and checking we have at least once IOMMU.
>>
>> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>
>> ---
>>
>> v3: Tweak to use the ops-based check rather than group-based, to
>> properly match the existing behaviour
>> v4: Just look for IOMMU instances instead of managed devices
>> ---
>> drivers/iommu/iommu.c | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 5a3ce293a5de..7bb92e8b7a49 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2000,9 +2000,28 @@ int bus_iommu_probe(const struct bus_type *bus)
>> return 0;
>> }
>> +/**
>> + * iommu_present() - make platform-specific assumptions about an IOMMU
>> + * @bus: bus to check
>> + *
>> + * Do not use this function. You want device_iommu_mapped() instead.
>> + *
>> + * Return: true if some IOMMU is present and aware of devices on the
>> given bus;
>> + * in general it may not be the only IOMMU, and it may not have
>> anything to do
>> + * with whatever device you are ultimately interested in.
>> + */
>> bool iommu_present(const struct bus_type *bus)
>> {
>> - return bus->iommu_ops != NULL;
>> + bool ret = false;
>> +
>> + for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
>> + if (iommu_buses[i] == bus) {
>> + spin_lock(&iommu_device_lock);
>> + ret = !list_empty(&iommu_device_list);
>> + spin_unlock(&iommu_device_lock);
>> + }
>> + }
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(iommu_present);
On Thu, Oct 12, 2023 at 12:40:01PM +0100, Robin Murphy wrote:
> On 2023-10-12 07:05, Baolu Lu wrote:
> > On 10/12/23 2:14 AM, Robin Murphy wrote:
> > > Much as I'd like to remove iommu_present(), the final remaining users
> > > are proving stubbornly difficult to clean up, so kick that can down the
> > > road and just rework it to preserve the current behaviour without
> > > depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
> >
> > The iommu_present() is only used in below two drivers.
> >
> > $ git grep iommu_present
> > drivers/gpu/drm/mediatek/mtk_drm_drv.c: if
> > (!iommu_present(&platform_bus_type))
> > drivers/gpu/drm/tegra/drm.c: if (host1x_drm_wants_iommu(dev) &&
> > iommu_present(&platform_bus_type)) {
> >
> > Both are platform drivers and have the device pointer passed in. Just
> > out of curiosity, why not replacing them with device_iommu_mapped()
> > instead? Sorry if I overlooked previous discussion.
>
> Yes, we've already gone round in circles on this several times, that's why
> it's explicitly called out as "stubbornly difficult" in the commit message.
> The Mediatek one is entirely redundant, but it seems I have yet to figure
> out the right CC list to get anyone to care about that patch[1].
Please just have Joerg take such a trivial patch, there is no reason
we need to torture outselves because DRM side is not behaving well. :(
Jason
On 2023/10/12 20:37, Jason Gunthorpe wrote:
> On Thu, Oct 12, 2023 at 12:40:01PM +0100, Robin Murphy wrote:
>> On 2023-10-12 07:05, Baolu Lu wrote:
>>> On 10/12/23 2:14 AM, Robin Murphy wrote:
>>>> Much as I'd like to remove iommu_present(), the final remaining users
>>>> are proving stubbornly difficult to clean up, so kick that can down the
>>>> road and just rework it to preserve the current behaviour without
>>>> depending on bus ops. Since commit 57365a04c921 ("iommu: Move bus setup
>>>
>>> The iommu_present() is only used in below two drivers.
>>>
>>> $ git grep iommu_present
>>> drivers/gpu/drm/mediatek/mtk_drm_drv.c: if
>>> (!iommu_present(&platform_bus_type))
>>> drivers/gpu/drm/tegra/drm.c: if (host1x_drm_wants_iommu(dev) &&
>>> iommu_present(&platform_bus_type)) {
>>>
>>> Both are platform drivers and have the device pointer passed in. Just
>>> out of curiosity, why not replacing them with device_iommu_mapped()
>>> instead? Sorry if I overlooked previous discussion.
>>
>> Yes, we've already gone round in circles on this several times, that's why
>> it's explicitly called out as "stubbornly difficult" in the commit message.
>> The Mediatek one is entirely redundant, but it seems I have yet to figure
>> out the right CC list to get anyone to care about that patch[1].
I see now. Thanks for the explanation.
>
> Please just have Joerg take such a trivial patch, there is no reason
> we need to torture outselves because DRM side is not behaving well. :(
I was not object to the patch. Just want to make sure that I understand
the reason why device_iommu_mapped() can't be used in those two drivers.
It's fine to me. I will add my r-b.
Best regards,
baolu
© 2016 - 2026 Red Hat, Inc.