Re: [PATCH rc v6] iommu: Fix nested pci_dev_reset_iommu_prepare/done()

[Jason Gunthorpe]

On Tue, Apr 07, 2026 at 12:46:44PM -0700, Nicolin Chen wrote:
> Shuai found that cxl_reset_bus_function() calls pci_reset_bus_function()
> internally while both are calling pci_dev_reset_iommu_prepare/done().
> 
> As pci_dev_reset_iommu_prepare() doesn't support re-entry, the inner call
> will trigger a WARN_ON and return -EBUSY, resulting in failing the entire
> device reset.
> 
> On the other hand, removing the outer calls in the PCI callers is unsafe.
> As pointed out by Kevin, device-specific quirks like reset_hinic_vf_dev()
> execute custom firmware waits after their inner pcie_flr() completes. If
> the IOMMU protection relies solely on the inner reset, the IOMMU will be
> unblocked prematurely while the device is still resetting.
> 
> Instead, fix this by making pci_dev_reset_iommu_prepare/done() reentrant.
> 
> Given the IOMMU core tracks the resetting state per iommu_group while the
> reset is per device, this has to track at the group_device level as well.
> 
> Introduce a 'reset_depth' and a 'blocked' flag to struct group_device, to
> handle the re-entries on the same device. This allows multi-device groups
> to isolate concurrent device resets independently.
> 
> Note that iommu_deferred_attach() and iommu_driver_get_domain_for_dev()
> both now check the per-device 'gdev->blocked' flag instead of a per-group
> flag like 'group->resetting_domain'. This is actually more precise. Also,
> this 'gdev->blocked' will be useful in the future work to flag the device
> blocked by an ongoing/failed reset or quarantine.
> 
> As the reset routine is per gdev, it cannot clear group->resetting_domain
> without iterating over the device list to ensure no other device is being
> reset. Simplify it by replacing the resetting_domain with a 'recovery_cnt'
> in the struct iommu_group.
> 
> Since both helpers are now per gdev, call the per-device set_dev_pasid op
> to recover PASID domains. And add 'max_pasids > 0' checks in both helpers.
> 
> Fixes: c279e83953d9 ("iommu: Introduce pci_dev_reset_iommu_prepare/done()")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/absKsk7qQOwzhpzv@Asurada-Nvidia/
> Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> ---
> Changelog
>  v6:
>   * Update inline comments and commit message
>   * Add "max_pasids > 0" condition in both helpers
>  v5:
>   https://lore.kernel.org/all/20260404050243.141366-1-nicolinc@xxxxxxxxxx/
>   * Add 'blocked' to fix iommu_driver_get_domain_for_dev() return.
>  v4:
>   https://lore.kernel.org/all/20260324014056.36103-1-nicolinc@xxxxxxxxxx/
>   * Rename 'reset_cnt' to 'recovery_cnt'
>  v3:
>   https://lore.kernel.org/all/20260321223930.10836-1-nicolinc@xxxxxxxxxx/
>   * Turn prepare()/done() to be per-gdev
>   * Use reset_depth to track nested re-entries
>   * Replace group->resetting_domain with a reset_cnt
>  v2:
>   https://lore.kernel.org/all/20260319043135.1153534-1-nicolinc@xxxxxxxxxx/
>   * Fix in the helpers by allowing re-entry
>  v1:
>   https://lore.kernel.org/all/20260318220028.1146905-1-nicolinc@xxxxxxxxxx/
> 
>  drivers/iommu/iommu.c | 148 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 110 insertions(+), 38 deletions(-)

This looks reasonable to me.

Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

Jason