queue-6.8/iommu-arm-smmu-v3-hold-arm_smmu_asid_lock-during-all.patch

   1 From 12392232511538aa602f1b3a79e986e6597daf5e Mon Sep 17 00:00:00 2001
   2 From: Sasha Levin <sashal@kernel.org>
   3 Date: Mon, 26 Feb 2024 13:07:16 -0400
   4 Subject: iommu/arm-smmu-v3: Hold arm_smmu_asid_lock during all of attach_dev
   5
   6 From: Jason Gunthorpe <jgg@nvidia.com>
   7
   8 [ Upstream commit 9f7c68911579bc15c57d227d021ccd253da2b635 ]
   9
  10 The BTM support wants to be able to change the ASID of any smmu_domain.
  11 When it goes to do this it holds the arm_smmu_asid_lock and iterates over
  12 the target domain's devices list.
  13
  14 During attach of a S1 domain we must ensure that the devices list and
  15 CD are in sync, otherwise we could miss CD updates or a parallel CD update
  16 could push an out of date CD.
  17
  18 This is pretty complicated, and almost works today because
  19 arm_smmu_detach_dev() removes the master from the linked list before
  20 working on the CD entries, preventing parallel update of the CD.
  21
  22 However, it does have an issue where the CD can remain programed while the
  23 domain appears to be unattached. arm_smmu_share_asid() will then not clear
  24 any CD entriess and install its own CD entry with the same ASID
  25 concurrently. This creates a small race window where the IOMMU can see two
  26 ASIDs pointing to different translations.
  27
  28        CPU0                                   CPU1
  29 arm_smmu_attach_dev()
  30    arm_smmu_detach_dev()
  31      spin_lock_irqsave(&smmu_domain->devices_lock, flags);
  32      list_del(&master->domain_head);
  33      spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
  34
  35                                       arm_smmu_mmu_notifier_get()
  36                                        arm_smmu_alloc_shared_cd()
  37                                         arm_smmu_share_asid():
  38                                           // Does nothing due to list_del above
  39                                           arm_smmu_update_ctx_desc_devices()
  40                                           arm_smmu_tlb_inv_asid()
  41                                        arm_smmu_write_ctx_desc()
  42                                          ** Now the ASID is in two CDs
  43                                             with different translation
  44
  45      arm_smmu_write_ctx_desc(master, IOMMU_NO_PASID, NULL);
  46
  47 Solve this by wrapping most of the attach flow in the
  48 arm_smmu_asid_lock. This locks more than strictly needed to prepare for
  49 the next patch which will reorganize the order of the linked list, STE and
  50 CD changes.
  51
  52 Move arm_smmu_detach_dev() till after we have initialized the domain so
  53 the lock can be held for less time.
  54
  55 Reviewed-by: Michael Shavit <mshavit@google.com>
  56 Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
  57 Reviewed-by: Mostafa Saleh <smostafa@google.com>
  58 Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
  59 Tested-by: Nicolin Chen <nicolinc@nvidia.com>
  60 Tested-by: Moritz Fischer <moritzf@google.com>
  61 Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
  62 Link: https://lore.kernel.org/r/5-v6-96275f25c39d+2d4-smmuv3_newapi_p1_jgg@nvidia.com
  63 Signed-off-by: Will Deacon <will@kernel.org>
  64 Signed-off-by: Sasha Levin <sashal@kernel.org>
  65 ---
  66  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 ++++++++++++---------
  67  1 file changed, 13 insertions(+), 9 deletions(-)
  68
  69 diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
  70 index 0ffb1cf17e0b2..f3f2e47b6d488 100644
  71 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
  72 +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
  73 @@ -2398,8 +2398,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
  74                 return -EBUSY;
  75         }
  76
  77 -       arm_smmu_detach_dev(master);
  78 -
  79         mutex_lock(&smmu_domain->init_mutex);
  80
  81         if (!smmu_domain->smmu) {
  82 @@ -2414,6 +2412,16 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
  83         if (ret)
  84                 return ret;
  85
  86 +       /*
  87 +        * Prevent arm_smmu_share_asid() from trying to change the ASID
  88 +        * of either the old or new domain while we are working on it.
  89 +        * This allows the STE and the smmu_domain->devices list to
  90 +        * be inconsistent during this routine.
  91 +        */
  92 +       mutex_lock(&arm_smmu_asid_lock);
  93 +
  94 +       arm_smmu_detach_dev(master);
  95 +
  96         master->domain = smmu_domain;
  97
  98         /*
  99 @@ -2439,13 +2447,7 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 100                         }
 101                 }
 102
 103 -               /*
 104 -                * Prevent SVA from concurrently modifying the CD or writing to
 105 -                * the CD entry
 106 -                */
 107 -               mutex_lock(&arm_smmu_asid_lock);
 108                 ret = arm_smmu_write_ctx_desc(master, IOMMU_NO_PASID, &smmu_domain->cd);
 109 -               mutex_unlock(&arm_smmu_asid_lock);
 110                 if (ret) {
 111                         master->domain = NULL;
 112                         goto out_list_del;
 113 @@ -2455,13 +2457,15 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 114         arm_smmu_install_ste_for_dev(master);
 115
 116         arm_smmu_enable_ats(master);
 117 -       return 0;
 118 +       goto out_unlock;
 119
 120  out_list_del:
 121         spin_lock_irqsave(&smmu_domain->devices_lock, flags);
 122         list_del(&master->domain_head);
 123         spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 124
 125 +out_unlock:
 126 +       mutex_unlock(&arm_smmu_asid_lock);
 127         return ret;
 128  }
 129
 130 --
 131 2.43.0
 132