Re: [PATCH v10 3/7] drm/tyr: add Memory Management Unit (MMU) support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing MmuCommand::Unlock in as_end_update() leaves the address space locked.
--

--- Patch [3]: [PATCH v10 3/7] drm/tyr: add Memory Management Unit (MMU) support ---
commit 4673c22526eac7702f0e2418212a28a74a5e56d8
Author: Boris Brezillon <boris.brezillon@collabora.com>

drm/tyr: add Memory Management Unit (MMU) support
    
Add Memory Management Unit (MMU) support in Tyr, which wraps a SlotManager
instance to allocate MMU address-space slots for virtual memory. This includes
an AddressSpaceManager for MMU hardware register operations such as locking
and flushing address spaces.

> diff --git a/drivers/gpu/drm/tyr/mmu/address_space.rs b/drivers/gpu/drm/tyr/mmu/address_space.rs
> new file mode 100644
> index 0000000000000..76e3d2df6afd4
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/mmu/address_space.rs

[ ... ]

> +    /// Locks a region of the translation tables for an atomic update.
> +    ///
> +    /// Programs the MMU LOCKADDR register for the given address space and issues
> +    /// the lock command. The hardware rounds the requested range up to a
> +    /// power-of-two region aligned to its size.
> +    ///
> +    /// Returns an error if the slot is invalid or if register writes/commands fail.
> +    fn as_start_update(&mut self, as_nr: usize, region: &Range<u64>) -> Result {

[ ... ]

> +        self.as_send_cmd_and_wait(as_nr, MmuCommand::Lock)
> +    }
> +
> +    /// Completes an atomic translation table update.
> +    ///
> +    /// Returns an error if the slot is invalid or if the flush command fails.
> +    fn as_end_update(&mut self, as_nr: usize) -> Result {
> +        self.validate_as_slot(as_nr)?;
> +        self.as_send_cmd_and_wait(as_nr, MmuCommand::FlushPt)?;

[Severity: High]
Does this function also need to send MmuCommand::Unlock to release the locked
region?

The start of the update sequence in as_start_update() locks the region by
sending MmuCommand::Lock, but as_end_update() currently only sends
MmuCommand::FlushPt and omits the unlock command. 

If the address space remains locked, GPU accesses to the updated virtual
memory region could be permanently blocked. This might lead to GPU hangs or
command timeouts when userspace triggers VM mapping or unmapping operations.

> +        Ok(())
> +    }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-fw-boot-b4-v10-0-9187aefa3f2f@collabora.com?part=3