[PATCH 14/27] iommu/arm-smmu-v3: Make changing domains be hitless for ATS

[Michael Shavit]

On Thu, Oct 12, 2023 at 7:26 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
[...]
> +struct attach_state {
> +       bool existing_master_domain : 1;
> +       bool want_ats : 1;
> +};
> +
> +/*
> + * Prepare to attach a domain to a master. This always goes in the direction of
> + * enabling the ATS.
> + */
> +static int arm_smmu_attach_prepare(struct arm_smmu_master *master,
> +                                  struct arm_smmu_domain *smmu_domain,
> +                                  struct attach_state *state)
> +{
> +       struct arm_smmu_master_domain *cur_master_domain;
> +       struct arm_smmu_master_domain *master_domain;
> +       unsigned long flags;
> +
> +       /*
> +        * arm_smmu_share_asid() must not see two domains pointing to the same
> +        * arm_smmu_master_domain contents otherwise it could randomly write one
> +        * or the other to the CD.
> +        */
> +       lockdep_assert_held(&arm_smmu_asid_lock);
> +
> +       master_domain = kzalloc(sizeof(*master_domain), GFP_KERNEL);
> +       if (!master_domain)
> +               return -ENOMEM;
> +       master_domain->master = master;
> +
> +       state->want_ats = arm_smmu_ats_supported(master);
> +
> +       /*
> +        * During prepare we want the current smmu_domain and new smmu_domain to
> +        * be in the devices list before we change any HW. This ensures that
> +        * both domains will send ATS invalidations to the master until we are
> +        * done.
> +        *
> +        * It is tempting to make this list only track masters that are using
> +        * ATS, but arm_smmu_share_asid() also uses this to change the ASID of a
> +        * domain, unrelated to ATS.
> +        */
> +       spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> +       cur_master_domain = arm_smmu_find_master_domain(smmu_domain, master);
> +       if (cur_master_domain) {
> +               kfree(master_domain);
> +               state->existing_master_domain = true;

I'm confused, why would we expect to find that the domain is already
attached to the master??? If the iommu core really allows such
attach() calls, can we check for that earlier and simply return early
from attach_dev()?