All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: devicetree@vger.kernel.org, Will Deacon <will@kernel.org>,
	iommu@lists.linux-foundation.org,
	Nicolin Chen <nicolinc@nvidia.com>,
	linux-tegra@vger.kernel.org, Dmitry Osipenko <digetx@gmail.com>,
	Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
Date: Thu, 20 May 2021 17:03:06 -0500	[thread overview]
Message-ID: <20210520220306.GA1976116@robh.at.kernel.org> (raw)
In-Reply-To: <20210423163234.3651547-2-thierry.reding@gmail.com>

On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Reserved memory region phandle references can be accompanied by a
> specifier that provides additional information about how that specific
> reference should be treated.
> 
> One use-case is to mark a memory region as needing an identity mapping
> in the system's IOMMU for the device that references the region. This is
> needed for example when the bootloader has set up hardware (such as a
> display controller) to actively access a memory region (e.g. a boot
> splash screen framebuffer) during boot. The operating system can use the
> identity mapping flag from the specifier to make sure an IOMMU identity
> mapping is set up for the framebuffer before IOMMU translations are
> enabled for the display controller.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../reserved-memory/reserved-memory.txt       | 21 +++++++++++++++++++
>  include/dt-bindings/reserved-memory.h         |  8 +++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 include/dt-bindings/reserved-memory.h

Sorry for being slow on this. I have 2 concerns.

First, this creates an ABI issue. A DT with cells in 'memory-region' 
will not be understood by an existing OS. I'm less concerned about this 
if we address that with a stable fix. (Though I'm pretty sure we've 
naively added #?-cells in the past ignoring this issue.)

Second, it could be the bootloader setting up the reserved region. If a 
node already has 'memory-region', then adding more regions is more 
complicated compared to adding new properties. And defining what each 
memory-region entry is or how many in schemas is impossible.

Both could be addressed with a new property. Perhaps something like 
'iommu-memory-region = <&phandle>;'. I think the 'iommu' prefix is 
appropriate given this is entirely because of the IOMMU being in the 
mix. I might feel differently if we had other uses for cells, but I 
don't really see it in this case. 

Rob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Krishna Reddy <vdumpa@nvidia.com>,
	Dmitry Osipenko <digetx@gmail.com>,
	devicetree@vger.kernel.org, iommu@lists.linux-foundation.org,
	linux-tegra@vger.kernel.org
Subject: Re: [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier
Date: Thu, 20 May 2021 17:03:06 -0500	[thread overview]
Message-ID: <20210520220306.GA1976116@robh.at.kernel.org> (raw)
In-Reply-To: <20210423163234.3651547-2-thierry.reding@gmail.com>

On Fri, Apr 23, 2021 at 06:32:30PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Reserved memory region phandle references can be accompanied by a
> specifier that provides additional information about how that specific
> reference should be treated.
> 
> One use-case is to mark a memory region as needing an identity mapping
> in the system's IOMMU for the device that references the region. This is
> needed for example when the bootloader has set up hardware (such as a
> display controller) to actively access a memory region (e.g. a boot
> splash screen framebuffer) during boot. The operating system can use the
> identity mapping flag from the specifier to make sure an IOMMU identity
> mapping is set up for the framebuffer before IOMMU translations are
> enabled for the display controller.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../reserved-memory/reserved-memory.txt       | 21 +++++++++++++++++++
>  include/dt-bindings/reserved-memory.h         |  8 +++++++
>  2 files changed, 29 insertions(+)
>  create mode 100644 include/dt-bindings/reserved-memory.h

Sorry for being slow on this. I have 2 concerns.

First, this creates an ABI issue. A DT with cells in 'memory-region' 
will not be understood by an existing OS. I'm less concerned about this 
if we address that with a stable fix. (Though I'm pretty sure we've 
naively added #?-cells in the past ignoring this issue.)

Second, it could be the bootloader setting up the reserved region. If a 
node already has 'memory-region', then adding more regions is more 
complicated compared to adding new properties. And defining what each 
memory-region entry is or how many in schemas is impossible.

Both could be addressed with a new property. Perhaps something like 
'iommu-memory-region = <&phandle>;'. I think the 'iommu' prefix is 
appropriate given this is entirely because of the IOMMU being in the 
mix. I might feel differently if we had other uses for cells, but I 
don't really see it in this case. 

Rob

  reply	other threads:[~2021-05-20 22:03 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23 16:32 [PATCH v2 0/5] iommu: Support identity mappings of reserved-memory regions Thierry Reding
2021-04-23 16:32 ` Thierry Reding
2021-04-23 16:32 ` [PATCH v2 1/5] dt-bindings: reserved-memory: Document memory region specifier Thierry Reding
2021-04-23 16:32   ` Thierry Reding
2021-05-20 22:03   ` Rob Herring [this message]
2021-05-20 22:03     ` Rob Herring
2021-05-28 16:54     ` Thierry Reding
2021-05-28 16:54       ` Thierry Reding
2021-06-08 16:51       ` Thierry Reding
2021-06-08 16:51         ` Thierry Reding
2021-07-01 18:14         ` Thierry Reding
2021-07-01 18:14           ` Thierry Reding
2021-07-02 14:16           ` Dmitry Osipenko
2021-07-02 14:16             ` Dmitry Osipenko
2021-09-01 14:13             ` Thierry Reding
2021-09-01 14:13               ` Thierry Reding
2021-09-03 13:20               ` Rob Herring
2021-09-03 13:20                 ` Rob Herring
2021-09-03 13:52                 ` Thierry Reding
2021-09-03 13:52                   ` Thierry Reding
2021-09-03 14:36                   ` Rob Herring
2021-09-03 14:36                     ` Rob Herring
2021-09-03 15:35                     ` Thierry Reding
2021-09-03 15:35                       ` Thierry Reding
2021-09-07 15:33                       ` Rob Herring
2021-09-07 15:33                         ` Rob Herring
2021-09-07 17:44                         ` Thierry Reding
2021-09-07 17:44                           ` Thierry Reding
2021-09-15 15:19                           ` Thierry Reding
2021-09-15 15:19                             ` Thierry Reding
2022-02-06 22:27                             ` Janne Grunau
2022-02-06 22:27                               ` Janne Grunau
2022-02-06 22:27                               ` Janne Grunau
2022-02-09 16:31                               ` Thierry Reding
2022-02-09 16:31                                 ` Thierry Reding
2022-02-09 16:31                                 ` Thierry Reding
2022-02-10 23:15                                 ` Janne Grunau
2022-02-10 23:15                                   ` Janne Grunau
2022-02-10 23:15                                   ` Janne Grunau
2022-03-31 16:25                                   ` Thierry Reding
2022-03-31 16:25                                     ` Thierry Reding
2022-03-31 16:25                                     ` Thierry Reding
2022-04-01 17:08                                     ` Janne Grunau
2022-04-01 17:08                                       ` Janne Grunau
2022-04-01 17:08                                       ` Janne Grunau
2021-04-23 16:32 ` [PATCH v2 2/5] iommu: Implement of_iommu_get_resv_regions() Thierry Reding
2021-04-23 16:32   ` Thierry Reding
2021-04-24  1:59   ` kernel test robot
2021-07-02 14:05   ` Dmitry Osipenko
2021-07-02 14:05     ` Dmitry Osipenko
2021-07-16 14:41     ` Rob Herring
2021-07-16 14:41       ` Rob Herring
2021-07-17 11:07       ` Dmitry Osipenko
2021-07-17 11:07         ` Dmitry Osipenko
2021-07-30 12:18         ` Will Deacon
2021-07-30 12:18           ` Will Deacon
2021-04-23 16:32 ` [PATCH v2 3/5] iommu: dma: Use of_iommu_get_resv_regions() Thierry Reding
2021-04-23 16:32   ` Thierry Reding
2021-04-23 16:32 ` [PATCH v2 4/5] iommu/tegra-smmu: Add support for reserved regions Thierry Reding
2021-04-23 16:32   ` Thierry Reding
2021-04-23 16:32 ` [PATCH v2 5/5] iommu/tegra-smmu: Support managed domains Thierry Reding
2021-04-23 16:32   ` Thierry Reding
2021-10-11 23:25   ` Dmitry Osipenko
2021-10-11 23:25     ` Dmitry Osipenko
2021-04-24  7:26 ` [PATCH v2 0/5] iommu: Support identity mappings of reserved-memory regions Dmitry Osipenko
2021-04-24  7:26   ` Dmitry Osipenko
2021-04-27 18:30   ` Krishna Reddy
2021-04-27 18:30     ` Krishna Reddy
2021-04-28  5:44     ` Dmitry Osipenko
2021-04-28  5:44       ` Dmitry Osipenko
2021-04-29  5:51       ` Krishna Reddy
2021-04-29  5:51         ` Krishna Reddy
2021-04-29 12:43         ` Dmitry Osipenko
2021-04-29 12:43           ` Dmitry Osipenko
2021-04-28  5:51 ` Dmitry Osipenko
2021-04-28  5:51   ` Dmitry Osipenko
2021-04-28  5:57   ` Mikko Perttunen
2021-04-28  5:57     ` Mikko Perttunen
2021-04-28  7:55     ` Dmitry Osipenko
2021-04-28  7:55       ` Dmitry Osipenko
2021-04-28  5:59 ` Dmitry Osipenko
2021-04-28  5:59   ` Dmitry Osipenko
2021-10-03  1:09 ` Dmitry Osipenko
2021-10-03  1:09   ` Dmitry Osipenko
2021-10-04 19:23   ` Thierry Reding
2021-10-04 19:23     ` Thierry Reding
2021-10-04 20:32     ` Dmitry Osipenko
2021-10-04 20:32       ` Dmitry Osipenko

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=20210520220306.GA1976116@robh.at.kernel.org \
    --to=robh@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=digetx@gmail.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=nicolinc@nvidia.com \
    --cc=robin.murphy@arm.com \
    --cc=thierry.reding@gmail.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.