linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,"
	<iommu@lists.linux-foundation.org>,
	Rob Clark <robdclark@chromium.org>,
	Douglas Anderson <dianders@chromium.org>,
	Akhil P Oommen <akhilpo@codeaurora.org>,
	Jonathan Marek <jonathan@marek.ca>, Eric Anholt <eric@anholt.net>,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Sharat Masetty <smasetty@codeaurora.org>,
	Konrad Dybcio <konrad.dybcio@somainline.org>,
	Sean Paul <sean@poorly.run>,
	Jordan Crouse <jcrouse@codeaurora.org>,
	"Kristian H. Kristensen" <hoegsberg@google.com>,
	Daniel Vetter <daniel@ffwll.ch>,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@somainline.org>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	freedreno <freedreno@lists.freedesktop.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 3/5] drm/msm: Improve the a6xx page fault handler
Date: Fri, 25 Jun 2021 08:42:59 -0700	[thread overview]
Message-ID: <CAF6AEGvTjTUQXqom-xhdh456tdLscbVFPQ+iud1H1gHc8A2=hA@mail.gmail.com> (raw)
In-Reply-To: <YNVP6rfQ699BejsI@yoga>

On Thu, Jun 24, 2021 at 8:39 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 10 Jun 16:44 CDT 2021, Rob Clark wrote:
> [..]
> > diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> > index 50d881794758..6975b95c3c29 100644
> > --- a/drivers/gpu/drm/msm/msm_iommu.c
> > +++ b/drivers/gpu/drm/msm/msm_iommu.c
> > @@ -211,8 +211,17 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
> >               unsigned long iova, int flags, void *arg)
> >  {
> >       struct msm_iommu *iommu = arg;
> > +     struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(iommu->base.dev);
> > +     struct adreno_smmu_fault_info info, *ptr = NULL;
> > +
> > +     if (adreno_smmu->get_fault_info) {
>
> This seemed reasonable when I read it last time, but I didn't realize
> that the msm_fault_handler() is installed for all msm_iommu instances.
>
> So while we're trying to recover from the boot splash and setup the new
> framebuffer we end up here with iommu->base.dev being the mdss device.
> Naturally drvdata of mdss is not a struct adreno_smmu_priv.
>
> > +             adreno_smmu->get_fault_info(adreno_smmu->cookie, &info);
>
> So here we just jump straight out into hyperspace, never to return.
>
> Not sure how to wire this up to avoid the problem, but right now I don't
> think we can boot any device with a boot splash.
>

I think we could do:

------------------------
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index eed2a762e9dd..30ee8866154e 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -29,6 +29,9 @@ static struct msm_iommu_pagetable
*to_pagetable(struct msm_mmu *mmu)
  return container_of(mmu, struct msm_iommu_pagetable, base);
 }

+static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
+ unsigned long iova, int flags, void *arg);
+
 static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova,
  size_t size)
 {
@@ -151,6 +154,8 @@ struct msm_mmu *msm_iommu_pagetable_create(struct
msm_mmu *parent)
  struct io_pgtable_cfg ttbr0_cfg;
  int ret;

+ iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu);
+
  /* Get the pagetable configuration from the domain */
  if (adreno_smmu->cookie)
  ttbr1_cfg = adreno_smmu->get_ttbr1_cfg(adreno_smmu->cookie);
@@ -300,7 +305,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);

------------------------

That would have the result of setting the same fault handler multiple
times, but that looks harmless.  Mostly the fault handling stuff is to
make it easier to debug userspace issues, the fallback dmesg spam from
arm-smmu should be sufficient for any kernel side issues.

BR,
-R

  reply	other threads:[~2021-06-25 15:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 21:44 [PATCH v5 0/5] iommu/arm-smmu: adreno-smmu page fault handling Rob Clark
2021-06-10 21:44 ` [PATCH v5 1/5] iommu/arm-smmu: Add support for driver IOMMU fault handlers Rob Clark
2021-06-14 17:26   ` Bjorn Andersson
2021-06-10 21:44 ` [PATCH v5 2/5] iommu/arm-smmu-qcom: Add an adreno-smmu-priv callback to get pagefault info Rob Clark
2021-06-14 17:30   ` Bjorn Andersson
2021-06-10 21:44 ` [PATCH v5 3/5] drm/msm: Improve the a6xx page fault handler Rob Clark
2021-06-14 17:46   ` Bjorn Andersson
2021-06-25  3:39   ` Bjorn Andersson
2021-06-25 15:42     ` Rob Clark [this message]
2021-06-10 21:44 ` [PATCH v5 4/5] iommu/arm-smmu-qcom: Add stall support Rob Clark
2021-06-11 13:49   ` Jordan Crouse
2021-06-14 17:54   ` Bjorn Andersson
2021-06-10 21:44 ` [PATCH v5 5/5] drm/msm: devcoredump iommu fault support Rob Clark
2021-06-11 13:49   ` Jordan Crouse
2021-07-04 12:53 ` [PATCH v5 0/5] iommu/arm-smmu: adreno-smmu page fault handling Dmitry Baryshkov
2021-07-04 18:20   ` Rob Clark
2021-07-06 21:36     ` Bjorn Andersson
2021-07-07  5:12     ` John Stultz
2021-07-07 17:38       ` Rob Clark
2021-07-06 14:27 [PATCH v5 3/5] drm/msm: Improve the a6xx page fault handler Yassine Oudjana

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAF6AEGvTjTUQXqom-xhdh456tdLscbVFPQ+iud1H1gHc8A2=hA@mail.gmail.com' \
    --to=robdclark@gmail.com \
    --cc=airlied@linux.ie \
    --cc=akhilpo@codeaurora.org \
    --cc=angelogioacchino.delregno@somainline.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=freedreno@lists.freedesktop.org \
    --cc=hoegsberg@google.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jcrouse@codeaurora.org \
    --cc=jonathan@marek.ca \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=robdclark@chromium.org \
    --cc=sean@poorly.run \
    --cc=smasetty@codeaurora.org \
    --subject='Re: [PATCH v5 3/5] drm/msm: Improve the a6xx page fault handler' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).