All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Will Deacon <will@kernel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Robin Murphy <robin.murphy@arm.com>,
	iommu@lists.linux.dev, joro@8bytes.org,
	linux-arm-kernel@lists.infradead.org, nicolinc@nvidia.com,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v2 5/5] iommu/arm-smmu: Convert to domain_alloc_paging()
Date: Tue, 13 Feb 2024 07:54:23 -0400	[thread overview]
Message-ID: <20240213115423.GG4048826@nvidia.com> (raw)
In-Reply-To: <20240213111623.GA28517@willie-the-truck>

On Tue, Feb 13, 2024 at 11:16:23AM +0000, Will Deacon wrote:
> On Tue, Feb 13, 2024 at 12:55:38PM +0200, Dmitry Baryshkov wrote:
> > On Tue, 13 Feb 2024 at 12:20, Robin Murphy <robin.murphy@arm.com> wrote:
> > > On 2024-02-13 7:51 am, Dmitry Baryshkov wrote:
> > > > On Sat, 10 Feb 2024 at 00:23, Jason Gunthorpe <jgg@nvidia.com> wrote:
> > > >> And then we may get a clue from the backtraces it generates. I only
> > > >> saw one iommu group reported in your log so I'd expect one trace?
> > > >
> > > > I added dev_info + mdelays() around the arm_smmu_init_domain_context()
> > > > and I can see that it crashes within that function.
> > >
> > > Yeah, this is totally broken. We can't just call the unmodified
> > > arm_smmu_init_domain_context() at domain allocation because half of what
> > > it's doing belongs to the attach operation. We should not be allocating
> > > context banks, IRQs, etc. for a not-yet-attached domain, and we
> > > certainly shouldn't be touching hardware there outside of RPM.
> > 
> > Should I send a revert?
> 
> If reverting the patch fixes the issue for you, then yes please!

Not the whole thing though, just remove the 'if (dev)' like you
tested, thanks.

If you want I will send it

> Hopefully you can help Jason test a reworked verson for the future, as
> it's evident that Tegra doesn't tickle the power management side of things
> in the same way.

It can stay, as long as it uses the alloc_domain_paging() that is
enough for the core code to move forward.

I included it only because we were able to test it, most of the other
drivers I did not try to move their "finalize".

Jason

WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
To: Will Deacon <will@kernel.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Robin Murphy <robin.murphy@arm.com>,
	iommu@lists.linux.dev, joro@8bytes.org,
	linux-arm-kernel@lists.infradead.org, nicolinc@nvidia.com,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v2 5/5] iommu/arm-smmu: Convert to domain_alloc_paging()
Date: Tue, 13 Feb 2024 07:54:23 -0400	[thread overview]
Message-ID: <20240213115423.GG4048826@nvidia.com> (raw)
In-Reply-To: <20240213111623.GA28517@willie-the-truck>

On Tue, Feb 13, 2024 at 11:16:23AM +0000, Will Deacon wrote:
> On Tue, Feb 13, 2024 at 12:55:38PM +0200, Dmitry Baryshkov wrote:
> > On Tue, 13 Feb 2024 at 12:20, Robin Murphy <robin.murphy@arm.com> wrote:
> > > On 2024-02-13 7:51 am, Dmitry Baryshkov wrote:
> > > > On Sat, 10 Feb 2024 at 00:23, Jason Gunthorpe <jgg@nvidia.com> wrote:
> > > >> And then we may get a clue from the backtraces it generates. I only
> > > >> saw one iommu group reported in your log so I'd expect one trace?
> > > >
> > > > I added dev_info + mdelays() around the arm_smmu_init_domain_context()
> > > > and I can see that it crashes within that function.
> > >
> > > Yeah, this is totally broken. We can't just call the unmodified
> > > arm_smmu_init_domain_context() at domain allocation because half of what
> > > it's doing belongs to the attach operation. We should not be allocating
> > > context banks, IRQs, etc. for a not-yet-attached domain, and we
> > > certainly shouldn't be touching hardware there outside of RPM.
> > 
> > Should I send a revert?
> 
> If reverting the patch fixes the issue for you, then yes please!

Not the whole thing though, just remove the 'if (dev)' like you
tested, thanks.

If you want I will send it

> Hopefully you can help Jason test a reworked verson for the future, as
> it's evident that Tegra doesn't tickle the power management side of things
> in the same way.

It can stay, as long as it uses the alloc_domain_paging() that is
enough for the core code to move forward.

I included it only because we were able to test it, most of the other
drivers I did not try to move their "finalize".

Jason

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2024-02-13 11:54 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-17 18:11 [PATCH v2 0/5] Convert SMMU to domain_alloc_paging() Jason Gunthorpe
2023-10-17 18:11 ` Jason Gunthorpe
2023-10-17 18:11 ` [PATCH v2 1/5] iommu/arm-smmu: Reorganize arm_smmu_domain_add_master() Jason Gunthorpe
2023-10-17 18:11   ` Jason Gunthorpe
2023-10-17 18:11 ` [PATCH v2 2/5] iommu/arm-smmu: Convert to a global static identity domain Jason Gunthorpe
2023-10-17 18:11   ` Jason Gunthorpe
2023-12-12 13:27   ` Will Deacon
2023-12-12 13:27     ` Will Deacon
2023-12-12 14:15     ` Jason Gunthorpe
2023-12-12 14:15       ` Jason Gunthorpe
2023-12-13 13:26       ` Will Deacon
2023-12-13 13:26         ` Will Deacon
2023-12-13 13:32         ` Jason Gunthorpe
2023-12-13 13:32           ` Jason Gunthorpe
2023-10-17 18:11 ` [PATCH v2 3/5] iommu/arm-smmu: Implement IOMMU_DOMAIN_BLOCKED Jason Gunthorpe
2023-10-17 18:11   ` Jason Gunthorpe
2023-10-17 18:11 ` [PATCH v2 4/5] iommu/arm-smmu: Pass arm_smmu_domain to internal functions Jason Gunthorpe
2023-10-17 18:11   ` Jason Gunthorpe
2023-10-17 18:11 ` [PATCH v2 5/5] iommu/arm-smmu: Convert to domain_alloc_paging() Jason Gunthorpe
2023-10-17 18:11   ` Jason Gunthorpe
2023-12-12 13:26   ` Will Deacon
2023-12-12 13:26     ` Will Deacon
2023-12-12 14:03     ` Jason Gunthorpe
2023-12-12 14:03       ` Jason Gunthorpe
2023-12-12 14:10       ` Will Deacon
2023-12-12 14:10         ` Will Deacon
2023-12-13 13:27         ` Will Deacon
2023-12-13 13:27           ` Will Deacon
2023-12-13 13:32           ` Jason Gunthorpe
2023-12-13 13:32             ` Jason Gunthorpe
2024-02-09 20:05   ` Dmitry Baryshkov
2024-02-09 20:05     ` Dmitry Baryshkov
2024-02-09 22:23     ` Jason Gunthorpe
2024-02-09 22:23       ` Jason Gunthorpe
2024-02-12 23:18       ` Dmitry Baryshkov
2024-02-12 23:18         ` Dmitry Baryshkov
2024-02-13  0:19         ` Jason Gunthorpe
2024-02-13  0:19           ` Jason Gunthorpe
2024-02-13  7:51       ` Dmitry Baryshkov
2024-02-13  7:51         ` Dmitry Baryshkov
2024-02-13 10:20         ` Robin Murphy
2024-02-13 10:20           ` Robin Murphy
2024-02-13 10:55           ` Dmitry Baryshkov
2024-02-13 10:55             ` Dmitry Baryshkov
2024-02-13 11:16             ` Will Deacon
2024-02-13 11:16               ` Will Deacon
2024-02-13 11:54               ` Jason Gunthorpe [this message]
2024-02-13 11:54                 ` Jason Gunthorpe
2023-12-13 17:25 ` [PATCH v2 0/5] Convert SMMU " Will Deacon
2023-12-13 17:25   ` Will Deacon

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=20240213115423.GG4048826@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=will@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.