Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3706 discussions

Re: [PATCH] dma-buf: system_heap: Reject private mappings in system_heap_mmap()

by T.J. Mercier

On Fri, Oct 30, 2026 at 1:44 PM Chris Wiseman <chriswiseman(a)google.com> wrote: > > When commit 381295758792 ("dma-buf: system_heap: Rework system heap to > use sgtables instead of pagelists") stopped using heap_helpers.c and > added a custom system_heap_mmap(), the check rejecting private mappings > was dropped. > > Other exporters and buffer allocators reject private or copy-on-write > mappings in their mmap handlers: cma_heap_mmap() and mmap_udmabuf() > check that VM_SHARED or VM_MAYSHARE is set, and drm_gem_shmem_mmap() > and ttm_bo_mmap_obj() reject vma_is_cow_mapping(vma). > > However, system_heap_mmap() omits this check and calls remap_pfn_range() > even when userspace requests MAP_PRIVATE with PROT_WRITE. As a result, > subsequent CPU writes to a MAP_PRIVATE mapping of a system heap buffer > fault into do_wp_page() -> wp_page_copy(), silently allocating private > anonymous copy-on-write pages whose writes never reach the underlying > system heap buffer pages. > > Reject non-shared mappings in system_heap_mmap() with -EINVAL to match > cma_heap_mmap() and mmap_udmabuf(). > > Fixes: 381295758792 ("dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists") > Cc: stable(a)vger.kernel.org > Signed-off-by: Chris Wiseman <chriswiseman(a)google.com> Reviewed-by: T.J. Mercier <tjmercier(a)google.com> I wonder if we should have this in dma_buf_mmap_internal() though. Any thoughts Sumit or Christian?

1 week, 1 day

Re: [PATCH v4 00/25] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 02/11/2026 11:08, Ekansh Gupta wrote: > On 02-11-2026 14:23, Krzysztof Kozlowski wrote: >> On 01/11/2026 23:21, Ekansh Gupta wrote: >>> drivers/accel/qda/qda_soc.c | 66 ++ >>> drivers/accel/qda/qda_soc.h | 61 ++ >>> drivers/misc/Kconfig | 1 + >>> include/uapi/drm/qda_accel.h | 282 +++++++ >>> 33 files changed, 5877 insertions(+), 1 deletion(-) >>> --- >>> base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c >>> change-id: 20261102-v4-e8feb321263f >> >> What is happening here? How are we supposed to work with this? >> >> v1: 20260223-qda-firstpost-4ab05249e2cc >> v2: 20260519-qda-series-78c2bf0ed78b >> v2 again: 20260817-qda-v2-78e2d1f10529 >> >> And now completely different one? >> >> This makes the change id completely unusable. b4 diff obviously does not >> work. Any tracking of this does not work. >> >> NAK, really, I already complained internally months ago about such >> practice, so apparently not enough. >> >> Best regards, >> Krzysztof > > Sorry about this. I lost the original b4 branch and re-enrolled the You lost original branch four times. It's not a coincidence, it's not random data loss because then after first loss you would solve the problem. It's a broken workflow. Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 00/25] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > drivers/accel/qda/qda_soc.c | 66 ++ > drivers/accel/qda/qda_soc.h | 61 ++ > drivers/misc/Kconfig | 1 + > include/uapi/drm/qda_accel.h | 282 +++++++ > 33 files changed, 5877 insertions(+), 1 deletion(-) > --- > base-commit: 7649ba73f9e7e1e04d0b46213e39c79ae09dd55c > change-id: 20261102-v4-e8feb321263f What is happening here? How are we supposed to work with this? v1: 20260223-qda-firstpost-4ab05249e2cc v2: 20260519-qda-series-78c2bf0ed78b v2 again: 20260817-qda-v2-78e2d1f10529 And now completely different one? This makes the change id completely unusable. b4 diff obviously does not work. Any tracking of this does not work. NAK, really, I already complained internally months ago about such practice, so apparently not enough. Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 21/25] accel/qda: Add Audio protection domain attach and its remote heap

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > > +/* > + * The Audio PD keeps the remote heap across daemon restarts, so only the first > + * attach on a device hands it to the DSP. The lock is held across the attach > + * so that a concurrent one cannot skip the heap while the first is still in > + * flight, and the flag is set only once the DSP has accepted it. > + */ > +static int qda_attach_audio_pd(struct qda_session *session, struct drm_qda_remote_attach *args) > +{ > + struct qda_dev *qdev = session->qda_dev; > + struct qda_static_pd_args sargs = {}; > + char *name; > + int err; > + > + if (strcmp(qdev->dsp_name, "adsp") || !qdev->remote_heap_size) That's value from DTB, so where did you document the ABI? I see such undocumented ABI in multiple places. Usually requirement would be to document it, but not label. Why label 'ADSP' is not correct? Why "Audio DSP" would not be correct? Drop all this label dependency, completely (and do not introduce other names, like node name). Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 16/25] accel/qda: Add DSP process creation and release to the legacy shim

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > snode->sdev = sdev; > + snode->secure = secure; > snode->miscdev.minor = MISC_DYNAMIC_MINOR; > snode->miscdev.name = snode->name; > snode->miscdev.fops = &qda_shim_fops; > @@ -293,8 +346,6 @@ static int qda_shim_probe(struct auxiliary_device *auxdev, > const struct auxiliary_device_id *id) > { > struct qda_dev *qdev = dev_get_platdata(&auxdev->dev); > - bool secure_channel = !of_property_read_bool(qdev->dev->of_node, > - "qcom,non-secure-domain"); You just added these lines two patches earlier. You are adding COMPLETELY new driver, so adding completely new code and then removing it is confusing. It means you added something which was not needed. Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 23/25] accel/qda: Add DSP capability query

by Arnd Bergmann

On Sun, Nov 1, 2026, at 23:21, Ekansh Gupta wrote: > * > * This structure is used with DRM_IOCTL_QDA_QUERY to query device attributes > - * based on @query_type. > + * based on @query_type. @dsp_name and @cap share storage, as only one query > + * is made per call. > */ > struct drm_qda_query { > __u32 query_type; > - __u8 dsp_name[16]; > + union { > + __u8 dsp_name[16]; > + struct { > + __u32 attribute_id; > + __u32 capability; > + } cap; > + }; > }; I think it would be better to make this separate ioctl commands instead of a multiplexer for each thing you query. ioctl numbers are cheap, but complexity is not. Arnd

1 week, 1 day

Re: [PATCH v4 04/25] accel/qda: Assign a context bank to each session

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > + > +static int qda_count_cb_nodes(struct device_node *parent_node) > +{ > + int count = 0; > + > + for_each_available_child_of_node_scoped(parent_node, child) I don't see a need for scoped loop. > + if (of_device_is_compatible(child, "qcom,fastrpc-compute-cb")) > + count++; > + > + return count; > } Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 03/25] accel/qda: Add compute context bank devices

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > diff --git a/drivers/accel/qda/qda_rpmsg.c b/drivers/accel/qda/qda_rpmsg.c > index 6bd977e8d68a..78b5f31974c1 100644 > --- a/drivers/accel/qda/qda_rpmsg.c > +++ b/drivers/accel/qda/qda_rpmsg.c > @@ -5,18 +5,18 @@ > #include <linux/rpmsg.h> > #include <drm/drm_print.h> > > +#include "qda_cb.h" > #include "qda_drv.h" > +#include "qda_soc.h" > > /* > * Both compatibles the binding defines are matched here. The glymur, hawi, > * maili and nord nodes name "qcom,kaanapali-fastrpc" as their fallback > - * compatible and so are matched by that entry. Per-SoC DSP parameters are > - * attached as match data by a later patch, once there is something that > - * consumes them. You just added this in previous commit. Why adding invalid documentation, which is immediately removed? > + * compatible and so are matched by that entry. Best regards, Krzysztof

1 week, 1 day

Re: [PATCH v4 21/25] accel/qda: Add Audio protection domain attach and its remote heap

by Arnd Bergmann

On Sun, Nov 1, 2026, at 23:21, Ekansh Gupta wrote: > struct drm_qda_remote_attach { > __u32 pd_type; > - __u32 pad; > + __u32 namelen; > + __u64 name; > }; > This is an ABI change in the middle of the series, which isn't great even though it is harmless as there are no users that would see the initial ABI. I'd suggest defining the structure in the final length from the start. Arnd

1 week, 1 day

Re: [PATCH v4 02/25] accel/qda: Add initial QDA DRM accelerator driver

by Krzysztof Kozlowski

On 01/11/2026 23:21, Ekansh Gupta wrote: > + > +#include "qda_drv.h" > + > +/* > + * Both compatibles the binding defines are matched here. The glymur, hawi, > + * maili and nord nodes name "qcom,kaanapali-fastrpc" as their fallback > + * compatible and so are matched by that entry. Per-SoC DSP parameters are > + * attached as match data by a later patch, once there is something that So you add empty or non-working code. No, add these once you actually have complete and working table. > + * consumes them. > + */ Drop comment, redundant. Referencing future patches makes no sense in source code, because there is no "patch" when looking at the code. > +static const struct of_device_id qda_rpmsg_of_match[] = { > + { .compatible = "qcom,kaanapali-fastrpc" }, > + { .compatible = "qcom,fastrpc" }, > + { } Best regards, Krzysztof

1 week, 1 day

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig