Re: [PATCH v6 04/10] drm/tyr: add per-file VM pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/tyr: off-by-one IdPool capacity in VmPool::new() on
  32-bit builds
--

commit 6b8d6be457daf3e5e905977c364059ce3d3249e4
Author: Alvin Sun <alvin.sun@linux.dev>

drm/tyr: add per-file VM pool

This commit introduces VmPool to manage up to 32 GPU address spaces per open
file using an IdPool and an XArray, tearing down remaining VMs when the pool
is dropped.

> diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
> index 1dd11b2b12532..6f5518652a03f 100644
> --- a/drivers/gpu/drm/tyr/vm.rs
> +++ b/drivers/gpu/drm/tyr/vm.rs

[ ... ]

> @@ -947,3 +953,135 @@ fn pt_unmap(dev: &Device, pt: &IoPageTable<'_, ARM64LPAES1>, range: Range<u64>)
>  
>      Ok(())
>  }
> +
> +/// Maximum number of VMs a single file may hold, matching panthor's
> +/// `PANTHOR_MAX_VMS_PER_FILE`.
> +const MAX_VMS_PER_FILE: usize = 32;

[ ... ]

> +impl<'drm> VmPool<'drm> {
> +    /// Creates a new [`VmPool`].
> +    #[expect(dead_code)]
> +    pub(crate) fn new(dev: &'drm Device<Bound>) -> impl PinInit<Self> {
> +        let ids = IdPool::new();
> +        pin_init!(Self {
> +            dev,
> +            ids <- new_mutex!(ids),
> +            vms <- XArray::new(AllocKind::Alloc),
> +        })
> +    }
> +
> +    /// Stores the VM and returns the allocated ID.
> +    ///
> +    /// On failure - ID space exhausted or a store failure - the VM is
> +    /// returned in an [`AddError`] so the caller owns the teardown.
> +    // TODO: allocate IDs with the XArray directly (once it grows range
> +    // allocation, the equivalent of C's `XA_LIMIT`) and drop the IdPool.
> +    #[expect(dead_code)]
> +    pub(crate) fn add(&self, vm: Arc<Vm<'drm>>) -> Result<u32, AddError<'drm>> {
> +        let id = (|| {
> +            let mut ids = self.ids.lock();
> +            let unused = ids.find_unused_id(1).ok_or(ENOSPC)?;
> +            if unused.as_usize() > MAX_VMS_PER_FILE {
> +                return Err(ENOSPC);
> +            }
> +            Ok(unused.acquire())
> +        })();

[Severity: Medium]
Can VmPool::add() fail with ENOSPC on the 32nd VM on 32-bit builds?

IdPool::new() creates an inline bitmap of length usize::BITS, which is 32 on
32-bit platforms (indices 0..=31).  Because find_unused_id(1) skips bit 0,
allocating 32 VMs requires bit index 32, so only 31 VMs (IDs 1..=31) can be
allocated before find_unused_id(1) returns None.  Searching from bit 0 and
adding 1 to form the 1-based VM ID (subtracting 1 on release_id()) would fit
all 32 VMs within indices 0..=31.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-tyr-ioctls-v6-0-b2896bf2b3c9@kylinos.cn?part=4