Messages in this thread | | | Date | Tue, 28 Nov 2023 15:07:58 -0400 | From | Jason Gunthorpe <> | Subject | Re: [PATCH 1/2] iommufd/selftest: Use a fwnode to distinguish devices |
| |
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
| |