When devices are deassigned/assigned, SMMU global fault is observed
because SMEs are freed in detach function and not allocated again when
the device is assigned back to the guest.

Don't free the SMEs when devices are deassigned, set the s2cr to type
fault. This way the SMMU will generate a fault if a DMA access is done
by a device not assigned to a guest.

Remove the arm_smmu_master_free_smes() as this is not needed anymore,
arm_smmu_write_s2cr will be used to set the s2cr to type fault.

Fixes: 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation")
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
Changes in v2:
 - fix minor comment in commit msg and added fixes tag.
 - make smmu_domain const in function arm_smmu_domain_remove_master
 - remove return in arm_smmu_detach_dev
---
---
 xen/drivers/passthrough/arm/smmu.c | 33 +++++++++++++++---------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index 69511683b4..0a514821b3 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -1598,21 +1598,6 @@ out_err:
 	return ret;
 }
 
-static void arm_smmu_master_free_smes(struct arm_smmu_master_cfg *cfg)
-{
-    struct arm_smmu_device *smmu = cfg->smmu;
-	int i, idx;
-	struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
-
-	spin_lock(&smmu->stream_map_lock);
-	for_each_cfg_sme(cfg, i, idx, fwspec->num_ids) {
-		if (arm_smmu_free_sme(smmu, idx))
-			arm_smmu_write_sme(smmu, idx);
-		cfg->smendx[i] = INVALID_SMENDX;
-	}
-	spin_unlock(&smmu->stream_map_lock);
-}
-
 static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 				      struct arm_smmu_master_cfg *cfg)
 {
@@ -1635,6 +1620,21 @@ static int arm_smmu_domain_add_master(struct arm_smmu_domain *smmu_domain,
 	return 0;
 }
 
+static void arm_smmu_domain_remove_master(
+				const struct arm_smmu_domain *smmu_domain,
+				struct arm_smmu_master_cfg *cfg)
+{
+	uint32_t i, idx;
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_s2cr *s2cr = smmu->s2crs;
+	const struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
+
+	for_each_cfg_sme(cfg, i, idx, fwspec->num_ids) {
+		s2cr[idx] = s2cr_init_val;
+		arm_smmu_write_s2cr(smmu, idx);
+	}
+}
+
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	int ret;
@@ -1684,10 +1684,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 
 static void arm_smmu_detach_dev(struct iommu_domain *domain, struct device *dev)
 {
+	struct arm_smmu_domain *smmu_domain = domain->priv;
 	struct arm_smmu_master_cfg *cfg = find_smmu_master_cfg(dev);
 
 	if (cfg)
-		arm_smmu_master_free_smes(cfg);
+		arm_smmu_domain_remove_master(smmu_domain, cfg);
 
 }
 
-- 
2.25.1

Hi Rahul,

On 11/08/2022 16:42, Rahul Singh wrote:
> When devices are deassigned/assigned, SMMU global fault is observed
> because SMEs are freed in detach function and not allocated again when
> the device is assigned back to the guest.
> 
> Don't free the SMEs when devices are deassigned, set the s2cr to type
> fault. This way the SMMU will generate a fault if a DMA access is done
> by a device not assigned to a guest.
> 
> Remove the arm_smmu_master_free_smes() as this is not needed anymore,
> arm_smmu_write_s2cr will be used to set the s2cr to type fault.

NIT: I would write arm_smmu_write_s2cr() so it is consistent with the 
line above.

> 
> Fixes: 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation")
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall

> On 12 Aug 2022, at 10:17, Julien Grall <julien@xen.org> wrote:
> 
> Hi Rahul,
> 
> On 11/08/2022 16:42, Rahul Singh wrote:
>> When devices are deassigned/assigned, SMMU global fault is observed
>> because SMEs are freed in detach function and not allocated again when
>> the device is assigned back to the guest.
>> Don't free the SMEs when devices are deassigned, set the s2cr to type
>> fault. This way the SMMU will generate a fault if a DMA access is done
>> by a device not assigned to a guest.
>> Remove the arm_smmu_master_free_smes() as this is not needed anymore,
>> arm_smmu_write_s2cr will be used to set the s2cr to type fault.
> 
> NIT: I would write arm_smmu_write_s2cr() so it is consistent with the line above.
> 
>> Fixes: 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation")
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

@Julien: could you fix the NIT on commit ?

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall

Hi Bertrand,

On 23/08/2022 11:34, Bertrand Marquis wrote:
> 
> 
>> On 12 Aug 2022, at 10:17, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Rahul,
>>
>> On 11/08/2022 16:42, Rahul Singh wrote:
>>> When devices are deassigned/assigned, SMMU global fault is observed
>>> because SMEs are freed in detach function and not allocated again when
>>> the device is assigned back to the guest.
>>> Don't free the SMEs when devices are deassigned, set the s2cr to type
>>> fault. This way the SMMU will generate a fault if a DMA access is done
>>> by a device not assigned to a guest.
>>> Remove the arm_smmu_master_free_smes() as this is not needed anymore,
>>> arm_smmu_write_s2cr will be used to set the s2cr to type fault.
>>
>> NIT: I would write arm_smmu_write_s2cr() so it is consistent with the line above.
>>
>>> Fixes: 0435784cc75d ("xen/arm: smmuv1: Intelligent SMR allocation")
>>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> @Julien: could you fix the NIT on commit ?

Yes. The patch is now committed.

Cheers,

-- 
Julien Grall