From: Dmitry Osipenko <digetx@gmail.com>
To: Nicolin Chen <nicoleotsuka@gmail.com>,
thierry.reding@gmail.com, joro@8bytes.org
Cc: vdumpa@nvidia.com, jonathanh@nvidia.com,
linux-tegra@vger.kernel.org, iommu@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/3] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
Date: Sat, 3 Oct 2020 17:19:06 +0300 [thread overview]
Message-ID: <386737ab-83e3-9424-66d9-afff04092599@gmail.com> (raw)
In-Reply-To: <20201003065947.18671-3-nicoleotsuka@gmail.com>
03.10.2020 09:59, Nicolin Chen пишет:
> The bus_set_iommu() in tegra_smmu_probe() enumerates all clients
> to call in tegra_smmu_probe_device() where each client searches
> its DT node for smmu pointer and swgroup ID, so as to configure
> an fwspec. But this requires a valid smmu pointer even before mc
> and smmu drivers are probed. So in tegra_smmu_probe() we added a
> line of code to fill mc->smmu, marking "a bit of a hack".
>
> This works for most of clients in the DTB, however, doesn't work
> for a client that doesn't exist in DTB, a PCI device for example.
>
> Actually, if we return ERR_PTR(-ENODEV) in ->probe_device() when
> it's called from bus_set_iommu(), iommu core will let everything
> carry on. Then when a client gets probed, of_iommu_configure() in
> iommu core will search DTB for swgroup ID and call ->of_xlate()
> to prepare an fwspec, similar to tegra_smmu_probe_device() and
> tegra_smmu_configure(). Then it'll call tegra_smmu_probe_device()
> again, and this time we shall return smmu->iommu pointer properly.
>
> So we can get rid of tegra_smmu_find() and tegra_smmu_configure()
> along with DT polling code by letting the iommu core handle every
> thing, except a problem that we search iommus property in DTB not
> only for swgroup ID but also for mc node to get mc->smmu pointer
> to call dev_iommu_priv_set() and return the smmu->iommu pointer.
> So we'll need to find another way to get smmu pointer.
>
> Referencing the implementation of sun50i-iommu driver, of_xlate()
> has client's dev pointer, mc node and swgroup ID. This means that
> we can call dev_iommu_priv_set() in of_xlate() instead, so we can
> simply get smmu pointer in ->probe_device().
>
> This patch reworks tegra_smmu_probe_device() by:
> 1) Removing mc->smmu hack in tegra_smmu_probe() so as to return
> ERR_PTR(-ENODEV) in tegra_smmu_probe_device() during stage of
> tegra_smmu_probe/tegra_mc_probe().
> 2) Moving dev_iommu_priv_set() to of_xlate() so we can get smmu
> pointer in tegra_smmu_probe_device() to replace DTB polling.
> 3) Removing tegra_smmu_configure() accordingly since iommu core
> takes care of it.
>
> This also fixes a problem that previously we added all clients to
> iommu groups before iommu core initializes its default domain:
> ubuntu@jetson:~$ dmesg | grep iommu
> platform smmu_benchmark: Adding to iommu group 0
> platform 1003000.pcie: Adding to iommu group 1
> platform 50000000.host1x: Adding to iommu group 2
> platform 57000000.gpu: Adding to iommu group 3
> platform 7000c400.i2c: Adding to iommu group 4
> platform 7000c500.i2c: Adding to iommu group 4
> platform 7000c700.i2c: Adding to iommu group 4
> platform 7000d000.i2c: Adding to iommu group 4
> iommu: Default domain type: Translated
>
> Though it works fine with IOMMU_DOMAIN_UNMANAGED, but will have
> warnings if switching to IOMMU_DOMAIN_DMA:
> iommu: Failed to allocate default IOMMU domain of type 0 for
> group (null) - Falling back to IOMMU_DOMAIN_DMA
> iommu: Failed to allocate default IOMMU domain of type 0 for
> group (null) - Falling back to IOMMU_DOMAIN_DMA
>
> Now, bypassing the first probe_device() call from bus_set_iommu()
> fixes the sequence:
> ubuntu@jetson:~$ dmesg | grep iommu
> iommu: Default domain type: Translated
> tegra-i2c 7000c400.i2c: Adding to iommu group 0
> tegra-i2c 7000c500.i2c: Adding to iommu group 0
> tegra-i2c 7000d000.i2c: Adding to iommu group 0
> tegra-pcie 1003000.pcie: Adding to iommu group 1
> ...
>
> Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED.
>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
Everything looks good to me, apart from the very minor pending question
about the NULL-checking. Thanks!
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
next prev parent reply other threads:[~2020-10-03 14:19 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-03 6:59 [PATCH v5 0/3] iommu/tegra-smmu: Add PCI support Nicolin Chen
2020-10-03 6:59 ` [PATCH v5 1/3] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev Nicolin Chen
2020-10-03 14:14 ` Dmitry Osipenko
2020-10-03 6:59 ` [PATCH v5 2/3] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen
2020-10-03 14:05 ` Dmitry Osipenko
2020-10-04 22:04 ` Nicolin Chen
2020-10-03 14:06 ` Dmitry Osipenko
2020-10-04 21:57 ` Nicolin Chen
2020-10-05 8:41 ` Dmitry Osipenko
2020-10-05 10:56 ` Thierry Reding
2020-10-06 1:14 ` Nicolin Chen
2020-10-03 14:19 ` Dmitry Osipenko [this message]
2020-10-03 6:59 ` [PATCH v5 3/3] iommu/tegra-smmu: Add PCI support Nicolin Chen
2020-10-03 14:16 ` Dmitry Osipenko
2020-10-04 22:08 ` Nicolin Chen
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=386737ab-83e3-9424-66d9-afff04092599@gmail.com \
--to=digetx@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jonathanh@nvidia.com \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=nicoleotsuka@gmail.com \
--cc=thierry.reding@gmail.com \
--cc=vdumpa@nvidia.com \
/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 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).