lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <30aa1b85-5c38-4e96-9217-7e0208fd0f6b@amd.com>
Date: Thu, 5 Sep 2024 10:01:59 +0200
From: Christian König <christian.koenig@....com>
To: Jens Wiklander <jens.wiklander@...aro.org>
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-media@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linaro-mm-sig@...ts.linaro.org, op-tee@...ts.trustedfirmware.org,
 linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 Olivier Masse <olivier.masse@....com>,
 Thierry Reding <thierry.reding@...il.com>, Yong Wu <yong.wu@...iatek.com>,
 Sumit Semwal <sumit.semwal@...aro.org>,
 Benjamin Gaignard <benjamin.gaignard@...labora.com>,
 Brian Starkey <Brian.Starkey@....com>, John Stultz <jstultz@...gle.com>,
 "T . J . Mercier" <tjmercier@...gle.com>, Sumit Garg
 <sumit.garg@...aro.org>, Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Subject: Re: [RFC PATCH 1/4] dma-buf: heaps: restricted_heap: add no_map
 attribute

Am 05.09.24 um 08:56 schrieb Jens Wiklander:
> On Fri, Aug 30, 2024 at 10:47 AM Christian König
> <christian.koenig@....com> wrote:
>> Am 30.08.24 um 09:03 schrieb Jens Wiklander:
>>> Add a no_map attribute to struct restricted_heap_attachment and struct
>>> restricted_heap to skip the call to dma_map_sgtable() if set. This
>>> avoids trying to map a dma-buf that doens't refer to memory accessible
>>> by the kernel.
>> You seem to have a misunderstanding here dma_map_sgtable() is called to
>> map a table into IOMMU and not any kernel address space.
>>
>> So please explain why you need that?
> You're right, I had misunderstood dma_map_sgtable(). There's no need
> for the no_map attribute, so I'll remove it.
>
> Perhaps you have a suggestion on how to fix a problem when using
> dma_map_sgtable()?
>
> Without it, I'll have to assign a pointer to teedev->dev.dma_mask when
> using the restricted heap with the OP-TEE driver or there will be a
> warning in __dma_map_sg_attrs() ending with a failure when trying to
> register the dma-buf fd. OP-TEE is probed with a platform device, and
> taking the dma_mask pointer from that device works. Is that a good
> approach or is there a better way of assigning dma_mask?

Mhm, I don't know the full picture so I have to make some assumptions.

The teedev is just a virtual device which represents the restricted 
memory access paths of a real device?

If that is true then taking the dma-mask from the real device is most 
likely the right thing to do.

Regards,
Christian.

>
> Thanks,
> Jens
>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Jens Wiklander <jens.wiklander@...aro.org>
>>> ---
>>>    drivers/dma-buf/heaps/restricted_heap.c | 17 +++++++++++++----
>>>    drivers/dma-buf/heaps/restricted_heap.h |  2 ++
>>>    2 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/heaps/restricted_heap.c b/drivers/dma-buf/heaps/restricted_heap.c
>>> index 8bc8a5e3f969..4bf28e3727ca 100644
>>> --- a/drivers/dma-buf/heaps/restricted_heap.c
>>> +++ b/drivers/dma-buf/heaps/restricted_heap.c
>>> @@ -16,6 +16,7 @@
>>>    struct restricted_heap_attachment {
>>>        struct sg_table                 *table;
>>>        struct device                   *dev;
>>> +     bool no_map;
>>>    };
>>>
>>>    static int
>>> @@ -54,6 +55,8 @@ restricted_heap_memory_free(struct restricted_heap *rheap, struct restricted_buf
>>>    static int restricted_heap_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attachment)
>>>    {
>>>        struct restricted_buffer *restricted_buf = dmabuf->priv;
>>> +     struct dma_heap *heap = restricted_buf->heap;
>>> +     struct restricted_heap *rheap = dma_heap_get_drvdata(heap);
>>>        struct restricted_heap_attachment *a;
>>>        struct sg_table *table;
>>>
>>> @@ -70,6 +73,7 @@ static int restricted_heap_attach(struct dma_buf *dmabuf, struct dma_buf_attachm
>>>        sg_dma_mark_restricted(table->sgl);
>>>        a->table = table;
>>>        a->dev = attachment->dev;
>>> +     a->no_map = rheap->no_map;
>>>        attachment->priv = a;
>>>
>>>        return 0;
>>> @@ -92,9 +96,12 @@ restricted_heap_map_dma_buf(struct dma_buf_attachment *attachment,
>>>        struct sg_table *table = a->table;
>>>        int ret;
>>>
>>> -     ret = dma_map_sgtable(attachment->dev, table, direction, DMA_ATTR_SKIP_CPU_SYNC);
>>> -     if (ret)
>>> -             return ERR_PTR(ret);
>>> +     if (!a->no_map) {
>>> +             ret = dma_map_sgtable(attachment->dev, table, direction,
>>> +                                   DMA_ATTR_SKIP_CPU_SYNC);
>>> +             if (ret)
>>> +                     return ERR_PTR(ret);
>>> +     }
>>>        return table;
>>>    }
>>>
>>> @@ -106,7 +113,9 @@ restricted_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_t
>>>
>>>        WARN_ON(a->table != table);
>>>
>>> -     dma_unmap_sgtable(attachment->dev, table, direction, DMA_ATTR_SKIP_CPU_SYNC);
>>> +     if (!a->no_map)
>>> +             dma_unmap_sgtable(attachment->dev, table, direction,
>>> +                               DMA_ATTR_SKIP_CPU_SYNC);
>>>    }
>>>
>>>    static int
>>> diff --git a/drivers/dma-buf/heaps/restricted_heap.h b/drivers/dma-buf/heaps/restricted_heap.h
>>> index 7dec4b8a471b..94cc0842f70d 100644
>>> --- a/drivers/dma-buf/heaps/restricted_heap.h
>>> +++ b/drivers/dma-buf/heaps/restricted_heap.h
>>> @@ -27,6 +27,8 @@ struct restricted_heap {
>>>        unsigned long           cma_paddr;
>>>        unsigned long           cma_size;
>>>
>>> +     bool                    no_map;
>>> +
>>>        void                    *priv_data;
>>>    };
>>>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ