From: Thierry Reding <thierry.reding@gmail.com>
To: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
jonathanh@nvidia.com, linux-tegra@vger.kernel.org
Subject: Re: [PATCH RESEND 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
Date: Fri, 20 Nov 2020 17:25:51 +0100 [thread overview]
Message-ID: <20201120162551.GF3870099@ulmo> (raw)
In-Reply-To: <20201111222129.15736-5-nicoleotsuka@gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3959 bytes --]
On Wed, Nov 11, 2020 at 02:21:28PM -0800, Nicolin Chen wrote:
> 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 could add clients to
> iommu groups before iommu core initializes its default domain:
> ubuntu@jetson:~$ dmesg | grep iommu
> platform 50000000.host1x: Adding to iommu group 1
> platform 57000000.gpu: Adding to iommu group 2
> iommu: Default domain type: Translated
> platform 54200000.dc: Adding to iommu group 3
> platform 54240000.dc: Adding to iommu group 3
> platform 54340000.vic: Adding to iommu group 4
>
> 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-host1x 50000000.host1x: Adding to iommu group 0
> tegra-dc 54200000.dc: Adding to iommu group 1
> tegra-dc 54240000.dc: Adding to iommu group 1
> tegra-vic 54340000.vic: Adding to iommu group 2
> nouveau 57000000.gpu: Adding to iommu group 3
>
> Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
> drivers/iommu/tegra-smmu.c | 96 ++++++--------------------------------
> 1 file changed, 15 insertions(+), 81 deletions(-)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 156 bytes --]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, linux-tegra@vger.kernel.org,
jonathanh@nvidia.com, vdumpa@nvidia.com
Subject: Re: [PATCH RESEND 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device()
Date: Fri, 20 Nov 2020 17:25:51 +0100 [thread overview]
Message-ID: <20201120162551.GF3870099@ulmo> (raw)
In-Reply-To: <20201111222129.15736-5-nicoleotsuka@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]
On Wed, Nov 11, 2020 at 02:21:28PM -0800, Nicolin Chen wrote:
> 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 could add clients to
> iommu groups before iommu core initializes its default domain:
> ubuntu@jetson:~$ dmesg | grep iommu
> platform 50000000.host1x: Adding to iommu group 1
> platform 57000000.gpu: Adding to iommu group 2
> iommu: Default domain type: Translated
> platform 54200000.dc: Adding to iommu group 3
> platform 54240000.dc: Adding to iommu group 3
> platform 54340000.vic: Adding to iommu group 4
>
> 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-host1x 50000000.host1x: Adding to iommu group 0
> tegra-dc 54200000.dc: Adding to iommu group 1
> tegra-dc 54240000.dc: Adding to iommu group 1
> tegra-vic 54340000.vic: Adding to iommu group 2
> nouveau 57000000.gpu: Adding to iommu group 3
>
> Note that dmesg log above is testing with IOMMU_DOMAIN_UNMANAGED.
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
> Tested-by: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
> drivers/iommu/tegra-smmu.c | 96 ++++++--------------------------------
> 1 file changed, 15 insertions(+), 81 deletions(-)
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-11-20 16:26 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-11 22:21 [PATCH RESEND 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-11 22:21 ` [PATCH RESEND 1/5] iommu/tegra-smmu: Unwrap tegra_smmu_group_get Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-11 22:21 ` [PATCH RESEND 2/5] iommu/tegra-smmu: Expand mutex protection range Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-11 22:21 ` [PATCH RESEND 3/5] iommu/tegra-smmu: Use fwspec in tegra_smmu_(de)attach_dev Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-20 16:23 ` Thierry Reding
2020-11-20 16:23 ` Thierry Reding
2020-11-11 22:21 ` [PATCH RESEND 4/5] iommu/tegra-smmu: Rework tegra_smmu_probe_device() Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-20 16:25 ` Thierry Reding [this message]
2020-11-20 16:25 ` Thierry Reding
2020-11-11 22:21 ` [PATCH RESEND 5/5] iommu/tegra-smmu: Add PCI support Nicolin Chen
2020-11-11 22:21 ` Nicolin Chen
2020-11-20 16:26 ` Thierry Reding
2020-11-20 16:26 ` Thierry Reding
2020-11-24 21:21 ` [PATCH RESEND 0/5] iommu/tegra-smmu: Some pending reviewed changes Nicolin Chen
2020-11-24 21:21 ` Nicolin Chen
2020-11-24 23:05 ` Dmitry Osipenko
2020-11-24 23:05 ` Dmitry Osipenko
2020-11-24 23:12 ` Nicolin Chen
2020-11-24 23:12 ` Nicolin Chen
2020-11-25 9:55 ` Will Deacon
2020-11-25 9:55 ` Will Deacon
2020-11-25 10:08 ` Nicolin Chen
2020-11-25 10: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=20201120162551.GF3870099@ulmo \
--to=thierry.reding@gmail.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jonathanh@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=nicoleotsuka@gmail.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 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.