iommu/arm-smmu-v3: Do not change the STE twice during arm_smmu_attach_dev()

This was needed because the STE code required the STE to be in
ABORT/BYPASS inorder to program a cdtable or S2 STE. Now that the STE code
can automatically handle all transitions we can remove this step
from the attach_dev flow.

A few small bugs exist because of this:

1) If the core code does BLOCKED -> UNMANAGED with disable_bypass=false
   then there will be a moment where the STE points at BYPASS. Since
   this can be done by VFIO/IOMMUFD it is a small security race.

2) If the core code does IDENTITY -> DMA then any IOMMU_RESV_DIRECT
   regions will temporarily become BLOCKED. We'd like drivers to
   work in a way that allows IOMMU_RESV_DIRECT to be continuously
   functional during these transitions.

Make arm_smmu_release_device() put the STE back to the correct
ABORT/BYPASS setting. Fix a bug where a IOMMU_RESV_DIRECT was ignored on
this path.

As noted before the reordering of the linked list/STE/CD changes is OK
against concurrent arm_smmu_share_asid() because of the
arm_smmu_asid_lock.

Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Moritz Fischer <moritzf@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/7-v6-96275f25c39d+2d4-smmuv3_newapi_p1_jgg@nvidia.com
Signed-off-by: Will Deacon <will@kernel.org>

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 6cdf075e9a7ee7e0bf8d01c698d4a21d80b5eae4..597a8c5f96589900433eb75e08af748e84d6635b 100644 (file)
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2509,7 +2509,6 @@ static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
 static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 {
        unsigned long flags;
-       struct arm_smmu_ste target;
        struct arm_smmu_domain *smmu_domain = master->domain;
 
        if (!smmu_domain)
@@ -2523,11 +2522,6 @@ static void arm_smmu_detach_dev(struct arm_smmu_master *master)
 
        master->domain = NULL;
        master->ats_enabled = false;
-       if (disable_bypass)
-               arm_smmu_make_abort_ste(&target);
-       else
-               arm_smmu_make_bypass_ste(&target);
-       arm_smmu_install_ste_for_dev(master, &target);
        /*
         * Clearing the CD entry isn't strictly required to detach the domain
         * since the table is uninstalled anyway, but it helps avoid confusion
@@ -2875,9 +2869,18 @@ err_free_master:
 static void arm_smmu_release_device(struct device *dev)
 {
        struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+       struct arm_smmu_ste target;
 
        if (WARN_ON(arm_smmu_master_sva_enabled(master)))
                iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
+
+       /* Put the STE back to what arm_smmu_init_strtab() sets */
+       if (disable_bypass && !dev->iommu->require_direct)
+               arm_smmu_make_abort_ste(&target);
+       else
+               arm_smmu_make_bypass_ste(&target);
+       arm_smmu_install_ste_for_dev(master, &target);
+
        arm_smmu_detach_dev(master);
        arm_smmu_disable_pasid(master);
        arm_smmu_remove_master(master);