From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com>
The property "mediatek,gce-events" is used for GCE event ID corresponding
to a hardware event signal sent by the hardware or a software driver.
If the mailbox providers or consumers want to manipulate the value of
the event ID, they need to know the specific event ID.
Since mediatek,gce-events property is used for both mailbox producers
and consumers, we add a mediatek,gce-props.yaml to place the common GCE
properties like mediatek,gce-events.
Change in v4:
1. Fix some typo.
2. Change maxItems of gce-events from 1024 to 32.
Change in v3:
1. Add more description and fix typo and grammar.
2. Fix $ref as full path.
Change in v2:
1. Add mediatek,gce-props.yaml for other binding reference.
Jason-JH.Lin (3):
dt-bindings: mailbox: Add mediatek,gce-props.yaml
dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to
reference
dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece
.../bindings/mailbox/mediatek,gce-props.yaml | 52 +++++++++++++++++++
.../bindings/media/mediatek,mdp3-rdma.yaml | 11 ++--
.../bindings/media/mediatek,mdp3-rsz.yaml | 12 ++---
.../bindings/media/mediatek,mdp3-wrot.yaml | 12 ++---
.../bindings/soc/mediatek/mediatek,ccorr.yaml | 12 ++---
.../bindings/soc/mediatek/mediatek,mutex.yaml | 11 ++--
.../bindings/soc/mediatek/mediatek,wdma.yaml | 12 ++---
7 files changed, 74 insertions(+), 48 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
--
2.18.0
Hello Felix Kuehling,
The patch 1819200166ce: "drm/amdkfd: Export DMABufs from KFD using
GEM handles" from Aug 24, 2023 (linux-next), leads to the following
Smatch static checker warning:
drivers/dma-buf/dma-buf.c:729 dma_buf_get()
warn: fd used after fd_install() 'fd'
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
809 static int kfd_mem_export_dmabuf(struct kgd_mem *mem)
810 {
811 if (!mem->dmabuf) {
812 struct amdgpu_device *bo_adev;
813 struct dma_buf *dmabuf;
814 int r, fd;
815
816 bo_adev = amdgpu_ttm_adev(mem->bo->tbo.bdev);
817 r = drm_gem_prime_handle_to_fd(&bo_adev->ddev, bo_adev->kfd.client.file,
818 mem->gem_handle,
819 mem->alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE ?
820 DRM_RDWR : 0, &fd);
^^^
The drm_gem_prime_handle_to_fd() function does an fd_install() and
returns the result as "fd".
821 if (r)
822 return r;
823 dmabuf = dma_buf_get(fd);
^^
Then we do another fget() inside dma_buf_get(). I'm not an expert,
but this looks wrong. We can't assume that the dmabuf here is the
same one from drm_gem_prime_handle_to_fd() because the user could
change it after the fd_install(). I suspect drm_gem_prime_handle_to_fd()
should pass the dmabuf back instead.
We had several CVEs similar to this such as CVE-2022-1998.
824 close_fd(fd);
825 if (WARN_ON_ONCE(IS_ERR(dmabuf)))
826 return PTR_ERR(dmabuf);
827 mem->dmabuf = dmabuf;
828 }
829
830 return 0;
831 }
regards,
dan carpenter
From: Jason-jh Lin <jason-jh.lin(a)mediatek.corp-partner.google.com>
The property "mediatek,gce-events" is used for GCE event ID corresponding
to a hardware event signal sent by the hardware or a sofware driver.
If the mailbox providers or consumers want to manipulate the value of
the event ID, they need to know the specific event ID.
Since mediatek,gce-events property is used for both mailbox producers
and consumers, we add a mediatek,gce-props.yaml to place the common GCE
properties like mediatek,gce-events.
Change in v3:
1. Add more description and fix typo and grammar.
2. Fix $ref as full path.
Change in v2:
1. Add mediatek,gce-props.yaml for other binding reference.
Jason-JH.Lin (3):
dt-bindings: mailbox: Add mediatek,gce-props.yaml
dt-bindings: media: mediatek: mdp: Change mediatek,gce-events to
reference
dt-bindings: soc: mediatek: Change mediatek,gce-events to refernece
.../bindings/mailbox/mediatek,gce-props.yaml | 52 +++++++++++++++++++
.../bindings/media/mediatek,mdp3-rdma.yaml | 11 ++--
.../bindings/media/mediatek,mdp3-rsz.yaml | 12 ++---
.../bindings/media/mediatek,mdp3-wrot.yaml | 12 ++---
.../bindings/soc/mediatek/mediatek,ccorr.yaml | 12 ++---
.../bindings/soc/mediatek/mediatek,mutex.yaml | 11 ++--
.../bindings/soc/mediatek/mediatek,wdma.yaml | 12 ++---
7 files changed, 74 insertions(+), 48 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
--
2.18.0
In an effort to separate intentional arithmetic wrap-around from
unexpected wrap-around, we need to refactor places that depend on this
kind of math. One of the most common code patterns of this is:
VAR + value < VAR
Notably, this is considered "undefined behavior" for signed and pointer
types, which the kernel works around by using the -fno-strict-overflow
option in the build[1] (which used to just be -fwrapv). Regardless, we
want to get the kernel source to the position where we can meaningfully
instrument arithmetic wrap-around conditions and catch them when they
are unexpected, regardless of whether they are signed[2], unsigned[3],
or pointer[4] types.
Refactor open-coded unsigned wrap-around addition test to use
check_add_overflow(), retaining the result for later usage (which removes
the redundant open-coded addition). This paves the way to enabling the
wrap-around sanitizers in the future.
Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1]
Link: https://github.com/KSPP/linux/issues/26 [2]
Link: https://github.com/KSPP/linux/issues/27 [3]
Link: https://github.com/KSPP/linux/issues/344 [4]
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: "Christian König" <christian.koenig(a)amd.com>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: linaro-mm-sig(a)lists.linaro.org
Signed-off-by: Kees Cook <keescook(a)chromium.org>
---
drivers/dma-buf/dma-buf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 8fe5aa67b167..3743c63a9b59 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -1458,6 +1458,8 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_end_cpu_access, DMA_BUF);
int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
unsigned long pgoff)
{
+ unsigned long sum;
+
if (WARN_ON(!dmabuf || !vma))
return -EINVAL;
@@ -1466,12 +1468,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma,
return -EINVAL;
/* check for offset overflow */
- if (pgoff + vma_pages(vma) < pgoff)
+ if (check_add_overflow(pgoff, vma_pages(vma), &sum))
return -EOVERFLOW;
/* check for overflowing the buffer's size */
- if (pgoff + vma_pages(vma) >
- dmabuf->size >> PAGE_SHIFT)
+ if (sum > dmabuf->size >> PAGE_SHIFT)
return -EINVAL;
/* readjust the vma */
--
2.34.1
Hi Frank,
Le vendredi 19 janvier 2024 à 16:49 -0500, Frank Li a écrit :
> On Wed, Jan 17, 2024 at 01:26:43PM +0100, Paul Cercueil wrote:
> > Add a new 'sg_was_mapped' field to the struct usb_request. This
> > field
> > can be used to indicate that the scatterlist associated to the USB
> > transfer has already been mapped into the DMA space, and it does
> > not
> > have to be done internally.
> >
> > Signed-off-by: Paul Cercueil <paul(a)crapouillou.net>
> > ---
> > drivers/usb/gadget/udc/core.c | 7 ++++++-
> > include/linux/usb/gadget.h | 2 ++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/udc/core.c
> > b/drivers/usb/gadget/udc/core.c
> > index d59f94464b87..9d4150124fdb 100644
> > --- a/drivers/usb/gadget/udc/core.c
> > +++ b/drivers/usb/gadget/udc/core.c
> > @@ -903,6 +903,11 @@ int usb_gadget_map_request_by_dev(struct
> > device *dev,
> > if (req->length == 0)
> > return 0;
> >
> > + if (req->sg_was_mapped) {
> > + req->num_mapped_sgs = req->num_sgs;
> > + return 0;
> > + }
> > +
> > if (req->num_sgs) {
> > int mapped;
> >
> > @@ -948,7 +953,7 @@ EXPORT_SYMBOL_GPL(usb_gadget_map_request);
> > void usb_gadget_unmap_request_by_dev(struct device *dev,
> > struct usb_request *req, int is_in)
> > {
> > - if (req->length == 0)
> > + if (req->length == 0 || req->sg_was_mapped)
> > return;
> >
> > if (req->num_mapped_sgs) {
> > diff --git a/include/linux/usb/gadget.h
> > b/include/linux/usb/gadget.h
> > index a771ccc038ac..c529e4e06997 100644
> > --- a/include/linux/usb/gadget.h
> > +++ b/include/linux/usb/gadget.h
> > @@ -52,6 +52,7 @@ struct usb_ep;
> > * @short_not_ok: When reading data, makes short packets be
> > * treated as errors (queue stops advancing till cleanup).
> > * @dma_mapped: Indicates if request has been mapped to DMA
> > (internal)
> > + * @sg_was_mapped: Set if the scatterlist has been mapped before
> > the request
> > * @complete: Function called when request completes, so this
> > request and
> > * its buffer may be re-used. The function will always be
> > called with
> > * interrupts disabled, and it must not sleep.
> > @@ -111,6 +112,7 @@ struct usb_request {
> > unsigned zero:1;
> > unsigned short_not_ok:1;
> > unsigned dma_mapped:1;
> > + unsigned sg_was_mapped:1;
>
> why not use dma_mapped direclty?
Because of the unmap case. We want to know whether we should unmap or
not.
>
> Frank
Cheers,
-Paul
>
> >
> > void (*complete)(struct usb_ep *ep,
> > struct usb_request *req);
> > --
> > 2.43.0
> >
Hi,
This is the v4 of my patchset that adds a new DMABUF import interface to
FunctionFS. It addresses the points that Daniel raised on the v3 - see
changelog below.
This interface is being used at Analog Devices, to transfer data from
high-speed transceivers to USB in a zero-copy fashion, using also the
DMABUF import interface to the IIO subsystem which is being upstreamed
in parallel [1]. The two are used by the Libiio software [2].
On a ZCU102 board with a FMComms3 daughter board, using the combination
of these two new interfaces yields a drastic improvement of the
throughput, from about 127 MiB/s using IIO's buffer read/write interface
+ read/write to the FunctionFS endpoints, to about 274 MiB/s when
passing around DMABUFs, for a lower CPU usage (0.85 load avg. before,
vs. 0.65 after).
Right now, *technically* there are no users of this interface, as
Analog Devices wants to wait until both interfaces are accepted upstream
to merge the DMABUF code in Libiio into the main branch, and Jonathan
wants to wait and see if this patchset is accepted to greenlight the
DMABUF interface in IIO as well. I think this isn't really a problem;
once everybody is happy with its part of the cake, we can merge them all
at once.
This is obviously for 5.9, and based on next-20240117.
Changelog:
- [3/4]:
- Protect the dmabufs list with a mutex
- Use incremental sequence number for the dma_fences
- Unref attachments and DMABUFs in workers
- Remove dead code in ffs_dma_resv_lock()
- Fix non-block actually blocking
- Use dma_fence_begin/end_signalling()
- Add comment about cache-management and dma_buf_unmap_attachment()
- Make sure dma_buf_map_attachment() is called with the dma-resv locked
Cheers,
-Paul
[1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.…
[2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api
Paul Cercueil (4):
usb: gadget: Support already-mapped DMA SGs
usb: gadget: functionfs: Factorize wait-for-endpoint code
usb: gadget: functionfs: Add DMABUF import interface
Documentation: usb: Document FunctionFS DMABUF API
Documentation/usb/functionfs.rst | 36 ++
drivers/usb/gadget/function/f_fs.c | 500 ++++++++++++++++++++++++++--
drivers/usb/gadget/udc/core.c | 7 +-
include/linux/usb/gadget.h | 2 +
include/uapi/linux/usb/functionfs.h | 41 +++
5 files changed, 565 insertions(+), 21 deletions(-)
--
2.43.0
Hi,
This small patchset adds three new IOCTLs that can be used to attach,
detach, or transfer from/to a DMABUF object.
This interface is being used at Analog Devices, to transfer data from
high-speed transceivers to USB in a zero-copy fashion, using also the
DMABUF import interface to the IIO subsystem which is being upstreamed
in parallel [1] (and has high chances to be accepted for 5.9, I think
Jonathan can confirm). The two are used by the Libiio software [2].
On a ZCU102 board with a FMComms3 daughter board, using the combination
of these two new interfaces yields a drastic improvement of the
throughput, from about 127 MiB/s using IIO's buffer read/write interface
+ read/write to the FunctionFS endpoints, to about 274 MiB/s when
passing around DMABUFs, for a lower CPU usage (0.85 load avg. before,
vs. 0.65 after).
Right now, *technically* there are no users of this interface, as
Analog Devices wants to wait until both interfaces are accepted upstream
to merge the DMABUF code in Libiio into the main branch, and Jonathan
wants to wait and see if this patchset is accepted to greenlight the
DMABUF interface in IIO as well. I think this isn't really a problem;
once everybody is happy with its part of the cake, we can merge them all
at once.
This is obviously for 5.9, and based on next-20240108.
Changelog:
- [3/4]:
- Inline to_ffs_dma_fence() which was called only once.
- Simplify ffs_dma_resv_lock()
- Add comment explaining why we unref twice in ffs_dmabuf_detach()
- Document uapi struct usb_ffs_dmabuf_transfer_req and IOCTLs
- [4/4]: New patch, as I figured out having documentation wouldn't hurt.
Cheers,
-Paul
[1] https://lore.kernel.org/linux-iio/219abc43b4fdd4a13b307ed2efaa0e6869e68e3f.…
[2] https://github.com/analogdevicesinc/libiio/tree/pcercuei/dev-new-dmabuf-api
Paul Cercueil (4):
usb: gadget: Support already-mapped DMA SGs
usb: gadget: functionfs: Factorize wait-for-endpoint code
usb: gadget: functionfs: Add DMABUF import interface
Documentation: usb: Document FunctionFS DMABUF API
Documentation/usb/functionfs.rst | 36 +++
drivers/usb/gadget/function/f_fs.c | 463 ++++++++++++++++++++++++++--
drivers/usb/gadget/udc/core.c | 7 +-
include/linux/usb/gadget.h | 2 +
include/uapi/linux/usb/functionfs.h | 41 +++
5 files changed, 528 insertions(+), 21 deletions(-)
--
2.43.0
Hi Vegard,
Le mardi 09 janvier 2024 à 14:08 +0100, Vegard Nossum a écrit :
> On 08/01/2024 13:00, Paul Cercueil wrote:
> > Add documentation for the three ioctls used to attach or detach
> > externally-created DMABUFs, and to request transfers from/to
> > previously
> > attached DMABUFs.
> >
> > Signed-off-by: Paul Cercueil <paul(a)crapouillou.net>
> >
> > ---
> > v3: New patch
> > ---
> > Documentation/usb/functionfs.rst | 36
> > ++++++++++++++++++++++++++++++++
> > 1 file changed, 36 insertions(+)
>
> Hi,
>
> I'd like to point out that this file (usb/functionfs.rst) is
> currently
> included by Documentation/subsystem-apis.rst, the top-level file for
> the
> "Kernel subsystem documentation" set of books, which describe
> internal
> APIs: "These books get into the details of how specific kernel
> subsystems work from the point of view of a kernel developer".
>
> However, functionfs.rst (and especially your new additions) are
> documenting a userspace API, so it really belongs somewhere in
> Documentation/userspace-api/ -- that's where /proc, /sys, /dev and
> ioctl
> descriptions for userspace programmers belong.
Agreed. Even the original content prior to my additions describe a
userspace API.
>
> I'm not NAKing the patch -- I just want to draw attention to this
> discrepancy. Maybe we can separate the kernel-implementation details
> (stuff about __init sections and stuff) from the new ioctl() info?
>
> Looking at <https://docs.kernel.org/usb/> I see that there are many
> other adjacent documents that are also not really documenting kernel
> implementation details, rough categorization as follows:
>
> USB support
> -----------
>
> - Linux ACM driver v0.16 ==> admin/user info
> - Authorizing (or not) your USB devices to connect to the system ==>
> admin/user info
> - ChipIdea Highspeed Dual Role Controller Driver => admin/user info
> - DWC3 driver ==> driver TODOs (can be moved into source code?)
> - EHCI driver ==> technical info + driver details
> - How FunctionFS works
> - Linux USB gadget configured through configfs ==> userspace API +
> implementation
> - Linux USB HID gadget driver ==> implementation + userspace API
> - Multifunction Composite Gadget ==> technical + user info
> - Linux USB Printer Gadget Driver ==> userspace API
> - Linux Gadget Serial Driver v2.0 ==> user/admin + userspace API
> - Linux UVC Gadget Driver ==> user/admin + userspace API
> - Gadget Testing ==> user/admin + userspace API
> - Infinity Usb Unlimited Readme ==> user/admin
> - Mass Storage Gadget (MSG) ==> user/admin
> - USB 7-Segment Numeric Display ==> user/admin
> - mtouchusb driver ==> user/admin
> - OHCI ==> technical info
> - USB Raw Gadget ==> userspace API
> - USB/IP protocol ==> technical info
> - usbmon ==> user/admin + userspace API
> - USB serial ==> user/admin + technical info
> - USB references
> - Linux CDC ACM inf
> - Linux inf
> - USB devfs drop permissions source
> - Credits
>
> By "admin/user info", I mean things that a user would have to do or
> run
> (e.g. modprobe + flags) to make use of a driver; "technical info" is
> more like device specifications (transfer speeds, modes of operation,
> etc.); "userspace API" is stuff like configfs and ioctls; "driver
> details" is really implementation details and internal
> considerations.
>
> The last ones I don't even really know how to categorize.
>
> I'm guessing nobody is really enthralled by the idea of splitting
> Documentation/usb/ up like this?
>
> Documentation/admin-guide/usb/
> Documentation/driver-api/usb/ (this one actually exists already)
> Documentation/userspace-api/usb/
>
> For the stuff that is _actually_ internal to a specific driver (so
> not
> useful for end users, not useful for admins, not generic USB info,
> and
> not useful for userspace programmers), I would honestly propose to
> just
> move it directly into the driver's source code, or, if the text is
> obsolete, just get rid of it completely.
>
> The distinction between user/admin and userspace API is pretty clear
> (one is for end users, the other is for userspace _programmers_), but
> it
> can sometimes be hard to determine whether something falls in one or
> the
> other category.
>
> In any case -- it looks like almost all of the usb/ directory does
> not
> document "how specific kernel subsystems work from the point of view
> of
> a kernel developer" so maybe we should just move the include to
> userspace-api/ for now as an obvious improvement (if still not 100%
> correct):
>
> diff --git a/Documentation/subsystem-apis.rst
> b/Documentation/subsystem-apis.rst
> index 2d353fb8ea26..fe972f57bf4c 100644
> --- a/Documentation/subsystem-apis.rst
> +++ b/Documentation/subsystem-apis.rst
> @@ -81,7 +81,6 @@ Storage interfaces
> security/index
> crypto/index
> bpf/index
> - usb/index
> PCI/index
> misc-devices/index
> peci/index
> diff --git a/Documentation/userspace-api/index.rst
> b/Documentation/userspace-api/index.rst
> index 82f9dbd228f5..e60cd9174ada 100644
> --- a/Documentation/userspace-api/index.rst
> +++ b/Documentation/userspace-api/index.rst
> @@ -41,6 +41,7 @@ Subsystem-specific documentation:
> tee
> isapnp
> dcdbas
> + ../usb/index
>
> Kernel ABIs: These documents describe the the ABI between the Linux
> kernel and userspace, and the relative stability of these
> interfaces.
>
>
> Thoughts?
Makes sense to me. There's definitely some cleanup to be done in the
USB documentation.
> Vegard
Cheers,
-Paul
Fix spelling mistakes as reported by codespell.
Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linaro-mm-sig(a)lists.linaro.org
---
drivers/dma-buf/dma-resv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff -- a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -405,7 +405,7 @@ static void dma_resv_iter_walk_unlocked(
*
* Beware that the iterator can be restarted. Code which accumulates statistics
* or similar needs to check for this with dma_resv_iter_is_restarted(). For
- * this reason prefer the locked dma_resv_iter_first() whenver possible.
+ * this reason prefer the locked dma_resv_iter_first() whenever possible.
*
* Returns the first fence from an unlocked dma_resv obj.
*/
@@ -428,7 +428,7 @@ EXPORT_SYMBOL(dma_resv_iter_first_unlock
*
* Beware that the iterator can be restarted. Code which accumulates statistics
* or similar needs to check for this with dma_resv_iter_is_restarted(). For
- * this reason prefer the locked dma_resv_iter_next() whenver possible.
+ * this reason prefer the locked dma_resv_iter_next() whenever possible.
*
* Returns the next fence from an unlocked dma_resv obj.
*/
Fix spelling mistakes as reported by codespell.
Signed-off-by: Randy Dunlap <rdunlap(a)infradead.org>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Gustavo Padovan <gustavo(a)padovan.org>
Cc: linux-media(a)vger.kernel.org
Cc: dri-devel(a)lists.freedesktop.org
Cc: Christian König <christian.koenig(a)amd.com>
Cc: linaro-mm-sig(a)lists.linaro.org
---
drivers/dma-buf/dma-fence.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff -- a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -102,7 +102,7 @@ static atomic64_t dma_fence_context_coun
*
* * Drivers are allowed to call dma_fence_wait() from their &mmu_notifier
* respectively &mmu_interval_notifier callbacks. This means any code required
- * for fence completeion cannot allocate memory with GFP_NOFS or GFP_NOIO.
+ * for fence completion cannot allocate memory with GFP_NOFS or GFP_NOIO.
* Only GFP_ATOMIC is permissible, which might fail.
*
* Note that only GPU drivers have a reasonable excuse for both requiring
@@ -522,7 +522,7 @@ dma_fence_wait_timeout(struct dma_fence
EXPORT_SYMBOL(dma_fence_wait_timeout);
/**
- * dma_fence_release - default relese function for fences
+ * dma_fence_release - default release function for fences
* @kref: &dma_fence.recfount
*
* This is the default release functions for &dma_fence. Drivers shouldn't call
@@ -974,8 +974,8 @@ void dma_fence_set_deadline(struct dma_f
EXPORT_SYMBOL(dma_fence_set_deadline);
/**
- * dma_fence_describe - Dump fence describtion into seq_file
- * @fence: the 6fence to describe
+ * dma_fence_describe - Dump fence description into seq_file
+ * @fence: the fence to describe
* @seq: the seq_file to put the textual description into
*
* Dump a textual description of the fence and it's state into the seq_file.
This patchset is for secure video playback and enables other potential
uses in the future. The 'secure dma-heap' will be used to allocate dma_buf
objects that reference memory in the secure world that is inaccessible/
unmappable by the non-secure (i.e.kernel/userspace) world. That memory
will be used by the secure world to store secure information (i.e.
decrypted media content). The dma_bufs allocated from the kernel will be
passed to V4L2 for video decoding (as input and output). They will also be
used by the drm system for rendering of the content.
This patchset adds two secure heaps and they will be used v4l2[1] and drm[2].
1) secure_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path).
The buffer is reserved for the secure world after bootup and it is used
for vcodec's ES/working buffer;
2) secure_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is
dynamically reserved for the secure world and will be got when we start
playing secure videos, Once the security video playing is complete, the
CMA will be released. This heap is used for the vcodec's frame buffer.
[1] https://lore.kernel.org/linux-mediatek/20231206081538.17056-1-yunfei.dong@m…
[2] https://lore.kernel.org/linux-mediatek/20231023044549.21412-1-jason-jh.lin@…
Change note:
v3: Base on v6.7-rc1.
1) Separate the secure heap into a common file(secure_heap.c) and a mtk special file
(secure_heap_mtk.c), and put all tee related code into our special file.
2) About dt-binding,
a) Add "mediatek," prefix since this is Mediatek TEE firmware definition.
b) Mute dt-binding check waring.
3) Remove the normal CMA heap which is a draft for qcom.
v2: https://lore.kernel.org/linux-mediatek/20231111111559.8218-1-yong.wu@mediat…
1) Move John's patches into the vcodec patchset since they use the new
dma heap interface directly.
https://lore.kernel.org/linux-mediatek/20231106120423.23364-1-yunfei.dong@m…
2) Reword the dt-binding description.
3) Rename the heap name from mtk_svp to secure_mtk_cm.
This means the current vcodec/DRM upstream code doesn't match this.
4) Add a normal CMA heap. currently it should be a draft version.
5) Regarding the UUID, I still use hard code, but put it in a private
data which allow the others could set their own UUID. What's more, UUID
is necessary for the session with TEE. If we don't have it, we can't
communicate with the TEE, including the get_uuid interface, which tries
to make uuid more generic, not working. If there is other way to make
UUID more general, please free to tell me.
v1: https://lore.kernel.org/linux-mediatek/20230911023038.30649-1-yong.wu@media…
Base on v6.6-rc1.
Yong Wu (7):
dt-bindings: reserved-memory: Add mediatek,dynamic-secure-region
dma-buf: heaps: Initialize a secure heap
dma-buf: heaps: secure_heap: Add private heap ops
dma-buf: heaps: secure_heap: Add dma_ops
dma-buf: heaps: secure_heap: Add MediaTek secure heap and heap_init
dma-buf: heaps: secure_heap_mtk: Add tee memory service call
dma_buf: heaps: secure_heap_mtk: Add a new CMA heap
.../mediatek,dynamic-secure-region.yaml | 43 +++
drivers/dma-buf/heaps/Kconfig | 13 +
drivers/dma-buf/heaps/Makefile | 2 +
drivers/dma-buf/heaps/secure_heap.c | 234 +++++++++++++
drivers/dma-buf/heaps/secure_heap.h | 43 +++
drivers/dma-buf/heaps/secure_heap_mtk.c | 321 ++++++++++++++++++
6 files changed, 656 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,dynamic-secure-region.yaml
create mode 100644 drivers/dma-buf/heaps/secure_heap.c
create mode 100644 drivers/dma-buf/heaps/secure_heap.h
create mode 100644 drivers/dma-buf/heaps/secure_heap_mtk.c
--
2.18.0
On Mon, Dec 11, 2023 at 2:58 AM Hans Verkuil <hverkuil-cisco(a)xs4all.nl> wrote:
>
> On 06/12/2023 09:15, Yunfei Dong wrote:
> > From: Jeffrey Kardatzke <jkardatzke(a)google.com>
> >
> > Verfies in the dmabuf implementations that if the secure memory flag is
>
> Verfies -> Verifies
Thanks. Yunfei, change that please.
>
> > set for a queue that the dmabuf submitted to the queue is unmappable.
> >
> > Signed-off-by: Jeffrey Kardatzke <jkardatzke(a)google.com>
> > Signed-off-by: Yunfei Dong <yunfei.dong(a)mediatek.com>
> > ---
> > drivers/media/common/videobuf2/videobuf2-dma-contig.c | 6 ++++++
> > drivers/media/common/videobuf2/videobuf2-dma-sg.c | 6 ++++++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > index 3d4fd4ef5310..ad58ef8dc231 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> > @@ -710,6 +710,12 @@ static int vb2_dc_map_dmabuf(void *mem_priv)
> > return -EINVAL;
> > }
> >
> > + /* verify the dmabuf is secure if we are in secure mode */
> > + if (buf->vb->vb2_queue->secure_mem && sg_page(sgt->sgl)) {
>
> This needs a bit more explanation. I guess that for secure memory
> sg_page returns NULL?
How about if we change it to:
/* verify the dmabuf is secure if we are in secure mode, this is done
by validating there is no page entry for the dmabuf */
>
> > + pr_err("secure queue requires secure dma_buf");
> > + return -EINVAL;
> > + }
> > +
> > /* checking if dmabuf is big enough to store contiguous chunk */
> > contig_size = vb2_dc_get_contiguous_size(sgt);
> > if (contig_size < buf->size) {
> > diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > index 28f3fdfe23a2..55428c73c380 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> > @@ -564,6 +564,12 @@ static int vb2_dma_sg_map_dmabuf(void *mem_priv)
> > return -EINVAL;
> > }
> >
> > + /* verify the dmabuf is secure if we are in secure mode */
> > + if (buf->vb->vb2_queue->secure_mem && !sg_dma_secure(sgt->sgl)) {
>
> I can't find the sg_dma_secure function. I suspect this patch series
> depends on another series?
That was an oversight, it should be the same as in
videobuf2-dma-contig.c. Yunfei, can you change this to match what's in
videobuf2-dma-contig.c after the comment is reworded?
>
> > + pr_err("secure queue requires secure dma_buf");
> > + return -EINVAL;
> > + }
> > +
> > buf->dma_sgt = sgt;
> > buf->vaddr = NULL;
> >
>
> Regards,
>
> Hans
On Mon, Dec 11, 2023 at 3:05 AM Hans Verkuil <hverkuil-cisco(a)xs4all.nl> wrote:
>
> On 06/12/2023 09:15, Yunfei Dong wrote:
> > From: Jeffrey Kardatzke <jkardatzke(a)google.com>
> >
> > Adds documentation for V4L2_MEMORY_FLAG_SECURE.
> >
> > Signed-off-by: Jeffrey Kardatzke <jkardatzke(a)google.com>
> > Signed-off-by: Yunfei Dong <yunfei.dong(a)mediatek.com>
> > ---
> > Documentation/userspace-api/media/v4l/buffer.rst | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> > index 52bbee81c080..a5a7d1c72d53 100644
> > --- a/Documentation/userspace-api/media/v4l/buffer.rst
> > +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> > @@ -696,7 +696,7 @@ enum v4l2_memory
> >
> > .. _memory-flags:
> >
> > -Memory Consistency Flags
> > +Memory Flags
> > ------------------------
> >
> > .. raw:: latex
> > @@ -728,6 +728,12 @@ Memory Consistency Flags
> > only if the buffer is used for :ref:`memory mapping <mmap>` I/O and the
> > queue reports the :ref:`V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS
> > <V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS>` capability.
> > + * .. _`V4L2-MEMORY-FLAG-SECURE`:
> > +
> > + - ``V4L2_MEMORY_FLAG_SECURE``
> > + - 0x00000002
> > + - DMA bufs passed into the queue will be validated to ensure they were
> > + allocated from a secure dma-heap.
>
> Hmm, that needs a bit more work. How about:
>
> - The queued buffers are expected to be in secure memory. If not, an error will be
> returned. This flag can only be used with ``V4L2_MEMORY_DMABUF``. Typically
> secure buffers are allocated using a secure dma-heap. This flag can only be
> specified if the ``V4L2_BUF_CAP_SUPPORTS_SECURE_MEM`` is set.
>
Thanks Hans. Yunfei, can you integrate this change into the patch please?
> In addition, the title of this table is currently "Memory Consistency Flags": that
> should be renamed to "Memory Flags".
Hans, the patch is already renaming the table as you suggested. :)
(unless there's some other spot I'm missing)
>
> Regards,
>
> Hans
>
> >
> > .. raw:: latex
> >
>
Am 28.12.23 um 03:57 schrieb Qi Zheng:
>
>
> On 2023/12/28 04:51, syzbot wrote:
>> Hello,
>>
>> syzbot found the following issue on:
>>
>> HEAD commit: 5254c0cbc92d Merge tag 'block-6.7-2023-12-22' of
>> git://git..
>> git tree: upstream
>> console+strace: https://syzkaller.appspot.com/x/log.txt?x=10cc6995e80000
>> kernel config:
>> https://syzkaller.appspot.com/x/.config?x=314e9ad033a7d3a7
>> dashboard link:
>> https://syzkaller.appspot.com/bug?extid=59dcc2e7283a6f5f5ba1
>> compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils
>> for Debian) 2.40
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13e35809e80000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=155d5fd6e80000
>>
>> Downloadable assets:
>> disk image:
>> https://storage.googleapis.com/syzbot-assets/ebe09a5995ee/disk-5254c0cb.raw…
>> vmlinux:
>> https://storage.googleapis.com/syzbot-assets/02178d7f5f98/vmlinux-5254c0cb.…
>> kernel image:
>> https://storage.googleapis.com/syzbot-assets/12307f47d87c/bzImage-5254c0cb.…
>>
>> The issue was bisected to:
>>
>> commit ea4452de2ae987342fadbdd2c044034e6480daad
>> Author: Qi Zheng <zhengqi.arch(a)bytedance.com>
>> Date: Fri Nov 18 10:00:11 2022 +0000
>>
>> mm: fix unexpected changes to {failslab|fail_page_alloc}.attr
>>
>> bisection log:
>> https://syzkaller.appspot.com/x/bisect.txt?x=13027f76e80000
>> final oops: https://syzkaller.appspot.com/x/report.txt?x=10827f76e80000
>> console output: https://syzkaller.appspot.com/x/log.txt?x=17027f76e80000
>>
>> IMPORTANT: if you fix the issue, please add the following tag to the
>> commit:
>> Reported-by: syzbot+59dcc2e7283a6f5f5ba1(a)syzkaller.appspotmail.com
>> Fixes: ea4452de2ae9 ("mm: fix unexpected changes to
>> {failslab|fail_page_alloc}.attr")
>>
>> R10: 0000000000000000 R11: 0000000000000246 R12: 00007efe98069194
>> R13: 00007efe97fd2210 R14: 0000000000000002 R15: 6972642f7665642f
>> </TASK>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 5107 at drivers/gpu/drm/drm_prime.c:227
>> drm_prime_destroy_file_private+0x43/0x60 drivers/gpu/drm/drm_prime.c:227
>
> The warning is caused by !RB_EMPTY_ROOT(&prime_fpriv->dmabufs):
>
> drm_prime_destroy_file_private
> --> WARN_ON(!RB_EMPTY_ROOT(&prime_fpriv->dmabufs));
>
> It seems irrelevant to the logic of fault injection. So I don't see
> why the commit ea4452de2ae9 can cause this warning. :(
Making an educated guess I strongly think syzbot incorrectly bisected this.
What basically happens is that a DRM test case crashes because a file
private data structure is destroyed before all DMA-bufs referring to it
are destroyed.
Looks like a random race condition in a test case to me. Question is
really what test is syzbot running and who is maintaining this test case?
Regards,
Christian.
>
>> Modules linked in:
>> CPU: 0 PID: 5107 Comm: syz-executor227 Not tainted
>> 6.7.0-rc6-syzkaller-00248-g5254c0cbc92d #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine,
>> BIOS Google 11/17/2023
>> RIP: 0010:drm_prime_destroy_file_private+0x43/0x60
>> drivers/gpu/drm/drm_prime.c:227
>> Code: 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 75 21 48 8b 83
>> 90 00 00 00 48 85 c0 75 06 5b e9 13 f1 93 fc e8 0e f1 93 fc 90 <0f>
>> 0b 90 5b e9 04 f1 93 fc e8 3f 9b ea fc eb d8 66 66 2e 0f 1f 84
>> RSP: 0018:ffffc90003bdf9e0 EFLAGS: 00010293
>> RAX: 0000000000000000 RBX: ffff888019f28378 RCX: ffffc90003bdf9b0
>> RDX: ffff888018ff9dc0 RSI: ffffffff84f380c2 RDI: ffff888019f28408
>> RBP: ffff888019f28000 R08: 0000000000000001 R09: 0000000000000001
>> R10: ffffffff8f193a57 R11: 0000000000000000 R12: ffff88814829a000
>> R13: ffff888019f282a8 R14: ffff88814829a068 R15: ffff88814829a0a0
>> FS: 0000000000000000(0000) GS:ffff8880b9800000(0000)
>> knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007efe98050410 CR3: 000000006d1ff000 CR4: 0000000000350ef0
>> Call Trace:
>> <TASK>
>> drm_file_free.part.0+0x738/0xb90 drivers/gpu/drm/drm_file.c:290
>> drm_file_free drivers/gpu/drm/drm_file.c:247 [inline]
>> drm_close_helper.isra.0+0x180/0x1f0 drivers/gpu/drm/drm_file.c:307
>> drm_release+0x22a/0x4f0 drivers/gpu/drm/drm_file.c:494
>> __fput+0x270/0xb70 fs/file_table.c:394
>> task_work_run+0x14d/0x240 kernel/task_work.c:180
>> exit_task_work include/linux/task_work.h:38 [inline]
>> do_exit+0xa8a/0x2ad0 kernel/exit.c:869
>> do_group_exit+0xd4/0x2a0 kernel/exit.c:1018
>> get_signal+0x23b5/0x2790 kernel/signal.c:2904
>> arch_do_signal_or_restart+0x90/0x7f0 arch/x86/kernel/signal.c:309
>> exit_to_user_mode_loop kernel/entry/common.c:168 [inline]
>> exit_to_user_mode_prepare+0x121/0x240 kernel/entry/common.c:204
>> __syscall_exit_to_user_mode_work kernel/entry/common.c:285 [inline]
>> syscall_exit_to_user_mode+0x1e/0x60 kernel/entry/common.c:296
>> do_syscall_64+0x4d/0x110 arch/x86/entry/common.c:89
>> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>> RIP: 0033:0x7efe98014769
>> Code: Unable to access opcode bytes at 0x7efe9801473f.
>> RSP: 002b:00007efe97fd2208 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
>> RAX: fffffffffffffe00 RBX: 00007efe9809c408 RCX: 00007efe98014769
>> RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00007efe9809c408
>> RBP: 00007efe9809c400 R08: 0000000000003131 R09: 0000000000003131
>> R10: 0000000000000000 R11: 0000000000000246 R12: 00007efe98069194
>> R13: 00007efe97fd2210 R14: 0000000000000002 R15: 6972642f7665642f
>> </TASK>
>>
>>
>> ---
>> This report is generated by a bot. It may contain errors.
>> See https://goo.gl/tpsmEJ for more information about syzbot.
>> syzbot engineers can be reached at syzkaller(a)googlegroups.com.
>>
>> syzbot will keep track of this issue. See:
>> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>> For information about bisection process see:
>> https://goo.gl/tpsmEJ#bisection
>>
>> If the report is already addressed, let syzbot know by replying with:
>> #syz fix: exact-commit-title
>>
>> If you want syzbot to run the reproducer, reply with:
>> #syz test: git://repo/address.git branch-or-commit-hash
>> If you attach or paste a git patch, syzbot will apply it before testing.
>>
>> If you want to overwrite report's subsystems, reply with:
>> #syz set subsystems: new-subsystem
>> (See the list of subsystem names on the web dashboard)
>>
>> If the report is a duplicate of another one, reply with:
>> #syz dup: exact-subject-of-another-report
>>
>> If you want to undo deduplication, reply with:
>> #syz undup
From: Rob Clark <robdclark(a)chromium.org>
Container fences have burner contexts, which makes the trick to store at
most one fence per context somewhat useless if we don't unwrap array or
chain fences.
Signed-off-by: Rob Clark <robdclark(a)chromium.org>
---
drivers/gpu/drm/scheduler/sched_main.c | 47 ++++++++++++++++++--------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index 9762464e3f99..16b550949c57 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -52,6 +52,7 @@
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/completion.h>
+#include <linux/dma-fence-unwrap.h>
#include <linux/dma-resv.h>
#include <uapi/linux/sched/types.h>
@@ -684,27 +685,14 @@ void drm_sched_job_arm(struct drm_sched_job *job)
}
EXPORT_SYMBOL(drm_sched_job_arm);
-/**
- * drm_sched_job_add_dependency - adds the fence as a job dependency
- * @job: scheduler job to add the dependencies to
- * @fence: the dma_fence to add to the list of dependencies.
- *
- * Note that @fence is consumed in both the success and error cases.
- *
- * Returns:
- * 0 on success, or an error on failing to expand the array.
- */
-int drm_sched_job_add_dependency(struct drm_sched_job *job,
- struct dma_fence *fence)
+static int drm_sched_job_add_single_dependency(struct drm_sched_job *job,
+ struct dma_fence *fence)
{
struct dma_fence *entry;
unsigned long index;
u32 id = 0;
int ret;
- if (!fence)
- return 0;
-
/* Deduplicate if we already depend on a fence from the same context.
* This lets the size of the array of deps scale with the number of
* engines involved, rather than the number of BOs.
@@ -728,6 +716,35 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job,
return ret;
}
+
+/**
+ * drm_sched_job_add_dependency - adds the fence as a job dependency
+ * @job: scheduler job to add the dependencies to
+ * @fence: the dma_fence to add to the list of dependencies.
+ *
+ * Note that @fence is consumed in both the success and error cases.
+ *
+ * Returns:
+ * 0 on success, or an error on failing to expand the array.
+ */
+int drm_sched_job_add_dependency(struct drm_sched_job *job,
+ struct dma_fence *fence)
+{
+ struct dma_fence_unwrap iter;
+ struct dma_fence *f;
+ int ret = 0;
+
+ dma_fence_unwrap_for_each (f, &iter, fence) {
+ dma_fence_get(f);
+ ret = drm_sched_job_add_single_dependency(job, f);
+ if (ret)
+ break;
+ }
+
+ dma_fence_put(fence);
+
+ return ret;
+}
EXPORT_SYMBOL(drm_sched_job_add_dependency);
/**
--
2.42.0
Hello,
syzbot found the following issue on:
HEAD commit: 2741f1b02117 string: use __builtin_memcpy() in strlcpy/str..
git tree: https://github.com/google/kmsan.git master
console+strace: https://syzkaller.appspot.com/x/log.txt?x=17bb33d1280000
kernel config: https://syzkaller.appspot.com/x/.config?x=753079601b2300f9
dashboard link: https://syzkaller.appspot.com/bug?extid=4fad2e57beb6397ab2fc
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16d669a5280000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14d8f095280000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/ebd05512d8d7/disk-2741f1b0.raw…
vmlinux: https://storage.googleapis.com/syzbot-assets/aa555b09582c/vmlinux-2741f1b0.…
kernel image: https://storage.googleapis.com/syzbot-assets/5ea0934e02cc/bzImage-2741f1b0.…
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4fad2e57beb6397ab2fc(a)syzkaller.appspotmail.com
=====================================================
BUG: KMSAN: uninit-value in drm_mode_setcrtc+0x1ad3/0x24a0 drivers/gpu/drm/drm_crtc.c:896
drm_mode_setcrtc+0x1ad3/0x24a0 drivers/gpu/drm/drm_crtc.c:896
drm_ioctl_kernel+0x5ae/0x730 drivers/gpu/drm/drm_ioctl.c:788
drm_ioctl+0xd12/0x1590 drivers/gpu/drm/drm_ioctl.c:891
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl+0x222/0x400 fs/ioctl.c:856
__x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
Uninit was created at:
slab_post_alloc_hook+0x12d/0xb60 mm/slab.h:716
slab_alloc_node mm/slub.c:3451 [inline]
__kmem_cache_alloc_node+0x4ff/0x8b0 mm/slub.c:3490
__do_kmalloc_node mm/slab_common.c:965 [inline]
__kmalloc+0x121/0x3c0 mm/slab_common.c:979
kmalloc_array include/linux/slab.h:596 [inline]
drm_mode_setcrtc+0x1dba/0x24a0 drivers/gpu/drm/drm_crtc.c:846
drm_ioctl_kernel+0x5ae/0x730 drivers/gpu/drm/drm_ioctl.c:788
drm_ioctl+0xd12/0x1590 drivers/gpu/drm/drm_ioctl.c:891
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl+0x222/0x400 fs/ioctl.c:856
__x64_sys_ioctl+0x96/0xe0 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
CPU: 1 PID: 4955 Comm: syz-executor275 Not tainted 6.4.0-rc4-syzkaller-g2741f1b02117 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/25/2023
=====================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller(a)googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
From: Rob Clark <robdclark(a)chromium.org>
Container fences have burner contexts, which makes the trick to store at
most one fence per context somewhat useless if we don't unwrap array or
chain fences.
Signed-off-by: Rob Clark <robdclark(a)chromium.org>
---
tbh, I'm not sure why we weren't doing this already, unless there is
something I'm overlooking
drivers/gpu/drm/scheduler/sched_main.c | 43 +++++++++++++++++---------
1 file changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index c2ee44d6224b..f59e5335afbb 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -41,20 +41,21 @@
* 4. Entities themselves maintain a queue of jobs that will be scheduled on
* the hardware.
*
* The jobs in a entity are always scheduled in the order that they were pushed.
*/
#include <linux/kthread.h>
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/completion.h>
+#include <linux/dma-fence-unwrap.h>
#include <linux/dma-resv.h>
#include <uapi/linux/sched/types.h>
#include <drm/drm_print.h>
#include <drm/drm_gem.h>
#include <drm/gpu_scheduler.h>
#include <drm/spsc_queue.h>
#define CREATE_TRACE_POINTS
#include "gpu_scheduler_trace.h"
@@ -665,41 +666,27 @@ void drm_sched_job_arm(struct drm_sched_job *job)
sched = entity->rq->sched;
job->sched = sched;
job->s_priority = entity->rq - sched->sched_rq;
job->id = atomic64_inc_return(&sched->job_id_count);
drm_sched_fence_init(job->s_fence, job->entity);
}
EXPORT_SYMBOL(drm_sched_job_arm);
-/**
- * drm_sched_job_add_dependency - adds the fence as a job dependency
- * @job: scheduler job to add the dependencies to
- * @fence: the dma_fence to add to the list of dependencies.
- *
- * Note that @fence is consumed in both the success and error cases.
- *
- * Returns:
- * 0 on success, or an error on failing to expand the array.
- */
-int drm_sched_job_add_dependency(struct drm_sched_job *job,
- struct dma_fence *fence)
+static int _add_dependency(struct drm_sched_job *job, struct dma_fence *fence)
{
struct dma_fence *entry;
unsigned long index;
u32 id = 0;
int ret;
- if (!fence)
- return 0;
-
/* Deduplicate if we already depend on a fence from the same context.
* This lets the size of the array of deps scale with the number of
* engines involved, rather than the number of BOs.
*/
xa_for_each(&job->dependencies, index, entry) {
if (entry->context != fence->context)
continue;
if (dma_fence_is_later(fence, entry)) {
dma_fence_put(entry);
@@ -709,20 +696,46 @@ int drm_sched_job_add_dependency(struct drm_sched_job *job,
}
return 0;
}
ret = xa_alloc(&job->dependencies, &id, fence, xa_limit_32b, GFP_KERNEL);
if (ret != 0)
dma_fence_put(fence);
return ret;
}
+
+/**
+ * drm_sched_job_add_dependency - adds the fence as a job dependency
+ * @job: scheduler job to add the dependencies to
+ * @fence: the dma_fence to add to the list of dependencies.
+ *
+ * Note that @fence is consumed in both the success and error cases.
+ *
+ * Returns:
+ * 0 on success, or an error on failing to expand the array.
+ */
+int drm_sched_job_add_dependency(struct drm_sched_job *job,
+ struct dma_fence *fence)
+{
+ struct dma_fence_unwrap iter;
+ struct dma_fence *f;
+ int ret = 0;
+
+ dma_fence_unwrap_for_each (f, &iter, fence) {
+ ret = _add_dependency(job, f);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
EXPORT_SYMBOL(drm_sched_job_add_dependency);
/**
* drm_sched_job_add_resv_dependencies - add all fences from the resv to the job
* @job: scheduler job to add the dependencies to
* @resv: the dma_resv object to get the fences from
* @usage: the dma_resv_usage to use to filter the fences
*
* This adds all fences matching the given usage from @resv to @job.
* Must be called with the @resv lock held.
--
2.39.2
From: Rob Clark <robdclark(a)chromium.org>
This is a re-post of the remaining patches from:
https://patchwork.freedesktop.org/series/114490/
Part of the hold-up of the remaining uabi patches was compositor
support, but now an MR for kwin exists:
https://invent.kde.org/plasma/kwin/-/merge_requests/4358
The syncobj userspace is:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21973
v1: https://patchwork.freedesktop.org/series/93035/
v2: Move filtering out of later deadlines to fence implementation
to avoid increasing the size of dma_fence
v3: Add support in fence-array and fence-chain; Add some uabi to
support igt tests and userspace compositors.
v4: Rebase, address various comments, and add syncobj deadline
support, and sync_file EPOLLPRI based on experience with perf/
freq issues with clvk compute workloads on i915 (anv)
v5: Clarify that this is a hint as opposed to a more hard deadline
guarantee, switch to using u64 ns values in UABI (still absolute
CLOCK_MONOTONIC values), drop syncobj related cap and driver
feature flag in favor of allowing count_handles==0 for probing
kernel support.
v6: Re-work vblank helper to calculate time of _start_ of vblank,
and work correctly if the last vblank event was more than a
frame ago. Add (mostly unrelated) drm/msm patch which also
uses the vblank helper. Use dma_fence_chain_contained(). More
verbose syncobj UABI comments. Drop DMA_FENCE_FLAG_HAS_DEADLINE_BIT.
v7: Fix kbuild complaints about vblank helper. Add more docs.
v8: Add patch to surface sync_file UAPI, and more docs updates.
v9: Repost the remaining patches that expose new uabi to userspace.
Rob Clark (3):
drm/syncobj: Add deadline support for syncobj waits
dma-buf/sync_file: Add SET_DEADLINE ioctl
dma-buf/sw_sync: Add fence deadline support
drivers/dma-buf/dma-fence.c | 3 +-
drivers/dma-buf/sw_sync.c | 82 ++++++++++++++++++++++++++++++++++
drivers/dma-buf/sync_debug.h | 2 +
drivers/dma-buf/sync_file.c | 19 ++++++++
drivers/gpu/drm/drm_syncobj.c | 64 ++++++++++++++++++++------
include/uapi/drm/drm.h | 17 +++++++
include/uapi/linux/sync_file.h | 22 +++++++++
7 files changed, 195 insertions(+), 14 deletions(-)
--
2.41.0
On Thu, Nov 30, 2023 at 06:14:16PM +0100, Marco Pagani wrote:
> This patch introduces an initial KUnit test suite for GEM objects
> backed by shmem buffers.
>
> Suggested-by: Javier Martinez Canillas <javierm(a)redhat.com>
> Signed-off-by: Marco Pagani <marpagan(a)redhat.com>
>
> v5:
> - using __drm_kunit_helper_alloc_drm_device() to avoid local struct
> v4:
> - Add missing MMU dependency for DRM_GEM_SHMEM_HELPER (kernel test robot)
> v3:
> - Explicitly cast pointers in the helpers
> - Removed unused pointer to parent dev in struct fake_dev
> - Test entries reordering in Kconfig and Makefile sent as a separate patch
> v2:
> - Improved description of test cases
> - Cleaner error handling using KUnit actions
> - Alphabetical order in Kconfig and Makefile
Applied to drm-misc-next, thanks!
Maxime
Am 22.11.23 um 17:05 schrieb Ramesh Errabolu:
> Fix the documentation of struct dma_buf members name and exp_name
> as to how these members are to be used and accessed.
>
> Signed-off-by: Ramesh Errabolu <Ramesh.Errabolu(a)amd.com>
Reviewed-by: Christian König <christian.koenig(a)amd.com>
> ---
> include/linux/dma-buf.h | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 3f31baa3293f..8ff4add71f88 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -343,16 +343,19 @@ struct dma_buf {
> /**
> * @exp_name:
> *
> - * Name of the exporter; useful for debugging. See the
> - * DMA_BUF_SET_NAME IOCTL.
> + * Name of the exporter; useful for debugging. Must not be NULL
> */
> const char *exp_name;
>
> /**
> * @name:
> *
> - * Userspace-provided name; useful for accounting and debugging,
> - * protected by dma_resv_lock() on @resv and @name_lock for read access.
> + * Userspace-provided name. Default value is NULL. If not NULL,
> + * length cannot be longer than DMA_BUF_NAME_LEN, including NIL
> + * char. Useful for accounting and debugging. Read/Write accesses
> + * are protected by @name_lock
> + *
> + * See the IOCTLs DMA_BUF_SET_NAME or DMA_BUF_SET_NAME_A/B
> */
> const char *name;
>
On Fri, Nov 24, 2023 at 11:15:12AM +0100, Marco Pagani wrote:
>
>
> On 2023-11-24 09:49, Maxime Ripard wrote:
> > Hi,
> >
> > On Thu, Nov 23, 2023 at 11:01:46AM +0100, Marco Pagani wrote:
> >> +static int drm_gem_shmem_test_init(struct kunit *test)
> >> +{
> >> + struct device *dev;
> >> + struct fake_dev {
> >> + struct drm_device drm_dev;
> >> + } *fdev;
> >> +
> >
> > [...]
> >
> >> +
> >> + /*
> >> + * The DRM core will automatically initialize the GEM core and create
> >> + * a DRM Memory Manager object which provides an address space pool
> >> + * for GEM objects allocation.
> >> + */
> >> + fdev = drm_kunit_helper_alloc_drm_device(test, dev, struct fake_dev,
> >> + drm_dev, DRIVER_GEM);
> >> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fdev);
> >
> > Sorry I missed it earlier, but you don't need the intermediate structure
> > if you use
> >
> > struct drm_device *drm;
> >
> > drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm), 0, DRIVER_GEM);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
> >
>
> I prefer to use drm_kunit_helper_alloc_drm_device() with the intermediate
> structure. It makes the code clearer, in my opinion. Initially, when
> developing the suite, I was using __drm_kunit_helper_alloc_drm_device()
> as most test suites do, but I feel the list of arguments including
> "sizeof(*drm), 0," is less straightforward to understand.
Then we can create an init helper, and you can allocate the drm_device
through drmm_kzalloc, but I'd like tests to use consistent constructs.
This can of course be made as a later patch: you use the same construct
the other tests do here, and later we work on the init function and
convert all tests to use it.
Maxime