| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACSVV036k+VwtLEvX4ATG7h7fTpXAzUuOZr9AY4WzwJWFvw0zQ@mail.gmail.com>
Date: Thu, 18 Sep 2025 07:46:32 -0700
From: Rob Clark <rob.clark@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>,
Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Sumit Semwal <sumit.semwal@...aro.org>,
Christian König <christian.koenig@....com>,
Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v5 2/5] drm/msm: split VM_BIND from the rest of GEM VMA code
On Wed, Sep 17, 2025 at 8:51 PM Dmitry Baryshkov
<dmitry.baryshkov@....qualcomm.com> wrote:
>
> In preparation to disabling GPU functionality split VM_BIND-related
> functions (which are used only for the GPU) from the rest of the GEM VMA
> implementation.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/msm_gem_vm_bind.c | 1116 +++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/msm_gem_vma.c | 1177 +--------------------------------
> drivers/gpu/drm/msm/msm_gem_vma.h | 105 +++
> 4 files changed, 1225 insertions(+), 1174 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index 0c0dfb25f01b193b10946fae20138caf32cf0ed2..d7876c154b0aa2cb0164c4b1fb7900b1a42db46b 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -115,6 +115,7 @@ msm-y += \
> msm_gem_shrinker.o \
> msm_gem_submit.o \
> msm_gem_vma.o \
> + msm_gem_vm_bind.o \
> msm_gpu.o \
> msm_gpu_devfreq.o \
> msm_io_utils.o \
[snip]
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.h b/drivers/gpu/drm/msm/msm_gem_vma.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..f702f81529e72b86bffb4960408f1912bc65851a
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2016 Red Hat
> + * Author: Rob Clark <robdclark@...il.com>
> + */
> +
> +#ifndef _MSM_GEM_VMA_H_
> +#define _MSM_GEM_VMA_H_
> +
> +#define vm_dbg(fmt, ...) pr_debug("%s:%d: "fmt"\n", __func__, __LINE__, ##__VA_ARGS__)
> +
> +/**
> + * struct msm_vm_map_op - create new pgtable mapping
> + */
> +struct msm_vm_map_op {
> + /** @iova: start address for mapping */
> + uint64_t iova;
> + /** @range: size of the region to map */
> + uint64_t range;
> + /** @offset: offset into @sgt to map */
> + uint64_t offset;
> + /** @sgt: pages to map, or NULL for a PRR mapping */
> + struct sg_table *sgt;
> + /** @prot: the mapping protection flags */
> + int prot;
> +
> + /**
> + * @queue_id: The id of the submitqueue the operation is performed
> + * on, or zero for (in particular) UNMAP ops triggered outside of
> + * a submitqueue (ie. process cleanup)
> + */
> + int queue_id;
> +};
> +
> +/**
> + * struct msm_vm_unmap_op - unmap a range of pages from pgtable
> + */
> +struct msm_vm_unmap_op {
> + /** @iova: start address for unmap */
> + uint64_t iova;
> + /** @range: size of region to unmap */
> + uint64_t range;
> +
> + /** @reason: The reason for the unmap */
> + const char *reason;
> +
> + /**
> + * @queue_id: The id of the submitqueue the operation is performed
> + * on, or zero for (in particular) UNMAP ops triggered outside of
> + * a submitqueue (ie. process cleanup)
> + */
> + int queue_id;
> +};
> +
> +static void
> +vm_log(struct msm_gem_vm *vm, const char *op, uint64_t iova, uint64_t range, int queue_id)
These would have to be static-inline
But overall I'm not sure how I feel about this.. I guess the goal is
to reduce the size of a kms-only driver? If so, I think you could do
better with some ugly ifdef (for ex, you could also remove scheduler
and other fields not used by kernel managed VMs from msm_gem_vm).
I'm not sure how much the savings would be, or if it is worth the pain
(ie. extra build configurations to test going forward, etc). Having
no GPU doesn't seem like a case worth optimizing for, tbh. You could
still have a single driver which binds to multiple different devices,
ie. if # of GPUs != # of DPUs without this with no change in
footprint.
BR,
-R
> +{
> + int idx;
> +
> + if (!vm->managed)
> + lockdep_assert_held(&vm->mmu_lock);
> +
> + vm_dbg("%s:%p:%d: %016llx %016llx", op, vm, queue_id, iova, iova + range);
> +
> + if (!vm->log)
> + return;
> +
> + idx = vm->log_idx;
> + vm->log[idx].op = op;
> + vm->log[idx].iova = iova;
> + vm->log[idx].range = range;
> + vm->log[idx].queue_id = queue_id;
> + vm->log_idx = (vm->log_idx + 1) & ((1 << vm->log_shift) - 1);
> +}
> +
> +static void
> +vm_unmap_op(struct msm_gem_vm *vm, const struct msm_vm_unmap_op *op)
> +{
> + const char *reason = op->reason;
> +
> + if (!reason)
> + reason = "unmap";
> +
> + vm_log(vm, reason, op->iova, op->range, op->queue_id);
> +
> + vm->mmu->funcs->unmap(vm->mmu, op->iova, op->range);
> +}
> +
> +static int
> +vm_map_op(struct msm_gem_vm *vm, const struct msm_vm_map_op *op)
> +{
> + vm_log(vm, "map", op->iova, op->range, op->queue_id);
> +
> + return vm->mmu->funcs->map(vm->mmu, op->iova, op->sgt, op->offset,
> + op->range, op->prot);
> +}
> +
> +int msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *_arg);
> +int msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg);
> +int msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *_arg);
> +
> +int msm_gem_vm_sched_init(struct msm_gem_vm *vm, struct drm_device *drm);
> +void msm_gem_vm_sched_fini(struct msm_gem_vm *vm);
> +
> +#endif
>
> --
> 2.47.3
>
Powered by blists - more mailing lists