Re: [PATCH 1/2] iommufd/selftest: Use a fwnode to distinguish devices

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Nov 28, 2023 at 05:36:33PM +0000, Robin Murphy wrote:

> You see this is exactly the kind of complexity I *don't* want, since the
> only thing it would foreseeably benefit is the one special case of the
> IOMMUFD selftest, which can far more trivially just adopt the other of the
> two "standard" usage models we have. I've been trying to get *away* from
> having to have boilerplate checks in all the drivers, and this would require
> bringing back a load of the ones I've just removed :(

I don't think we need to bring back the fwspec checks you removed, the
loop just needs to keep the NULL check:

 +	list_for_each_entry(iommu, &iommu_device_list, list) {
 +		if (iommu->ops != ops && !iommu->fwnode) {
 +			ops = iommu->ops;
 +			ret = iommu_init_device(pinf, iommu->ops);
 +			if (ret != -ENODEV)
 +				return ret;
 +		}
 +	}

Iterate over all the global driver ops only. Drivers with a fwnode
will never be called without a fwspec.

Also, does omap have problems now too? omap seems to set fwnode but
does some slightly different open coded non-fwspec parsing that worked
at bus time? Is it still OK? Does fwspec even find ops in omap's FW
description (ie it looks like it make iommu-cells optional or
something)?

> As I said before, I really want to avoid the perf_event_init model of
> calling round every driver saying "hey, do you want this?" since it's also
> error-prone if any of those drivers doesn't get the boilerplate exactly
> right and inadvertently fails to reject something it should have. 

The core missed an API that every driver needs: give me the struct
iommu_driver* the FW has referenced.

Instead every driver open codes something like
arm_smmu_get_by_fwnode(), or much worse.

If we force the drivers to say

 iommu_driver = iommu_fw_give_me_my_driver(dev, ops)

Then we automatically have a place to do all the rejection checks we
need, and driver's can't inadvertently skip this because they really
can't work without the iommu_driver at all.

Anyhow, I completed the series I talked about yesterday. It turned out
really nice I think, especially the driver facing API is much
cleaner. I'm just going through the last bits before I share it.

Jason