Re: [PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions

2022-05-18 Thread Thierry Reding
On Sun, May 15, 2022 at 12:35:44PM +0200, Janne Grunau wrote:
> Hej,
> 
> I'm working on the display controller for Apple silicon SoCs and will 
> add some comments with support for it in mind.
> 
> added as...@lists.linux.dev to CC for the Apple silicon related aspects
> 
> On 2022-05-12 21:00:47 +0200, Thierry Reding wrote:
> > 
> > this is another attempt at solving the problem of passing IOMMU
> > configuration via device tree. It has significantly evolved since the
> > last attempt, based on the discussion that followed. The discussion can
> > be found here:
> > 
> >   
> > https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
> > 
> > Rather than using a memory-region specifier, this new version introduces
> > a new "iommu-addresses" property for the reserved-memory regions
> > themselves.
> 
> If experimented with both proposed bindings for dcp and I think this 
> binding is easer to understand and to work with.
> 
> > These are used to describe either a static mapping or
> > reservation that should be created for a given device. If both "reg" and
> > "iommu-addresses" properties are given, a mapping will be created
> > (typically this would be an identity mapping)
> 
> dcp on Apple silicon will not use identity mappings. The IOMMU supports 
> identity mapping but the pre-configured mappings setup by Apple's system 
> firmware will not work with identity mapping. It maps multiple regions 
> which are incompatible with a linear identity mapping. In addition the 
> embbeded aarch64 micro controllers used in the display subsystem appears 
> to use a heap mapped at low IOVA space starting at 0.
> 
> > whereas if only an "iommu-addresses" property is specified, a 
> > reservation for the specified range will be installed.
> > 
> > An example is included in the DT bindings, but here is an extract of
> > what I've used to test this:
> > 
> > reserved-memory {
> > #address-cells = <2>;
> > #size-cells = <2>;
> > ranges;
> > 
> > /*
> >  * Creates an identity mapping for the framebuffer that
> >  * the firmware has setup to scan out a bootsplash from.
> >  */
> > fb: framebuffer@92cb2000 {
> > reg = <0x0 0x92cb2000 0x0 0x0080>;
> > iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
> > };
> 
> The binding supports mapping the same region to multiple devices. The 
> code supports that and it will be used on Apple silicon. Not necessary 
> to extend and complicate the example for I wanted to mention it 
> explicitly.
> 
> > 
> > /*
> >  * Creates a reservation in the IOVA space to prevent
> >  * any buffers from being mapped to that region. Note
> >  * that on Tegra the range is actually quite different
> >  * from this, but it would conflict with the display
> >  * driver that I tested this against, so this is just
> >  * a dummy region for testing.
> >  */
> > adsp: reservation-adsp {
> > iommu-addresses = < 0x0 0x9000 0x0 0x0001>;
> > };
> > };
> > 
> > host1x@5000 {
> > dc@5420 {
> > memory-region = <>, <>;
> > };
> > };
> > 
> > This is abbreviated a little to focus on the essentials. Note also that
> > the ADSP reservation is not actually used on this device and the driver
> > for this doesn't exist yet, but I wanted to include this variant for
> > testing, because we'll want to use these bindings for the reservation
> > use-case as well at some point.
> > 
> > Adding Alyssa and Janne who have in the past tried to make these
> > bindings work on Apple M1. Also adding Sameer from the Tegra audio team
> > to look at the ADSP reservation and double-check that this is suitable
> > for our needs.
> 
> The binding itself is sufficient for the needs of the display subsystem 
> on Apple silicon. The device tree parsing code for reserved regions is 
> of limited use in it's current form. We will have either to extend or 
> duplicate it to retrieve the non-identity mappings. That's our problem 
> to solve.

I had looked at it a bit to see if I could easily implement that, but
the direct mapping support in the IOMMU subsystem currently only
supports either reservations or identity mappings, so arbitrary mappings
would either have to be added to that code, or it would have to take a
different code path that basically goes through the same steps, except
that it uses different physical and I/O virtual addresses.

The easiest, I think, would be for struct iommu_resv_region to be
extended with a pair of start/length fields for the I/O virtual address
and then the rest of the code should mostly work. This shouldn't even be
very invasive, maybe just adding a version of iommu_alloc_resv_region()
that takes the I/O virtual addresses as 

Re: [PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions

2022-05-15 Thread Janne Grunau
Hej,

I'm working on the display controller for Apple silicon SoCs and will 
add some comments with support for it in mind.

added as...@lists.linux.dev to CC for the Apple silicon related aspects

On 2022-05-12 21:00:47 +0200, Thierry Reding wrote:
> 
> this is another attempt at solving the problem of passing IOMMU
> configuration via device tree. It has significantly evolved since the
> last attempt, based on the discussion that followed. The discussion can
> be found here:
> 
>   
> https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
> 
> Rather than using a memory-region specifier, this new version introduces
> a new "iommu-addresses" property for the reserved-memory regions
> themselves.

If experimented with both proposed bindings for dcp and I think this 
binding is easer to understand and to work with.

> These are used to describe either a static mapping or
> reservation that should be created for a given device. If both "reg" and
> "iommu-addresses" properties are given, a mapping will be created
> (typically this would be an identity mapping)

dcp on Apple silicon will not use identity mappings. The IOMMU supports 
identity mapping but the pre-configured mappings setup by Apple's system 
firmware will not work with identity mapping. It maps multiple regions 
which are incompatible with a linear identity mapping. In addition the 
embbeded aarch64 micro controllers used in the display subsystem appears 
to use a heap mapped at low IOVA space starting at 0.

> whereas if only an "iommu-addresses" property is specified, a 
> reservation for the specified range will be installed.
> 
> An example is included in the DT bindings, but here is an extract of
> what I've used to test this:
> 
>   reserved-memory {
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> 
>   /*
>* Creates an identity mapping for the framebuffer that
>* the firmware has setup to scan out a bootsplash from.
>*/
>   fb: framebuffer@92cb2000 {
>   reg = <0x0 0x92cb2000 0x0 0x0080>;
>   iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
>   };

The binding supports mapping the same region to multiple devices. The 
code supports that and it will be used on Apple silicon. Not necessary 
to extend and complicate the example for I wanted to mention it 
explicitly.

> 
>   /*
>* Creates a reservation in the IOVA space to prevent
>* any buffers from being mapped to that region. Note
>* that on Tegra the range is actually quite different
>* from this, but it would conflict with the display
>* driver that I tested this against, so this is just
>* a dummy region for testing.
>*/
>   adsp: reservation-adsp {
>   iommu-addresses = < 0x0 0x9000 0x0 0x0001>;
>   };
>   };
> 
>   host1x@5000 {
>   dc@5420 {
>   memory-region = <>, <>;
>   };
>   };
> 
> This is abbreviated a little to focus on the essentials. Note also that
> the ADSP reservation is not actually used on this device and the driver
> for this doesn't exist yet, but I wanted to include this variant for
> testing, because we'll want to use these bindings for the reservation
> use-case as well at some point.
> 
> Adding Alyssa and Janne who have in the past tried to make these
> bindings work on Apple M1. Also adding Sameer from the Tegra audio team
> to look at the ADSP reservation and double-check that this is suitable
> for our needs.

The binding itself is sufficient for the needs of the display subsystem 
on Apple silicon. The device tree parsing code for reserved regions is 
of limited use in it's current form. We will have either to extend or 
duplicate it to retrieve the non-identity mappings. That's our problem 
to solve.

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


[PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions

2022-05-12 Thread Thierry Reding
From: Thierry Reding 

Hi,

this is another attempt at solving the problem of passing IOMMU
configuration via device tree. It has significantly evolved since the
last attempt, based on the discussion that followed. The discussion can
be found here:

  https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/

Rather than using a memory-region specifier, this new version introduces
a new "iommu-addresses" property for the reserved-memory regions
themselves. These are used to describe either a static mapping or
reservation that should be created for a given device. If both "reg" and
"iommu-addresses" properties are given, a mapping will be created
(typically this would be an identity mapping) whereas if only an
"iommu-addresses" property is specified, a reservation for the specified
range will be installed.

An example is included in the DT bindings, but here is an extract of
what I've used to test this:

reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;

/*
 * Creates an identity mapping for the framebuffer that
 * the firmware has setup to scan out a bootsplash from.
 */
fb: framebuffer@92cb2000 {
reg = <0x0 0x92cb2000 0x0 0x0080>;
iommu-addresses = < 0x0 0x92cb2000 0x0 0x0080>;
};

/*
 * Creates a reservation in the IOVA space to prevent
 * any buffers from being mapped to that region. Note
 * that on Tegra the range is actually quite different
 * from this, but it would conflict with the display
 * driver that I tested this against, so this is just
 * a dummy region for testing.
 */
adsp: reservation-adsp {
iommu-addresses = < 0x0 0x9000 0x0 0x0001>;
};
};

host1x@5000 {
dc@5420 {
memory-region = <>, <>;
};
};

This is abbreviated a little to focus on the essentials. Note also that
the ADSP reservation is not actually used on this device and the driver
for this doesn't exist yet, but I wanted to include this variant for
testing, because we'll want to use these bindings for the reservation
use-case as well at some point.

Adding Alyssa and Janne who have in the past tried to make these
bindings work on Apple M1. Also adding Sameer from the Tegra audio team
to look at the ADSP reservation and double-check that this is suitable
for our needs.

Thierry

Navneet Kumar (1):
  iommu/tegra-smmu: Support managed domains

Thierry Reding (4):
  dt-bindings: reserved-memory: Document iommu-addresses
  iommu: Implement of_iommu_get_resv_regions()
  iommu: dma: Use of_iommu_get_resv_regions()
  iommu/tegra-smmu: Add support for reserved regions

 .../reserved-memory/reserved-memory.txt   |  1 -
 .../reserved-memory/reserved-memory.yaml  | 62 +
 drivers/iommu/dma-iommu.c |  3 +
 drivers/iommu/of_iommu.c  | 90 +++
 drivers/iommu/tegra-smmu.c| 82 +
 include/dt-bindings/reserved-memory.h |  8 ++
 include/linux/of_iommu.h  |  8 ++
 7 files changed, 235 insertions(+), 19 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
 create mode 100644 include/dt-bindings/reserved-memory.h

-- 
2.36.1

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