Re: [PATCH v3 05/11] drm/tyr: add user and MCU VM specifications

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

> On 28 Sep 2026, at 23:15, Ke Sun via B4 Relay <devnull+sunke.kylinos.cn@kernel.org> wrote:
> 
> From: Alvin Sun <alvin.sun@linux.dev>
> 
> The MCU and user VMs need different VA layouts. Give each a dedicated
> constructor: new_for_fw() builds the kernel-only 4G layout, while
> new_for_user() splits the address space by task_size or a user-provided
> size, rejecting oversized requests rather than clamping. The resulting
> user range is what VM_CREATE reports back as user_va_range.
> 
> Signed-off-by: Alvin Sun <alvin.sun@linux.dev>
> ---
> drivers/gpu/drm/tyr/fw.rs |  19 +++---
> drivers/gpu/drm/tyr/vm.rs | 170 ++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 164 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
> index 7edb5eff17077..d790b54e373e6 100644
> --- a/drivers/gpu/drm/tyr/fw.rs
> +++ b/drivers/gpu/drm/tyr/fw.rs
> @@ -52,7 +52,6 @@
>         KernelBoVaAlloc, //
>     },
>     gpu::GpuInfo,
> -
>     mmu::Mmu,
>     regs::{
>         gpu_control::{
> @@ -223,10 +222,10 @@ 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_for_fw(dev, ddev, mmu, gpu_info)?;
>         vm.activate()?;
> 
> -        let result = (|| {
> +        let sections = (|| -> Result<KVec<Section<'drm>>> {
>             let (fw, parsed_sections) = Self::load(dev, ddev, gpu_info)?;
>             let mut sections = KVec::new();
>             for parsed in parsed_sections {
> @@ -256,18 +255,18 @@ pub(crate) fn new(
>                 sections.push(Section { data, mem }, GFP_KERNEL)?;
>             }
> 
> -            Ok(Firmware {
> -                iomem,
> -                vm: vm.clone(),
> -                sections,
> -            })
> +            Ok(sections)
>         })();
> 
> -        if result.is_err() {
> +        if sections.is_err() {
>             vm.kill();
>         }
> 
> -        result
> +        Ok(Firmware {
> +            iomem,
> +            vm,
> +            sections: sections?,
> +        })
>     }
> 
>     pub(crate) fn boot(&self) -> Result {
> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index a2857820570cf..625ec7e95790f 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::{
> @@ -44,6 +45,8 @@
>     new_mutex,
>     prelude::*,
>     sizes::{
> +        LargeSizeConstants,
> +        SizeConstants,
>         SZ_1G,
>         SZ_2M,
>         SZ_4K, //
> @@ -159,6 +162,103 @@ 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).
> +    #[expect(dead_code)]
> +    pub(crate) fn from_uapi(v: u64) -> Self {
> +        match NonZeroU64::new(v) {
> +            Some(size) => Self::Fixed(size),
> +            None => Self::Auto,
> +        }
> +    }
> +}
> +
> +/// 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
> +            );

This pr_err seems to be removed in patch 7.

> +            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
> +                    );

Same here. In this case, just don’t add them in this patch to begin with.

> +                    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,
> +        })
> +    }
> +}
> +


With the changes above,

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>