diff mbox series

[25/27] iommu/arm-smmu-v3: Allow IDENTITY/BLOCKED to be set while PASID is used

Message ID 25-v1-afbb86647bbd+5-smmuv3_newapi_p2_jgg@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Update SMMUv3 to the modern iommu API (part 2/2) | expand

Commit Message

Jason Gunthorpe Oct. 11, 2023, 11:26 p.m. UTC
The HW supports this, use the S1DSS bits to configure the behavior
of SSID=0 which is the RID's translation.

If SSID's are currently being used in the CD table then just update the
S1DSS bits in the STE, remove the master_domain and leave ATS alone.

For iommufd the SMMUv3 HW has a small problem that once a CD table is
installed there is no way to abort transactions (either for untagged with
PASID or for PASID tagged) in a way that doesn't generate an event. This
means VFIO userspace and VM can flood the driver with advisory events.

For BLOCKED the F_STREAM_DISABLED (STRTAB_STE_1_S1DSS_TERMINATE) event is
generated on untagged traffic and a substream CD table entry with
V=0 (removed pasid) will generate C_BAD_CD.

As we don't yet support PASID in iommufd this is a problem to resolve
later.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 62 +++++++++++++--------
 1 file changed, 38 insertions(+), 24 deletions(-)

Comments

Jason Gunthorpe Oct. 25, 2023, 3:10 p.m. UTC | #1
On Wed, Oct 11, 2023 at 08:26:01PM -0300, Jason Gunthorpe wrote:
> -static int arm_smmu_attach_dev_ste(struct device *dev,
> -				   struct arm_smmu_ste *ste)
> +static void arm_smmu_attach_dev_ste(struct device *dev,
> +				    struct arm_smmu_ste *ste,
> +				    unsigned int s1dss)
>  {
>  	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> -
> -	if (arm_smmu_ssids_in_use(&master->cd_table))
> -		return -EBUSY;
> +	struct arm_smmu_domain *old_domain =
> +		to_smmu_domain_safe(iommu_get_domain_for_dev(master->dev));
>  
>  	/*
>  	 * Do not allow any ASID to be changed while are working on the STE,
> @@ -2755,6 +2760,19 @@ static int arm_smmu_attach_dev_ste(struct device *dev,
>  	 */
>  	mutex_lock(&master->smmu->asid_lock);
>  
> +	/*
> +	 * If the CD table is still in use then we need to keep it installed and
> +	 * use the S1DSS to change the mode.
> +	 */
> +	if (arm_smmu_ssids_in_use(&master->cd_table)) {
> +		arm_smmu_make_cdtable_ste(ste, master, &master->cd_table,
> +					  master->ats_enabled, s1dss);
> +		arm_smmu_remove_master_domain(master, old_domain,
> +					      IOMMU_NO_PASID);
> +	} else {
> +		arm_smmu_attach_remove(master, old_domain, IOMMU_NO_PASID);
> +	}
> +
>  	/*
>  	 * The SMMU does not support enabling ATS with bypass/abort. When the
>  	 * STE is in bypass (STE.Config[2:0] == 0b100), ATS Translation Requests
> @@ -2762,11 +2780,6 @@ static int arm_smmu_attach_dev_ste(struct device *dev,
>  	 * the stream (STE.EATS == 0b00), causing F_BAD_ATS_TREQ and
>  	 * F_TRANSL_FORBIDDEN events (IHI0070Ea 5.2 Stream Table Entry).
>  	 */
> -	arm_smmu_attach_remove(
> -		master,
> -		to_smmu_domain_safe(iommu_get_domain_for_dev(master->dev)),
> -		IOMMU_NO_PASID);
> -
>  	arm_smmu_install_ste_for_dev(master, ste);
>  	mutex_unlock(&master->smmu->asid_lock);

This is the last bit that had the ATC invalidation sequenced wrong. I
changed this entirely to look like this, with a clear order for the
ATC invalidation after the STE is changed. This probably eliminates
the need for the wmb() as the STE change and ATC invalidate will be
sequenced by the SMMU logic. Even if the device issues another ATS
after the invalidation is sent it will be rejected by the SMMU.

	/*
	 * If the CD table is not in use we can use the provided STE, otherwise
	 * we use a cdtable STE with the provided S1DSS.
	 */
	if (!arm_smmu_ssids_in_use(&master->cd_table)) {
		/*
		 * The SMMU does not support enabling ATS with bypass/abort.
		 * When the STE is in bypass (STE.Config[2:0] == 0b100), ATS
		 * Translation Requests and Translated transactions are denied
		 * as though ATS is disabled for the stream (STE.EATS == 0b00),
		 * causing F_BAD_ATS_TREQ and F_TRANSL_FORBIDDEN events
		 * (IHI0070Ea 5.2 Stream Table Entry).
		 */
		if (master->ats_enabled) {
			pci_disable_ats(to_pci_dev(master->dev));
			/*
			 * Ensure ATS is disabled at the endpoint before we
			 * issue the ATC invalidation via the SMMU.
			 */
			wmb();
		}
	} else {
		/*
		 * It also does not support ATS with S1DSS = bypass but we have
		 * no idea what the other PASIDs are doing so it has to be left
		 * on.
		 */
		arm_smmu_make_cdtable_ste(ste, master, &master->cd_table,
					  master->ats_enabled, s1dss);
	}

	arm_smmu_install_ste_for_dev(master, ste);

	if (old_domain) {
		if (master->ats_enabled)
			arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
		arm_smmu_remove_master_domain(master, old_domain,
					      IOMMU_NO_PASID);
	}

	if (!arm_smmu_ssids_in_use(&master->cd_table
diff mbox series

Patch

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 e784bacc58e098..83d288fef51249 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1460,7 +1460,7 @@  static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target)
 static void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 				      struct arm_smmu_master *master,
 				      struct arm_smmu_ctx_desc_cfg *cd_table,
-				      bool ats_enabled)
+				      bool ats_enabled, unsigned int s1dss)
 {
 	struct arm_smmu_device *smmu = master->smmu;
 
@@ -1473,7 +1473,7 @@  static void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 		FIELD_PREP(STRTAB_STE_0_S1CDMAX, cd_table->s1cdmax));
 
 	target->data[1] = cpu_to_le64(
-		FIELD_PREP(STRTAB_STE_1_S1DSS, STRTAB_STE_1_S1DSS_SSID0) |
+		FIELD_PREP(STRTAB_STE_1_S1DSS, s1dss) |
 		FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
 		FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
 		FIELD_PREP(STRTAB_STE_1_S1CSH, ARM_SMMU_SH_ISH) |
@@ -1486,7 +1486,11 @@  static void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
 		FIELD_PREP(STRTAB_STE_1_STRW,
 			   (smmu->features & ARM_SMMU_FEAT_E2H) ?
 				   STRTAB_STE_1_STRW_EL2 :
-				   STRTAB_STE_1_STRW_NSEL1));
+				   STRTAB_STE_1_STRW_NSEL1) |
+		FIELD_PREP(STRTAB_STE_1_SHCFG,
+			   s1dss == STRTAB_STE_1_S1DSS_BYPASS ?
+				   STRTAB_STE_1_SHCFG_INCOMING :
+				   0));
 }
 
 static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
@@ -2477,6 +2481,10 @@  static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
 	struct arm_smmu_master_domain *master_domain;
 	unsigned long flags;
 
+	/* NULL means the old domain is IDENTITY/BLOCKED which we don't track */
+	if (!smmu_domain)
+		return;
+
 	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
 	master_domain = arm_smmu_find_master_domain(smmu_domain, master, ssid);
 	if (master_domain) {
@@ -2589,10 +2597,7 @@  static void arm_smmu_attach_remove(struct arm_smmu_master *master,
 				   struct arm_smmu_domain *smmu_domain,
 				   ioasid_t ssid)
 {
-	if (!smmu_domain)
-		return;
-
-	if (ssid == IOMMU_NO_PASID && master->ats_enabled) {
+	if (master->ats_enabled) {
 		pci_disable_ats(to_pci_dev(master->dev));
 		/*
 		 * Ensure ATS is disabled at the endpoint before we issue the
@@ -2604,8 +2609,7 @@  static void arm_smmu_attach_remove(struct arm_smmu_master *master,
 
 	arm_smmu_remove_master_domain(master, smmu_domain, ssid);
 
-	if (ssid == IOMMU_NO_PASID)
-		master->ats_enabled = false;
+	master->ats_enabled = false;
 }
 
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
@@ -2666,7 +2670,8 @@  static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 		arm_smmu_write_cd_entry(master, IOMMU_NO_PASID, cdptr,
 					&target_cd);
 		arm_smmu_make_cdtable_ste(&target, master, &master->cd_table,
-					  state.want_ats);
+					  state.want_ats,
+					  STRTAB_STE_1_S1DSS_SSID0);
 		arm_smmu_install_ste_for_dev(master, &target);
 		break;
 	}
@@ -2736,18 +2741,18 @@  static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 	smmu_domain = to_smmu_domain(domain);
 
 	mutex_lock(&master->smmu->asid_lock);
-	arm_smmu_attach_remove(master, smmu_domain, pasid);
+	arm_smmu_remove_master_domain(master, smmu_domain, pasid);
 	arm_smmu_clear_cd(master, pasid);
 	mutex_unlock(&master->smmu->asid_lock);
 }
 
-static int arm_smmu_attach_dev_ste(struct device *dev,
-				   struct arm_smmu_ste *ste)
+static void arm_smmu_attach_dev_ste(struct device *dev,
+				    struct arm_smmu_ste *ste,
+				    unsigned int s1dss)
 {
 	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
-	if (arm_smmu_ssids_in_use(&master->cd_table))
-		return -EBUSY;
+	struct arm_smmu_domain *old_domain =
+		to_smmu_domain_safe(iommu_get_domain_for_dev(master->dev));
 
 	/*
 	 * Do not allow any ASID to be changed while are working on the STE,
@@ -2755,6 +2760,19 @@  static int arm_smmu_attach_dev_ste(struct device *dev,
 	 */
 	mutex_lock(&master->smmu->asid_lock);
 
+	/*
+	 * If the CD table is still in use then we need to keep it installed and
+	 * use the S1DSS to change the mode.
+	 */
+	if (arm_smmu_ssids_in_use(&master->cd_table)) {
+		arm_smmu_make_cdtable_ste(ste, master, &master->cd_table,
+					  master->ats_enabled, s1dss);
+		arm_smmu_remove_master_domain(master, old_domain,
+					      IOMMU_NO_PASID);
+	} else {
+		arm_smmu_attach_remove(master, old_domain, IOMMU_NO_PASID);
+	}
+
 	/*
 	 * The SMMU does not support enabling ATS with bypass/abort. When the
 	 * STE is in bypass (STE.Config[2:0] == 0b100), ATS Translation Requests
@@ -2762,11 +2780,6 @@  static int arm_smmu_attach_dev_ste(struct device *dev,
 	 * the stream (STE.EATS == 0b00), causing F_BAD_ATS_TREQ and
 	 * F_TRANSL_FORBIDDEN events (IHI0070Ea 5.2 Stream Table Entry).
 	 */
-	arm_smmu_attach_remove(
-		master,
-		to_smmu_domain_safe(iommu_get_domain_for_dev(master->dev)),
-		IOMMU_NO_PASID);
-
 	arm_smmu_install_ste_for_dev(master, ste);
 	mutex_unlock(&master->smmu->asid_lock);
 
@@ -2776,7 +2789,6 @@  static int arm_smmu_attach_dev_ste(struct device *dev,
 	 * descriptor from arm_smmu_share_asid().
 	 */
 	arm_smmu_clear_cd(master, IOMMU_NO_PASID);
-	return 0;
 }
 
 static int arm_smmu_attach_dev_identity(struct iommu_domain *domain,
@@ -2785,7 +2797,8 @@  static int arm_smmu_attach_dev_identity(struct iommu_domain *domain,
 	struct arm_smmu_ste ste;
 
 	arm_smmu_make_bypass_ste(&ste);
-	return arm_smmu_attach_dev_ste(dev, &ste);
+	arm_smmu_attach_dev_ste(dev, &ste, STRTAB_STE_1_S1DSS_BYPASS);
+	return 0;
 }
 
 static const struct iommu_domain_ops arm_smmu_identity_ops = {
@@ -2803,7 +2816,8 @@  static int arm_smmu_attach_dev_blocked(struct iommu_domain *domain,
 	struct arm_smmu_ste ste;
 
 	arm_smmu_make_abort_ste(&ste);
-	return arm_smmu_attach_dev_ste(dev, &ste);
+	arm_smmu_attach_dev_ste(dev, &ste, STRTAB_STE_1_S1DSS_TERMINATE);
+	return 0;
 }
 
 static const struct iommu_domain_ops arm_smmu_blocked_ops = {