Re: [PATCH 5/9] drm/tyr: add user and MCU VM specifications

[Daniel Almeida <daniel.almeida@collabora.com>]

> On 1 Sep 2026, at 13:09, Ke Sun via B4 Relay <devnull+sunke.kylinos.cn@kernel.org> wrote:
> 
> From: Alvin Sun <alvin.sun@linux.dev>
> 
> Distinguish MCU VMs from user VMs, and compute the user/kernel
> GPU VA split for user VMs.

Same comment as the previous commit: please write a few
more words here if possible :)

> 
> Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
> ---
> drivers/gpu/drm/tyr/fw.rs |   8 +--
> drivers/gpu/drm/tyr/vm.rs | 134 +++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 131 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
> index 47d25c901bd01..9b4b488521b85 100644
> --- a/drivers/gpu/drm/tyr/fw.rs
> +++ b/drivers/gpu/drm/tyr/fw.rs
> @@ -51,7 +51,6 @@
>         KernelBoVaAlloc, //
>     },
>     gpu::GpuInfo,
> -
>     mmu::Mmu,
>     regs::{
>         gpu_control::{
> @@ -66,7 +65,10 @@
>             JOB_IRQ_RAWSTAT, //
>         }, //
>     },
> -    vm::Vm, //
> +    vm::{
> +        Vm,
> +        VmSpec, //
> +    }, //
> };
> 
> mod parser;
> @@ -220,7 +222,7 @@ pub(crate) fn new(
>         mmu: ArcBorrow<'_, Mmu<'drm>>,
>         gpu_info: &GpuInfo,
>     ) -> Result<Firmware<'drm>> {
> -        let vm = Vm::new(dev, ddev, mmu, gpu_info)?;
> +        let vm = Vm::new(dev, ddev, mmu, gpu_info, VmSpec::Mcu)?;
>         vm.activate()?;
> 
>         let result = (|| {
> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index c5e307b1e2416..76c3d60bb2fe2 100644
> --- a/drivers/gpu/drm/tyr/vm.rs
> +++ b/drivers/gpu/drm/tyr/vm.rs
> @@ -8,6 +8,7 @@
> //! mapped into hardware address space (AS) slots for GPU execution.
> 
> use core::marker::PhantomData;
> +use core::num::NonZeroU64;
> use core::ops::Range;
> 
> use kernel::{
> @@ -43,6 +44,8 @@
>     new_mutex,
>     prelude::*,
>     sizes::{
> +        LargeSizeConstants,
> +        SizeConstants,
>         SZ_1G,
>         SZ_2M,
>         SZ_4K, //
> @@ -154,6 +157,109 @@ fn try_from(value: u32) -> Result<Self, Self::Error> {
>     }
> }
> 
> +/// User VA size request for a user VM.
> +pub(crate) enum UserVaRequest {
> +    /// Split based on `task_size()` and the GPU VA range.
> +    Auto,
> +    /// Caller-specified size; construction guarantees `> 0`.
> +    Fixed(NonZeroU64),
> +}
> +
> +impl UserVaRequest {
> +    /// UAPI boundary normalization: `0` -> [`Auto`](Self::Auto).
> +    pub(crate) fn from_uapi(v: u64) -> Self {
> +        match NonZeroU64::new(v) {
> +            Some(size) => Self::Fixed(size),
> +            None => Self::Auto,
> +        }
> +    }
> +}
> +
> +pub(crate) enum VmSpec {

Instead of having an enum, I think we could go with the current
tyr-dev design, i.e.:

- new_fw() (or, perhaps even better, new_for_fw())
- new_for_user()

> +    /// MCU/firmware VM, entirely kernel-managed.
> +    Mcu,
> +    /// User VM: full GPU VA range, split into user/kernel per `user_va`.
> +    User { user_va: UserVaRequest },
> +}
> +
> +/// Final user/kernel VA layout for a VM.
> +pub(crate) struct VmLayout {
> +    /// Full GPU VA range covered by this VM.
> +    pub(crate) full: Range<u64>,
> +    /// User-accessible VA range. Empty for MCU VMs.
> +    pub(crate) user: Range<u64>,
> +}
> +
> +impl VmLayout {
> +    /// Kernel VA range, reserved for future kernel object allocation.
> +    #[expect(dead_code)]
> +    pub(crate) fn kernel(&self) -> Range<u64> {
> +        self.user.end..self.full.end
> +    }
> +
> +    /// Compute a user/kernel split for a user VM from the full GPU VA range and
> +    /// a user request.
> +    pub(crate) fn compute(full: Range<u64>, req: UserVaRequest) -> Result<Self> {
> +        /// Minimum VA space reserved for kernel objects (heaps, ring buffers, ...).
> +        const MIN_KERNEL_VA: u64 = u64::SZ_256M;
> +
> +        if full.end <= MIN_KERNEL_VA {
> +            pr_err!(
> +                "Invalid VA range {:#x}..{:#x}, kernel VA min required: >{:#x}\n",
> +                full.start,
> +                full.end,
> +                MIN_KERNEL_VA
> +            );
> +            return Err(EINVAL);
> +        }
> +
> +        let user_max = full.end - MIN_KERNEL_VA;
> +
> +        let user_end = match req {
> +            UserVaRequest::Fixed(v) => {
> +                let user_size = v.get();
> +                if user_size > user_max {
> +                    pr_err!(
> +                        "Requested user VA range {:#x} exceeds maximum {:#x}\n",
> +                        user_size,
> +                        user_max
> +                    );
> +                    return Err(EINVAL);
> +                }
> +                user_size
> +            }
> +            UserVaRequest::Auto => {
> +                let task_size = current!().mm().map(|mm| mm.task_size());
> +                let candidate = match task_size {
> +                    // `task_size()` returns usize; widen to u64 for the comparison.
> +                    Some(t) if (t as u64) < full.end => t as u64,
> +                    None | Some(_) => {
> +                        // If the range exceeds 4G, split it in two so CPU and
> +                        // GPU share the same addresses (SVM).
> +                        if full.end > u64::SZ_4G {
> +                            full.end / 2
> +                        } else {
> +                            user_max
> +                        }
> +                    }
> +                };
> +                candidate.min(user_max)
> +            }
> +        };
> +
> +        let delta = full.end - user_end;
> +        // Pick a kernel VA range that's a power of two, to have a clear split.
> +        let kernel_va_range = 1u64 << delta.ilog2();
> +        let kernel_va_start = full.end - kernel_va_range;
> +        let full_start = full.start;
> +
> +        Ok(Self {
> +            full,
> +            user: full_start..kernel_va_start,
> +        })
> +    }
> +}
> +
> /// Arguments for a virtual memory map operation.
> struct VmMapArgs<'drm> {
>     /// Access permissions and caching behavior for the mapping.
> @@ -329,8 +435,8 @@ pub(crate) struct Vm<'drm> {
>     /// Non-core part of the GPUVM. Can be used for stuff that doesn't modify the
>     /// internal mapping tree, like GpuVm::obtain()
>     gpuvm: ARef<GpuVm<GpuVmData<'drm>>>,
> -    /// VA range for this VM.
> -    va_range: Range<u64>,
> +    /// VA layout for this VM.
> +    pub(crate) layout: VmLayout,
> }
> 
> impl<'drm> Vm<'drm> {
> @@ -343,6 +449,7 @@ pub(crate) fn new(
>         ddev: &TyrDrmDevice,
>         mmu: ArcBorrow<'_, Mmu<'drm>>,
>         gpu_info: &GpuInfo,
> +        spec: VmSpec,
>     ) -> Result<Arc<Vm<'drm>>> {
>         let mmu_features = MMU_FEATURES::from_raw(gpu_info.mmu_features);
>         let va_bits = mmu_features.va_bits().get();
> @@ -351,6 +458,14 @@ pub(crate) fn new(
>         let range = 0..(1u64 << va_bits);
>         let reserve_range = 0..0u64;
> 
> +        let layout = match spec {
> +            VmSpec::Mcu => VmLayout {
> +                full: range.clone(),
> +                user: 0..0u64,
> +            },
> +            VmSpec::User { user_va } => VmLayout::compute(range.clone(), user_va)?,
> +        };
> +
>         // dummy_obj is used to initialize the GPUVM tree.
>         let dummy_obj = gem::new_dummy_object(ddev).inspect_err(|e| {
>             dev_err!(dev, "Failed to create dummy GEM object: {:?}", e);
> @@ -380,7 +495,7 @@ pub(crate) fn new(
>                 mmu: mmu.into(),
>                 gpuvm,
>                 gpuvm_unique <- new_mutex!(gpuvm_unique),
> -                va_range: range,
> +                layout,
>             }),
>             GFP_KERNEL,
>         )?;
> @@ -414,7 +529,10 @@ pub(crate) fn kill(&self) {
>         // TODO: Turn the VM into a state where it can't be used.
>         let _ = self.deactivate();
>         let _ = self
> -            .unmap_range(self.va_range.start, self.va_range.end - self.va_range.start)
> +            .unmap_range(
> +                self.layout.full.start,
> +                self.layout.full.end - self.layout.full.start,
> +            )
>             .inspect_err(|e| {
>                 dev_err!(self.dev, "Failed to unmap range during deactivate: {:?}", e);
>             });
> @@ -551,14 +669,14 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
> 
>         let end = va.checked_add(size).ok_or(EINVAL)?;
> 
> -        if va < self.va_range.start || end > self.va_range.end {
> +        if va < self.layout.full.start || end > self.layout.full.end {
>             dev_err!(
>                 self.dev,
>                 "Unmap range {:#x}..{:#x} exceeds VM range {:#x}..{:#x}",
>                 va,
>                 end,
> -                self.va_range.start,
> -                self.va_range.end
> +                self.layout.full.start,
> +                self.layout.full.end
>             );
>             return Err(EINVAL);
>         }
> @@ -568,7 +686,7 @@ pub(crate) fn unmap_range(&self, va: u64, size: u64) -> Result {
>             region: va..end,
>         };
> 
> -        let full_vm = va == self.va_range.start && end == self.va_range.end;
> +        let full_vm = va == self.layout.full.start && end == self.layout.full.end;
> 
>         let mut resources = VmOpResources {
>             preallocated_gpuvas: if full_vm {
> 
> -- 
> 2.43.0
> 
>