Re: [RFC PATCH v6 08/11] iommufd: Add vIOMMU provider support

[Jason Gunthorpe]

On Tue, Sep 29, 2026 at 09:28:53PM +0530, Aneesh Kumar K.V wrote:
> Switching pdev->tsm to an RCU-protected pointer requires broader changes
> to the existing TSM code. To move this series forward, I will continue
> protecting it with pci_tsm_rwsem in the next revision, as that requires
> fewer changes. We can revisit an RCU conversion later if needed?

Sure, as long as we get the general big picture properties of no hot
unplug and a very simple lifecycle model

> With this approach, viommu_alloc() will do:
> 	struct tsm_dev *tsm_dev = NULL;
> 
> 
> 	tsm_dev = tsm_get_device(idev->dev);
> 	ops = tsm_dev ? tsm_viommu_get_ops(tsm_dev, idev->dev, cmd->type) : NULL;
> 	if (!ops) {
>                 tsm_dev = NULL;
> 		ops = iommu_dev->ops->get_viommu_ops(idev->dev, cmd->type);
>         }
> 
> 	viommu = (struct iommufd_viommu *)_iommufd_object_alloc_ucmd(
> 	viommu->tsm_dev = tsm_dev;
> 	tsm_dev = NULL;
> 
> 	rc = ops->viommu_init(viommu, idev->dev,....)
> 
> ....
> 	if (tsm_dev)
> 		tsm_put_device(tsm_dev);

This probably shouldn't be here? The way iommufd usually works is
these undos are alway here:

> void iommufd_viommu_destroy(struct iommufd_object *obj)
> {
> ..
> 	if (viommu->tsm_dev)
> 		tsm_put_device(viommu->tsm_dev);
> ...
> }


> struct tsm_dev *pci_tsm_get_device(struct pci_dev *pdev)
> {
> 	const struct pci_tsm_ops *ops;
> 	struct tsm_dev *tsm_dev;
> 
> 	guard(rwsem_read)(&pci_tsm_rwsem);
> 	if (!pdev->tsm)
> 		return NULL;
> 
> 	tsm_dev = pdev->tsm->tsm_dev;
> 
> 	ops = tsm_dev->pci_ops;
> 	if (!try_module_get(ops->owner)) // arm-cca-host
> 		return ERR_PTR(-ENODEV);
> 	if (!tsm_try_get(tsm_dev)) {

What/why is this tsm_try_get()? I wouldn't expect to see both
try_module_get() and tsm_try_get()?

> Are you suggesting setting suppress_bind_attrs = true for the
> arm-cca-host driver?

That or it blocks waiting on unbind.

No hot unplug of TSMs under active use. This is very hard to implement
correctly, and there is no use case.

> The driver model otherwise allows the driver to be unbound. 

Isn't this is a side effect of introducing smcc bus? We didn't used to
have actual drivers for this stuff that isn't physically hot
pluggable.

> We need to support tsm_unregister() in that
> case. I am also not sure whether there are other paths that can call
> tsm_unregister().  Currently, we register the cleanup callback via
>
> 	tsm_dev = tsm_register(&sdev->dev, &cca_link_pci_ops);
> 
> 	ret = devm_add_action_or_reset(&sdev->dev, cca_link_tsm_remove, tsm_dev);
> 	if (ret)

Yeah, since you have a driver it has to be structured like that, but
that doesn't mean we have to be forced into implementing full general
hot unplug across all of tsm.

Either block sysfs and use the module refcount, or hang in removal
until all the users naturally close (like vfio). Everything else is
too complicated to be justified..

Jason