* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 11:40 [PATCH RFC] drm/msm: move VM_BIND op validation to Rust Konrad Dybcio
@ 2026-10-26 14:39 ` Rob Clark
2026-10-26 15:07 ` Konrad Dybcio
2026-10-26 15:26 ` Rob Clark
2026-10-29 9:48 ` Alice Ryhl
2 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2026-10-26 14:39 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux, Konrad Dybcio
On Mon, Oct 26, 2026 at 4:40 AM Konrad Dybcio <konradybcio@kernel.org> wrote:
>
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Much of the VM handling code is a headache to read and maintain.
> There's a soup of mixed-signedness/size types which need to be compared
> and added and getting that wrong in C is very easy.
>
> The "(range + obj_offset) > obj->size" check could wrap for a large
> obj_offset, which let an out-of-range map be accepted and then mapped
> as nothing. Oxidize it and use checked_add() to fix that.
>
> This commit is a "get the foot in the door" type change to make sure
> that folks building kernels for Qualcomm SoCs have their Rust setup
> ready. It may/should be extended to the wider mm code once the
> appropriate abstractions are finalized.
>
> Worth noting, the C UERR() macro expands to dev_dbg() and the Rust
> implementation for that currently doesn't support dynamic_debug.
>
> The DRM_BRIDGE dependency is dropped because including RUST exposed
> some spaghetti dependencies ending at DEBUGFS and it's enabled anyway.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> (if this is rejected we may just patch the C code).
>
> Notably, this will regress some ancient configurations:
> -- arch/arm/Kconfig
> select HAVE_RUST if CPU_LITTLE_ENDIAN && CPU_32v7 && !KASAN
>
> Smoke-tested on Glymur
> ---
> drivers/gpu/drm/msm/Kconfig | 2 +-
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/msm_gem_vma.c | 66 +++------------
> drivers/gpu/drm/msm/msm_vm_bind.h | 16 ++++
> drivers/gpu/drm/msm/msm_vm_bind.rs | 160 +++++++++++++++++++++++++++++++++++++
> rust/uapi/uapi_helper.h | 1 +
> 6 files changed, 188 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index ed1c4775d1e9..f5c4749f8e95 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -3,6 +3,7 @@
> config DRM_MSM
> tristate "MSM DRM"
> depends on DRM
> + depends on RUST
> depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
> depends on COMMON_CLK
> depends on IOMMU_SUPPORT
> @@ -63,7 +64,6 @@ config DRM_MSM_VALIDATE_XML
> config DRM_MSM_KMS
> def_bool n
> depends on DRM_MSM
> - select DRM_BRIDGE
> select DRM_BRIDGE_CONNECTOR
> select DRM_CLIENT_SELECTION
> select DRM_DISPLAY_HELPER
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 1e246616f83f..826759a820a0 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -129,6 +129,7 @@ msm-y += \
> msm_ringbuffer.o \
> msm_submitqueue.o \
> msm_syncobj.o \
> + msm_vm_bind.o \
> msm_gpu_tracepoints.o \
>
> msm-$(CONFIG_DRM_MSM_KMS) += \
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index f687a629629d..3c453b034e8f 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -14,6 +14,7 @@
> #include "msm_gpu.h"
> #include "msm_mmu.h"
> #include "msm_syncobj.h"
> +#include "msm_vm_bind.h"
>
> #define vm_dbg(fmt, ...) pr_debug("%s:%d: "fmt"\n", __func__, __LINE__, ##__VA_ARGS__)
>
> @@ -981,23 +982,12 @@ vm_bind_job_create(struct drm_device *dev, struct drm_file *file, struct drm_gpu
> return job;
> }
>
> -static bool invalid_alignment(uint64_t addr)
> -{
> - /*
> - * Technically this is about GPU alignment, not CPU alignment. But
> - * I've not seen any qcom SoC where the SMMU does not support the
> - * CPU's smallest page size.
> - */
> - return !PAGE_ALIGNED(addr);
> -}
> -
> static int
> lookup_op(struct msm_vm_bind_job *job, const struct drm_msm_vm_bind_op *op)
> {
> struct drm_device *dev = job->vm->drm;
> struct msm_drm_private *priv = dev->dev_private;
> int i = job->nr_ops++;
> - int ret = 0;
>
> job->ops[i].op = op->op;
> job->ops[i].handle = op->handle;
> @@ -1006,48 +996,11 @@ lookup_op(struct msm_vm_bind_job *job, const struct drm_msm_vm_bind_op *op)
> job->ops[i].range = op->range;
> job->ops[i].flags = op->flags;
>
> - if (op->flags & ~MSM_VM_BIND_OP_FLAGS)
> - ret = UERR(EINVAL, dev, "invalid flags: %x\n", op->flags);
> -
> - if (invalid_alignment(op->iova))
> - ret = UERR(EINVAL, dev, "invalid address: %016llx\n", op->iova);
> -
> - if (invalid_alignment(op->obj_offset))
> - ret = UERR(EINVAL, dev, "invalid bo_offset: %016llx\n", op->obj_offset);
> -
> - if (invalid_alignment(op->range))
> - ret = UERR(EINVAL, dev, "invalid range: %016llx\n", op->range);
> -
> - if (!drm_gpuvm_range_valid(job->vm, op->iova, op->range))
> - ret = UERR(EINVAL, dev, "invalid range: %016llx, %016llx\n", op->iova, op->range);
> -
> - /*
> - * MAP must specify a valid handle. But the handle MBZ for
> - * UNMAP or MAP_NULL.
> - */
> - if (op->op == MSM_VM_BIND_OP_MAP) {
> - if (!op->handle)
> - ret = UERR(EINVAL, dev, "invalid handle\n");
> - } else if (op->handle) {
> - ret = UERR(EINVAL, dev, "handle must be zero\n");
> - }
> -
> - switch (op->op) {
> - case MSM_VM_BIND_OP_MAP:
> - case MSM_VM_BIND_OP_MAP_NULL:
> - case MSM_VM_BIND_OP_UNMAP:
> - break;
> - default:
> - ret = UERR(EINVAL, dev, "invalid op: %u\n", op->op);
> - break;
> - }
> -
> - if ((op->op == MSM_VM_BIND_OP_MAP_NULL) &&
> - !adreno_smmu_has_prr(priv->gpu)) {
> - ret = UERR(EINVAL, dev, "PRR not supported\n");
> - }
> -
> - return ret;
> + return msm_vm_bind_check_op(dev->dev, op->op, op->handle, op->obj_offset,
> + op->iova, op->range, op->flags, PAGE_SIZE,
> + drm_gpuvm_range_valid(job->vm, op->iova, op->range),
> + op->op == MSM_VM_BIND_OP_MAP_NULL &&
> + adreno_smmu_has_prr(priv->gpu));
> }
>
> /*
> @@ -1118,11 +1071,10 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
> op->obj = obj;
> cnt++;
>
> - if ((op->range + op->obj_offset) > obj->size) {
> - ret = UERR(EINVAL, dev, "invalid range: %016llx + %016llx > %016zx\n",
> - op->range, op->obj_offset, obj->size);
> + ret = msm_vm_bind_check_bo_range(dev->dev, op->obj_offset, op->range,
> + obj->size);
> + if (ret)
I think the ioctl surface area is a sensible place to start using
rust.. but seems like this patch could instead be a 1-ish line patch
to use check_add_overflow() here?
Big picture, I'm a bit more interested in the
goto-error-unwind-spagatti-monstors. Even there, just using more
cleanup.h would be an improvement.
BR,
-R
> goto out_unlock;
> - }
> }
>
> *nr_bos = cnt;
> diff --git a/drivers/gpu/drm/msm/msm_vm_bind.h b/drivers/gpu/drm/msm/msm_vm_bind.h
> new file mode 100644
> index 000000000000..e8aa6adb9181
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_vm_bind.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __MSM_VM_BIND_H__
> +#define __MSM_VM_BIND_H__
> +
> +#include <linux/types.h>
> +
> +struct device;
> +
> +int msm_vm_bind_check_op(struct device *dev, u32 op, u32 handle,
> + u64 obj_offset, u64 iova, u64 range, u32 flags,
> + u64 page_size, bool range_valid, bool prr_supported);
> +int msm_vm_bind_check_bo_range(struct device *dev, u64 obj_offset, u64 range,
> + u64 obj_size);
> +
> +#endif /* __MSM_VM_BIND_H__ */
> diff --git a/drivers/gpu/drm/msm/msm_vm_bind.rs b/drivers/gpu/drm/msm/msm_vm_bind.rs
> new file mode 100644
> index 000000000000..164208e0a1b4
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_vm_bind.rs
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//! VM_BIND operation validation for the MSM driver.
> +
> +use kernel::{
> + bindings,
> + device::Device,
> + error::code::EINVAL,
> + prelude::*,
> + uapi, //
> +};
> +
> +/// Validates a single VM_BIND operation.
> +///
> +/// Returns 0 if the operation is valid. Otherwise logs the reason and returns
> +/// `-EINVAL`.
> +///
> +/// `page_size` is the granularity that `iova`, `obj_offset` and `range` must be
> +/// aligned to. It is the GPU's alignment rather than the CPU's, but no known qcom
> +/// SMMU lacks support for the CPU's smallest page size.
> +/// `range_valid` is the result of `drm_gpuvm_range_valid()` for this operation.
> +/// `prr_supported` is only consulted for `MSM_VM_BIND_OP_MAP_NULL`.
> +///
> +/// # Safety
> +///
> +/// `dev` must point to a live `struct device`.
> +#[no_mangle]
> +pub unsafe extern "C" fn msm_vm_bind_check_op(
> + dev: *mut bindings::device,
> + op: u32,
> + handle: u32,
> + obj_offset: u64,
> + iova: u64,
> + range: u64,
> + flags: u32,
> + page_size: u64,
> + range_valid: bool,
> + prr_supported: bool,
> +) -> c_int {
> + // SAFETY: The caller guarantees that `dev` is a live `struct device`.
> + let dev: &Device = unsafe { Device::from_raw(dev) };
> +
> + match check_op(
> + dev,
> + op,
> + handle,
> + obj_offset,
> + iova,
> + range,
> + flags,
> + page_size,
> + range_valid,
> + prr_supported,
> + ) {
> + Ok(()) => 0,
> + Err(e) => e.to_errno(),
> + }
> +}
> +
> +fn check_op(
> + dev: &Device,
> + op: u32,
> + handle: u32,
> + obj_offset: u64,
> + iova: u64,
> + range: u64,
> + flags: u32,
> + page_size: u64,
> + range_valid: bool,
> + prr_supported: bool,
> +) -> Result {
> + let aligned = |addr: u64| page_size.is_power_of_two() && (addr & (page_size - 1)) == 0;
> +
> + if flags & !uapi::MSM_VM_BIND_OP_FLAGS != 0 {
> + dev_dbg!(dev, "invalid flags: {:x}\n", flags);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(iova) {
> + dev_dbg!(dev, "invalid address: {:016x}\n", iova);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(obj_offset) {
> + dev_dbg!(dev, "invalid bo_offset: {:016x}\n", obj_offset);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(range) {
> + dev_dbg!(dev, "invalid range: {:016x}\n", range);
> + return Err(EINVAL);
> + }
> +
> + if !range_valid {
> + dev_dbg!(dev, "invalid range: {:016x}, {:016x}\n", iova, range);
> + return Err(EINVAL);
> + }
> +
> + // MAP must name a handle; UNMAP and MAP_NULL must not.
> + match op {
> + uapi::MSM_VM_BIND_OP_MAP => {
> + if handle == 0 {
> + dev_dbg!(dev, "invalid handle\n");
> + return Err(EINVAL);
> + }
> + }
> + uapi::MSM_VM_BIND_OP_UNMAP | uapi::MSM_VM_BIND_OP_MAP_NULL => {
> + if handle != 0 {
> + dev_dbg!(dev, "handle must be zero\n");
> + return Err(EINVAL);
> + }
> + }
> + _ => {
> + dev_dbg!(dev, "invalid op: {}\n", op);
> + return Err(EINVAL);
> + }
> + }
> +
> + if op == uapi::MSM_VM_BIND_OP_MAP_NULL && !prr_supported {
> + dev_dbg!(dev, "PRR not supported\n");
> + return Err(EINVAL);
> + }
> +
> + Ok(())
> +}
> +
> +/// Checks that `[obj_offset, obj_offset + range)` lies within an object of
> +/// `obj_size` bytes.
> +///
> +/// Returns 0 if it does. Otherwise logs the reason and returns `-EINVAL`.
> +///
> +/// # Safety
> +///
> +/// `dev` must point to a live `struct device`.
> +#[no_mangle]
> +pub unsafe extern "C" fn msm_vm_bind_check_bo_range(
> + dev: *mut bindings::device,
> + obj_offset: u64,
> + range: u64,
> + obj_size: u64,
> +) -> c_int {
> + // SAFETY: The caller guarantees that `dev` is a live `struct device`.
> + let dev: &Device = unsafe { Device::from_raw(dev) };
> +
> + let fits = obj_offset
> + .checked_add(range)
> + .is_some_and(|end| end <= obj_size);
> +
> + if fits {
> + return 0;
> + }
> +
> + dev_dbg!(
> + dev,
> + "invalid range: {:016x} + {:016x} > {:016x}\n",
> + range,
> + obj_offset,
> + obj_size
> + );
> + EINVAL.to_errno()
> +}
> diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
> index 489748ef642c..38741220270a 100644
> --- a/rust/uapi/uapi_helper.h
> +++ b/rust/uapi/uapi_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <uapi/drm/drm.h>
> +#include <uapi/drm/msm_drm.h>
> #include <uapi/drm/nova_drm.h>
> #include <uapi/drm/panthor_drm.h>
> #include <uapi/linux/android/binder.h>
>
> ---
> base-commit: 20a33c6b6cbe7e4805dfb6cd68816937443aa148
> change-id: 20261026-topic-vm_bind_rs-5551b8014396
>
> Best regards,
> --
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 14:39 ` Rob Clark
@ 2026-10-26 15:07 ` Konrad Dybcio
0 siblings, 0 replies; 8+ messages in thread
From: Konrad Dybcio @ 2026-10-26 15:07 UTC (permalink / raw)
To: rob.clark, Konrad Dybcio
Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux
On 10/26/26 3:39 PM, Rob Clark wrote:
> On Mon, Oct 26, 2026 at 4:40 AM Konrad Dybcio <konradybcio@kernel.org> wrote:
>>
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> Much of the VM handling code is a headache to read and maintain.
>> There's a soup of mixed-signedness/size types which need to be compared
>> and added and getting that wrong in C is very easy.
[...]
>> @@ -1118,11 +1071,10 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
>> op->obj = obj;
>> cnt++;
>>
>> - if ((op->range + op->obj_offset) > obj->size) {
>> - ret = UERR(EINVAL, dev, "invalid range: %016llx + %016llx > %016zx\n",
>> - op->range, op->obj_offset, obj->size);
>> + ret = msm_vm_bind_check_bo_range(dev->dev, op->obj_offset, op->range,
>> + obj->size);
>> + if (ret)
>
> I think the ioctl surface area is a sensible place to start using
> rust.. but seems like this patch could instead be a 1-ish line patch
> to use check_add_overflow() here?
Yeah I said that under the --- line, my main objective here is to
encourage rs
> Big picture, I'm a bit more interested in the
> goto-error-unwind-spagatti-monstors. Even there, just using more
> cleanup.h would be an improvement.
Same. I'm done reviewing gotos. LLMs help a lot, but LLMs probably
prefer Rust too.
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 11:40 [PATCH RFC] drm/msm: move VM_BIND op validation to Rust Konrad Dybcio
2026-10-26 14:39 ` Rob Clark
@ 2026-10-26 15:26 ` Rob Clark
2026-10-26 15:30 ` Konrad Dybcio
2026-10-29 9:48 ` Alice Ryhl
2 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2026-10-26 15:26 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux, Konrad Dybcio
On Mon, Oct 26, 2026 at 4:40 AM Konrad Dybcio <konradybcio@kernel.org> wrote:
>
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> Much of the VM handling code is a headache to read and maintain.
> There's a soup of mixed-signedness/size types which need to be compared
> and added and getting that wrong in C is very easy.
>
> The "(range + obj_offset) > obj->size" check could wrap for a large
> obj_offset, which let an out-of-range map be accepted and then mapped
> as nothing. Oxidize it and use checked_add() to fix that.
>
> This commit is a "get the foot in the door" type change to make sure
> that folks building kernels for Qualcomm SoCs have their Rust setup
> ready. It may/should be extended to the wider mm code once the
> appropriate abstractions are finalized.
>
> Worth noting, the C UERR() macro expands to dev_dbg() and the Rust
> implementation for that currently doesn't support dynamic_debug.
>
> The DRM_BRIDGE dependency is dropped because including RUST exposed
> some spaghetti dependencies ending at DEBUGFS and it's enabled anyway.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> (if this is rejected we may just patch the C code).
>
> Notably, this will regress some ancient configurations:
> -- arch/arm/Kconfig
> select HAVE_RUST if CPU_LITTLE_ENDIAN && CPU_32v7 && !KASAN
Side note, I'm ok w/ drm/msm (or at least the gpu part) depending on
CPU_LITTLE_ENDIAN. The combination of BE CPU and LE GPU would be a
mess. I'll actively ignore bug reports, patches, etc related to BE.
So please, no BE!
The !KASAN bit is more worrying. But AFAIK the intersection between
things that can support VM_BIND and armv7 is the empty set.
BR,
-R
>
> Smoke-tested on Glymur
> ---
> drivers/gpu/drm/msm/Kconfig | 2 +-
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/msm_gem_vma.c | 66 +++------------
> drivers/gpu/drm/msm/msm_vm_bind.h | 16 ++++
> drivers/gpu/drm/msm/msm_vm_bind.rs | 160 +++++++++++++++++++++++++++++++++++++
> rust/uapi/uapi_helper.h | 1 +
> 6 files changed, 188 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index ed1c4775d1e9..f5c4749f8e95 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -3,6 +3,7 @@
> config DRM_MSM
> tristate "MSM DRM"
> depends on DRM
> + depends on RUST
> depends on ARCH_QCOM || SOC_IMX5 || COMPILE_TEST
> depends on COMMON_CLK
> depends on IOMMU_SUPPORT
> @@ -63,7 +64,6 @@ config DRM_MSM_VALIDATE_XML
> config DRM_MSM_KMS
> def_bool n
> depends on DRM_MSM
> - select DRM_BRIDGE
> select DRM_BRIDGE_CONNECTOR
> select DRM_CLIENT_SELECTION
> select DRM_DISPLAY_HELPER
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 1e246616f83f..826759a820a0 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -129,6 +129,7 @@ msm-y += \
> msm_ringbuffer.o \
> msm_submitqueue.o \
> msm_syncobj.o \
> + msm_vm_bind.o \
> msm_gpu_tracepoints.o \
>
> msm-$(CONFIG_DRM_MSM_KMS) += \
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index f687a629629d..3c453b034e8f 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -14,6 +14,7 @@
> #include "msm_gpu.h"
> #include "msm_mmu.h"
> #include "msm_syncobj.h"
> +#include "msm_vm_bind.h"
>
> #define vm_dbg(fmt, ...) pr_debug("%s:%d: "fmt"\n", __func__, __LINE__, ##__VA_ARGS__)
>
> @@ -981,23 +982,12 @@ vm_bind_job_create(struct drm_device *dev, struct drm_file *file, struct drm_gpu
> return job;
> }
>
> -static bool invalid_alignment(uint64_t addr)
> -{
> - /*
> - * Technically this is about GPU alignment, not CPU alignment. But
> - * I've not seen any qcom SoC where the SMMU does not support the
> - * CPU's smallest page size.
> - */
> - return !PAGE_ALIGNED(addr);
> -}
> -
> static int
> lookup_op(struct msm_vm_bind_job *job, const struct drm_msm_vm_bind_op *op)
> {
> struct drm_device *dev = job->vm->drm;
> struct msm_drm_private *priv = dev->dev_private;
> int i = job->nr_ops++;
> - int ret = 0;
>
> job->ops[i].op = op->op;
> job->ops[i].handle = op->handle;
> @@ -1006,48 +996,11 @@ lookup_op(struct msm_vm_bind_job *job, const struct drm_msm_vm_bind_op *op)
> job->ops[i].range = op->range;
> job->ops[i].flags = op->flags;
>
> - if (op->flags & ~MSM_VM_BIND_OP_FLAGS)
> - ret = UERR(EINVAL, dev, "invalid flags: %x\n", op->flags);
> -
> - if (invalid_alignment(op->iova))
> - ret = UERR(EINVAL, dev, "invalid address: %016llx\n", op->iova);
> -
> - if (invalid_alignment(op->obj_offset))
> - ret = UERR(EINVAL, dev, "invalid bo_offset: %016llx\n", op->obj_offset);
> -
> - if (invalid_alignment(op->range))
> - ret = UERR(EINVAL, dev, "invalid range: %016llx\n", op->range);
> -
> - if (!drm_gpuvm_range_valid(job->vm, op->iova, op->range))
> - ret = UERR(EINVAL, dev, "invalid range: %016llx, %016llx\n", op->iova, op->range);
> -
> - /*
> - * MAP must specify a valid handle. But the handle MBZ for
> - * UNMAP or MAP_NULL.
> - */
> - if (op->op == MSM_VM_BIND_OP_MAP) {
> - if (!op->handle)
> - ret = UERR(EINVAL, dev, "invalid handle\n");
> - } else if (op->handle) {
> - ret = UERR(EINVAL, dev, "handle must be zero\n");
> - }
> -
> - switch (op->op) {
> - case MSM_VM_BIND_OP_MAP:
> - case MSM_VM_BIND_OP_MAP_NULL:
> - case MSM_VM_BIND_OP_UNMAP:
> - break;
> - default:
> - ret = UERR(EINVAL, dev, "invalid op: %u\n", op->op);
> - break;
> - }
> -
> - if ((op->op == MSM_VM_BIND_OP_MAP_NULL) &&
> - !adreno_smmu_has_prr(priv->gpu)) {
> - ret = UERR(EINVAL, dev, "PRR not supported\n");
> - }
> -
> - return ret;
> + return msm_vm_bind_check_op(dev->dev, op->op, op->handle, op->obj_offset,
> + op->iova, op->range, op->flags, PAGE_SIZE,
> + drm_gpuvm_range_valid(job->vm, op->iova, op->range),
> + op->op == MSM_VM_BIND_OP_MAP_NULL &&
> + adreno_smmu_has_prr(priv->gpu));
> }
>
> /*
> @@ -1118,11 +1071,10 @@ vm_bind_job_lookup_ops(struct msm_vm_bind_job *job, struct drm_msm_vm_bind *args
> op->obj = obj;
> cnt++;
>
> - if ((op->range + op->obj_offset) > obj->size) {
> - ret = UERR(EINVAL, dev, "invalid range: %016llx + %016llx > %016zx\n",
> - op->range, op->obj_offset, obj->size);
> + ret = msm_vm_bind_check_bo_range(dev->dev, op->obj_offset, op->range,
> + obj->size);
> + if (ret)
> goto out_unlock;
> - }
> }
>
> *nr_bos = cnt;
> diff --git a/drivers/gpu/drm/msm/msm_vm_bind.h b/drivers/gpu/drm/msm/msm_vm_bind.h
> new file mode 100644
> index 000000000000..e8aa6adb9181
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_vm_bind.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __MSM_VM_BIND_H__
> +#define __MSM_VM_BIND_H__
> +
> +#include <linux/types.h>
> +
> +struct device;
> +
> +int msm_vm_bind_check_op(struct device *dev, u32 op, u32 handle,
> + u64 obj_offset, u64 iova, u64 range, u32 flags,
> + u64 page_size, bool range_valid, bool prr_supported);
> +int msm_vm_bind_check_bo_range(struct device *dev, u64 obj_offset, u64 range,
> + u64 obj_size);
> +
> +#endif /* __MSM_VM_BIND_H__ */
> diff --git a/drivers/gpu/drm/msm/msm_vm_bind.rs b/drivers/gpu/drm/msm/msm_vm_bind.rs
> new file mode 100644
> index 000000000000..164208e0a1b4
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_vm_bind.rs
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//! VM_BIND operation validation for the MSM driver.
> +
> +use kernel::{
> + bindings,
> + device::Device,
> + error::code::EINVAL,
> + prelude::*,
> + uapi, //
> +};
> +
> +/// Validates a single VM_BIND operation.
> +///
> +/// Returns 0 if the operation is valid. Otherwise logs the reason and returns
> +/// `-EINVAL`.
> +///
> +/// `page_size` is the granularity that `iova`, `obj_offset` and `range` must be
> +/// aligned to. It is the GPU's alignment rather than the CPU's, but no known qcom
> +/// SMMU lacks support for the CPU's smallest page size.
> +/// `range_valid` is the result of `drm_gpuvm_range_valid()` for this operation.
> +/// `prr_supported` is only consulted for `MSM_VM_BIND_OP_MAP_NULL`.
> +///
> +/// # Safety
> +///
> +/// `dev` must point to a live `struct device`.
> +#[no_mangle]
> +pub unsafe extern "C" fn msm_vm_bind_check_op(
> + dev: *mut bindings::device,
> + op: u32,
> + handle: u32,
> + obj_offset: u64,
> + iova: u64,
> + range: u64,
> + flags: u32,
> + page_size: u64,
> + range_valid: bool,
> + prr_supported: bool,
> +) -> c_int {
> + // SAFETY: The caller guarantees that `dev` is a live `struct device`.
> + let dev: &Device = unsafe { Device::from_raw(dev) };
> +
> + match check_op(
> + dev,
> + op,
> + handle,
> + obj_offset,
> + iova,
> + range,
> + flags,
> + page_size,
> + range_valid,
> + prr_supported,
> + ) {
> + Ok(()) => 0,
> + Err(e) => e.to_errno(),
> + }
> +}
> +
> +fn check_op(
> + dev: &Device,
> + op: u32,
> + handle: u32,
> + obj_offset: u64,
> + iova: u64,
> + range: u64,
> + flags: u32,
> + page_size: u64,
> + range_valid: bool,
> + prr_supported: bool,
> +) -> Result {
> + let aligned = |addr: u64| page_size.is_power_of_two() && (addr & (page_size - 1)) == 0;
> +
> + if flags & !uapi::MSM_VM_BIND_OP_FLAGS != 0 {
> + dev_dbg!(dev, "invalid flags: {:x}\n", flags);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(iova) {
> + dev_dbg!(dev, "invalid address: {:016x}\n", iova);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(obj_offset) {
> + dev_dbg!(dev, "invalid bo_offset: {:016x}\n", obj_offset);
> + return Err(EINVAL);
> + }
> +
> + if !aligned(range) {
> + dev_dbg!(dev, "invalid range: {:016x}\n", range);
> + return Err(EINVAL);
> + }
> +
> + if !range_valid {
> + dev_dbg!(dev, "invalid range: {:016x}, {:016x}\n", iova, range);
> + return Err(EINVAL);
> + }
> +
> + // MAP must name a handle; UNMAP and MAP_NULL must not.
> + match op {
> + uapi::MSM_VM_BIND_OP_MAP => {
> + if handle == 0 {
> + dev_dbg!(dev, "invalid handle\n");
> + return Err(EINVAL);
> + }
> + }
> + uapi::MSM_VM_BIND_OP_UNMAP | uapi::MSM_VM_BIND_OP_MAP_NULL => {
> + if handle != 0 {
> + dev_dbg!(dev, "handle must be zero\n");
> + return Err(EINVAL);
> + }
> + }
> + _ => {
> + dev_dbg!(dev, "invalid op: {}\n", op);
> + return Err(EINVAL);
> + }
> + }
> +
> + if op == uapi::MSM_VM_BIND_OP_MAP_NULL && !prr_supported {
> + dev_dbg!(dev, "PRR not supported\n");
> + return Err(EINVAL);
> + }
> +
> + Ok(())
> +}
> +
> +/// Checks that `[obj_offset, obj_offset + range)` lies within an object of
> +/// `obj_size` bytes.
> +///
> +/// Returns 0 if it does. Otherwise logs the reason and returns `-EINVAL`.
> +///
> +/// # Safety
> +///
> +/// `dev` must point to a live `struct device`.
> +#[no_mangle]
> +pub unsafe extern "C" fn msm_vm_bind_check_bo_range(
> + dev: *mut bindings::device,
> + obj_offset: u64,
> + range: u64,
> + obj_size: u64,
> +) -> c_int {
> + // SAFETY: The caller guarantees that `dev` is a live `struct device`.
> + let dev: &Device = unsafe { Device::from_raw(dev) };
> +
> + let fits = obj_offset
> + .checked_add(range)
> + .is_some_and(|end| end <= obj_size);
> +
> + if fits {
> + return 0;
> + }
> +
> + dev_dbg!(
> + dev,
> + "invalid range: {:016x} + {:016x} > {:016x}\n",
> + range,
> + obj_offset,
> + obj_size
> + );
> + EINVAL.to_errno()
> +}
> diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h
> index 489748ef642c..38741220270a 100644
> --- a/rust/uapi/uapi_helper.h
> +++ b/rust/uapi/uapi_helper.h
> @@ -7,6 +7,7 @@
> */
>
> #include <uapi/drm/drm.h>
> +#include <uapi/drm/msm_drm.h>
> #include <uapi/drm/nova_drm.h>
> #include <uapi/drm/panthor_drm.h>
> #include <uapi/linux/android/binder.h>
>
> ---
> base-commit: 20a33c6b6cbe7e4805dfb6cd68816937443aa148
> change-id: 20261026-topic-vm_bind_rs-5551b8014396
>
> Best regards,
> --
> Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 15:26 ` Rob Clark
@ 2026-10-26 15:30 ` Konrad Dybcio
2026-10-26 15:33 ` Rob Clark
0 siblings, 1 reply; 8+ messages in thread
From: Konrad Dybcio @ 2026-10-26 15:30 UTC (permalink / raw)
To: rob.clark, Konrad Dybcio
Cc: Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten, David Airlie, Simona Vetter, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux
On 10/26/26 4:26 PM, Rob Clark wrote:
> On Mon, Oct 26, 2026 at 4:40 AM Konrad Dybcio <konradybcio@kernel.org> wrote:
>>
>> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>
>> Much of the VM handling code is a headache to read and maintain.
>> There's a soup of mixed-signedness/size types which need to be compared
>> and added and getting that wrong in C is very easy.
>>
>> The "(range + obj_offset) > obj->size" check could wrap for a large
>> obj_offset, which let an out-of-range map be accepted and then mapped
>> as nothing. Oxidize it and use checked_add() to fix that.
>>
>> This commit is a "get the foot in the door" type change to make sure
>> that folks building kernels for Qualcomm SoCs have their Rust setup
>> ready. It may/should be extended to the wider mm code once the
>> appropriate abstractions are finalized.
>>
>> Worth noting, the C UERR() macro expands to dev_dbg() and the Rust
>> implementation for that currently doesn't support dynamic_debug.
>>
>> The DRM_BRIDGE dependency is dropped because including RUST exposed
>> some spaghetti dependencies ending at DEBUGFS and it's enabled anyway.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> ---
>> (if this is rejected we may just patch the C code).
>>
>> Notably, this will regress some ancient configurations:
>> -- arch/arm/Kconfig
>> select HAVE_RUST if CPU_LITTLE_ENDIAN && CPU_32v7 && !KASAN
>
> Side note, I'm ok w/ drm/msm (or at least the gpu part) depending on
> CPU_LITTLE_ENDIAN. The combination of BE CPU and LE GPU would be a
> mess. I'll actively ignore bug reports, patches, etc related to BE.
> So please, no BE!
arm64 is free of it now!
134b0fbc5c24 ("arm64: Kbuild: Remove vestigial big-endian support")
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 15:30 ` Konrad Dybcio
@ 2026-10-26 15:33 ` Rob Clark
0 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2026-10-26 15:33 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Konrad Dybcio, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, linux-kernel, linux-arm-msm,
dri-devel, freedreno, rust-for-linux
On Mon, Oct 26, 2026 at 8:30 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 10/26/26 4:26 PM, Rob Clark wrote:
> > On Mon, Oct 26, 2026 at 4:40 AM Konrad Dybcio <konradybcio@kernel.org> wrote:
> >>
> >> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >>
> >> Much of the VM handling code is a headache to read and maintain.
> >> There's a soup of mixed-signedness/size types which need to be compared
> >> and added and getting that wrong in C is very easy.
> >>
> >> The "(range + obj_offset) > obj->size" check could wrap for a large
> >> obj_offset, which let an out-of-range map be accepted and then mapped
> >> as nothing. Oxidize it and use checked_add() to fix that.
> >>
> >> This commit is a "get the foot in the door" type change to make sure
> >> that folks building kernels for Qualcomm SoCs have their Rust setup
> >> ready. It may/should be extended to the wider mm code once the
> >> appropriate abstractions are finalized.
> >>
> >> Worth noting, the C UERR() macro expands to dev_dbg() and the Rust
> >> implementation for that currently doesn't support dynamic_debug.
> >>
> >> The DRM_BRIDGE dependency is dropped because including RUST exposed
> >> some spaghetti dependencies ending at DEBUGFS and it's enabled anyway.
> >>
> >> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> >> ---
> >> (if this is rejected we may just patch the C code).
> >>
> >> Notably, this will regress some ancient configurations:
> >> -- arch/arm/Kconfig
> >> select HAVE_RUST if CPU_LITTLE_ENDIAN && CPU_32v7 && !KASAN
> >
> > Side note, I'm ok w/ drm/msm (or at least the gpu part) depending on
> > CPU_LITTLE_ENDIAN. The combination of BE CPU and LE GPU would be a
> > mess. I'll actively ignore bug reports, patches, etc related to BE.
> > So please, no BE!
>
> arm64 is free of it now!
>
> 134b0fbc5c24 ("arm64: Kbuild: Remove vestigial big-endian support")
And there was much rejoicing :-)
BR,
-R
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-26 11:40 [PATCH RFC] drm/msm: move VM_BIND op validation to Rust Konrad Dybcio
2026-10-26 14:39 ` Rob Clark
2026-10-26 15:26 ` Rob Clark
@ 2026-10-29 9:48 ` Alice Ryhl
2026-10-29 13:36 ` Konrad Dybcio
2 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-10-29 9:48 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux, Konrad Dybcio
On Mon, Oct 26, 2026 at 12:40:02PM +0100, Konrad Dybcio wrote:
> +#[no_mangle]
If you use the #[export] macro instead, then it has the same effect
as #[no_mangle], but it verifies at build-time that the signature
matches what's in the header file.
See for comparison:
drivers/gpu/drm/drm_panic_helper_qr.rs
Alice
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC] drm/msm: move VM_BIND op validation to Rust
2026-10-29 9:48 ` Alice Ryhl
@ 2026-10-29 13:36 ` Konrad Dybcio
0 siblings, 0 replies; 8+ messages in thread
From: Konrad Dybcio @ 2026-10-29 13:36 UTC (permalink / raw)
To: Alice Ryhl, Konrad Dybcio
Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, linux-kernel, linux-arm-msm, dri-devel,
freedreno, rust-for-linux
On 10/29/26 10:48 AM, Alice Ryhl wrote:
> On Mon, Oct 26, 2026 at 12:40:02PM +0100, Konrad Dybcio wrote:
>> +#[no_mangle]
>
> If you use the #[export] macro instead, then it has the same effect
> as #[no_mangle], but it verifies at build-time that the signature
> matches what's in the header file.
>
> See for comparison:
> drivers/gpu/drm/drm_panic_helper_qr.rs
Nice, thanks!
Konrad
^ permalink raw reply [flat|nested] 8+ messages in thread