IOVA allocation dependency between firmware buffer and remaining buffers

IOVA allocation dependency between firmware buffer and remaining buffers

[Ajay kumar]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

Hi All,

I have an IOMMU master which has limitations as mentioned below:
1) The IOMMU master internally executes a firmware, and the firmware memory
is allocated by the same master driver.
The firmware buffer address should be of the lowest range than other address
allocated by the device, or in other words, all the remaining buffer
addresses
should always be in a higher range than the firmware address.
2) None of the buffer addresses should go beyond 0xC000_0000

example:
If firmware buffer address is buf_fw = 0x8000_5000;
All other addresses given to the device should be greater than
(0x8000_5000 + firmware size) and less than 0xC000_0000

Currently, this is being handled with one of the below hacks:
1) By keeping dma_mask in lower range while allocating firmware buffer,
and then increasing the dma_mask to higher range for other buffers.
2) By reserving IOVA for firmware at the lowest range and creating direct
mappings for the same.

I want to know if there is a better way this can be handled with current
framework, or if anybody is facing similar problems with their devices,
please share how it is taken care.

I also think there should be some way the masters can specify the IOVA
range they want to limit to for current allocation.
Something like a new iommu_ops callback like below:
limit_iova_alloc_range(dev, iova_start, iova_end)

And, in my driver, the sequence will be:
limit_iova_alloc_range(dev, 0x0000_0000, 0x1000_0000); /* via helpers */
alloc( ) firmware buffer using DMA API
limit_iova_alloc_range(dev, 0x1000_0000, 0xC000_0000); /* via helpers */
alloc( ) other buffers using DMA API

Thanks,
Ajay Kumar

[-- Attachment #2: Type: text/html, Size: 2114 bytes --]

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Ajay kumar]

Can someone check this?

On Mon, Apr 20, 2020 at 9:24 PM Ajay kumar <ajaynumb@gmail.com> wrote:
>
> Hi All,
>
> I have an IOMMU master which has limitations as mentioned below:
> 1) The IOMMU master internally executes a firmware, and the firmware memory
> is allocated by the same master driver.
> The firmware buffer address should be of the lowest range than other address
> allocated by the device, or in other words, all the remaining buffer addresses
> should always be in a higher range than the firmware address.
> 2) None of the buffer addresses should go beyond 0xC000_0000
>
> example:
> If firmware buffer address is buf_fw = 0x8000_5000;
> All other addresses given to the device should be greater than
> (0x8000_5000 + firmware size) and less than 0xC000_0000
>
> Currently, this is being handled with one of the below hacks:
> 1) By keeping dma_mask in lower range while allocating firmware buffer,
> and then increasing the dma_mask to higher range for other buffers.
> 2) By reserving IOVA for firmware at the lowest range and creating direct mappings for the same.
>
> I want to know if there is a better way this can be handled with current framework,
> or if anybody is facing similar problems with their devices,
> please share how it is taken care.
>
> I also think there should be some way the masters can specify the IOVA
> range they want to limit to for current allocation.
> Something like a new iommu_ops callback like below:
> limit_iova_alloc_range(dev, iova_start, iova_end)
>
> And, in my driver, the sequence will be:
> limit_iova_alloc_range(dev, 0x0000_0000, 0x1000_0000); /* via helpers */
> alloc( ) firmware buffer using DMA API
> limit_iova_alloc_range(dev, 0x1000_0000, 0xC000_0000); /* via helpers */
> alloc( ) other buffers using DMA API
>
> Thanks,
> Ajay Kumar

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Robin Murphy]

On 2020-04-24 4:04 pm, Ajay kumar wrote:
> Can someone check this?
> 
> On Mon, Apr 20, 2020 at 9:24 PM Ajay kumar <ajaynumb@gmail.com> wrote:
>>
>> Hi All,
>>
>> I have an IOMMU master which has limitations as mentioned below:
>> 1) The IOMMU master internally executes a firmware, and the firmware memory
>> is allocated by the same master driver.
>> The firmware buffer address should be of the lowest range than other address
>> allocated by the device, or in other words, all the remaining buffer addresses
>> should always be in a higher range than the firmware address.
>> 2) None of the buffer addresses should go beyond 0xC000_0000

That particular constraint could (and perhaps should) be expressed as a 
DMA mask/limit for the device, but if you have specific requirements to 
place buffers at particular addresses then you might be better off 
managing your own IOMMU domain like some other (mostly DRM) drivers do. 
The DMA APIs don't offer any guarantees about what addresses you'll get 
other than that they won't exceed the appropriate mask.

>> example:
>> If firmware buffer address is buf_fw = 0x8000_5000;
>> All other addresses given to the device should be greater than
>> (0x8000_5000 + firmware size) and less than 0xC000_0000

Out of curiosity, how do you control that in the no-IOMMU or IOMMU 
passthrough cases?

Robin.

>> Currently, this is being handled with one of the below hacks:
>> 1) By keeping dma_mask in lower range while allocating firmware buffer,
>> and then increasing the dma_mask to higher range for other buffers.
>> 2) By reserving IOVA for firmware at the lowest range and creating direct mappings for the same.
>>
>> I want to know if there is a better way this can be handled with current framework,
>> or if anybody is facing similar problems with their devices,
>> please share how it is taken care.
>>
>> I also think there should be some way the masters can specify the IOVA
>> range they want to limit to for current allocation.
>> Something like a new iommu_ops callback like below:
>> limit_iova_alloc_range(dev, iova_start, iova_end)
>>
>> And, in my driver, the sequence will be:
>> limit_iova_alloc_range(dev, 0x0000_0000, 0x1000_0000); /* via helpers */
>> alloc( ) firmware buffer using DMA API
>> limit_iova_alloc_range(dev, 0x1000_0000, 0xC000_0000); /* via helpers */
>> alloc( ) other buffers using DMA API
>>
>> Thanks,
>> Ajay Kumar

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Shaik Ameer Basha]

On Fri, Apr 24, 2020 at 8:59 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-04-24 4:04 pm, Ajay kumar wrote:
> > Can someone check this?
> >
> > On Mon, Apr 20, 2020 at 9:24 PM Ajay kumar <ajaynumb@gmail.com> wrote:
> >>
> >> Hi All,
> >>
> >> I have an IOMMU master which has limitations as mentioned below:
> >> 1) The IOMMU master internally executes a firmware, and the firmware memory
> >> is allocated by the same master driver.
> >> The firmware buffer address should be of the lowest range than other address
> >> allocated by the device, or in other words, all the remaining buffer addresses
> >> should always be in a higher range than the firmware address.
> >> 2) None of the buffer addresses should go beyond 0xC000_0000
>
> That particular constraint could (and perhaps should) be expressed as a
> DMA mask/limit for the device, but if you have specific requirements to

Yes Robin. We do use 0xC000_0000 address to set the DMA mask in our driver.

> place buffers at particular addresses then you might be better off
> managing your own IOMMU domain like some other (mostly DRM) drivers do.

If you remember any of such drivers can you please point the driver path ?

> The DMA APIs don't offer any guarantees about what addresses you'll get
> other than that they won't exceed the appropriate mask.

True, we have gone through most of the APIs and didn't find any way to match our
requirements with the existing DMA APIs

>
> >> example:
> >> If firmware buffer address is buf_fw = 0x8000_5000;
> >> All other addresses given to the device should be greater than
> >> (0x8000_5000 + firmware size) and less than 0xC000_0000
>
> Out of curiosity, how do you control that in the no-IOMMU or IOMMU
> passthrough cases?

We manage the no-IOMMU or pass through cases using the reserved-memory.

>
> Robin.
>
> >> Currently, this is being handled with one of the below hacks:
> >> 1) By keeping dma_mask in lower range while allocating firmware buffer,
> >> and then increasing the dma_mask to higher range for other buffers.
> >> 2) By reserving IOVA for firmware at the lowest range and creating direct mappings for the same.
> >>
> >> I want to know if there is a better way this can be handled with current framework,
> >> or if anybody is facing similar problems with their devices,
> >> please share how it is taken care.
> >>
> >> I also think there should be some way the masters can specify the IOVA
> >> range they want to limit to for current allocation.
> >> Something like a new iommu_ops callback like below:
> >> limit_iova_alloc_range(dev, iova_start, iova_end)
> >>
> >> And, in my driver, the sequence will be:
> >> limit_iova_alloc_range(dev, 0x0000_0000, 0x1000_0000); /* via helpers */
> >> alloc( ) firmware buffer using DMA API
> >> limit_iova_alloc_range(dev, 0x1000_0000, 0xC000_0000); /* via helpers */
> >> alloc( ) other buffers using DMA API
> >>

Just want to understand more from you, on the new iommu_ops we suggested.
Shouldn't device have that flexibility to allocate IOVA as per it's requirement?
If you see our device as example, we need to have control on the
allocated IOVA region
based on where device is using this buffer.

If we have these callbacks in place, then the low level IOMMU driver
can implement and
manage such requests when needed.

If this can't be taken forward for some right reasons, then we will
definitely try to understand
on how to manage the IOMMU domain from our driver as per your suggestion

- Shaik.

> >> Thanks,
> >> Ajay Kumar

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Marek Szyprowski]

Hi Shaik,

I've run into similar problem while adapting S5P-MFC and Exynos4-IS 
drivers for generic IOMMU-DMA framework. Here is my first solution: 
https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprowski@samsung.com/T/ 


It allows to remap given buffer at the specific IOVA address, although 
it doesn't guarantee that those specific addresses won't be later used 
by the IOVA allocator. Probably it would make sense to add an API for 
generic IOMMU-DMA framework to mark the given IOVA range as 
reserved/unused to protect them.

Best regards
Marek Szyprowski, PhD
Samsung R&D Institute Poland

On 24.04.2020 18:15, Shaik Ameer Basha wrote:
> On Fri, Apr 24, 2020 at 8:59 PM Robin Murphy <robin.murphy@arm.com> wrote:
>> On 2020-04-24 4:04 pm, Ajay kumar wrote:
>>> Can someone check this?
>>>
>>> On Mon, Apr 20, 2020 at 9:24 PM Ajay kumar <ajaynumb@gmail.com> wrote:
>>>> Hi All,
>>>>
>>>> I have an IOMMU master which has limitations as mentioned below:
>>>> 1) The IOMMU master internally executes a firmware, and the firmware memory
>>>> is allocated by the same master driver.
>>>> The firmware buffer address should be of the lowest range than other address
>>>> allocated by the device, or in other words, all the remaining buffer addresses
>>>> should always be in a higher range than the firmware address.
>>>> 2) None of the buffer addresses should go beyond 0xC000_0000
>> That particular constraint could (and perhaps should) be expressed as a
>> DMA mask/limit for the device, but if you have specific requirements to
> Yes Robin. We do use 0xC000_0000 address to set the DMA mask in our driver.
>
>> place buffers at particular addresses then you might be better off
>> managing your own IOMMU domain like some other (mostly DRM) drivers do.
> If you remember any of such drivers can you please point the driver path ?
>
>> The DMA APIs don't offer any guarantees about what addresses you'll get
>> other than that they won't exceed the appropriate mask.
> True, we have gone through most of the APIs and didn't find any way to match our
> requirements with the existing DMA APIs
>
>>>> example:
>>>> If firmware buffer address is buf_fw = 0x8000_5000;
>>>> All other addresses given to the device should be greater than
>>>> (0x8000_5000 + firmware size) and less than 0xC000_0000
>> Out of curiosity, how do you control that in the no-IOMMU or IOMMU
>> passthrough cases?
> We manage the no-IOMMU or pass through cases using the reserved-memory.
>
>> Robin.
>>
>>>> Currently, this is being handled with one of the below hacks:
>>>> 1) By keeping dma_mask in lower range while allocating firmware buffer,
>>>> and then increasing the dma_mask to higher range for other buffers.
>>>> 2) By reserving IOVA for firmware at the lowest range and creating direct mappings for the same.
>>>>
>>>> I want to know if there is a better way this can be handled with current framework,
>>>> or if anybody is facing similar problems with their devices,
>>>> please share how it is taken care.
>>>>
>>>> I also think there should be some way the masters can specify the IOVA
>>>> range they want to limit to for current allocation.
>>>> Something like a new iommu_ops callback like below:
>>>> limit_iova_alloc_range(dev, iova_start, iova_end)
>>>>
>>>> And, in my driver, the sequence will be:
>>>> limit_iova_alloc_range(dev, 0x0000_0000, 0x1000_0000); /* via helpers */
>>>> alloc( ) firmware buffer using DMA API
>>>> limit_iova_alloc_range(dev, 0x1000_0000, 0xC000_0000); /* via helpers */
>>>> alloc( ) other buffers using DMA API
>>>>
> Just want to understand more from you, on the new iommu_ops we suggested.
> Shouldn't device have that flexibility to allocate IOVA as per it's requirement?
> If you see our device as example, we need to have control on the
> allocated IOVA region
> based on where device is using this buffer.
>
> If we have these callbacks in place, then the low level IOMMU driver
> can implement and
> manage such requests when needed.
>
> If this can't be taken forward for some right reasons, then we will
> definitely try to understand
> on how to manage the IOMMU domain from our driver as per your suggestion
>
> - Shaik.
>
>>>> Thanks,
>>>> Ajay Kumar

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Christoph Hellwig]

On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> Hi Shaik,
> 
> I've run into similar problem while adapting S5P-MFC and Exynos4-IS 
> drivers for generic IOMMU-DMA framework. Here is my first solution: 
> https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprowski@samsung.com/T/ 
> 
> 
> It allows to remap given buffer at the specific IOVA address, although 
> it doesn't guarantee that those specific addresses won't be later used 
> by the IOVA allocator. Probably it would make sense to add an API for 
> generic IOMMU-DMA framework to mark the given IOVA range as 
> reserved/unused to protect them.

If you want to use IOVA addresses in a device otherwise managed by
dma-iommu we need to expose an API through the dma API.  Can you please
include the iommu list in the discussion of your series?

I don't think using the raw IOMMU API is a very idea in these drivers,
we probably want a way to change the allocator algorithm or hint the
next IOVA and keep using the normal DMA API.  Maybe Robin has a better
idea.

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Ajay kumar]

Hello all,

We pretty much tried to solve the same issue here with a new API in DMA-IOMMU:
https://lore.kernel.org/linux-iommu/20200811054912.GA301@infradead.org/T/

Christopher- the user part would be MFC devices on exynos platforms

Thanks,
Ajay
On 9/23/20, Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>> Hi Shaik,
>>
>> I've run into similar problem while adapting S5P-MFC and Exynos4-IS
>> drivers for generic IOMMU-DMA framework. Here is my first solution:
>> https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprowski@samsung.com/T/
>>
>>
>>
>> It allows to remap given buffer at the specific IOVA address, although
>> it doesn't guarantee that those specific addresses won't be later used
>> by the IOVA allocator. Probably it would make sense to add an API for
>> generic IOMMU-DMA framework to mark the given IOVA range as
>> reserved/unused to protect them.
>
> If you want to use IOVA addresses in a device otherwise managed by
> dma-iommu we need to expose an API through the dma API.  Can you please
> include the iommu list in the discussion of your series?
>
> I don't think using the raw IOMMU API is a very idea in these drivers,
> we probably want a way to change the allocator algorithm or hint the
> next IOVA and keep using the normal DMA API.  Maybe Robin has a better
> idea.
>

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Ajay Kumar]

Hello all,

We pretty much tried to solve the same issue here with a new API in DMA-IOMMU:
https://lore.kernel.org/linux-iommu/20200811054912.GA301@infradead.org/T/

Christoph - the user part would be MFC devices on exynos platforms

Thanks,
Ajay


On Wed, Sep 23, 2020 at 12:28 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> > Hi Shaik,
> >
> > I've run into similar problem while adapting S5P-MFC and Exynos4-IS
> > drivers for generic IOMMU-DMA framework. Here is my first solution:
> > https://lore.kernel.org/linux-samsung-soc/20200918144833.14618-1-m.szyprowski@samsung.com/T/
> >
> >
> > It allows to remap given buffer at the specific IOVA address, although
> > it doesn't guarantee that those specific addresses won't be later used
> > by the IOVA allocator. Probably it would make sense to add an API for
> > generic IOMMU-DMA framework to mark the given IOVA range as
> > reserved/unused to protect them.
>
> If you want to use IOVA addresses in a device otherwise managed by
> dma-iommu we need to expose an API through the dma API.  Can you please
> include the iommu list in the discussion of your series?
>
> I don't think using the raw IOMMU API is a very idea in these drivers,
> we probably want a way to change the allocator algorithm or hint the
> next IOVA and keep using the normal DMA API.  Maybe Robin has a better
> idea.
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Christoph Hellwig]

On Wed, Sep 23, 2020 at 01:15:33PM +0530, Ajay kumar wrote:
> Hello all,
> 
> We pretty much tried to solve the same issue here with a new API in DMA-IOMMU:
> https://lore.kernel.org/linux-iommu/20200811054912.GA301@infradead.org/T/
> 
> Christopher- the user part would be MFC devices on exynos platforms

I still think we:

 a) need to wire it up through the DMA API with an ops vector,
    and an error for devices not using dma-iommu
 b) submit it together with an actual users (like the series from Marek)

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Joerg Roedel]

On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> It allows to remap given buffer at the specific IOVA address, although 
> it doesn't guarantee that those specific addresses won't be later used 
> by the IOVA allocator. Probably it would make sense to add an API for 
> generic IOMMU-DMA framework to mark the given IOVA range as 
> reserved/unused to protect them.

There is an API for that, the IOMMU driver can return IOVA reserved
regions per device and the IOMMU core code will take care of mapping
these regions and reserving them in the IOVA allocator, so that
DMA-IOMMU code will not use it for allocations.

Have a look at the iommu_ops->get_resv_regions() and
iommu_ops->put_resv_regions().

Regards,

	Joerg

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Marek Szyprowski]

Hi Joerg,

On 24.09.2020 10:28, Joerg Roedel wrote:
> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>> It allows to remap given buffer at the specific IOVA address, although
>> it doesn't guarantee that those specific addresses won't be later used
>> by the IOVA allocator. Probably it would make sense to add an API for
>> generic IOMMU-DMA framework to mark the given IOVA range as
>> reserved/unused to protect them.
> There is an API for that, the IOMMU driver can return IOVA reserved
> regions per device and the IOMMU core code will take care of mapping
> these regions and reserving them in the IOVA allocator, so that
> DMA-IOMMU code will not use it for allocations.
>
> Have a look at the iommu_ops->get_resv_regions() and
> iommu_ops->put_resv_regions().

I know about the reserved regions IOMMU API, but the main problem here, 
in case of Exynos, is that those reserved regions won't be created by 
the IOMMU driver but by the IOMMU client device. It is just a result how 
the media drivers manages their IOVA space. They simply have to load 
firmware at the IOVA address lower than the any address of the used 
buffers.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
> Hi Joerg,
> 
> On 24.09.2020 10:28, Joerg Roedel wrote:
> > On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> >> It allows to remap given buffer at the specific IOVA address, although
> >> it doesn't guarantee that those specific addresses won't be later used
> >> by the IOVA allocator. Probably it would make sense to add an API for
> >> generic IOMMU-DMA framework to mark the given IOVA range as
> >> reserved/unused to protect them.
> > There is an API for that, the IOMMU driver can return IOVA reserved
> > regions per device and the IOMMU core code will take care of mapping
> > these regions and reserving them in the IOVA allocator, so that
> > DMA-IOMMU code will not use it for allocations.
> >
> > Have a look at the iommu_ops->get_resv_regions() and
> > iommu_ops->put_resv_regions().
> 
> I know about the reserved regions IOMMU API, but the main problem here, 
> in case of Exynos, is that those reserved regions won't be created by 
> the IOMMU driver but by the IOMMU client device. It is just a result how 
> the media drivers manages their IOVA space. They simply have to load 
> firmware at the IOVA address lower than the any address of the used 
> buffers.

I've been working on adding a way to automatically add direct mappings
using reserved-memory regions parsed from device tree, see:

    https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/

Perhaps this can be of use? With that you should be able to add a
reserved-memory region somewhere in the lower range that you need for
firmware images and have that automatically added as a direct mapping
so that it won't be reused later on for dynamic allocations.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Robin Murphy]

On 2020-09-24 11:16, Thierry Reding wrote:
> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
>> Hi Joerg,
>>
>> On 24.09.2020 10:28, Joerg Roedel wrote:
>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>>>> It allows to remap given buffer at the specific IOVA address, although
>>>> it doesn't guarantee that those specific addresses won't be later used
>>>> by the IOVA allocator. Probably it would make sense to add an API for
>>>> generic IOMMU-DMA framework to mark the given IOVA range as
>>>> reserved/unused to protect them.
>>> There is an API for that, the IOMMU driver can return IOVA reserved
>>> regions per device and the IOMMU core code will take care of mapping
>>> these regions and reserving them in the IOVA allocator, so that
>>> DMA-IOMMU code will not use it for allocations.
>>>
>>> Have a look at the iommu_ops->get_resv_regions() and
>>> iommu_ops->put_resv_regions().
>>
>> I know about the reserved regions IOMMU API, but the main problem here,
>> in case of Exynos, is that those reserved regions won't be created by
>> the IOMMU driver but by the IOMMU client device. It is just a result how
>> the media drivers manages their IOVA space. They simply have to load
>> firmware at the IOVA address lower than the any address of the used
>> buffers.
> 
> I've been working on adding a way to automatically add direct mappings
> using reserved-memory regions parsed from device tree, see:
> 
>      https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
> 
> Perhaps this can be of use? With that you should be able to add a
> reserved-memory region somewhere in the lower range that you need for
> firmware images and have that automatically added as a direct mapping
> so that it won't be reused later on for dynamic allocations.

It can't easily be a *direct* mapping though - if the driver has to use 
the DMA masks to ensure that everything stays within the addressable 
range, then (as far as I'm aware) there's no physical memory that low 
down to equal the DMA addresses.

TBH I'm not convinced that this is a sufficiently common concern to 
justify new APIs, or even to try to make overly generic. I think just 
implementing a new DMA attribute to say "please allocate/map this 
particular request at the lowest DMA address possible" would be good 
enough. Such a thing could also serve PCI drivers that actually care 
about SAC/DAC to give us more of a chance of removing the "try a 32-bit 
mask first" trick from everyone's hotpath...

Robin.

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Marek Szyprowski]

Hi Thierry,

On 24.09.2020 12:16, Thierry Reding wrote:
> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
>> On 24.09.2020 10:28, Joerg Roedel wrote:
>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>>>> It allows to remap given buffer at the specific IOVA address, although
>>>> it doesn't guarantee that those specific addresses won't be later used
>>>> by the IOVA allocator. Probably it would make sense to add an API for
>>>> generic IOMMU-DMA framework to mark the given IOVA range as
>>>> reserved/unused to protect them.
>>> There is an API for that, the IOMMU driver can return IOVA reserved
>>> regions per device and the IOMMU core code will take care of mapping
>>> these regions and reserving them in the IOVA allocator, so that
>>> DMA-IOMMU code will not use it for allocations.
>>>
>>> Have a look at the iommu_ops->get_resv_regions() and
>>> iommu_ops->put_resv_regions().
>> I know about the reserved regions IOMMU API, but the main problem here,
>> in case of Exynos, is that those reserved regions won't be created by
>> the IOMMU driver but by the IOMMU client device. It is just a result how
>> the media drivers manages their IOVA space. They simply have to load
>> firmware at the IOVA address lower than the any address of the used
>> buffers.
> I've been working on adding a way to automatically add direct mappings
> using reserved-memory regions parsed from device tree, see:
>
>      https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
>
> Perhaps this can be of use? With that you should be able to add a
> reserved-memory region somewhere in the lower range that you need for
> firmware images and have that automatically added as a direct mapping
> so that it won't be reused later on for dynamic allocations.

Frankly, using that would be even bigger hack than what I've proposed in 
my workaround. I see no point polluting DT with such artificial regions 
just to ensure specific IOVA space layout.

I just looking for a nice way of reusing most of the IOMMU DMA-mapping 
code with a small addition of manual IOVA space management (just to 
remap the firmware buffer at the specific IOVA address).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Marek Szyprowski]

Hi Robin,

On 24.09.2020 12:40, Robin Murphy wrote:
> On 2020-09-24 11:16, Thierry Reding wrote:
>> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
>>> On 24.09.2020 10:28, Joerg Roedel wrote:
>>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>>>>> It allows to remap given buffer at the specific IOVA address, 
>>>>> although
>>>>> it doesn't guarantee that those specific addresses won't be later 
>>>>> used
>>>>> by the IOVA allocator. Probably it would make sense to add an API for
>>>>> generic IOMMU-DMA framework to mark the given IOVA range as
>>>>> reserved/unused to protect them.
>>>> There is an API for that, the IOMMU driver can return IOVA reserved
>>>> regions per device and the IOMMU core code will take care of mapping
>>>> these regions and reserving them in the IOVA allocator, so that
>>>> DMA-IOMMU code will not use it for allocations.
>>>>
>>>> Have a look at the iommu_ops->get_resv_regions() and
>>>> iommu_ops->put_resv_regions().
>>>
>>> I know about the reserved regions IOMMU API, but the main problem here,
>>> in case of Exynos, is that those reserved regions won't be created by
>>> the IOMMU driver but by the IOMMU client device. It is just a result 
>>> how
>>> the media drivers manages their IOVA space. They simply have to load
>>> firmware at the IOVA address lower than the any address of the used
>>> buffers.
>>
>> I've been working on adding a way to automatically add direct mappings
>> using reserved-memory regions parsed from device tree, see:
>>
>> https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
>>
>> Perhaps this can be of use? With that you should be able to add a
>> reserved-memory region somewhere in the lower range that you need for
>> firmware images and have that automatically added as a direct mapping
>> so that it won't be reused later on for dynamic allocations.
>
> It can't easily be a *direct* mapping though - if the driver has to 
> use the DMA masks to ensure that everything stays within the 
> addressable range, then (as far as I'm aware) there's no physical 
> memory that low down to equal the DMA addresses.
>
> TBH I'm not convinced that this is a sufficiently common concern to 
> justify new APIs, or even to try to make overly generic. I think just 
> implementing a new DMA attribute to say "please allocate/map this 
> particular request at the lowest DMA address possible" would be good 
> enough. Such a thing could also serve PCI drivers that actually care 
> about SAC/DAC to give us more of a chance of removing the "try a 
> 32-bit mask first" trick from everyone's hotpath...

Hmm, I like the idea of such DMA attribute! It should make things really 
simple, especially in the drivers. Thanks for the great idea! I will try 
to implement it then instead of the workarounds I've proposed in 
s5p-mfc/exynos4-is drivers.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Robin Murphy]

On 2020-09-24 11:47, Marek Szyprowski wrote:
> Hi Robin,
> 
> On 24.09.2020 12:40, Robin Murphy wrote:
>> On 2020-09-24 11:16, Thierry Reding wrote:
>>> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
>>>> On 24.09.2020 10:28, Joerg Roedel wrote:
>>>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>>>>>> It allows to remap given buffer at the specific IOVA address,
>>>>>> although
>>>>>> it doesn't guarantee that those specific addresses won't be later
>>>>>> used
>>>>>> by the IOVA allocator. Probably it would make sense to add an API for
>>>>>> generic IOMMU-DMA framework to mark the given IOVA range as
>>>>>> reserved/unused to protect them.
>>>>> There is an API for that, the IOMMU driver can return IOVA reserved
>>>>> regions per device and the IOMMU core code will take care of mapping
>>>>> these regions and reserving them in the IOVA allocator, so that
>>>>> DMA-IOMMU code will not use it for allocations.
>>>>>
>>>>> Have a look at the iommu_ops->get_resv_regions() and
>>>>> iommu_ops->put_resv_regions().
>>>>
>>>> I know about the reserved regions IOMMU API, but the main problem here,
>>>> in case of Exynos, is that those reserved regions won't be created by
>>>> the IOMMU driver but by the IOMMU client device. It is just a result
>>>> how
>>>> the media drivers manages their IOVA space. They simply have to load
>>>> firmware at the IOVA address lower than the any address of the used
>>>> buffers.
>>>
>>> I've been working on adding a way to automatically add direct mappings
>>> using reserved-memory regions parsed from device tree, see:
>>>
>>> https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
>>>
>>> Perhaps this can be of use? With that you should be able to add a
>>> reserved-memory region somewhere in the lower range that you need for
>>> firmware images and have that automatically added as a direct mapping
>>> so that it won't be reused later on for dynamic allocations.
>>
>> It can't easily be a *direct* mapping though - if the driver has to
>> use the DMA masks to ensure that everything stays within the
>> addressable range, then (as far as I'm aware) there's no physical
>> memory that low down to equal the DMA addresses.
>>
>> TBH I'm not convinced that this is a sufficiently common concern to
>> justify new APIs, or even to try to make overly generic. I think just
>> implementing a new DMA attribute to say "please allocate/map this
>> particular request at the lowest DMA address possible" would be good
>> enough. Such a thing could also serve PCI drivers that actually care
>> about SAC/DAC to give us more of a chance of removing the "try a
>> 32-bit mask first" trick from everyone's hotpath...
> 
> Hmm, I like the idea of such DMA attribute! It should make things really
> simple, especially in the drivers. Thanks for the great idea! I will try
> to implement it then instead of the workarounds I've proposed in
> s5p-mfc/exynos4-is drivers.

Right, I think it's fair to draw a line and say that anyone who wants a 
*specific* address needs to manage their own IOMMU domain.

In the backend I suspect it's going to be cleanest to implement a 
dedicated iova_alloc_low() (or similar) function in the IOVA API that 
sidesteps all of the existing allocation paths and goes straight to the 
rbtree.

Robin.

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Shaik Ameer Basha]

Hi Robin and Marek,


On Thu, Sep 24, 2020 at 4:36 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-09-24 11:47, Marek Szyprowski wrote:
> > Hi Robin,
> >
> > On 24.09.2020 12:40, Robin Murphy wrote:
> >> On 2020-09-24 11:16, Thierry Reding wrote:
> >>> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
> >>>> On 24.09.2020 10:28, Joerg Roedel wrote:
> >>>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> >>>>>> It allows to remap given buffer at the specific IOVA address,
> >>>>>> although
> >>>>>> it doesn't guarantee that those specific addresses won't be later
> >>>>>> used
> >>>>>> by the IOVA allocator. Probably it would make sense to add an API for
> >>>>>> generic IOMMU-DMA framework to mark the given IOVA range as
> >>>>>> reserved/unused to protect them.
> >>>>> There is an API for that, the IOMMU driver can return IOVA reserved
> >>>>> regions per device and the IOMMU core code will take care of mapping
> >>>>> these regions and reserving them in the IOVA allocator, so that
> >>>>> DMA-IOMMU code will not use it for allocations.
> >>>>>
> >>>>> Have a look at the iommu_ops->get_resv_regions() and
> >>>>> iommu_ops->put_resv_regions().
> >>>>
> >>>> I know about the reserved regions IOMMU API, but the main problem here,
> >>>> in case of Exynos, is that those reserved regions won't be created by
> >>>> the IOMMU driver but by the IOMMU client device. It is just a result
> >>>> how
> >>>> the media drivers manages their IOVA space. They simply have to load
> >>>> firmware at the IOVA address lower than the any address of the used
> >>>> buffers.
> >>>
> >>> I've been working on adding a way to automatically add direct mappings
> >>> using reserved-memory regions parsed from device tree, see:
> >>>
> >>> https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
> >>>
> >>> Perhaps this can be of use? With that you should be able to add a
> >>> reserved-memory region somewhere in the lower range that you need for
> >>> firmware images and have that automatically added as a direct mapping
> >>> so that it won't be reused later on for dynamic allocations.
> >>
> >> It can't easily be a *direct* mapping though - if the driver has to
> >> use the DMA masks to ensure that everything stays within the
> >> addressable range, then (as far as I'm aware) there's no physical
> >> memory that low down to equal the DMA addresses.
> >>
> >> TBH I'm not convinced that this is a sufficiently common concern to
> >> justify new APIs, or even to try to make overly generic. I think just
> >> implementing a new DMA attribute to say "please allocate/map this
> >> particular request at the lowest DMA address possible" would be good
> >> enough. Such a thing could also serve PCI drivers that actually care
> >> about SAC/DAC to give us more of a chance of removing the "try a
> >> 32-bit mask first" trick from everyone's hotpath...
> >
> > Hmm, I like the idea of such DMA attribute! It should make things really
> > simple, especially in the drivers. Thanks for the great idea! I will try
> > to implement it then instead of the workarounds I've proposed in
> > s5p-mfc/exynos4-is drivers.
>
> Right, I think it's fair to draw a line and say that anyone who wants a
> *specific* address needs to manage their own IOMMU domain.
>
> In the backend I suspect it's going to be cleanest to implement a
> dedicated iova_alloc_low() (or similar) function in the IOVA API that
> sidesteps all of the existing allocation paths and goes straight to the
> rbtree.

This is the place we started with..

But our solution was to provide an API which limits the allocation
range per device (dynamically) based on the driver request..
Something like,
    limit_iova_alloc_range(dev, low_iova, high_iova); /* via helpers */

When multiple devices use the same IOVA space, how we can handle api's
like " iova_alloc_low()" ?
And providing APIs like " limit_iova_alloc_range()" may cater similar
future requirements from drivers instead of worrying about
high/low/mid etc.

Again, flexibility should be there with user drivers to request the
range they want at any point of time...

Please let us know your inputs.

>
> Robin.

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]

On Thu, Sep 24, 2020 at 12:41:29PM +0200, Marek Szyprowski wrote:
> Hi Thierry,
> 
> On 24.09.2020 12:16, Thierry Reding wrote:
> > On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
> >> On 24.09.2020 10:28, Joerg Roedel wrote:
> >>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
> >>>> It allows to remap given buffer at the specific IOVA address, although
> >>>> it doesn't guarantee that those specific addresses won't be later used
> >>>> by the IOVA allocator. Probably it would make sense to add an API for
> >>>> generic IOMMU-DMA framework to mark the given IOVA range as
> >>>> reserved/unused to protect them.
> >>> There is an API for that, the IOMMU driver can return IOVA reserved
> >>> regions per device and the IOMMU core code will take care of mapping
> >>> these regions and reserving them in the IOVA allocator, so that
> >>> DMA-IOMMU code will not use it for allocations.
> >>>
> >>> Have a look at the iommu_ops->get_resv_regions() and
> >>> iommu_ops->put_resv_regions().
> >> I know about the reserved regions IOMMU API, but the main problem here,
> >> in case of Exynos, is that those reserved regions won't be created by
> >> the IOMMU driver but by the IOMMU client device. It is just a result how
> >> the media drivers manages their IOVA space. They simply have to load
> >> firmware at the IOVA address lower than the any address of the used
> >> buffers.
> > I've been working on adding a way to automatically add direct mappings
> > using reserved-memory regions parsed from device tree, see:
> >
> >      https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/
> >
> > Perhaps this can be of use? With that you should be able to add a
> > reserved-memory region somewhere in the lower range that you need for
> > firmware images and have that automatically added as a direct mapping
> > so that it won't be reused later on for dynamic allocations.
> 
> Frankly, using that would be even bigger hack than what I've proposed in 
> my workaround. I see no point polluting DT with such artificial regions 
> just to ensure specific IOVA space layout.

I think I misunderstood the requirements that you have. Sounds like
there are no actual restrictions for where exactly the memory resides
for the firmware, it just has to be lower than any of the buffer
allocations. I agree, in that case using reserved memory regions does
not make sense at all.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: IOVA allocation dependency between firmware buffer and remaining buffers

[Marek Szyprowski]

Hi Robin,

On 24.09.2020 13:06, Robin Murphy wrote:
> On 2020-09-24 11:47, Marek Szyprowski wrote:
>> On 24.09.2020 12:40, Robin Murphy wrote:
>>> On 2020-09-24 11:16, Thierry Reding wrote:
>>>> On Thu, Sep 24, 2020 at 10:46:46AM +0200, Marek Szyprowski wrote:
>>>>> On 24.09.2020 10:28, Joerg Roedel wrote:
>>>>>> On Wed, Sep 23, 2020 at 08:48:26AM +0200, Marek Szyprowski wrote:
>>>>>>> It allows to remap given buffer at the specific IOVA address,
>>>>>>> although
>>>>>>> it doesn't guarantee that those specific addresses won't be later
>>>>>>> used
>>>>>>> by the IOVA allocator. Probably it would make sense to add an 
>>>>>>> API for
>>>>>>> generic IOMMU-DMA framework to mark the given IOVA range as
>>>>>>> reserved/unused to protect them.
>>>>>> There is an API for that, the IOMMU driver can return IOVA reserved
>>>>>> regions per device and the IOMMU core code will take care of mapping
>>>>>> these regions and reserving them in the IOVA allocator, so that
>>>>>> DMA-IOMMU code will not use it for allocations.
>>>>>>
>>>>>> Have a look at the iommu_ops->get_resv_regions() and
>>>>>> iommu_ops->put_resv_regions().
>>>>>
>>>>> I know about the reserved regions IOMMU API, but the main problem 
>>>>> here,
>>>>> in case of Exynos, is that those reserved regions won't be created by
>>>>> the IOMMU driver but by the IOMMU client device. It is just a result
>>>>> how
>>>>> the media drivers manages their IOVA space. They simply have to load
>>>>> firmware at the IOVA address lower than the any address of the used
>>>>> buffers.
>>>>
>>>> I've been working on adding a way to automatically add direct mappings
>>>> using reserved-memory regions parsed from device tree, see:
>>>>
>>>> https://lore.kernel.org/lkml/20200904130000.691933-1-thierry.reding@gmail.com/ 
>>>>
>>>>
>>>> Perhaps this can be of use? With that you should be able to add a
>>>> reserved-memory region somewhere in the lower range that you need for
>>>> firmware images and have that automatically added as a direct mapping
>>>> so that it won't be reused later on for dynamic allocations.
>>>
>>> It can't easily be a *direct* mapping though - if the driver has to
>>> use the DMA masks to ensure that everything stays within the
>>> addressable range, then (as far as I'm aware) there's no physical
>>> memory that low down to equal the DMA addresses.
>>>
>>> TBH I'm not convinced that this is a sufficiently common concern to
>>> justify new APIs, or even to try to make overly generic. I think just
>>> implementing a new DMA attribute to say "please allocate/map this
>>> particular request at the lowest DMA address possible" would be good
>>> enough. Such a thing could also serve PCI drivers that actually care
>>> about SAC/DAC to give us more of a chance of removing the "try a
>>> 32-bit mask first" trick from everyone's hotpath...
>>
>> Hmm, I like the idea of such DMA attribute! It should make things really
>> simple, especially in the drivers. Thanks for the great idea! I will try
>> to implement it then instead of the workarounds I've proposed in
>> s5p-mfc/exynos4-is drivers.
>
> Right, I think it's fair to draw a line and say that anyone who wants 
> a *specific* address needs to manage their own IOMMU domain.
>
> In the backend I suspect it's going to be cleanest to implement a 
> dedicated iova_alloc_low() (or similar) function in the IOVA API that 
> sidesteps all of the existing allocation paths and goes straight to 
> the rbtree.

Just for the record - I've implemented this approach here: 
https://lore.kernel.org/lkml/20200925141218.13550-1-m.szyprowski@samsung.com/

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland