Hi Mykyta,
> On 16 Jun 2022, at 8:48 am, Mykyta Poturai <[email protected]> wrote:
>
> Hi Julien, Rahul
> I've encountered a similar problem with IMX8 GPU recently. It wasn't probing
> properly after the domain reboot. After some digging, I came to the same
> solution as Rahul and found this thread. I also encountered the occasional
> "Unexpected global fault, this could be serious" error message when destroying
> a domain with an actively-working GPU.
>
>> Hmmmm.... Looking at the code, arm_smmu_alloc_smes() doesn't seem to use
>> the domain information. So why would it need to be done every time it is
>> assigned?
> Indeed after removing the arm_smmu_master_free_smes() call, both reboot and
> global
> fault issues are gone. If I understand correctly, device removing is not yet
> supported, so I can't find a proper place for the arm_smmu_master_free_smes()
> call.
> Should we remove the function completely or just left it commented for later
> or
> something else?
>
> Rahul, are you still working on this or could I send my patch?
Yes, I have this on my to-do list but I was busy with other work and it got
delayed.
I created another solution for this issue, in which we don’t need to call
arm_smmu_master_free_smes()
in arm_smmu_detach_dev() but we can configure the s2cr value to type fault in
detach function.
Will call new function arm_smmu_domain_remove_master() in detach function that
will revert the changes done
by arm_smmu_domain_add_master() in attach function.
I don’t have any board to test the patch. If it is okay, Could you please test
the patch and let me know the result.
diff --git a/xen/drivers/passthrough/arm/smmu.c
b/xen/drivers/passthrough/arm/smmu.c
index 69511683b4..da3adf8e7f 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,20 @@ static int arm_smmu_domain_add_master(struct
arm_smmu_domain *smmu_domain,
return 0;
}
+static void arm_smmu_domain_remove_master(struct arm_smmu_domain *smmu_domain,
+ struct arm_smmu_master_cfg *cfg)
+{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_s2cr *s2cr = smmu->s2crs;
+ struct iommu_fwspec *fwspec = arm_smmu_get_fwspec(cfg);
+ int i, idx;
+
+ 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 +1683,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);
+ return arm_smmu_domain_remove_master(smmu_domain, cfg);
}
Regards,
Rahul
