* Re: [PATCH v2] pinctrl: qcom: Make important drivers default
From: Dmitry Baryshkov @ 2026-04-28 21:13 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Russell King, Bjorn Andersson, Konrad Dybcio, Linus Walleij,
linux-arm-kernel, linux-kernel, linux-arm-msm, linux-gpio,
Konrad Dybcio
In-Reply-To: <20260428163548.154392-2-krzysztof.kozlowski@oss.qualcomm.com>
On Tue, Apr 28, 2026 at 06:35:49PM +0200, Krzysztof Kozlowski wrote:
> The main SoC TLMM (Top-Level Multiplexer) pin controller drivers are
> essential for booting up SoCs and are not really optional for a given
> platform. Kernel should not ask users choice of drivers when that
> choice is obvious and known to the developers that answer should be
> 'yes' or 'module'.
>
> Switch all Qualcomm TLMM pin controller drivers to a default 'yes' for
> ARCH_QCOM. This has impact:
>
> 1. arm64 defconfig: enable PINCTRL_SM7150, PINCTRL_IPQ9650 and
> PINCTRL_HAWI, which were not selected before but should be, because
> these platforms need them for proper boot.
>
> 2. arm qcom_defconfig: no changes.
>
> 3. arm multi_v7 defconfig: enable drivers necessary to boot ARM 32-bit
> platforms, which are already enabled on qcom_defconfig.
>
> 4. COMPILE_TEST builds: enable by default all drivers for arm or arm64
> builds, whenever ARCH_QCOM is selected. This has impact on build
> time and feels logical, because if one selects ARCH_QCOM then
> probably by default wants to build test it entirely. Kernels with
> COMPILE_TEST are not supposed to be used for booting.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> I did similar change for clocks and I will be continuing with
> interconnect and other missing pieces.
>
> Changes in v2:
> 1. Rebase on Linus W. devel branch. This should go with pinctrl, I
> think.
> 2. Add tags.
> 3. Update also PINCTRL_IPQ9650.
> ---
> arch/arm/configs/multi_v7_defconfig | 8 ----
> arch/arm/configs/qcom_defconfig | 15 -------
> arch/arm64/configs/defconfig | 46 ----------------------
> drivers/pinctrl/qcom/Kconfig | 1 +
> drivers/pinctrl/qcom/Kconfig.msm | 61 +++++++++++++++++++++++++++++
I think it would have been better split into 3 patches, but this way is
also fine.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 5 files changed, 62 insertions(+), 69 deletions(-)
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH 3/3] DO NOT MERGE: selftests: perf_events: Add device memory callchain unwinding test
From: Fredrik Markstrom @ 2026-04-28 20:49 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Santosh Shilimkar, Olof Johansson, Tony Lindgren
Cc: linux-arm-kernel, linux-kernel, linux-kselftest, linux-perf-users,
Nicolas Pitre, Fredrik Markstrom, Ivar Holmqvist, Malin Jonsson
In-Reply-To: <20260428-master-with-pfix-v3-v1-0-c384d3e53092@est.tech>
The device memory callchain guard introduced earlier in this series
needs a regression test to ensure future refactoring does not
silently reintroduce the vulnerability where perf follows frame
pointers into device memory.
Add a kselftest that exercises the exact attack vector: a process
mmaps /dev/mem (creating a device memory mapping), points its frame
pointer into it, and is sampled by perf with frame-pointer
callchains. The test passes if both the process and kernel survive.
The default MMIO address is 0xc0000000; override via the MMIO_ADDR
environment variable if that is unsuitable. The address must be a
physical address that is not backed by any responding device, so
that an access produces a synchronous external abort rather than
returning data. On QEMU's virt machine and many modern arm64
platforms, 0xc0000000 falls in an unused region of the MMIO
address space and works for this purpose.
Since the test only sets the frame pointer to the address (never
reads it directly), the only reads come from the perf unwinder —
which the guard blocks.
The /dev/mem mmap must happen in the child process after fork.
fork() does not copy PTEs for VM_PFNMAP regions, so mapping before
fork leaves the child with empty page tables — the unwinder gets a
translation fault (caught by extable) instead of a synchronous
external abort.
arm64-only; skipped on other architectures.
Assisted-by: Kiro:claude-opus-4.6 [kiro-cli]
Signed-off-by: Fredrik Markstrom <fredrik.markstrom@est.tech>
Reviewed-by: Ivar Holmqvist <ivar.holmqvist@est.tech>
Reviewed-by: Malin Jonsson <malin.jonsson@est.tech>
---
MAINTAINERS | 1 +
tools/testing/selftests/perf_events/Makefile | 2 +-
.../testing/selftests/perf_events/test_perf_vmio.c | 114 +++++++++++++++++++++
3 files changed, 116 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd16388f590a77c04e08d2d6d002f5cc..5416f80c4aac28a5f1d780c76bb23110283dcdc3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20881,6 +20881,7 @@ F: include/uapi/linux/perf_event.h
F: kernel/events/*
F: tools/lib/perf/
F: tools/perf/
+F: tools/testing/selftests/perf_events/
PERFORMANCE EVENTS TOOLING ARM64
R: John Garry <john.g.garry@oracle.com>
diff --git a/tools/testing/selftests/perf_events/Makefile b/tools/testing/selftests/perf_events/Makefile
index 2e5d85770dfeadd909196dbf980fd334b9580477..a432e24d9e493f77951092571989249703d22351 100644
--- a/tools/testing/selftests/perf_events/Makefile
+++ b/tools/testing/selftests/perf_events/Makefile
@@ -2,5 +2,5 @@
CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES)
LDFLAGS += -lpthread
-TEST_GEN_PROGS := sigtrap_threads remove_on_exec watermark_signal mmap
+TEST_GEN_PROGS := sigtrap_threads remove_on_exec watermark_signal mmap test_perf_vmio
include ../lib.mk
diff --git a/tools/testing/selftests/perf_events/test_perf_vmio.c b/tools/testing/selftests/perf_events/test_perf_vmio.c
new file mode 100644
index 0000000000000000000000000000000000000000..780c5800dd6bd3b7a9d3813b490d4621da876da3
--- /dev/null
+++ b/tools/testing/selftests/perf_events/test_perf_vmio.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Device memory perf callchain unwinding test (arm64 only).
+ *
+ * Maps a physical address via /dev/mem (creating a device memory mapping),
+ * launches perf record to sample this process with frame-pointer
+ * callchains, then points FP (x29) into the mapping and spins.
+ * The test passes if the kernel survives without crashing.
+ *
+ * The default MMIO address is 0xc0000000; override via environment:
+ * MMIO_ADDR=0x10000000 ./test_perf_vmio
+ */
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+#define DEFAULT_MMIO_ADDR 0xc0000000UL
+
+TEST(device_memory_callchain)
+{
+#ifndef __aarch64__
+ SKIP(return, "arm64 only");
+#else
+ unsigned long pa = DEFAULT_MMIO_ADDR;
+ unsigned long page, off;
+ pid_t spin_pid, perf_pid;
+ char pid_str[16];
+ int fd, pst;
+ void *m, *fp;
+ char *env;
+
+ if (getuid() != 0)
+ SKIP(return, "need root");
+
+ env = getenv("MMIO_ADDR");
+ if (env)
+ pa = strtoul(env, NULL, 16);
+
+ page = pa & ~0xFFFUL;
+ off = pa - page;
+
+ fd = open("/dev/mem", O_RDWR | O_SYNC);
+ if (fd < 0)
+ SKIP(return, "cannot open /dev/mem");
+
+ /* Fork a spinner child with FP pointing into device memory */
+ spin_pid = fork();
+ ASSERT_GE(spin_pid, 0);
+ if (spin_pid == 0) {
+ /*
+ * mmap /dev/mem in the child so remap_pfn_range populates
+ * PTEs directly. fork() does not copy PTEs for VM_PFNMAP
+ * regions, so mapping before fork leaves the child with
+ * empty page tables — the unwinder would get a translation
+ * fault instead of a synchronous external abort.
+ */
+ m = mmap(NULL, off + 4096, PROT_READ | PROT_WRITE,
+ MAP_SHARED, fd, page);
+ if (m == MAP_FAILED)
+ _exit(1);
+ fp = (char *)m + off;
+ __asm__ volatile(
+ "mov x29, %0\n"
+ "1: b 1b\n"
+ : : "r"(fp) : "x29", "memory");
+ _exit(0);
+ }
+
+ /* Launch perf to sample the spinner */
+ snprintf(pid_str, sizeof(pid_str), "%d", spin_pid);
+
+ perf_pid = fork();
+ if (perf_pid < 0) {
+ kill(spin_pid, SIGKILL);
+ waitpid(spin_pid, NULL, 0);
+ close(fd);
+ ASSERT_GE(perf_pid, 0);
+ }
+ if (perf_pid == 0) {
+ char *const perf_argv[] = {
+ "perf", "record", "-g", "--call-graph", "fp",
+ "-p", pid_str, "--", "sleep", "3", NULL
+ };
+
+ if (chdir("/tmp"))
+ _exit(1);
+ execvp(perf_argv[0], perf_argv);
+ _exit(1);
+ }
+
+ waitpid(perf_pid, &pst, 0);
+
+ kill(spin_pid, SIGKILL);
+ waitpid(spin_pid, NULL, 0);
+ close(fd);
+
+ if (WIFEXITED(pst) && WEXITSTATUS(pst) == 1)
+ SKIP(return, "perf not available");
+
+ /*
+ * The real test is that the kernel survived. If we got here
+ * without a synchronous external abort, the guard worked.
+ */
+ TH_LOG("kernel survived perf sampling with FP in device memory");
+#endif /* __aarch64__ */
+}
+
+TEST_HARNESS_MAIN
--
2.51.0
^ permalink raw reply related
* [PATCH 2/3] DO NOT MERGE: arm64: perf: Add skip_vmio parameter to control device memory callchain guard
From: Fredrik Markstrom @ 2026-04-28 20:48 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Santosh Shilimkar, Olof Johansson, Tony Lindgren
Cc: linux-arm-kernel, linux-kernel, linux-kselftest, linux-perf-users,
Nicolas Pitre, Fredrik Markstrom, Ivar Holmqvist, Malin Jonsson
In-Reply-To: <20260428-master-with-pfix-v3-v1-0-c384d3e53092@est.tech>
Reproducing the synchronous external abort that the device memory
guard prevents requires disabling the guard at runtime. Without
this, there is no way to verify the guard is actually needed or
to regression-test the crash path.
Add a module parameter (skip_vmio, default true) that controls
whether the guard is active. Set to 0 to disable it:
Boot: stacktrace.skip_vmio=0
Runtime: echo 0 > /sys/module/stacktrace/parameters/skip_vmio
When disabled, perf follows frame pointers into device memory
regions, triggering a synchronous external abort and kernel panic
on arm64.
Assisted-by: Kiro:claude-opus-4.6 [kiro-cli]
Signed-off-by: Fredrik Markstrom <fredrik.markstrom@est.tech>
Reviewed-by: Ivar Holmqvist <ivar.holmqvist@est.tech>
Reviewed-by: Malin Jonsson <malin.jonsson@est.tech>
---
arch/arm64/kernel/stacktrace.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 6426a307b8f86ae756ea444247ae329591a89b4b..ebe909012e4edee7bb1ddbebac2d3c49bdb91665 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -11,6 +11,7 @@
#include <linux/filter.h>
#include <linux/ftrace.h>
#include <linux/kprobes.h>
+#include <linux/moduleparam.h>
#include <linux/pgtable.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
@@ -112,6 +113,10 @@ static bool addr_is_device_mem(unsigned long addr)
#endif
}
+static bool skip_vmio = true;
+module_param(skip_vmio, bool, 0644);
+MODULE_PARM_DESC(skip_vmio, "Skip device memory during user callchain unwinding");
+
enum kunwind_source {
KUNWIND_SOURCE_UNKNOWN,
KUNWIND_SOURCE_FRAME,
@@ -616,7 +621,7 @@ unwind_user_frame(struct frame_tail __user *tail, void *cookie,
if (!access_ok(tail, sizeof(buftail)))
return NULL;
- if (addr_is_device_mem((unsigned long)tail))
+ if (READ_ONCE(skip_vmio) && addr_is_device_mem((unsigned long)tail))
return NULL;
pagefault_disable();
@@ -667,7 +672,7 @@ unwind_compat_user_frame(struct compat_frame_tail __user *tail, void *cookie,
if (!access_ok(tail, sizeof(buftail)))
return NULL;
- if (addr_is_device_mem((unsigned long)tail))
+ if (READ_ONCE(skip_vmio) && addr_is_device_mem((unsigned long)tail))
return NULL;
pagefault_disable();
--
2.51.0
^ permalink raw reply related
* [PATCH 1/3] arm64: perf: Skip device memory during user callchain unwinding
From: Fredrik Markstrom @ 2026-04-28 20:48 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Santosh Shilimkar, Olof Johansson, Tony Lindgren
Cc: linux-arm-kernel, linux-kernel, linux-kselftest, linux-perf-users,
Nicolas Pitre, Fredrik Markstrom, Ivar Holmqvist, Malin Jonsson
In-Reply-To: <20260428-master-with-pfix-v3-v1-0-c384d3e53092@est.tech>
Perf callchain unwinding follows userspace frame pointers via
copy_from_user. A corrupted frame pointer can point into device
I/O memory mapped into the process (e.g. via UIO), causing the
kernel to read from MMIO regions. Reads from device memory can
have side effects, trigger bus errors, or produce faults that
crash the kernel.
Add a lockless page table walk that inspects the MAIR attribute
index in the leaf PTE before reading. If the PTE indicates
device memory (MT_DEVICE_nGnRnE or MT_DEVICE_nGnRE), the frame
is skipped. The walk uses the same lockless accessors as
perf_get_pgtable_size() with local_irq_save/restore to ensure
page table pages are not freed during the walk, as
arch_stack_walk_user() can also be reached from process
context via ftrace (stack_trace_save_user).
The walk is guarded by #ifdef CONFIG_HAVE_GUP_FAST to match
perf_get_pgtable_size(), though the lockless helpers all have
generic fallbacks and the guard may not be strictly necessary.
Without this guard the kernel panics:
Internal error: synchronous external abort: 0000000096000010 [#1] SMP
CPU: 1 UID: 0 PID: 33 Comm: test_perf_vmio Tainted: G M 7.0.0+ #37 PREEMPTLAZY
Tainted: [M]=MACHINE_CHECK
Hardware name: linux,dummy-virt (DT)
pstate: 800000c5 (Nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __arch_copy_from_user+0xb8/0x23c
lr : arch_stack_walk_user+0x218/0x258
sp : ffff800080433ba0
x29: ffff800080433ba0 x28: ffff00000097ed40 x27: 0000000000000000
x26: 000000000000001f x25: ffffffffffffffff x24: ffff00000097ed40
x23: 000ffffffffffff0 x22: ffff800080433c78 x21: ffff800080022db8
x20: ffff80008032bc60 x19: 0000ffff9e575000 x18: 0000000000000000
x17: ffff7fffbfac5000 x16: ffff800080430000 x15: 0000ffff9e575000
x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
x8 : 0000000000000001 x7 : 000000000000007f x6 : ffff800080433c10
x5 : ffff800080433c20 x4 : 0000000000000000 x3 : 0000000000000010
x2 : 0000000000000010 x1 : 0000ffff9e575000 x0 : ffff800080433c10
Call trace:
__arch_copy_from_user+0xb8/0x23c (P)
perf_callchain_user+0x1c/0x24
get_perf_callchain+0x130/0x138
perf_callchain+0xac/0xc4
perf_prepare_sample+0xac/0x5d8
perf_event_output_forward+0x44/0xa0
__perf_event_overflow+0x190/0x230
perf_event_overflow+0x18/0x20
armv8pmu_handle_irq+0x154/0x194
armpmu_dispatch_irq+0x28/0x54
handle_percpu_devid_irq+0xf0/0x11c
handle_irq_desc+0x3c/0x50
generic_handle_domain_irq+0x14/0x1c
gic_handle_irq+0x80/0x98
call_on_irq_stack+0x30/0x4c
do_interrupt_handler+0x5c/0x84
el0_interrupt+0x58/0x8c
__el0_irq_handler_common+0x14/0x1c
el0t_64_irq_handler+0xc/0x14
el0t_64_irq+0x154/0x158
Code: f8400827 f8408828 91004021 a88120c7 (f8400827)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: synchronous external abort: Fatal exception in interrupt
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x0000000,000d0000,00040000,0400400b
Memory Limit: none
Assisted-by: Kiro:claude-opus-4.6 [kiro-cli]
Fixes: 030896885ade ("arm64: Performance counters support")
Signed-off-by: Fredrik Markstrom <fredrik.markstrom@est.tech>
Reviewed-by: Ivar Holmqvist <ivar.holmqvist@est.tech>
Reviewed-by: Malin Jonsson <malin.jonsson@est.tech>
---
arch/arm64/kernel/stacktrace.c | 98 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 3ebcf8c53fb04050488ffc110ff2059028b6772d..6426a307b8f86ae756ea444247ae329591a89b4b 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -4,12 +4,14 @@
*
* Copyright (C) 2012 ARM Ltd.
*/
+#include <linux/bitfield.h>
#include <linux/kernel.h>
#include <linux/efi.h>
#include <linux/export.h>
#include <linux/filter.h>
#include <linux/ftrace.h>
#include <linux/kprobes.h>
+#include <linux/pgtable.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
#include <linux/sched/task_stack.h>
@@ -17,9 +19,99 @@
#include <asm/efi.h>
#include <asm/irq.h>
+#include <asm/memory.h>
#include <asm/stack_pointer.h>
#include <asm/stacktrace.h>
+/*
+ * addr_is_device_mem - check if a userspace address maps device memory
+ *
+ * Walks the current task's page tables without taking the mmap lock,
+ * using the same lockless pattern as perf_get_pgtable_size() in
+ * kernel/events/core.c. Inspects the MAIR attribute index in the
+ * leaf PTE to detect device memory types.
+ *
+ * Returns true for device memory (MT_DEVICE_nGnRnE, MT_DEVICE_nGnRE)
+ * or if the mapping cannot be determined. Safe to call from IRQ/NMI
+ * context.
+ */
+static bool addr_is_device_mem(unsigned long addr)
+{
+#ifdef CONFIG_HAVE_GUP_FAST
+ struct mm_struct *mm = current->mm;
+ pgd_t *pgdp, pgd;
+ p4d_t *p4dp, p4d;
+ pud_t *pudp, pud;
+ pmd_t *pmdp, pmd;
+ pte_t *ptep, pte;
+ unsigned long flags;
+ unsigned int idx;
+ bool is_dev;
+
+ if (!mm)
+ return true;
+
+ local_irq_save(flags);
+
+ pgdp = pgd_offset(mm, addr);
+ pgd = pgdp_get(pgdp);
+ if (pgd_none(pgd))
+ goto err;
+
+ p4dp = p4d_offset_lockless(pgdp, pgd, addr);
+ p4d = p4dp_get(p4dp);
+ if (!p4d_present(p4d))
+ goto err;
+
+ pudp = pud_offset_lockless(p4dp, p4d, addr);
+ pud = pudp_get(pudp);
+ if (!pud_present(pud))
+ goto err;
+
+ if (pud_leaf(pud)) {
+ pte = pud_pte(pud);
+ goto check;
+ }
+
+ pmdp = pmd_offset_lockless(pudp, pud, addr);
+again:
+ pmd = pmdp_get_lockless(pmdp);
+ if (!pmd_present(pmd))
+ goto err;
+
+ if (pmd_leaf(pmd)) {
+ pte = pmd_pte(pmd);
+ goto check;
+ }
+
+ ptep = pte_offset_map(&pmd, addr);
+ if (!ptep)
+ goto again;
+
+ pte = ptep_get_lockless(ptep);
+ pte_unmap(ptep);
+
+ if (!pte_present(pte))
+ goto err;
+check:
+ idx = FIELD_GET(PTE_ATTRINDX_MASK, pte_val(pte));
+ is_dev = idx == MT_DEVICE_nGnRnE || idx == MT_DEVICE_nGnRE;
+ local_irq_restore(flags);
+ return is_dev;
+err:
+ local_irq_restore(flags);
+ return true;
+#else
+ /*
+ * Without GUP-fast lockless page table helpers we cannot
+ * inspect the PTE. Preserve the existing behavior (no
+ * device memory check) rather than unconditionally blocking
+ * all unwinding.
+ */
+ return false;
+#endif
+}
+
enum kunwind_source {
KUNWIND_SOURCE_UNKNOWN,
KUNWIND_SOURCE_FRAME,
@@ -524,6 +616,9 @@ unwind_user_frame(struct frame_tail __user *tail, void *cookie,
if (!access_ok(tail, sizeof(buftail)))
return NULL;
+ if (addr_is_device_mem((unsigned long)tail))
+ return NULL;
+
pagefault_disable();
err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail));
pagefault_enable();
@@ -572,6 +667,9 @@ unwind_compat_user_frame(struct compat_frame_tail __user *tail, void *cookie,
if (!access_ok(tail, sizeof(buftail)))
return NULL;
+ if (addr_is_device_mem((unsigned long)tail))
+ return NULL;
+
pagefault_disable();
err = __copy_from_user_inatomic(&buftail, tail, sizeof(buftail));
pagefault_enable();
--
2.51.0
^ permalink raw reply related
* [PATCH 0/3] arm64: perf: Skip device memory during user callchain unwinding
From: Fredrik Markstrom @ 2026-04-28 20:48 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Shuah Khan, Peter Zijlstra,
Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Santosh Shilimkar, Olof Johansson, Tony Lindgren
Cc: linux-arm-kernel, linux-kernel, linux-kselftest, linux-perf-users,
Nicolas Pitre, Fredrik Markstrom, Ivar Holmqvist, Malin Jonsson
Perf callchain unwinding follows userspace frame pointers via
copy_from_user. A corrupted or malicious frame pointer can point
into device I/O memory mapped into the process (e.g. via UIO or
/dev/mem), causing the kernel to read from MMIO regions in PMU
interrupt context. Such reads can have side effects on hardware
(clearing status registers, advancing FIFOs, triggering DMA) and
on arm64 can produce a synchronous external abort that panics the
kernel.
This series adds a guard that detects device memory before each
frame pointer read and skips the frame.
Patch 1: Lockless page table walk checking the MAIR attribute index
in the leaf PTE to identify device memory types
(MT_DEVICE_nGnRnE, MT_DEVICE_nGnRE). Follows the same
pattern as perf_get_pgtable_size() in kernel/events/core.c.
Patch 2: (DO NOT MERGE) Module parameter to disable the guard at
runtime for regression testing.
Patch 3: (DO NOT MERGE) kselftest that exercises the attack vector:
maps /dev/mem, points FP into it, and verifies the kernel
survives perf sampling.
Alternatives considered:
- VMA lookup (mmap_read_trylock + vma_lookup checking VM_IO):
requires the mmap lock on every frame.
- RCU maple tree lookup: lock-free but still a tree traversal
per frame.
- lock_vma_under_rcu: sleeping lock, unusable from IRQ context.
The page table walk requires no locks and costs only 4 pointer
dereferences per frame.
Limitations:
- The MAIR attribute check is arm64-specific. Other architectures
use different mechanisms to identify device memory and would need
their own PTE inspection logic.
- The walk only detects memory types visible in the PTE. If a VM_IO
region has not been faulted in, the walk sees no PTE and fails
safe (skips the frame). This is conservative — it may skip frames
that would not actually fault.
A QEMU-based reproducer is available at:
https://gitlab.com/frma71/qemu-kernel-tests/-/tree/vmio_perf_test?ref_type=tags
Signed-off-by: Fredrik Markstrom <fredrik.markstrom@est.tech>
---
Fredrik Markstrom (3):
arm64: perf: Skip device memory during user callchain unwinding
DO NOT MERGE: arm64: perf: Add skip_vmio parameter to control device memory callchain guard
DO NOT MERGE: selftests: perf_events: Add device memory callchain unwinding test
MAINTAINERS | 1 +
arch/arm64/kernel/stacktrace.c | 103 +++++++++++++++++++
tools/testing/selftests/perf_events/Makefile | 2 +-
.../testing/selftests/perf_events/test_perf_vmio.c | 114 +++++++++++++++++++++
4 files changed, 219 insertions(+), 1 deletion(-)
---
base-commit: dca922e019dd758b4c1b4bec8f1d509efddeaab4
change-id: 20260427-master-with-pfix-v3-ae7173f538ca
Best regards,
--
Fredrik Markstrom <fredrik.markstrom@est.tech>
^ permalink raw reply
* Re: [PATCH v2 00/12] clk: add new flag CLK_ROUNDING_NOOP
From: Brian Masney @ 2026-04-28 20:38 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Sudeep Holla, Abel Vesa,
Andrea della Porta, Baolin Wang, Bjorn Andersson, Chanwoo Choi,
Frank Li, Geert Uytterhoeven, Krzysztof Kozlowski, Orson Zhai,
Sascha Hauer, Sylwester Nawrocki, Tudor Ambarus, Alim Akhtar,
arm-scmi, Chunyan Zhang, Cristian Marussi, Fabio Estevam, imx,
linux-arm-kernel, linux-arm-msm, linux-renesas-soc,
linux-samsung-soc, Peng Fan, Pengutronix Kernel Team,
Dmitry Baryshkov, Krzysztof Kozlowski
In-Reply-To: <20260309-clk-det-rate-fw-managed-v2-0-c48ef5a3100a@redhat.com>
Hi Stephen,
On Mon, Mar 09, 2026 at 10:38:39AM -0400, Brian Masney wrote:
> There are some clocks where the determine_rate clk op is just an empty
> function that returns 0. This can be either because the rounding is
> managed by the firmware, or the clock is capable of any rate. Add a
> new flag for these type of clocks, and update the clk core so that the
> determine_rate() clk op is not required when this flag is set.
>
> Based on discussions with Stephen at Linux Plumbers Conference, he
> suggested adding a flag for this particular case. So let's add a new
> flag, and update the clk core so that the determine_rate() clk op is
> not required when this flag is set.
>
> This series adds the flag, some kunit tests, and updates all of the
> relevant drivers under drivers/clk to use the new flag.
>
> Once this is merged, and in Linus's tree, I can update the few remaining
> clk drivers that are outside of drivers/clk via those subsystems at a
> later time.
>
> Merge Strategy
> --------------
> All of this needs to be directly merged by Stephen as one series into
> his tree. Subsystem maintainers: please leave a Reviewed-by or Acked-by.
> To reduce the noise, I am only CCing people on their respective drivers.
>
> Note this series depends on 3 previously-posted patches in this git pull
> to Stephen for v7.1.
> https://lore.kernel.org/linux-clk/aZuK4-QJCXUeSxtL@redhat.com/
>
> Signed-off-by: Brian Masney <bmasney@redhat.com>
We talked about this change at LPC in Tokyo, and you were the one that
suggested adding a new flag. I initially wanted to add a new shared noop
function to drivers/clk/clk.c. This series implements everything we
talked about in person. The only thing that we didn't talk about in
person was the name of this new flag.
Anyways, let me know if you are interested in this change, and if you
want any changes. If there's no changes then would it be possible to
pick this up?
Thanks,
Brian
^ permalink raw reply
* Re: [PATCH v4 35/49] KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive
From: Marc Zyngier @ 2026-04-28 20:37 UTC (permalink / raw)
To: Vishnu Pajjuri
Cc: Fuad Tabba, Joey Gouly, Suzuki K Poulose, Oliver Upton,
Zenghui Yu, Christoffer Dall, Mark Brown, kvm, linux-arm-kernel,
kvmarm, Darren Hart
In-Reply-To: <87ik9dbys7.wl-maz@kernel.org>
On Sun, 26 Apr 2026 15:07:36 +0100,
Marc Zyngier <maz@kernel.org> wrote:
>
> On Sun, 26 Apr 2026 10:14:11 +0100,
> Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Wed, 22 Apr 2026 15:57:44 +0100,
> > Vishnu Pajjuri <vishnu@os.amperecomputing.com> wrote:
> > >
> > > Hi Marc,
> > >
> > > On 22-04-2026 12:25, Marc Zyngier wrote:
> > > >
> > > > Have you made progress on this? I can't reproduce it at all despite my
> > > > best effort. I'm perfectly happy to help, but you need to give me
> > > > *something* to go on.
> > >
> > >
> > > Thanks for your support!!
> > >
> > > The issue is triggered as soon as the timer interrupt (IRQ 27) is
> > > deactivated. Preventing the deactivation of IRQ 27 during nested VGIC
> > > state transitions prevents the failure from reproducing.
> >
> > Which level of deactivation? From L2 to L1? Or L1 to L0? The former
> > should just be a an update to the irq structure, while the latter is
> > effectively a write to ICC_DIR_EL1, and *that* is a new behaviour
> > introduced by this patch.
> >
> > I wonder if your implementation is such that ICC_DIR_EL1 is trapped by
> > ICH_HCR_EL2.TDIR, which is allowed by the architecture, but that none
> > of the two implementations I have actually enforce (the trap only
> > applies to ICV_DIR_EL1). Can you try the hack below which disables the
> > traps much earlier, and let me know if that helps?
> >
> > Even if that's the case, this should result in an EL2->EL2 exception,
> > and that should be caught as an unhandled exception in entry-common.c,
> > so something else is afoot.
>
> Actually, this should never happen. ICH_HCR_EL2.TDIR is constructed
> like all the other GICv3 trap bits, in the sense that it only traps
> accesses from EL1, not EL2 (for sanity reasons, I'm not considering
> the possibility of a trap to EL3...).
>
> Still, I'm interested in finding out if that hack helps at all.
Any update?
I've been racking my brain on this one, and I may have another
potential avenue to explore. Does your CPU implement SEIS
(ICC_CTLR_EL1.SEIS == 1)?
I think there is a corner case where we can end-up doing a double
deactivation. On its own, that's a completely harmless thing to
do. Except on an implementation with SEIS, which could deliver an
SError and route that to EL3. Add quality firmware to the mix, and you
could be in for a treat.
Obviously, implementing SEIS is a terrible and pointless thing to do
(it's been long deprecated), but in case someone felt adventurous in
the RTL department, let me know what the following hack does for you.
M.
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 9e841e7afd4a7..c92ca6969e85f 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -275,7 +275,7 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val)
lr = vgic_v3_compute_lr(vcpu, irq) & ~ICH_LR_ACTIVE_BIT;
}
- if (lr & ICH_LR_HW)
+ if ((lr & ICH_LR_HW) && !vgic_state_is_nested(vcpu))
vgic_v3_deactivate_phys(FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));
vgic_v3_fold_lr(vcpu, lr);
--
Jazz isn't dead. It just smells funny.
^ permalink raw reply related
* Re: [PATCH] media: mediatek: vcodec: free working buf in vdec_vp9_slice_setup_single()
From: Nicolas Dufresne @ 2026-04-28 20:37 UTC (permalink / raw)
To: Haoxiang Li, tiffany.lin, andrew-ct.chen, yunfei.dong, mchehab,
matthias.bgg, angelogioacchino.delregno, hverkuil+cisco,
laurent.pinchart, p.zabel, benjamin.gaignard, xiaoyong.lu,
mingjia.zhang
Cc: linux-media, linux-kernel, linux-arm-kernel, linux-mediatek,
stable
In-Reply-To: <20260330021110.2733458-1-lihaoxiang@isrc.iscas.ac.cn>
[-- Attachment #1: Type: text/plain, Size: 1500 bytes --]
Le lundi 30 mars 2026 à 10:11 +0800, Haoxiang Li a écrit :
> Add an error path label in vdec_vp9_slice_setup_single()
> and call vdec_vp9_slice_free_working_buffer() to free
> working buffer.
>
> Fixes: b0f407c19648 ("media: mediatek: vcodec: add vp9 decoder driver for mt8186")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
> ---
> .../mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> index cd1935014d76..b3ecb94bebb3 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> @@ -1811,14 +1811,16 @@ static int vdec_vp9_slice_setup_single(struct vdec_vp9_slice_instance *instance,
>
> ret = vdec_vp9_slice_setup_prob_buffer(instance, vsi);
This function never fails, remove its return value and remove the error code
below.
> if (ret)
> - goto err;
> + goto alloc_err;
>
> ret = vdec_vp9_slice_setup_tile_buffer(instance, vsi, bs);
> if (ret)
> - goto err;
> + goto alloc_err;
Ack.
cheers,
Nicolas
>
> return 0;
>
> +alloc_err:
> + vdec_vp9_slice_free_working_buffer(instance);
> err:
> return ret;
> }
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] media: mediatek: vcodec: free working buf on error path in vdec_vp9_slice_setup_lat()
From: Nicolas Dufresne @ 2026-04-28 20:35 UTC (permalink / raw)
To: Haoxiang Li, tiffany.lin, andrew-ct.chen, yunfei.dong, mchehab,
matthias.bgg, angelogioacchino.delregno, laurent.pinchart,
hverkuil+cisco, benjamin.gaignard, p.zabel, george.sun
Cc: linux-media, linux-kernel, linux-arm-kernel, linux-mediatek,
stable
In-Reply-To: <20260330020224.2729252-1-lihaoxiang@isrc.iscas.ac.cn>
[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]
Le lundi 30 mars 2026 à 10:02 +0800, Haoxiang Li a écrit :
> Add an error path label in vdec_vp9_slice_setup_lat()
> and call vdec_vp9_slice_free_working_buffer() to free
> working buffer to prevent potential memory leak.
>
> Fixes: 5d418351ca8f ("media: mediatek: vcodec: support stateless VP9 decoding")
> Cc: stable@vger.kernel.org
> Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
> ---
> .../mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> index cd1935014d76..3dadb5cc8876 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> @@ -1168,7 +1168,7 @@ static int vdec_vp9_slice_setup_lat(struct vdec_vp9_slice_instance *instance,
>
> ret = vdec_vp9_slice_setup_lat_buffer(instance, vsi, bs, lat_buf);
This function never fails, please remove its return value and remove the error
code below instead.
> if (ret)
> - goto err;
> + goto alloc_err;
>
> vdec_vp9_slice_setup_seg_buffer(instance, vsi, &instance->seg[0]);
>
> @@ -1176,14 +1176,16 @@ static int vdec_vp9_slice_setup_lat(struct vdec_vp9_slice_instance *instance,
>
> ret = vdec_vp9_slice_setup_prob_buffer(instance, vsi);
Same issue, that function should not have had a return value.
> if (ret)
> - goto err;
> + goto alloc_err;
>
> ret = vdec_vp9_slice_setup_tile_buffer(instance, vsi, bs);
> if (ret)
> - goto err;
> + goto alloc_err;
Ack for this one.
>
> return 0;
>
> +alloc_err:
> + vdec_vp9_slice_free_working_buffer(instance);
> err:
> return ret;
> }
cheers,
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 03/15] clk: scmi: Use new determine_rate clock operation
From: Brian Masney @ 2026-04-28 20:33 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc, sudeep.holla, philip.radford, james.quinlan,
f.fainelli, vincent.guittot, etienne.carriere, peng.fan,
michal.simek, geert+renesas, kuninori.morimoto.gx,
marek.vasut+renesas, Michael Turquette, Stephen Boyd
In-Reply-To: <20260428201522.903875-4-cristian.marussi@arm.com>
Hi Cristian,
On Tue, Apr 28, 2026 at 09:15:10PM +0100, Cristian Marussi wrote:
> Use the Clock protocol layer determine_rate logic to calculate the closest
> rate that can be supported by a specific clock.
>
> No functional change.
>
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> @brian: I'd modify further this clk-scmi driver, with a patch on top of
> this series, to properly use your new CLK_ROUNDING_NOOP flag once your
> series AND another (already reviewed) series on clk-scmi from Peng are in.
I don't know if Stephen is going to pick up my CLK_ROUNDING_NOOP series.
We talked about it in person at LPC in Tokyo, and he was the one that
suggested the flag rather than a new shared noop function. However he
didn't pick it up last development cycle.
I would recommend NOT basing on that series of mine to reduce
dependencies, and so that your stuff doesn't get held up by series.
Brian
^ permalink raw reply
* Re: [PATCH v3 08/11] drm/bridge: adv7511: switch to of_drm_get_bridge_by_endpoint()
From: Dmitry Baryshkov @ 2026-04-28 20:33 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, Xinliang Liu, Sumit Semwal, Yongqin Liu,
John Stultz, Tomi Valkeinen, Michal Simek, Hui Pu, Ian Ray,
Thomas Petazzoni, dri-devel, linux-kernel, linux-arm-msm,
freedreno, linux-arm-kernel
In-Reply-To: <20260428-drm-bridge-alloc-getput-panel_or_bridge-v3-8-a537b5567add@bootlin.com>
On Tue, Apr 28, 2026 at 05:30:57PM +0200, Luca Ceresoli wrote:
> This driver calls drm_of_find_panel_or_bridge() with a NULL pointer in the
> @panel parameter, thus using a reduced feature set of that function.
> Replace this call with the simpler of_drm_get_bridge_by_endpoint().
>
> Since of_drm_get_bridge_by_endpoint() increases the refcount of the
> returned bridge, ensure it is put on removal. To achieve this, instead of
> adding an explicit drm_bridge_put(), migrate to the bridge::next_bridge
> pointer which is automatically put when the bridge is eventually freed.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> Changes in v3:
> - fix ERR_PTR deref when -ENODEV is returned
Biju, thanks for pointing this out!
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v3 03/11] drm/msm/hdmi: switch to of_drm_get_bridge_by_endpoint()
From: Dmitry Baryshkov @ 2026-04-28 20:32 UTC (permalink / raw)
To: Luca Ceresoli
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, Xinliang Liu, Sumit Semwal, Yongqin Liu,
John Stultz, Tomi Valkeinen, Michal Simek, Hui Pu, Ian Ray,
Thomas Petazzoni, dri-devel, linux-kernel, linux-arm-msm,
freedreno, linux-arm-kernel
In-Reply-To: <20260428-drm-bridge-alloc-getput-panel_or_bridge-v3-3-a537b5567add@bootlin.com>
On Tue, Apr 28, 2026 at 05:30:52PM +0200, Luca Ceresoli wrote:
> This driver calls drm_of_find_panel_or_bridge() with a NULL pointer in the
> @panel parameter, thus using a reduced feature set of that function.
> Replace this call with the simpler of_drm_get_bridge_by_endpoint().
>
> Since of_drm_get_bridge_by_endpoint() increases the refcount of the
> returned bridge, ensure it is put on removal.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v4 01/14] media: mediatek: vcodec: add driver to support vcp
From: Nicolas Dufresne @ 2026-04-28 20:29 UTC (permalink / raw)
To: Yunfei Dong, Nícolas F . R . A . Prado, Sebastian Fricke,
Hans Verkuil, AngeloGioacchino Del Regno, Benjamin Gaignard,
Nathan Hebert, Daniel Almeida
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho, linux-media,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260328051630.7937-2-yunfei.dong@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 26306 bytes --]
Le samedi 28 mars 2026 à 13:16 +0800, Yunfei Dong a écrit :
> The processor is changed from scp to vcp in mt8196 platform.
> Adding new firmware interface to communicate kernel with vcp
> for the communication method is changed.
>
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
> .../media/platform/mediatek/vcodec/Kconfig | 4 +
> .../platform/mediatek/vcodec/common/Makefile | 4 +
> .../mediatek/vcodec/common/mtk_vcodec_fw.c | 3 +
> .../mediatek/vcodec/common/mtk_vcodec_fw.h | 1 +
> .../vcodec/common/mtk_vcodec_fw_priv.h | 12 +
> .../vcodec/common/mtk_vcodec_fw_vcp.c | 505 ++++++++++++++++++
> .../vcodec/common/mtk_vcodec_fw_vcp.h | 139 +++++
> 7 files changed, 668 insertions(+)
> create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> create mode 100644 drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
>
> diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig b/drivers/media/platform/mediatek/vcodec/Kconfig
> index bc8292232530..d23dad5c78ce 100644
> --- a/drivers/media/platform/mediatek/vcodec/Kconfig
> +++ b/drivers/media/platform/mediatek/vcodec/Kconfig
> @@ -1,4 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +config VIDEO_MEDIATEK_VCODEC_VCP
> + bool
> +
> config VIDEO_MEDIATEK_VCODEC_SCP
> bool
>
> @@ -21,6 +24,7 @@ config VIDEO_MEDIATEK_VCODEC
> select V4L2_MEM2MEM_DEV
> select VIDEO_MEDIATEK_VCODEC_VPU if VIDEO_MEDIATEK_VPU
> select VIDEO_MEDIATEK_VCODEC_SCP if MTK_SCP
> + select VIDEO_MEDIATEK_VCODEC_VCP if MTK_VCP_RPROC
> select V4L2_H264
> select V4L2_VP9
> select MEDIA_CONTROLLER
> diff --git a/drivers/media/platform/mediatek/vcodec/common/Makefile b/drivers/media/platform/mediatek/vcodec/common/Makefile
> index d0479914dfb3..2f68692e8c98 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/Makefile
> +++ b/drivers/media/platform/mediatek/vcodec/common/Makefile
> @@ -14,6 +14,10 @@ ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP),)
> mtk-vcodec-common-y += mtk_vcodec_fw_scp.o
> endif
>
> +ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP),)
> +mtk-vcodec-common-y += mtk_vcodec_fw_vcp.o
> +endif
> +
> ifneq ($(CONFIG_DEBUG_FS),)
> obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec-dbgfs.o
>
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> index 08949b08fbc6..fc547afa4ebf 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.c
> @@ -3,6 +3,7 @@
> #include "../decoder/mtk_vcodec_dec_drv.h"
> #include "../encoder/mtk_vcodec_enc_drv.h"
> #include "mtk_vcodec_fw_priv.h"
> +#include "mtk_vcodec_fw_vcp.h"
>
> struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type type,
> enum mtk_vcodec_fw_use fw_use)
> @@ -19,6 +20,8 @@ struct mtk_vcodec_fw *mtk_vcodec_fw_select(void *priv, enum mtk_vcodec_fw_type t
> return mtk_vcodec_fw_vpu_init(priv, fw_use);
> case SCP:
> return mtk_vcodec_fw_scp_init(priv, fw_use);
> + case VCP:
> + return mtk_vcodec_fw_vcp_init(priv, fw_use);
This applies to the entire series. Do you have rational toward using the switch
and if/else approach over the entire series ? The ops pattern is much more
command for multi-platform driver. Using this as an example, you'd have a
vpu_init ops, in your pdata, and simply call this without the runtime switches
and conditions.
Please apply changes according to this comment for the entire series, keeping
mtk_vcodec_enc_drv.h a free from VCP/SCP specific code as possible.
Nicolas
> default:
> dev_err(&plat_dev->dev, "Invalid vcodec fw type");
> return ERR_PTR(-EINVAL);
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> index 300363a40158..c1642fb09b42 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw.h
> @@ -14,6 +14,7 @@ struct mtk_vcodec_enc_dev;
> enum mtk_vcodec_fw_type {
> VPU,
> SCP,
> + VCP,
> };
>
> enum mtk_vcodec_fw_use {
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> index 99603accd82e..0a2a9b010244 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_priv.h
> @@ -4,6 +4,7 @@
> #define _MTK_VCODEC_FW_PRIV_H_
>
> #include "mtk_vcodec_fw.h"
> +#include "mtk_vcodec_fw_vcp.h"
>
> struct mtk_vcodec_dec_dev;
> struct mtk_vcodec_enc_dev;
> @@ -13,6 +14,7 @@ struct mtk_vcodec_fw {
> const struct mtk_vcodec_fw_ops *ops;
> struct platform_device *pdev;
> struct mtk_scp *scp;
> + struct mtk_vcp *vcp;
> enum mtk_vcodec_fw_use fw_use;
> };
>
> @@ -49,4 +51,14 @@ mtk_vcodec_fw_scp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
> }
> #endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_SCP */
>
> +#if IS_ENABLED(CONFIG_VIDEO_MEDIATEK_VCODEC_VCP)
> +struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use);
> +#else
> +static inline struct mtk_vcodec_fw *
> +mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +#endif /* CONFIG_VIDEO_MEDIATEK_VCODEC_VCP */
> +
> #endif /* _MTK_VCODEC_FW_PRIV_H_ */
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> new file mode 100644
> index 000000000000..9fee52fed181
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> @@ -0,0 +1,505 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 MediaTek Inc.
> + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/dma-direction.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/iommu.h>
> +#include <linux/remoteproc/mtk_vcp_public.h>
> +#include <linux/firmware/mediatek/mtk-vcp-ipc.h>
> +
> +#include "../decoder/mtk_vcodec_dec_drv.h"
> +#include "../decoder/vdec_ipi_msg.h"
> +#include "mtk_vcodec_fw_priv.h"
> +
> +#define IPI_SEND_TIMEOUT_MS 100U
> +#define IPI_TIMEOUT_MS 100U
> +
> +#define VCP_IPI_HEADER_SIZE (sizeof(u32) * 2)
> +#define VCP_IPI_ALIGN (4)
> +
> +static struct mutex *mtk_vcodec_vcp_get_ipi_lock(struct mtk_vcp *vcp, u32 ipi_id)
> +{
> + return &vcp->ipi_desc[ipi_id].lock;
> +}
> +
> +static void mtk_vcodec_vcp_ipi_lock(struct mtk_vcp *vcp, u32 ipi_id)
> +{
> + struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp, ipi_id);
> +
> + if (!lock)
> + return;
> +
> + mutex_lock(lock);
> +}
> +
> +static void mtk_vcodec_vcp_ipi_unlock(struct mtk_vcp *vcp, u32 ipi_id)
> +{
> + struct mutex *lock = mtk_vcodec_vcp_get_ipi_lock(vcp, ipi_id);
> +
> + if (!lock)
> + return;
> +
> + lockdep_assert_held(lock);
> + mutex_unlock(lock);
> +}
> +
> +static spinlock_t *mtk_vcodec_vcp_get_msg_queue_lock(struct mtk_vcodec_fw *fw)
> +{
> + return &fw->vcp->msg_queue.lock;
> +}
> +
> +static void mtk_vcodec_vcp_msq_queue_lock(struct mtk_vcodec_fw *fw, unsigned long *flags)
> +{
> + spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
> +
> + if (!lock)
> + return;
> +
> + spin_lock_irqsave(lock, *flags);
> +}
> +
> +static void mtk_vcodec_vcp_msq_queue_unlock(struct mtk_vcodec_fw *fw, unsigned long *flags)
> +{
> + spinlock_t *lock = mtk_vcodec_vcp_get_msg_queue_lock(fw);
> +
> + if (!lock)
> + return;
> +
> + spin_unlock_irqrestore(lock, *flags);
> +}
> +
> +static int mtk_vcodec_vcp_notifier(struct notifier_block *nb, unsigned long event, void *ptr)
> +{
> + struct mtk_vcp *vcp = container_of(nb, struct mtk_vcp, vcp_notify);
> +
> + switch (event) {
> + case VCP_EVENT_SUSPEND:
> + case VCP_EVENT_STOP:
> + dev_dbg(&vcp->pdev->dev, "vcp notifier suspend");
> + break;
> + case VCP_EVENT_READY:
> + case VCP_EVENT_RESUME:
> + dev_dbg(&vcp->pdev->dev, "vcp notifier ready");
> + break;
> + }
> +
> + return NOTIFY_DONE;
> +}
> +
> +static void mtk_vcodec_vcp_free_msg_node(struct mtk_vcodec_fw *fw,
> + struct mtk_vcp_msg_node *msg_node)
> +{
> + unsigned long flags;
> +
> + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> + list_add(&msg_node->list, &fw->vcp->msg_queue.node_list);
> + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> +}
> +
> +static int mtk_vcodec_vcp_ipi_register(struct mtk_vcp *vcp, u32 ipi_id, vcp_ipi_handler_t handler,
> + void *priv)
> +{
> + if (!vcp)
> + return -EPROBE_DEFER;
> +
> + if (WARN_ON(ipi_id >= VCP_IPI_MAX) || WARN_ON(!handler))
> + return -EINVAL;
> +
> + mtk_vcodec_vcp_ipi_lock(vcp, ipi_id);
> + vcp->ipi_desc[ipi_id].handler = handler;
> + vcp->ipi_desc[ipi_id].priv = priv;
> + mtk_vcodec_vcp_ipi_unlock(vcp, ipi_id);
> +
> + return 0;
> +}
> +
> +static int mtk_vcodec_vcp_msg_process_thread(void *arg)
> +{
> + struct mtk_vcodec_fw *fw = arg;
> + struct vdec_vpu_ipi_ack *msg = NULL;
> + struct mtk_vcp_share_obj *obj;
> + struct mtk_vcp_msg_node *msg_node;
> + vcp_ipi_handler_t handler;
> + unsigned long flags;
> + int ret = 0;
> +
> + do {
> + ret = wait_event_interruptible(fw->vcp->msg_queue.wq,
> + atomic_read(&fw->vcp->msg_queue.cnt) > 0);
> + if (ret < 0) {
> + dev_err(&fw->pdev->dev, "wait msg queue ack timeout %d %d\n",
> + ret, atomic_read(&fw->vcp->msg_queue.cnt));
> + continue;
> + }
> +
> + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> + msg_node = list_entry(fw->vcp->msg_queue.msg_list.next,
> + struct mtk_vcp_msg_node, list);
> + list_del(&msg_node->list);
> + atomic_dec(&fw->vcp->msg_queue.cnt);
> + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> +
> + obj = &msg_node->ipi_data;
> + msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
> +
> + if (!msg->ap_inst_addr) {
> + dev_err(&fw->pdev->dev, "invalid message address\n");
> + mtk_vcodec_vcp_free_msg_node(fw, msg_node);
> + continue;
> + }
> +
> + dev_dbg(&fw->pdev->dev, "msg ack id %d len %d msg_id 0x%x\n", obj->id, obj->len,
> + msg->msg_id);
> +
> + mtk_vcodec_vcp_ipi_lock(fw->vcp, obj->id);
> + handler = fw->vcp->ipi_desc[obj->id].handler;
> + if (!handler) {
> + dev_err(&fw->pdev->dev, "invalid ack ipi handler id = %d\n", obj->id);
> + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
> + mtk_vcodec_vcp_free_msg_node(fw, msg_node);
> + return -EINVAL;
> + }
> +
> + handler(msg, obj->len, fw->vcp->ipi_desc[obj->id].priv);
> + mtk_vcodec_vcp_ipi_unlock(fw->vcp, obj->id);
> +
> + fw->vcp->msg_signaled[obj->id] = true;
> + wake_up(&fw->vcp->msg_wq[obj->id]);
> +
> + mtk_vcodec_vcp_free_msg_node(fw, msg_node);
> + } while (!kthread_should_stop());
> +
> + return ret;
> +}
> +
> +static int mtk_vcodec_vcp_msg_ack_isr(unsigned int id, void *prdata, void *data, unsigned int len)
> +{
> + struct mtk_vcodec_fw *fw = prdata;
> + struct mtk_vcp_msg_queue *msg_queue = &fw->vcp->msg_queue;
> + struct mtk_vcp_msg_node *msg_node;
> + struct vdec_vpu_ipi_ack *msg = NULL;
> + struct mtk_vcp_share_obj *obj = data;
> + unsigned long flags;
> +
> + msg = (struct vdec_vpu_ipi_ack *)obj->share_buf;
> +
> + mtk_vcodec_vcp_msq_queue_lock(fw, &flags);
> + if (!list_empty(&msg_queue->node_list)) {
> + msg_node = list_entry(msg_queue->node_list.next, struct mtk_vcp_msg_node, list);
> +
> + memcpy(&msg_node->ipi_data, obj, sizeof(*obj));
> + list_move_tail(&msg_node->list, &msg_queue->msg_list);
> + atomic_inc(&msg_queue->cnt);
> + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> +
> + dev_dbg(&fw->pdev->dev, "push ipi_id %x msg_id %x, msg cnt %d\n",
> + obj->id, msg->msg_id, atomic_read(&msg_queue->cnt));
> +
> + wake_up(&msg_queue->wq);
> + } else {
> + mtk_vcodec_vcp_msq_queue_unlock(fw, &flags);
> + dev_err(&fw->pdev->dev, "no free nodes in msg queue\n");
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_vcodec_vcp_msg_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
> + unsigned int len, unsigned int wait)
> +{
> + struct mtk_vcp *vcp = fw->vcp;
> + struct mtk_vcp_device *vcp_device = vcp->vcp_device;
> + bool *msg_signaled = &vcp->msg_signaled[id];
> + wait_queue_head_t *msg_wq = &vcp->msg_wq[id];
> + int ret, ipi_size, feature_id, mailbox_id, retry_cnt = 0;
> + unsigned long timeout_jiffies = 0;
> + struct mtk_vcp_share_obj obj = {0};
> + unsigned int *data;
> +
> + if (!vcp_device) {
> + dev_dbg(&fw->pdev->dev, "vcp device is null\n");
> + return -EINVAL;
> + }
> +
> + mutex_lock(&vcp->ipi_mutex);
> + feature_id = VDEC_FEATURE_ID;
> + mailbox_id = IPI_OUT_VDEC_1;
> +
> + timeout_jiffies = jiffies + msecs_to_jiffies(VCP_SYNC_TIMEOUT_MS);
> + while (!vcp_device->ops->vcp_is_ready(feature_id)) {
> + if (time_after(jiffies, timeout_jiffies)) {
> + vcp->ipi_id_ack[id] = -EINVAL;
> + ret = -EINVAL;
> + goto error;
> + }
> + mdelay(1);
> + }
> +
> + if (len > VCP_SHARE_BUF_SIZE) {
> + vcp->ipi_id_ack[id] = -EINVAL;
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + obj.id = id;
> + obj.len = len;
> + memcpy(obj.share_buf, buf, len);
> +
> + ipi_size = round_up(VCP_IPI_HEADER_SIZE + len, VCP_IPI_ALIGN);
> + data = (unsigned int *)obj.share_buf;
> + dev_dbg(&fw->pdev->dev, "vcp send message: id %d len %d data 0x%x\n",
> + obj.id, obj.len, data[0]);
> +
> + ret = mtk_vcp_ipc_send(vcp_get_ipidev(vcp_device), mailbox_id, &obj, ipi_size);
> + if (ret != IPI_ACTION_DONE) {
> + vcp->ipi_id_ack[id] = -EIO;
> + ret = -EIO;
> + goto error;
> + }
> +
> +wait_ack:
> + /* wait for VCP's ACK */
> + ret = wait_event_timeout(*msg_wq, *msg_signaled, msecs_to_jiffies(IPI_TIMEOUT_MS));
> + if (!ret || retry_cnt > 5) {
> + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> + dev_err(&fw->pdev->dev, "wait ipi ack timeout! %d %d\n", ret, vcp->ipi_id_ack[id]);
> + } else if (ret == -ERESTARTSYS) {
> + dev_err(&fw->pdev->dev, "wait ipi ack err (%d)\n", vcp->ipi_id_ack[id]);
> + retry_cnt++;
> + goto wait_ack;
> + } else if (ret < 0) {
> + dev_err(&fw->pdev->dev, "wait ipi ack fail ret %d %d\n", ret, vcp->ipi_id_ack[id]);
> + vcp->ipi_id_ack[id] = VCODEC_IPI_MSG_STATUS_FAIL;
> + }
> +
> + dev_dbg(&fw->pdev->dev, "receive message: id %d len %d data 0x%x\n",
> + obj.id, obj.len, data[0]);
> +
> + *msg_signaled = false;
> + mutex_unlock(&vcp->ipi_mutex);
> +
> + return vcp->ipi_id_ack[id];
> +
> +error:
> + mutex_unlock(&vcp->ipi_mutex);
> + dev_err(&fw->pdev->dev, "send msg error type:%d msg:%d > %d ret:%d\n", fw->type, len,
> + VCP_SHARE_BUF_SIZE, ret);
> +
> + return ret;
> +}
> +
> +static int check_vcp_loaded(struct mtk_vcodec_fw *fw)
> +{
> + struct device *dev = &fw->pdev->dev;
> + struct device_driver *drv;
> +
> + drv = driver_find("mtk-vcp", &platform_bus_type);
> + if (!drv) {
> + dev_err(dev, "find mtk-vcp driver failed, need to reload.");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_vcodec_vcp_get_vcp_device(struct mtk_vcodec_fw *fw)
> +{
> + struct device *dev = &fw->pdev->dev;
> + int retry = 0, retry_cnt = 10000;
> + phandle vcp_phandle;
> +
> + while (try_then_request_module(check_vcp_loaded(fw), "mtk-vcp")) {
> + if (++retry > retry_cnt) {
> + dev_err(dev, "failed to load mtk-vcp module");
> + return -EPROBE_DEFER;
> + }
> + msleep(1);
> + }
> +
> + if (of_property_read_u32(dev->of_node, "mediatek,vcp", &vcp_phandle)) {
> + dev_err(dev, "can't get vcp handle.\n");
> + return -ENODEV;
> + }
> +
> + fw->vcp->vcp_device = mtk_vcp_get_by_phandle(vcp_phandle);
> + if (!fw->vcp->vcp_device) {
> + dev_err(dev, "get vcp device failed\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static int mtk_vcodec_vcp_load_firmware(struct mtk_vcodec_fw *fw)
> +{
> + struct mtk_vcp_device *vcp_device;
> + int ret, feature_id, mem_id, mailbox_id, ipi_id;
> +
> + if (fw->vcp->is_init_done) {
> + dev_dbg(&fw->pdev->dev, "vcp has already been initialized done.\n");
> + return 0;
> + }
> +
> + if (mtk_vcodec_vcp_get_vcp_device(fw) < 0) {
> + dev_err(&fw->pdev->dev, "vcp device is null.\n");
> + return -EINVAL;
> + }
> +
> + vcp_device = fw->vcp->vcp_device;
> +
> + feature_id = VDEC_FEATURE_ID;
> + mem_id = VDEC_MEM_ID;
> + mailbox_id = IPI_IN_VDEC_1;
> + ipi_id = VCP_IPI_LAT_DECODER;
> +
> + ret = mtk_vcp_mbox_ipc_register(vcp_get_ipidev(vcp_device), mailbox_id,
> + mtk_vcodec_vcp_msg_ack_isr, fw, &fw->vcp->share_data);
> + if (ret) {
> + dev_dbg(&fw->pdev->dev, "ipi register fail %d %d %d %d\n", ret, feature_id,
> + mem_id, mailbox_id);
> + return -EINVAL;
> + }
> +
> + fw->vcp->vcp_notify.notifier_call = mtk_vcodec_vcp_notifier;
> + fw->vcp->vcp_notify.priority = 1;
> + vcp_device->ops->vcp_register_notify(feature_id, &fw->vcp->vcp_notify);
> +
> + if (!fw->vcp->is_register_done) {
> + ret = vcp_device->ops->vcp_register_feature(vcp_device, feature_id);
> + if (ret < 0) {
> + dev_err(&fw->pdev->dev, "%d register to vcp fail(%d)\n", feature_id, ret);
> + return -EINVAL;
> + }
> +
> + fw->vcp->is_register_done = true;
> + }
> +
> + fw->vcp->is_init_done = true;
> +
> + mutex_init(&fw->vcp->ipi_desc[ipi_id].lock);
> + mutex_init(&fw->vcp->ipi_mutex);
> +
> + kthread_run(mtk_vcodec_vcp_msg_process_thread, fw, "vcp_vdec_msq_thread");
> +
> + fw->vcp->vsi_addr = vcp_device->ops->vcp_get_mem_virt(mem_id);
> + fw->vcp->vsi_core_addr = fw->vcp->vsi_addr + VCODEC_VSI_LEN;
> + fw->vcp->vsi_size = vcp_device->ops->vcp_get_mem_size(mem_id);
> + fw->vcp->iova_addr = vcp_device->ops->vcp_get_mem_iova(mem_id);
> +
> + init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_LAT_DECODER]);
> + init_waitqueue_head(&fw->vcp->msg_wq[VCP_IPI_CORE_DECODER]);
> +
> + dev_dbg(&fw->pdev->dev, "vdec vcp init done => va: %p size:0x%x iova:%p.\n",
> + fw->vcp->vsi_addr, fw->vcp->vsi_size, &fw->vcp->iova_addr);
> +
> + return 0;
> +}
> +
> +static unsigned int mtk_vcodec_vcp_get_vdec_capa(struct mtk_vcodec_fw *fw)
> +{
> + return MTK_VDEC_FORMAT_MM21 | MTK_VDEC_FORMAT_H264_SLICE | MTK_VDEC_FORMAT_VP9_FRAME |
> + MTK_VDEC_FORMAT_AV1_FRAME | MTK_VDEC_FORMAT_HEVC_FRAME |
> + MTK_VDEC_IS_SUPPORT_10BIT | MTK_VDEC_IS_SUPPORT_EXT;
> +}
> +
> +static void *mtk_vcodec_vcp_dm_addr(struct mtk_vcodec_fw *fw, u32 dtcm_dmem_addr)
> +{
> + return NULL;
> +}
> +
> +static int mtk_vcodec_vcp_set_ipi_register(struct mtk_vcodec_fw *fw, int id,
> + mtk_vcodec_ipi_handler handler,
> + const char *name, void *priv)
> +{
> + return mtk_vcodec_vcp_ipi_register(fw->vcp, id, handler, priv);
> +}
> +
> +static int mtk_vcodec_vcp_ipi_send(struct mtk_vcodec_fw *fw, int id, void *buf,
> + unsigned int len, unsigned int wait)
> +{
> + return mtk_vcodec_vcp_msg_ipi_send(fw, id, buf, len, wait);
> +}
> +
> +static void mtk_vcodec_vcp_release(struct mtk_vcodec_fw *fw)
> +{
> + struct mtk_vcp_device *vcp_device = fw->vcp->vcp_device;
> + struct device *dev = &fw->pdev->dev;
> + int ret, feature_id;
> +
> + if (!fw->vcp->vcp_device) {
> + dev_err(dev, "vcp device is null\n");
> + return;
> + }
> +
> + if (!fw->vcp->is_register_done)
> + return;
> +
> + feature_id = VDEC_FEATURE_ID;
> + ret = vcp_device->ops->vcp_deregister_feature(vcp_device, VDEC_FEATURE_ID);
> + if (ret < 0) {
> + dev_err(dev, "deregister feature_id(%d) fail(%d)\n", feature_id, ret);
> + return;
> + }
> +
> + fw->vcp->is_register_done = false;
> +
> +}
> +
> +static const struct mtk_vcodec_fw_ops mtk_vcodec_vcp_msg = {
> + .load_firmware = mtk_vcodec_vcp_load_firmware,
> + .get_vdec_capa = mtk_vcodec_vcp_get_vdec_capa,
> + .map_dm_addr = mtk_vcodec_vcp_dm_addr,
> + .ipi_register = mtk_vcodec_vcp_set_ipi_register,
> + .ipi_send = mtk_vcodec_vcp_ipi_send,
> + .release = mtk_vcodec_vcp_release,
> +};
> +
> +struct mtk_vcodec_fw *mtk_vcodec_fw_vcp_init(void *priv, enum mtk_vcodec_fw_use fw_use)
> +{
> + struct mtk_vcp_msg_node *msg_node;
> + struct platform_device *plat_dev;
> + struct mtk_vcodec_fw *fw;
> + int i;
> +
> + if (fw_use == DECODER) {
> + struct mtk_vcodec_dec_dev *dec_dev = priv;
> +
> + plat_dev = dec_dev->plat_dev;
> + } else {
> + pr_err("Invalid fw_use %d (use a reasonable fw id here)\n", fw_use);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + fw = devm_kzalloc(&plat_dev->dev, sizeof(*fw), GFP_KERNEL);
> + if (!fw)
> + return ERR_PTR(-ENOMEM);
> +
> + fw->type = VCP;
> + fw->pdev = plat_dev;
> + fw->fw_use = fw_use;
> + fw->ops = &mtk_vcodec_vcp_msg;
> + fw->vcp = devm_kzalloc(&plat_dev->dev, sizeof(*fw->vcp), GFP_KERNEL);
> + if (!fw->vcp)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&fw->vcp->msg_queue.msg_list);
> + INIT_LIST_HEAD(&fw->vcp->msg_queue.node_list);
> + spin_lock_init(&fw->vcp->msg_queue.lock);
> + init_waitqueue_head(&fw->vcp->msg_queue.wq);
> + atomic_set(&fw->vcp->msg_queue.cnt, 0);
> + fw->vcp->pdev = plat_dev;
> +
> + for (i = 0; i < VCP_MAX_MQ_NODE_CNT; i++) {
> + msg_node = devm_kzalloc(&plat_dev->dev, sizeof(*msg_node), GFP_KERNEL);
> + if (!msg_node)
> + return ERR_PTR(-ENOMEM);
> +
> + list_add(&msg_node->list, &fw->vcp->msg_queue.node_list);
> + }
> +
> + return fw;
> +}
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> new file mode 100644
> index 000000000000..fada786124d5
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
> @@ -0,0 +1,139 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2025 MediaTek Inc.
> + * Author: Yunfei Dong <yunfei.dong@mediatek.com>
> + */
> +
> +#ifndef _MTK_VCODEC_FW_VCP_H_
> +#define _MTK_VCODEC_FW_VCP_H_
> +
> +typedef void (*vcp_ipi_handler_t) (void *data, unsigned int len, void *priv);
> +
> +#define VCP_MAX_MQ_NODE_CNT 6
> +#define VCP_SHARE_BUF_SIZE 64
> +
> +#define VCODEC_VSI_LEN (0x2000)
> +
> +/* enum mtk_vcp_ipi_index - index used to separate different hardware */
> +enum mtk_vcp_ipi_index {
> + VCP_IPI_LAT_DECODER,
> + VCP_IPI_CORE_DECODER,
> + VCP_IPI_MAX,
> +};
> +
> +/**
> + * struct mtk_vcp_msg_queue - process the vcp message between kernel with vcp
> + *
> + * @msg_list: store share buffer list which from vcp to kernel
> + * @wq: waitqueue that can be used to wait for vcp message
> + * @lock: protect msg list
> + * @cnt: the count of share obj in msg list
> + * @node_list: share obj list
> + */
> +struct mtk_vcp_msg_queue {
> + struct list_head msg_list;
> + wait_queue_head_t wq;
> + spinlock_t lock;
> + atomic_t cnt;
> + struct list_head node_list;
> +};
> +
> +/**
> + * struct mtk_vcp_ipi_desc - store the ack handler
> + *
> + * @lock: protect ack handler data
> + * @handler: calling this handler when kernel receive ack
> + * @priv: private data when calling handler to process
> + */
> +struct mtk_vcp_ipi_desc {
> + struct mutex lock;
> + vcp_ipi_handler_t handler;
> + void *priv;
> +};
> +
> +/**
> + * struct mtk_vcp_share_obj - share buffer used to send data to vcp
> + *
> + * @id: message index
> + * @len: message size
> + * @share_buf: message data
> + */
> +struct mtk_vcp_share_obj {
> + unsigned int id;
> + unsigned int len;
> + unsigned char share_buf[VCP_SHARE_BUF_SIZE];
> +};
> +
> +/* enum mtk_vcp_ipi_msg_status - the status when send message to vcp */
> +enum mtk_vcp_ipi_msg_status {
> + VCODEC_IPI_MSG_STATUS_OK = 0,
> + VCODEC_IPI_MSG_STATUS_FAIL = -1,
> + VCODEC_IPI_MSG_STATUS_MAX_INST = -2,
> + VCODEC_IPI_MSG_STATUS_ILSEQ = -3,
> + VCODEC_IPI_MSG_STATUS_INVALID_ID = -4,
> + VCODEC_IPI_MSG_STATUS_DMA_FAIL = -5,
> +};
> +
> +/**
> + * struct mtk_vcp_msg_node - share buffer used to send data to vcp
> + *
> + * @ipi_data: share obj data
> + * @list: list to store msg node
> + */
> +struct mtk_vcp_msg_node {
> + struct mtk_vcp_share_obj ipi_data;
> + struct list_head list;
> +};
> +
> +/**
> + * struct mtk_vcp - vcp firmware private data
> + *
> + * @is_init_done: vcp is ready to use
> + *
> + * @ipi_mutex: used to protect ipi data
> + * @msg_signaled: whether receive ack from vcp
> + * @msg_wq: wake message queue
> + *
> + * @ipi_desc: store ack handler
> + * @ipi_id_ack: the ack handler status
> + *
> + * @msg_queue: process vcp message
> + * @share_data: temp share obj data
> + *
> + * @vcp_notify: register notifier to vcp
> + *
> + * @vsi_addr: vsi virtual data address
> + * @vsi_core_addr: vsi core virtual data address
> + * @iova_addr: vsi iova address
> + * @vsi_size: vsi size
> + *
> + * @pdev: platform device
> + * @vcp_device: vcp private data
> + * @is_register_done: register vcodec to vcp
> + */
> +struct mtk_vcp {
> + bool is_init_done;
> +
> + struct mutex ipi_mutex;
> + bool msg_signaled[VCP_IPI_MAX];
> + wait_queue_head_t msg_wq[VCP_IPI_MAX];
> +
> + struct mtk_vcp_ipi_desc ipi_desc[VCP_IPI_MAX];
> + bool ipi_id_ack[VCP_IPI_MAX];
> +
> + struct mtk_vcp_msg_queue msg_queue;
> + struct mtk_vcp_share_obj share_data;
> +
> + struct notifier_block vcp_notify;
> +
> + void *vsi_addr;
> + void *vsi_core_addr;
> + dma_addr_t iova_addr;
> + int vsi_size;
> +
> + struct platform_device *pdev;
> + struct mtk_vcp_device *vcp_device;
> + bool is_register_done;
> +};
> +
> +#endif
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/3] mailbox: exynos: Add support for Exynos850 mailbox
From: Alexey Klimov @ 2026-04-28 20:26 UTC (permalink / raw)
To: Tudor Ambarus
Cc: Krzysztof Kozlowski, Sylwester Nawrocki, Chanwoo Choi,
Alim Akhtar, Sam Protsenko, Michael Turquette, Stephen Boyd,
Rob Herring, Conor Dooley, Jassi Brar, Krzysztof Kozlowski,
Peter Griffin, linux-samsung-soc, linux-arm-kernel, linux-clk,
devicetree, linux-kernel
In-Reply-To: <DHNSP3FVR4ZQ.1PIRHF0KJGKI5@linaro.org>
On Wed Apr 8, 2026 at 2:08 PM BST, Alexey Klimov wrote:
[...]
> On Thu Apr 2, 2026 at 9:42 AM BST, Tudor Ambarus wrote:
>>> static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
>>> @@ -57,7 +104,8 @@ static int exynos_mbox_send_data(struct mbox_chan *chan, void *data)
>>> return -EINVAL;
>>> }
>>>
>>> - writel(BIT(msg->chan_id), exynos_mbox->regs + EXYNOS_MBOX_INTGR1);
>>> + writel(BIT(msg->chan_id) << exynos_mbox->data->irq_doorbell_shift,
>>> + exynos_mbox->regs + exynos_mbox->data->irq_doorbell_offset);
>>
>> Use FIELD_PREP from <linux/bitfield.h> please. You will use a mask instead of
>> a shift.
>>
>> I would rename irq_doorbell_offset to intgr. It aligns with the register name
>> from the datasheet. You won't need to prepend _offset to the name, we already
>> see it's an offset when doing the writel().
>
> Sure. Thanks. Let's use FIELD_PREP.
+ /* Ring the doorbell */
+ writel(BIT(msg->chan_id) << exynos_mbox->data->intgr_shift,
+ exynos_mbox->regs + exynos_mbox->data->intgr);
FIELD_PREP() wants a mask as a compile-time constant. Unless you want me to
add switch/case for different SoCs or I misunderstood something I don't see
how this is gonna fly.
Best regards,
Alexey
^ permalink raw reply
* Re: [PATCH v18 7/8] ring-buffer: Cleanup persistent ring buffer validation
From: Steven Rostedt @ 2026-04-28 20:24 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Catalin Marinas, Will Deacon, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177701357903.2223789.1327164275262010250.stgit@mhiramat.tok.corp.google.com>
On Fri, 24 Apr 2026 15:52:59 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Cleanup rb_meta_validate_events() function to make it easier to read.
> This includes the following cleanups:
> - Introduce rb_validatation_state to hold working variables in
> validation.
> - Move repleated validation state updates into rb_validate_buffer().
> - Move reader_page injection code outside of rb_meta_validate_events().
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> kernel/trace/ring_buffer.c | 186 ++++++++++++++++++++++----------------------
> 1 file changed, 95 insertions(+), 91 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index de653a8e3cec..9850a0d8d24b 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1883,8 +1883,16 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
> return events;
> }
>
> -static int rb_validate_buffer(struct buffer_page *bpage, int cpu,
> - struct ring_buffer_cpu_meta *meta, u64 prev_ts, u64 next_ts)
> +struct rb_validation_state {
> + unsigned long entries;
> + unsigned long entry_bytes;
> + int discarded;
> + u64 ts;
> +};
> +
> +static int __rb_validate_buffer(struct buffer_page *bpage, int cpu,
> + struct ring_buffer_cpu_meta *meta,
> + u64 prev_ts, u64 next_ts)
> {
This can still use those comments (from patch 4).
Also, could you rebase on top of v7.1-rc1?
Thanks Masami!
-- Steve
> struct buffer_data_page *dpage = bpage->page;
> unsigned long long ts;
> @@ -1914,16 +1922,82 @@ static int rb_validate_buffer(struct buffer_page *bpage, int cpu,
> return ret;
> }
^ permalink raw reply
* Re: [PATCH v4 00/14] media: mediatek: vcodec: support video decoder in mt8196
From: Nicolas Dufresne @ 2026-04-28 20:24 UTC (permalink / raw)
To: Yunfei Dong, Nícolas F . R . A . Prado, Sebastian Fricke,
Hans Verkuil, AngeloGioacchino Del Regno, Benjamin Gaignard,
Nathan Hebert, Daniel Almeida
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho, linux-media,
devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260328051630.7937-1-yunfei.dong@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 4308 bytes --]
Le samedi 28 mars 2026 à 13:16 +0800, Yunfei Dong a écrit :
> This patch set adds support for the video decoder on the MediaTek MT8196
> platform, leveraging the VCP microprocessor for enhanced performance. It
> introduces new firmware interfaces for kernel-side communication with VCP,
> adds MT8196 compatible, codec levels/profiles, and private data. Rewrites
> the AV1 driver to support extended VSI structures, then change irq table
> and cdf table size.
>
> This patch set depends on "Add VCP support for mt8196"[1]
>
> [1]
> https://patchwork.kernel.org/project/linux-remoteproc/patch/20250402092134.12293-2-xiangzhi.tang@mediatek.com/
I see this had changes requested, will mark the series accordingly.
>
> Compliance Test Result:
> - v4l2-compliance: 48/49 tests passed, 1 failed (known issue)
> - fail: v4l2-test-controls.cpp(981): ret (got 13)
> - test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
>
> Fluster Test Result:
> - H264:
> - Ran 95/135 tests successfully in 73.540 secs
> - H265:
> - Ran 142/147 tests successfully in 100.168 secs
> - VP9:
> - Ran 276/305 tests successfully in 106.804 secs
> - AV1:
> - Ran 240/242 tests successfully in 23.991 secs
> ---
> Changed in v4:
> - Rebase to latest codebase
> - Move dt-bindings and compatible to the after of vcp driver ready
> - MT8189 base on MT8196 patch set
>
> Changed in v3:
> - Depends on change no update
> - Add Compliance and Fluster test result
> - Re-write the commit message for patch 12
> - Change the patches according to review suggestion for patch: 3/4/5/6/14
>
> Changed in v2:
> - re-write the commit message for patch 1
> ---
> Yunfei Dong (14):
> media: mediatek: vcodec: add driver to support vcp
> media: mediatek: vcodec: add driver to support vcp encoder
> media: mediatek: vcodec: get different firmware ipi id
> media: mediatek: vcodec: get share memory address
> media: mediatek: vcodec: define MT8196 vcodec levels.
> media: mediatek: vcodec: support vcp architecture
> media: mediatek: vcodec: support 36bit iova address
> media: mediatek: vcodec: clean xpc status
> media: mediatek: vcodec: add debug information
> media: mediatek: vcodec: send share memory address to vcp
> dt-bindings: media: mediatek: vcodec: add decoder dt-bindings for
> mt8196
> media: mediatek: vcodec: add decoder compatible to support mt8196
> media: mediatek: decoder: fill av1 buffer size with picinfo
> media: mediatek: decoder: support av1 extend vsi
>
> .../media/mediatek,vcodec-subdev-decoder.yaml | 1 +
> .../media/platform/mediatek/vcodec/Kconfig | 4 +
> .../platform/mediatek/vcodec/common/Makefile | 4 +
> .../mediatek/vcodec/common/mtk_vcodec_dbgfs.c | 21 +-
> .../mediatek/vcodec/common/mtk_vcodec_fw.c | 16 +
> .../mediatek/vcodec/common/mtk_vcodec_fw.h | 2 +
> .../vcodec/common/mtk_vcodec_fw_priv.h | 12 +
> .../vcodec/common/mtk_vcodec_fw_vcp.c | 561 ++++++++++++++++++
> .../vcodec/common/mtk_vcodec_fw_vcp.h | 153 +++++
> .../vcodec/decoder/mtk_vcodec_dec_drv.c | 16 +
> .../vcodec/decoder/mtk_vcodec_dec_drv.h | 1 +
> .../vcodec/decoder/mtk_vcodec_dec_hw.c | 28 +
> .../vcodec/decoder/mtk_vcodec_dec_hw.h | 13 +-
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 6 +
> .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 113 +++-
> .../decoder/vdec/vdec_h264_req_multi_if.c | 11 +-
> .../decoder/vdec/vdec_hevc_req_multi_if.c | 12 +-
> .../vcodec/decoder/vdec/vdec_vp8_req_if.c | 5 +-
> .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 27 +-
> .../mediatek/vcodec/decoder/vdec_ipi_msg.h | 2 +
> .../mediatek/vcodec/decoder/vdec_vpu_if.c | 12 +-
> .../mediatek/vcodec/encoder/mtk_vcodec_enc.c | 1 -
> .../mediatek/vcodec/encoder/mtk_vcodec_enc.h | 2 +
> 23 files changed, 985 insertions(+), 38 deletions(-)
> create mode 100644
> drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.c
> create mode 100644
> drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vcp.h
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v18 4/8] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer
From: Steven Rostedt @ 2026-04-28 20:21 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Catalin Marinas, Will Deacon, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, Ian Rogers, linux-arm-kernel
In-Reply-To: <177701355481.2223789.8929563537404268817.stgit@mhiramat.tok.corp.google.com>
On Fri, 24 Apr 2026 15:52:35 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> @@ -1892,9 +1895,19 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu,
> * subbuf_size is considered invalid.
> */
> tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
> - if (tail > meta->subbuf_size - BUF_PAGE_HDR_SIZE)
> - return -1;
> - return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
> + if (tail <= meta->subbuf_size - BUF_PAGE_HDR_SIZE)
> + ret = rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
> +
This code seriously needs comments.
-- Steve
> + if (ret < 0 || (prev_ts && prev_ts > ts) || (next_ts && ts > next_ts)) {
> + local_set(&bpage->entries, 0);
> + local_set(&bpage->page->commit, 0);
> + bpage->page->time_stamp = prev_ts ? prev_ts : next_ts;
> + ret = -1;
> + } else {
> + local_set(&bpage->entries, ret);
> + }
> +
> + return ret;
> }
>
^ permalink raw reply
* [PATCH v3 12/15] firmware: arm_scmi: Use proper iter_response_bound_cleanup() name
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
From: Geert Uytterhoeven <geert+renesas@glider.be>
The documentation speaks of the "iter_response_bound_cleanup()" protocol
helper, while the actual helper is called "iter_response_cleanup()".
Settle on the former name, because the helper is only needed when using
bound-iterators.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
[Cristian: removed Fixed tag referring the same series]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 6 +++---
drivers/firmware/arm_scmi/protocols.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index fd031a8d40df..57785c0c0424 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1875,7 +1875,7 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en
return 0;
}
-static void scmi_iterator_cleanup(void *iter)
+static void scmi_iterator_bound_cleanup(void *iter)
{
struct scmi_iterator *i = iter;
@@ -1888,7 +1888,7 @@ static int scmi_iterator_run(void *iter)
int ret;
ret = __scmi_iterator_run(iter, NULL, NULL);
- scmi_iterator_cleanup(iter);
+ scmi_iterator_bound_cleanup(iter);
return ret;
}
@@ -2078,7 +2078,7 @@ static const struct scmi_proto_helpers_ops helpers_ops = {
.iter_response_init = scmi_iterator_init,
.iter_response_run = scmi_iterator_run,
.iter_response_run_bound = scmi_iterator_run_bound,
- .iter_response_cleanup = scmi_iterator_cleanup,
+ .iter_response_bound_cleanup = scmi_iterator_bound_cleanup,
.protocol_msg_check = scmi_protocol_msg_check,
.fastchannel_init = scmi_common_fastchannel_init,
.fastchannel_db_ring = scmi_common_fastchannel_db_ring,
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index e2ef604c16ef..15ad5162e37a 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -286,7 +286,7 @@ struct scmi_proto_helpers_ops {
int (*iter_response_run)(void *iter);
int (*iter_response_run_bound)(void *iter,
unsigned int *start, unsigned int *end);
- void (*iter_response_cleanup)(void *iter);
+ void (*iter_response_bound_cleanup)(void *iter);
int (*protocol_msg_check)(const struct scmi_protocol_handle *ph,
u32 message_id, u32 *attributes);
void (*fastchannel_init)(const struct scmi_protocol_handle *ph,
--
2.53.0
^ permalink raw reply related
* [PATCH v3 14/15] firmware: arm_scmi: Fix OOB in scmi_clock_describe_rates_get_lazy()
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Lazy discovery of discrete rates works as follows:
A. Grab the first three rates,
B. Grab the last rate, if there are more than three rates.
It is up to the SCMI provider implementation to decide how many rates
are returned in response to a single CLOCK_DESCRIBE_RATES command. Each
rate received is stored in the scmi_clock_rates.rates[] array, and
.num_rates is updated accordingly.
When more than 3 rates have been received after step A, the last rate
may have been received already, and stored in scmi_clock_rates.rates[]
(which has space for scmi_clock_desc.tot_rates entries). Hence grabbing
the last rate again will store it a second time, beyond the end of the
array.
Fix this by only grabbing the last rate when we don't already have it.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
[Cristian: removed Fixed tag referring the same series]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/clock.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 15a963b1edb9..ba25a9c6d3ae 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -582,15 +582,18 @@ scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph,
if (ret)
goto out;
- /* If discrete grab the last value, which should be the max */
- if (clkd->rate_discrete && clkd->tot_rates > 3) {
+ /*
+ * If discrete and we don't already have it, grab the last value, which
+ * should be the max
+ */
+ if (clkd->rate_discrete && clkd->tot_rates > clkd->num_rates) {
first = clkd->tot_rates - 1;
last = clkd->tot_rates - 1;
ret = ph->hops->iter_response_run_bound(iter, &first, &last);
}
out:
- ph->hops->iter_response_cleanup(iter);
+ ph->hops->iter_response_bound_cleanup(iter);
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v3 15/15] firmware: arm_scmi: Introduce all_rates_get clock operation
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi, Peng Fan
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
Add a clock operation to get the whole set of rates available to a specific
clock: when needed this request could transparently trigger a full rate
discovery enumeration if this specific clock-rates were previously only
lazily enumerated.
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v2 --> v3
- collected Reviewed tags
---
drivers/firmware/arm_scmi/clock.c | 85 +++++++++++++++++++++----------
include/linux/scmi_protocol.h | 9 ++++
2 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index ba25a9c6d3ae..316d3cd90c81 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -159,10 +159,8 @@ struct scmi_clock_rate_notify_payld {
struct scmi_clock_desc {
u32 id;
- bool rate_discrete;
unsigned int tot_rates;
- unsigned int num_rates;
- u64 *rates;
+ struct scmi_clock_rates r;
#define RATE_MIN 0
#define RATE_MAX 1
#define RATE_STEP 2
@@ -469,10 +467,10 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st,
flags = le32_to_cpu(r->num_rates_flags);
st->num_remaining = NUM_REMAINING(flags);
st->num_returned = NUM_RETURNED(flags);
- p->clkd->rate_discrete = RATE_DISCRETE(flags);
+ p->clkd->r.rate_discrete = RATE_DISCRETE(flags);
/* Warn about out of spec replies ... */
- if (!p->clkd->rate_discrete &&
+ if (!p->clkd->r.rate_discrete &&
(st->num_returned != 3 || st->num_remaining != 0)) {
dev_warn(p->dev,
"Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n",
@@ -486,9 +484,9 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st,
if (!st->max_resources) {
unsigned int tot_rates = st->num_returned + st->num_remaining;
- p->clkd->rates = devm_kcalloc(p->dev, tot_rates,
- sizeof(*p->clkd->rates), GFP_KERNEL);
- if (!p->clkd->rates)
+ p->clkd->r.rates = devm_kcalloc(p->dev, tot_rates,
+ sizeof(*p->clkd->r.rates), GFP_KERNEL);
+ if (!p->clkd->r.rates)
return -ENOMEM;
/* max_resources is used by the iterators to control bounds */
@@ -507,10 +505,10 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
struct scmi_clk_ipriv *p = priv;
const struct scmi_msg_resp_clock_describe_rates *r = response;
- p->clkd->rates[p->clkd->num_rates] = RATE_TO_U64(r->rate[st->loop_idx]);
+ p->clkd->r.rates[p->clkd->r.num_rates] = RATE_TO_U64(r->rate[st->loop_idx]);
/* Count only effectively discovered rates */
- p->clkd->num_rates++;
+ p->clkd->r.num_rates++;
return 0;
}
@@ -531,7 +529,13 @@ scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph,
.dev = ph->dev,
};
- iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_DESCRIBE_RATES,
+ /*
+ * Using tot_rates as max_resources parameter here so as to trigger
+ * the dynamic allocation only when strictly needed: when trying a
+ * full enumeration after a lazy one tot_rates will be non-zero.
+ */
+ iter = ph->hops->iter_response_init(ph, &ops, clkd->tot_rates,
+ CLOCK_DESCRIBE_RATES,
sizeof(struct scmi_msg_clock_describe_rates),
&cpriv);
if (IS_ERR(iter))
@@ -542,12 +546,12 @@ scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph,
return ret;
/* empty set ? */
- if (!clkd->num_rates)
+ if (!clkd->r.num_rates)
return 0;
- if (clkd->rate_discrete)
- sort(clkd->rates, clkd->num_rates,
- sizeof(clkd->rates[0]), rate_cmp_func, NULL);
+ if (clkd->r.rate_discrete && PROTOCOL_REV_MAJOR(ph->version) == 0x1)
+ sort(clkd->r.rates, clkd->r.num_rates,
+ sizeof(clkd->r.rates[0]), rate_cmp_func, NULL);
return 0;
}
@@ -586,7 +590,7 @@ scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph,
* If discrete and we don't already have it, grab the last value, which
* should be the max
*/
- if (clkd->rate_discrete && clkd->tot_rates > clkd->num_rates) {
+ if (clkd->r.rate_discrete && clkd->tot_rates > clkd->r.num_rates) {
first = clkd->tot_rates - 1;
last = clkd->tot_rates - 1;
ret = ph->hops->iter_response_run_bound(iter, &first, &last);
@@ -618,16 +622,16 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph,
if (ret)
return ret;
- clkd->info.min_rate = clkd->rates[RATE_MIN];
- if (!clkd->rate_discrete) {
- clkd->info.max_rate = clkd->rates[RATE_MAX];
+ clkd->info.min_rate = clkd->r.rates[RATE_MIN];
+ if (!clkd->r.rate_discrete) {
+ clkd->info.max_rate = clkd->r.rates[RATE_MAX];
dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
- clkd->rates[RATE_MIN], clkd->rates[RATE_MAX],
- clkd->rates[RATE_STEP]);
+ clkd->r.rates[RATE_MIN], clkd->r.rates[RATE_MAX],
+ clkd->r.rates[RATE_STEP]);
} else {
- clkd->info.max_rate = clkd->rates[clkd->num_rates - 1];
+ clkd->info.max_rate = clkd->r.rates[clkd->r.num_rates - 1];
dev_dbg(ph->dev, "Clock:%s DISCRETE:%d -> Min %llu Max %llu\n",
- clkd->info.name, clkd->rate_discrete,
+ clkd->info.name, clkd->r.rate_discrete,
clkd->info.min_rate, clkd->info.max_rate);
}
@@ -732,7 +736,7 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
* If we can't figure out what rate it will be, so just return the
* rate back to the caller.
*/
- if (clkd->rate_discrete)
+ if (clkd->r.rate_discrete)
return 0;
fmin = clk->min_rate;
@@ -746,14 +750,40 @@ static int scmi_clock_determine_rate(const struct scmi_protocol_handle *ph,
}
ftmp = *rate - fmin;
- ftmp += clkd->rates[RATE_STEP] - 1; /* to round up */
- ftmp = div64_ul(ftmp, clkd->rates[RATE_STEP]);
+ ftmp += clkd->r.rates[RATE_STEP] - 1; /* to round up */
+ ftmp = div64_ul(ftmp, clkd->r.rates[RATE_STEP]);
- *rate = ftmp * clkd->rates[RATE_STEP] + fmin;
+ *rate = ftmp * clkd->r.rates[RATE_STEP] + fmin;
return 0;
}
+static const struct scmi_clock_rates *
+scmi_clock_all_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id)
+{
+ struct clock_info *ci = ph->get_priv(ph);
+ struct scmi_clock_desc *clkd;
+ struct scmi_clock_info *clk;
+
+ clk = scmi_clock_domain_lookup(ci, clk_id);
+ if (IS_ERR(clk) || !clk->name[0])
+ return NULL;
+
+ clkd = to_desc(clk);
+ /* Needs full enumeration ? */
+ if (clkd->r.rate_discrete && clkd->tot_rates != clkd->r.num_rates) {
+ int ret;
+
+ /* rates[] is already allocated BUT we need to re-enumerate */
+ clkd->r.num_rates = 0;
+ ret = scmi_clock_describe_rates_get_full(ph, clkd);
+ if (ret)
+ return NULL;
+ }
+
+ return &clkd->r;
+}
+
static int
scmi_clock_config_set(const struct scmi_protocol_handle *ph, u32 clk_id,
enum clk_state state,
@@ -1067,6 +1097,7 @@ static const struct scmi_clk_proto_ops clk_proto_ops = {
.rate_get = scmi_clock_rate_get,
.rate_set = scmi_clock_rate_set,
.determine_rate = scmi_clock_determine_rate,
+ .all_rates_get = scmi_clock_all_rates_get,
.enable = scmi_clock_enable,
.disable = scmi_clock_disable,
.state_get = scmi_clock_state_get,
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 5552ac04c820..c710107c2120 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -40,6 +40,12 @@ struct scmi_revision_info {
char sub_vendor_id[SCMI_SHORT_NAME_MAX_SIZE];
};
+struct scmi_clock_rates {
+ bool rate_discrete;
+ unsigned int num_rates;
+ u64 *rates;
+};
+
struct scmi_clock_info {
char name[SCMI_MAX_STR_SIZE];
unsigned int enable_latency;
@@ -85,6 +91,7 @@ enum scmi_clock_oem_config {
* clock calculating the closest allowed rate.
* Note that @rate is an input/output parameter used both to
* describe the requested rate and report the closest match
+ * @all_rates_get: get the list of all available rates for the specified clock.
* @enable: enables the specified clock
* @disable: disables the specified clock
* @state_get: get the status of the specified clock
@@ -104,6 +111,8 @@ struct scmi_clk_proto_ops {
u64 rate);
int (*determine_rate)(const struct scmi_protocol_handle *ph, u32 clk_id,
unsigned long *rate);
+ const struct scmi_clock_rates __must_check *(*all_rates_get)
+ (const struct scmi_protocol_handle *ph, u32 clk_id);
int (*enable)(const struct scmi_protocol_handle *ph, u32 clk_id,
bool atomic);
int (*disable)(const struct scmi_protocol_handle *ph, u32 clk_id,
--
2.53.0
^ permalink raw reply related
* [PATCH v3 13/15] firmware: arm_scmi: Use bound iterators to minimize discovered rates
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
Clock rates are guaranteed to be returned in ascending order for SCMI clock
protocol versions greater than 1.0: in such a case, use bounded iterators
to minimize the number of message exchanges needed to discover min and max
rate.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v1 --> v2
- fixed final ret value in scmi_clock_describe_get
---
drivers/firmware/arm_scmi/clock.c | 90 +++++++++++++++++++++++++++----
1 file changed, 81 insertions(+), 9 deletions(-)
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 8ce889dfc87b..15a963b1edb9 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -160,6 +160,7 @@ struct scmi_clock_rate_notify_payld {
struct scmi_clock_desc {
u32 id;
bool rate_discrete;
+ unsigned int tot_rates;
unsigned int num_rates;
u64 *rates;
#define RATE_MIN 0
@@ -483,15 +484,16 @@ iter_clk_describe_update_state(struct scmi_iterator_state *st,
}
if (!st->max_resources) {
- int num_rates = st->num_returned + st->num_remaining;
+ unsigned int tot_rates = st->num_returned + st->num_remaining;
- p->clkd->rates = devm_kcalloc(p->dev, num_rates,
+ p->clkd->rates = devm_kcalloc(p->dev, tot_rates,
sizeof(*p->clkd->rates), GFP_KERNEL);
if (!p->clkd->rates)
return -ENOMEM;
/* max_resources is used by the iterators to control bounds */
- st->max_resources = st->num_returned + st->num_remaining;
+ p->clkd->tot_rates = tot_rates;
+ st->max_resources = tot_rates;
}
return 0;
@@ -514,8 +516,8 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
}
static int
-scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
- struct clock_info *cinfo)
+scmi_clock_describe_rates_get_full(const struct scmi_protocol_handle *ph,
+ struct scmi_clock_desc *clkd)
{
int ret;
void *iter;
@@ -524,7 +526,6 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
.update_state = iter_clk_describe_update_state,
.process_response = iter_clk_describe_process_response,
};
- struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id];
struct scmi_clk_ipriv cpriv = {
.clkd = clkd,
.dev = ph->dev,
@@ -544,17 +545,88 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
if (!clkd->num_rates)
return 0;
+ if (clkd->rate_discrete)
+ sort(clkd->rates, clkd->num_rates,
+ sizeof(clkd->rates[0]), rate_cmp_func, NULL);
+
+ return 0;
+}
+
+static int
+scmi_clock_describe_rates_get_lazy(const struct scmi_protocol_handle *ph,
+ struct scmi_clock_desc *clkd)
+{
+ struct scmi_iterator_ops ops = {
+ .prepare_message = iter_clk_describe_prepare_message,
+ .update_state = iter_clk_describe_update_state,
+ .process_response = iter_clk_describe_process_response,
+ };
+ struct scmi_clk_ipriv cpriv = {
+ .clkd = clkd,
+ .dev = ph->dev,
+ };
+ unsigned int first, last;
+ void *iter;
+ int ret;
+
+ iter = ph->hops->iter_response_init(ph, &ops, 0, CLOCK_DESCRIBE_RATES,
+ sizeof(struct scmi_msg_clock_describe_rates),
+ &cpriv);
+ if (IS_ERR(iter))
+ return PTR_ERR(iter);
+
+ /* Try to grab a triplet, so that in case is NON-discrete we are done */
+ first = 0;
+ last = 2;
+ ret = ph->hops->iter_response_run_bound(iter, &first, &last);
+ if (ret)
+ goto out;
+
+ /* If discrete grab the last value, which should be the max */
+ if (clkd->rate_discrete && clkd->tot_rates > 3) {
+ first = clkd->tot_rates - 1;
+ last = clkd->tot_rates - 1;
+ ret = ph->hops->iter_response_run_bound(iter, &first, &last);
+ }
+
+out:
+ ph->hops->iter_response_cleanup(iter);
+
+ return ret;
+}
+
+static int
+scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph,
+ u32 clk_id, struct clock_info *cinfo)
+{
+ struct scmi_clock_desc *clkd = &cinfo->clkds[clk_id];
+ int ret;
+
+ /*
+ * Since only after SCMI Clock v1.0 the returned rates are guaranteed to
+ * be discovered in ascending order, lazy enumeration cannot be use for
+ * SCMI Clock v1.0 protocol.
+ */
+ if (PROTOCOL_REV_MAJOR(ph->version) > 0x1)
+ ret = scmi_clock_describe_rates_get_lazy(ph, clkd);
+ else
+ ret = scmi_clock_describe_rates_get_full(ph, clkd);
+
+ if (ret)
+ return ret;
+
+ clkd->info.min_rate = clkd->rates[RATE_MIN];
if (!clkd->rate_discrete) {
clkd->info.max_rate = clkd->rates[RATE_MAX];
dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
clkd->rates[RATE_MIN], clkd->rates[RATE_MAX],
clkd->rates[RATE_STEP]);
} else {
- sort(clkd->rates, clkd->num_rates,
- sizeof(clkd->rates[0]), rate_cmp_func, NULL);
clkd->info.max_rate = clkd->rates[clkd->num_rates - 1];
+ dev_dbg(ph->dev, "Clock:%s DISCRETE:%d -> Min %llu Max %llu\n",
+ clkd->info.name, clkd->rate_discrete,
+ clkd->info.min_rate, clkd->info.max_rate);
}
- clkd->info.min_rate = clkd->rates[RATE_MIN];
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v3 10/15] firmware: arm_scmi: Add bound iterators support
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
SCMI core stack provides some common helpers to handle in a unified way
multipart message replies: such iterator-helpers, when run, currently
process by default the whole set of discovered resources.
Introduce an alternative way to run the initialized iterator on a limited
range of resources.
Note that the subset of resources that can be chosen is anyway limited by
the SCMI protocol specification, since you are only allowed to choose the
start-index on a multi-part enumeration NOT the end-index, so that the
effective number of returned items by a bound iterators depends really
on platform side decisions.
Suggested-by: Etienne Carriere <etienne.carriere@foss.st.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v2 --> v3
- fixed typos in commit message
---
drivers/firmware/arm_scmi/clock.c | 3 +-
drivers/firmware/arm_scmi/driver.c | 58 +++++++++++++++++++--------
drivers/firmware/arm_scmi/protocols.h | 13 +++++-
3 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index d07cfef243fd..8ce889dfc87b 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -505,8 +505,7 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
struct scmi_clk_ipriv *p = priv;
const struct scmi_msg_resp_clock_describe_rates *r = response;
- p->clkd->rates[st->desc_index + st->loop_idx] =
- RATE_TO_U64(r->rate[st->loop_idx]);
+ p->clkd->rates[p->clkd->num_rates] = RATE_TO_U64(r->rate[st->loop_idx]);
/* Count only effectively discovered rates */
p->clkd->num_rates++;
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 66cb64c8ed3d..cb4865fd8af2 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1813,48 +1813,50 @@ static void *scmi_iterator_init(const struct scmi_protocol_handle *ph,
return no_free_ptr(i);
}
-static int scmi_iterator_run(void *iter)
+static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *end)
{
int ret;
struct scmi_iterator_ops *iops;
const struct scmi_protocol_handle *ph;
struct scmi_iterator_state *st;
+ struct scmi_iterator *i;
if (!iter)
return -EINVAL;
- /* Take ownership of the iterator */
- struct scmi_iterator *i __free(kfree) = iter;
-
+ i = iter;
iops = i->ops;
ph = i->ph;
st = &i->state;
+ /* Reinitialize state for next run */
+ st->num_returned = 0;
+ st->num_remaining = 0;
+ st->desc_index = start ? *start : 0;
+
do {
iops->prepare_message(i->msg, st->desc_index, i->priv);
ret = ph->xops->do_xfer(ph, i->t);
if (ret)
- break;
+ return ret;
st->rx_len = i->t->rx.len;
ret = iops->update_state(st, i->resp, i->priv);
if (ret)
- break;
+ return ret;
if (st->num_returned > st->max_resources - st->desc_index) {
dev_err(ph->dev,
"No. of resources can't exceed %d\n",
st->max_resources);
- ret = -EINVAL;
- break;
+ return -EINVAL;
}
- for (st->loop_idx = 0; !ret && st->loop_idx < st->num_returned;
- st->loop_idx++)
+ for (st->loop_idx = 0; st->loop_idx < st->num_returned; st->loop_idx++) {
ret = iops->process_response(ph, i->resp, st, i->priv);
-
- if (ret)
- break;
+ if (ret)
+ return ret;
+ }
st->desc_index += st->num_returned;
ph->xops->reset_rx_to_maxsz(ph, i->t);
@@ -1862,14 +1864,36 @@ static int scmi_iterator_run(void *iter)
* check for both returned and remaining to avoid infinite
* loop due to buggy firmware
*/
- } while (st->num_returned && st->num_remaining);
+ } while (st->num_returned && st->num_remaining &&
+ (!end || (st->desc_index <= min(*end, st->max_resources - 1))));
- /* Finalize and destroy iterator */
- ph->xops->xfer_put(ph, i->t);
+ return 0;
+}
+
+static void scmi_iterator_cleanup(void *iter)
+{
+ struct scmi_iterator *i = iter;
+
+ i->ph->xops->xfer_put(i->ph, i->t);
+ kfree(i);
+}
+
+static int scmi_iterator_run(void *iter)
+{
+ int ret;
+
+ ret = __scmi_iterator_run(iter, NULL, NULL);
+ scmi_iterator_cleanup(iter);
return ret;
}
+static int scmi_iterator_run_bound(void *iter, unsigned int *start,
+ unsigned int *end)
+{
+ return __scmi_iterator_run(iter, start, end);
+}
+
struct scmi_msg_get_fc_info {
__le32 domain;
__le32 message_id;
@@ -2048,6 +2072,8 @@ static const struct scmi_proto_helpers_ops helpers_ops = {
.get_max_msg_size = scmi_common_get_max_msg_size,
.iter_response_init = scmi_iterator_init,
.iter_response_run = scmi_iterator_run,
+ .iter_response_run_bound = scmi_iterator_run_bound,
+ .iter_response_cleanup = scmi_iterator_cleanup,
.protocol_msg_check = scmi_protocol_msg_check,
.fastchannel_init = scmi_common_fastchannel_init,
.fastchannel_db_ring = scmi_common_fastchannel_db_ring,
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index f51245aca259..e2ef604c16ef 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -259,7 +259,15 @@ struct scmi_fc_info {
* multi-part responses using the custom operations
* provided in @ops.
* @iter_response_run: A common helper to trigger the run of a previously
- * initialized iterator.
+ * initialized iterator. Note that unbound iterators are
+ * automatically cleaned up.
+ * @iter_response_run_bound: A common helper to trigger the run of a previously
+ * initialized iterator, but only within the
+ * specified, optional, @start and @end resource
+ * indexes. Note that these bound-iterators need
+ * explicit cleanup via @iter_response_bound_cleanup.
+ * @iter_response_bound_cleanup: A common helper to finally release the iterator
+ * for bound iterators.
* @protocol_msg_check: A common helper to check is a specific protocol message
* is supported.
* @fastchannel_init: A common helper used to initialize FC descriptors by
@@ -276,6 +284,9 @@ struct scmi_proto_helpers_ops {
unsigned int max_resources, u8 msg_id,
size_t tx_size, void *priv);
int (*iter_response_run)(void *iter);
+ int (*iter_response_run_bound)(void *iter,
+ unsigned int *start, unsigned int *end);
+ void (*iter_response_cleanup)(void *iter);
int (*protocol_msg_check)(const struct scmi_protocol_handle *ph,
u32 message_id, u32 *attributes);
void (*fastchannel_init)(const struct scmi_protocol_handle *ph,
--
2.53.0
^ permalink raw reply related
* [PATCH v3 11/15] firmware: arm_scmi: Fix bound iterators returning too many items
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
From: Geert Uytterhoeven <geert+renesas@glider.be>
When using a bound-iterator with an upper bound, commands are sent, and
responses are received, until the upper bound is reached. However, it
is up to the SCMI provider implementation to decide how many rates are
returned in response to a single CLOCK_DESCRIBE_RATES command. If the
last response contains rates beyond the specified upper bound, they are
still passed up for further processing. This may lead to buffer
overflows in unprepared callsites.
While the imprecise bound handling may have been intentional (it was
mentioned in the commit message introducing the code), it is still
confusing for users, and may cause hard to debug crashes. Fix this by
strictly enforcing the upper bound.
Note that this may cause an increase in the number of
CLOCK_DESCRIBE_RATES commands issued, as retrieving the last rate may no
longer be done inadvertentently, but require its own command.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
[Cristian: removed Fixed tag referring the same series]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/driver.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index cb4865fd8af2..fd031a8d40df 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -1820,6 +1820,7 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en
const struct scmi_protocol_handle *ph;
struct scmi_iterator_state *st;
struct scmi_iterator *i;
+ unsigned int n;
if (!iter)
return -EINVAL;
@@ -1852,13 +1853,17 @@ static int __scmi_iterator_run(void *iter, unsigned int *start, unsigned int *en
return -EINVAL;
}
- for (st->loop_idx = 0; st->loop_idx < st->num_returned; st->loop_idx++) {
+ if (end)
+ n = min(st->num_returned, *end - st->desc_index + 1);
+ else
+ n = st->num_returned;
+ for (st->loop_idx = 0; st->loop_idx < n; st->loop_idx++) {
ret = iops->process_response(ph, i->resp, st, i->priv);
if (ret)
return ret;
}
- st->desc_index += st->num_returned;
+ st->desc_index += n;
ph->xops->reset_rx_to_maxsz(ph, i->t);
/*
* check for both returned and remaining to avoid infinite
--
2.53.0
^ permalink raw reply related
* [PATCH v3 09/15] firmware: arm_scmi: Refactor iterators internal allocation
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi, Peng Fan
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
Use cleanup handlers to manage iterator data structures.
No functional change.
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v2 --> v3
- collected Reviewed tags
---
drivers/firmware/arm_scmi/driver.c | 35 +++++++++++++++---------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index f167194f7cf6..66cb64c8ed3d 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -17,6 +17,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/bitmap.h>
+#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/export.h>
@@ -1789,39 +1790,41 @@ static void *scmi_iterator_init(const struct scmi_protocol_handle *ph,
size_t tx_size, void *priv)
{
int ret;
- struct scmi_iterator *i;
- i = devm_kzalloc(ph->dev, sizeof(*i), GFP_KERNEL);
+ struct scmi_iterator *i __free(kfree) = kzalloc(sizeof(*i), GFP_KERNEL);
if (!i)
return ERR_PTR(-ENOMEM);
+ if (!ops || !ph)
+ return ERR_PTR(-EINVAL);
+
i->ph = ph;
i->ops = ops;
i->priv = priv;
ret = ph->xops->xfer_get_init(ph, msg_id, tx_size, 0, &i->t);
- if (ret) {
- devm_kfree(ph->dev, i);
+ if (ret)
return ERR_PTR(ret);
- }
i->state.max_resources = max_resources;
i->msg = i->t->tx.buf;
i->resp = i->t->rx.buf;
- return i;
+ return no_free_ptr(i);
}
static int scmi_iterator_run(void *iter)
{
- int ret = -EINVAL;
+ int ret;
struct scmi_iterator_ops *iops;
const struct scmi_protocol_handle *ph;
struct scmi_iterator_state *st;
- struct scmi_iterator *i = iter;
- if (!i || !i->ops || !i->ph)
- return ret;
+ if (!iter)
+ return -EINVAL;
+
+ /* Take ownership of the iterator */
+ struct scmi_iterator *i __free(kfree) = iter;
iops = i->ops;
ph = i->ph;
@@ -1846,12 +1849,12 @@ static int scmi_iterator_run(void *iter)
break;
}
- for (st->loop_idx = 0; st->loop_idx < st->num_returned;
- st->loop_idx++) {
+ for (st->loop_idx = 0; !ret && st->loop_idx < st->num_returned;
+ st->loop_idx++)
ret = iops->process_response(ph, i->resp, st, i->priv);
- if (ret)
- goto out;
- }
+
+ if (ret)
+ break;
st->desc_index += st->num_returned;
ph->xops->reset_rx_to_maxsz(ph, i->t);
@@ -1861,10 +1864,8 @@ static int scmi_iterator_run(void *iter)
*/
} while (st->num_returned && st->num_remaining);
-out:
/* Finalize and destroy iterator */
ph->xops->xfer_put(ph, i->t);
- devm_kfree(ph->dev, i);
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v3 06/15] firmware: arm_scmi: Drop unused clock rate interfaces
From: Cristian Marussi @ 2026-04-28 20:15 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, arm-scmi, linux-clk,
linux-renesas-soc
Cc: sudeep.holla, philip.radford, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek,
geert+renesas, kuninori.morimoto.gx, marek.vasut+renesas,
Cristian Marussi, Peng Fan
In-Reply-To: <20260428201522.903875-1-cristian.marussi@arm.com>
Only the unified interface exposing min_rate/max_rate is now used.
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v1 --> v2
- Collected Peng Reviewed-by tag
---
include/linux/scmi_protocol.h | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 7283302b0c85..d97b4e734744 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -53,17 +53,6 @@ struct scmi_clock_info {
bool extended_config;
u64 min_rate;
u64 max_rate;
- union {
- struct {
- int num_rates;
- u64 rates[SCMI_MAX_NUM_RATES];
- } list;
- struct {
- u64 min_rate;
- u64 max_rate;
- u64 step_size;
- } range;
- };
int num_parents;
u32 *parents;
};
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox