Re: [PATCH v2] xen/arm: smmuv3: Add support for removing devices

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Mykyta,

> On 14 Apr 2026, at 14:15, Mykyta Poturai <Mykyta_Poturai@xxxxxxxx> wrote:
> 
> Allow for removing devices from SMMUv3. arm_smmu_deassign_dev handles
> most of the work by disabling ATS and zeroing STEs. Additionally, unset
> the dt_device_is_protected flag and free no longer needed smmu_master.
> 
> Rework dt_device_set_protected to accept a boolean parameter, update
> callsites.
> 

Should ...

> Tested on QEMU with SRIOV series[1] by repeatedly enabling/disabling
> VFs.
> 
> [1]: https://patchew.org/Xen/cover.1772806036.git.mykyta._5Fpoturai@xxxxxxxx/

this be omitted from commit message?

> 
> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> ---
> V1-V2:
> * check for phantom functions
> * simplify pci/dt device split
> * improve error handling
> * don't try to free master for unprotected devices
> * rework dt_device_set_protected
> ---
> xen/drivers/passthrough/arm/ipmmu-vmsa.c |  2 +-
> xen/drivers/passthrough/arm/smmu-v3.c    | 57 +++++++++++++++++++++++-
> xen/drivers/passthrough/arm/smmu.c       |  4 +-
> xen/include/xen/device_tree.h            |  5 ++-
> 4 files changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c 
> b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index fa9ab9cb13..0648f9b407 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -1367,7 +1367,7 @@ static int ipmmu_add_device(u8 devfn, struct device 
> *dev)
>         }
> 
>         /* Let Xen know that the master device is protected by an IOMMU. */
> -        dt_device_set_protected(dev_to_dt(dev));
> +        dt_device_set_protected(dev_to_dt(dev), true);
>     }
> #ifdef CONFIG_HAS_PCI
>     if ( dev_is_pci(dev) )
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index bf153227db..8e080cd7d0 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -1493,6 +1493,60 @@ static int arm_smmu_assign_dev(struct domain *d, u8 
> devfn, struct device *dev,
> static int arm_smmu_deassign_dev(struct domain *d, uint8_t devfn,
> struct device *dev);
> 
> +static int arm_smmu_remove_device(u8 devfn, struct device *dev)
> +{
> + struct arm_smmu_master *master;
> + struct iommu_fwspec *fwspec;

I’m looking into the arm_smmu_remove_device, at some point there we allocate
the iommu_fwspec, but we are not using iommu_fwspec_free() here, I’ve tried
to look around and I’m not able to see what frees that structure apart form
iommu_remove_dt_device().
But pci_remove_device() -> iommu_remove_device() can call this function as 
well, I’m not sure I’ve understood
correctly the framework here so maybe someone with more PCI experience can help
@Stewart Hildebrand ?


> + struct domain *d = NULL;
> +
> + fwspec = dev_iommu_fwspec_get(dev);
> + if ( !fwspec )
> + return -ENODEV;
> +
> + master = dev_iommu_priv_get(dev);
> + if ( !master )
> + return -ENODEV;
> +
> + if ( IS_ENABLED(CONFIG_HAS_PCI) && dev_is_pci(dev) )
> + {
> + struct pci_dev *pdev = dev_to_pci(dev);
> +
> + /* Ignore calls for phantom functions */
> + if ( devfn != pdev->devfn )
> + return 0;
> +
> + d = pdev->domain;
> + }
> + else
> + {
> + if ( !dt_device_is_protected(dev_to_dt(dev)) )
> + {
> + dev_err(dev, "Not added to SMMUv3\n");
> + return -ENODEV;
> + }
> +
> + dt_device_set_protected(dev_to_dt(dev), false);
> + if ( master->domain && master->domain->d )
> + d = master->domain->d;
> + }
> +
> + if ( d )
> + {
> + int ret = arm_smmu_deassign_dev(d, devfn, dev);
> + /* This should never fail because we already checked the domain */
> + ASSERT(!ret);
> + }
> +
> + arm_smmu_disable_pasid(master);
> +
> + dev_info(dev, "Removed master device (SMMUv3 %s StreamIds %u)\n",
> + dev_name(fwspec->iommu_dev), fwspec->num_ids);
> +
> + xfree(master);
> + dev_iommu_priv_set(dev, NULL);
> + return 0;
> +}
> +

Cheers,
Luca