| Message ID | 20210707180113.840741-1-robdclark@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: Fix display fault handling | expand |
On Wed, Jul 7 2021 at 21:57:05 +0400, Rob Clark <robdclark@gmail.com> wrote: > From: Rob Clark <robdclark@chromium.org> > > It turns out that when the display is enabled by the bootloader, we > can > get some transient iommu faults from the display. Which doesn't go > over > too well when we install a fault handler that is gpu specific. To > avoid > this, defer installing the fault handler until we get around to > setting > up per-process pgtables (which is adreno_smmu specific). The arm-smmu > fallback error reporting is sufficient for reporting display related > faults (and in fact was all we had prior to > f8f934c180f629bb927a04fd90d) > > Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reported-by: Yassine Oudjana <y.oudjana@protonmail.com> > Fixes: 2a574cc05d38 ("drm/msm: Improve the a6xx page fault handler") > Signed-off-by: Rob Clark <robdclark@chromium.org> > Tested-by: John Stultz <john.stultz@linaro.org> > --- > drivers/gpu/drm/msm/msm_iommu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_iommu.c > b/drivers/gpu/drm/msm/msm_iommu.c > index eed2a762e9dd..bcaddbba564d 100644 > --- a/drivers/gpu/drm/msm/msm_iommu.c > +++ b/drivers/gpu/drm/msm/msm_iommu.c > @@ -142,6 +142,9 @@ static const struct iommu_flush_ops null_tlb_ops > = { > .tlb_add_page = msm_iommu_tlb_add_page, > }; > > +static int msm_fault_handler(struct iommu_domain *domain, struct > device *dev, > + unsigned long iova, int flags, void *arg); > + > struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) > { > struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(parent->dev); > @@ -157,6 +160,13 @@ struct msm_mmu > *msm_iommu_pagetable_create(struct msm_mmu *parent) > if (!ttbr1_cfg) > return ERR_PTR(-ENODEV); > > + /* > + * Defer setting the fault handler until we have a valid adreno_smmu > + * to avoid accidentially installing a GPU specific fault handler > for > + * the display's iommu > + */ > + iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu); > + > pagetable = kzalloc(sizeof(*pagetable), GFP_KERNEL); > if (!pagetable) > return ERR_PTR(-ENOMEM); > @@ -300,7 +310,6 @@ struct msm_mmu *msm_iommu_new(struct device *dev, > struct iommu_domain *domain) > > iommu->domain = domain; > msm_mmu_init(&iommu->base, dev, &funcs, MSM_MMU_IOMMU); > - iommu_set_fault_handler(domain, msm_fault_handler, iommu); > > atomic_set(&iommu->pagetables, 0); > > -- > 2.31.1 > Tested-by: Yassine Oudjana <y.oudjana@protonmail.com>
On Wed, Jul 7, 2021 at 10:57 AM Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > It turns out that when the display is enabled by the bootloader, we can > get some transient iommu faults from the display. Which doesn't go over > too well when we install a fault handler that is gpu specific. To avoid > this, defer installing the fault handler until we get around to setting > up per-process pgtables (which is adreno_smmu specific). The arm-smmu > fallback error reporting is sufficient for reporting display related > faults (and in fact was all we had prior to f8f934c180f629bb927a04fd90d) > > Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reported-by: Yassine Oudjana <y.oudjana@protonmail.com> > Fixes: 2a574cc05d38 ("drm/msm: Improve the a6xx page fault handler") > Signed-off-by: Rob Clark <robdclark@chromium.org> > Tested-by: John Stultz <john.stultz@linaro.org> > --- Hey folks! Just wanted to follow up on this, as it's still missing from 5.14-rc3 and is critical for resolving a boot regression on db845c. Is there anything keeping this from heading upstream? thanks -john
On Wed 07 Jul 11:01 PDT 2021, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > It turns out that when the display is enabled by the bootloader, we can > get some transient iommu faults from the display. Which doesn't go over > too well when we install a fault handler that is gpu specific. To avoid > this, defer installing the fault handler until we get around to setting > up per-process pgtables (which is adreno_smmu specific). The arm-smmu > fallback error reporting is sufficient for reporting display related > faults (and in fact was all we had prior to f8f934c180f629bb927a04fd90d) > > Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Reported-by: Yassine Oudjana <y.oudjana@protonmail.com> > Fixes: 2a574cc05d38 ("drm/msm: Improve the a6xx page fault handler") > Signed-off-by: Rob Clark <robdclark@chromium.org> > Tested-by: John Stultz <john.stultz@linaro.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > drivers/gpu/drm/msm/msm_iommu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c > index eed2a762e9dd..bcaddbba564d 100644 > --- a/drivers/gpu/drm/msm/msm_iommu.c > +++ b/drivers/gpu/drm/msm/msm_iommu.c > @@ -142,6 +142,9 @@ static const struct iommu_flush_ops null_tlb_ops = { > .tlb_add_page = msm_iommu_tlb_add_page, > }; > > +static int msm_fault_handler(struct iommu_domain *domain, struct device *dev, > + unsigned long iova, int flags, void *arg); > + > struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) > { > struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(parent->dev); > @@ -157,6 +160,13 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) > if (!ttbr1_cfg) > return ERR_PTR(-ENODEV); > > + /* > + * Defer setting the fault handler until we have a valid adreno_smmu > + * to avoid accidentially installing a GPU specific fault handler for > + * the display's iommu > + */ > + iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu); > + > pagetable = kzalloc(sizeof(*pagetable), GFP_KERNEL); > if (!pagetable) > return ERR_PTR(-ENOMEM); > @@ -300,7 +310,6 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain) > > iommu->domain = domain; > msm_mmu_init(&iommu->base, dev, &funcs, MSM_MMU_IOMMU); > - iommu_set_fault_handler(domain, msm_fault_handler, iommu); > > atomic_set(&iommu->pagetables, 0); > > -- > 2.31.1 >
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index eed2a762e9dd..bcaddbba564d 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -142,6 +142,9 @@ static const struct iommu_flush_ops null_tlb_ops = { .tlb_add_page = msm_iommu_tlb_add_page, }; +static int msm_fault_handler(struct iommu_domain *domain, struct device *dev, + unsigned long iova, int flags, void *arg); + struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) { struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(parent->dev); @@ -157,6 +160,13 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent) if (!ttbr1_cfg) return ERR_PTR(-ENODEV); + /* + * Defer setting the fault handler until we have a valid adreno_smmu + * to avoid accidentially installing a GPU specific fault handler for + * the display's iommu + */ + iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu); + pagetable = kzalloc(sizeof(*pagetable), GFP_KERNEL); if (!pagetable) return ERR_PTR(-ENOMEM); @@ -300,7 +310,6 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain) iommu->domain = domain; msm_mmu_init(&iommu->base, dev, &funcs, MSM_MMU_IOMMU); - iommu_set_fault_handler(domain, msm_fault_handler, iommu); atomic_set(&iommu->pagetables, 0);