[PATCH v4 00/17] Consolidate the error handling around device attachment

Jason Gunthorpe
2023-04-12T13:51:57Z

Device attachment has a bunch of different flows open coded in different
ways throughout the code.

One of the things that became apparently recently is that error handling
is important and we do need to consistently treat errors during attach and
have some strategy to unwind back to a safe state.

Implement a single algorithm for this in one function. It will call each
device's attach, if it fails it will try to go back to the prior domain or
as a contingency against a UAF crash try to go to a blocking domain.

As part of this we consolidate how the default domain is created and
attached as well into one place with a consistent flow.

The new worker functions are called __iommu_device_set_domain() and
__iommu_group_set_domain_internal(), each has sensible error handling
internally. At the end __iommu_group_set_domain_internal() is the only
function that stores to group->domain, and must be called to change this
value.

Some flags tell the intent of the caller, if the caller cannot accept a
failure, or if the caller is a first attach and wants to do the deferred
logic.

Several of the confusing workflows where we store things in group->domain
or group->default_domain before they are fully setup are removed.

This has a followup series that does a similar de-duplication to the probe
path:

https://github.com/jgunthorpe/linux/commits/iommu_err_unwind

v4:
- Update comments and commit messages
- New patch "Remove iommu_group_do_dma_first_attach() from iommu_group_add_device()"
- Redo "Replace __iommu_group_dma_first_attach() with set_domain"
to avoid the flag
- Drop "Make iommu_group_do_dma_first_attach() work with owned groups"
since the fix happens implicitly now
- Use __iommu_group_set_domain() instead of
__iommu_group_set_domain_internal() in most places
- Make sure iommu_create_device_direct_mappings() is always called to
update the default domain, even if the current attached domain is not
the default domain
- Make the error case for inconsistent iommu_get_def_domain_type()'s
always use iommu_def_domain_type
- Handle errrors for the first default domain attach more cleanly
v3: https://lore.kernel.org/r/0-v3-e89a9bb522f5+8c87-iommu_err_unwind_jgg@nvidia.com
- New patch to do iommu_group_create_direct_mappings() before
attach on the hotplug path based on Lu and Robin's remarks
- Fix to return 0 if the group has conflicting default domain types like
the original code
- Put iommu_group_create_direct_mappings() before attach in setup_domains
- Split up the alloc changes from the setup_domain patch to their own
patch, implement Robin's point about how the iommu_def_domain_type should
work
- New patch to optionally do iommu_group_create_direct_mappings() after
attach
- Reword the setup_domain patch's commit message
v2: https://lore.kernel.org/r/0-v2-cd32667d2ba6+70bd1-iommu_err_unwind_jgg@nvidia.com
- New patch to remove iommu_group_device_count()
- New patch to add a for_each helper: for_each_group_device()
- Rebase on Joerg's tree
- IOMMU_SET_DOMAIN_MUST_SUCCEED instead of true
- Split patch to fix owned groups during first attach
- Change iommu_create_device_direct_mappings to accept a domain not a
group
- Significantly revise the "iommu: Consolidate the default_domain setup to
one function" patch to de-duplicate the domain type calculation logic
too
- New patch to clean the flow inside iommu_group_store_type()
v1: https://lore.kernel.org/r/0-v1-20507a7e6b7e+2d6-iommu_err_unwind_jgg@nvidia.com

Jason Gunthorpe (17):
iommu: Replace iommu_group_device_count() with list_count_nodes()
iommu: Add for_each_group_device()
iommu: Make __iommu_group_set_domain() handle error unwind
iommu: Use __iommu_group_set_domain() for __iommu_attach_group()
iommu: Use __iommu_group_set_domain() in iommu_change_dev_def_domain()
iommu: Replace __iommu_group_dma_first_attach() with set_domain
iommu: Remove iommu_group_do_dma_first_attach() from
iommu_group_add_device()
iommu: Replace iommu_group_do_dma_first_attach with
__iommu_device_set_domain
iommu: Fix iommu_probe_device() to attach the right domain
iommu: Do iommu_group_create_direct_mappings() before attach
iommu: Remove the assignment of group->domain during default domain
alloc
iommu: Consolidate the code to calculate the target default domain
type
iommu: Revise iommu_group_alloc_default_domain()
iommu: Consolidate the default_domain setup to one function
iommu: Allow IOMMU_RESV_DIRECT to work on ARM
iommu: Remove __iommu_group_for_each_dev()
iommu: Tidy the control flow in iommu_group_store_type()

.clang-format | 1 +
drivers/iommu/iommu.c | 669 +++++++++++++++++++++---------------------
2 files changed, 331 insertions(+), 339 deletions(-)

base-commit: 3578e36f238ef81a1967e849e0a3106c9dd37e68
--
2.40.0