2022-06-01 17:56:34

by Saravana Kannan

Subject: [PATCH v2 8/9] iommu/of: Delete usage of driver_deferred_probe_check_state()

Now that fw_devlink=on and fw_devlink.strict=1 by default and fw_devlink
supports iommu DT properties, the execution will never get to the point
where driver_deferred_probe_check_state() is called before the supplier
has probed successfully or before deferred probe timeout has expired.

So, delete the call and replace it with -ENODEV.

Signed-off-by: Saravana Kannan <[email protected]>
---
drivers/iommu/of_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 5696314ae69e..41f4eb005219 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -40,7 +40,7 @@ static int of_iommu_xlate(struct device *dev,
* a proper probe-ordering dependency mechanism in future.
*/
if (!ops)
- return driver_deferred_probe_check_state(dev);
+ return -ENODEV;

if (!try_module_get(ops->owner))
return -ENODEV;
--
2.36.1.255.ge46751e96f-goog

2022-08-19 14:44:36

by Jean-Philippe Brucker

Subject: Re: [PATCH v2 8/9] iommu/of: Delete usage of driver_deferred_probe_check_state()

Hi Saravana,

On Wed, Jun 01, 2022 at 12:07:04AM -0700, Saravana Kannan via iommu wrote:
> Now that fw_devlink=on and fw_devlink.strict=1 by default and fw_devlink
> supports iommu DT properties, the execution will never get to the point
> where driver_deferred_probe_check_state() is called before the supplier
> has probed successfully or before deferred probe timeout has expired.
>
> So, delete the call and replace it with -ENODEV.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> ---
> drivers/iommu/of_iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5696314ae69e..41f4eb005219 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -40,7 +40,7 @@ static int of_iommu_xlate(struct device *dev,
> * a proper probe-ordering dependency mechanism in future.
> */
> if (!ops)
> - return driver_deferred_probe_check_state(dev);
> + return -ENODEV;

Unfortunately this breaks virtio-iommu on v6.0-rc1. Could you add it to
your revert series [1]?

Eventually I'd really like to use fw_devlink in order to enable module
support, so I'm looking for a proper fix. Our DT description is peculiar
because the virtio-iommu, when it uses the virtio-pci transport, can
appear on the same PCI bus than the endpoints it manages. So the PCI node
looks like this:

pcie@10000000 {
compatible = "pci-host-ecam-generic";
...
iommu-map = <0x00 &viommu 0x00 0x20>
<0x21 &viommu 0x21 0xffdf>;

viommu: virtio_iommu@32 {
#iommu-cells = <0x01>;
reg = <0x2000 0x00 0x00 0x00 0x00>;
compatible = "virtio,pci-iommu";
};
};

The iommu-map property points to the IOMMU sub-node for all endpoints
except the IOMMU itself (device 0x20 - 00:04.0). fw_devlink currently
rejects this:

[ 0.270150] OF: Not linking pcie@10000000 to virtio_iommu@32 - is descendant
[ 0.270324] OF: Not linking pcie@10000000 to virtio_iommu@32 - is descendant

I don't know yet if this kind of topology could ever work with fw_devlink,
but I thought I'd notify you before digging further.

Thanks,
Jean

[1] https://lore.kernel.org/all/[email protected]/#t

>
> if (!try_module_get(ops->owner))
> return -ENODEV;
> --
> 2.36.1.255.ge46751e96f-goog
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu