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

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Jul 09, 2026 at 02:36:43PM -0700, Deborah Brouwer wrote:
> From: Boris Brezillon <boris.brezillon@collabora.com>
> 
> Add Memory Management Unit (MMU) support in Tyr. The MMU module wraps a
> SlotManager instance to allocate MMU address-space slots for use by
> virtual memory (VM) address spaces. The MMU's SlotManager uses an
> AddressSpaceManager to handle the hardware-specific callbacks. For
> example, the AddressSpaceManager activates and evicts VMs from slots by
> writing commands to the MMU registers.
> 
> Add an implementation block for the MMU's MEMATTR register to provide
> a method for translating the Memory Attribute Indirection Register (MAIR)
> format from the pagetable configuration to a format understood by the MMU.
> 
> Create an mmu instance during probe, it will be used by subsequent patches
> in this series.
> 
> Wrap the iomem stored in TyrDrmRegistrationData in an Arc. The iomem
> is stored in the mmu through its AddressSpaceManager. In anticipation
> of the iomem also being stored in the firmware object, set up shared
> ownership of the iomem now.
> 
> Update Kconfig to add the new MMU and IOMMU dependencies required
> by this MMU module.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Co-developed-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>

> +//! Memory Management Unit (MMU) module.
> +//!
> +//! The GPU MMU provides a limited number of memory address spaces for use by command streams.
> +//! The MMU translates virtual addresses to physical addresses and manages memory configuration
> +//! and access permissions.
> +//!
> +//! This MMU module is essentially a locked wrapper around a [`SlotManager`] instance.
> +//! The [`SlotManager`] manages the assignment of virtual address spaces to hardware address-space
> +//! (AS) slots. MMU commands such as updates and flushes are carried out by the
> +//! [`AddressSpaceManager`] which actually writes to the MMU registers.
> +#![allow(dead_code)]

This occurs in a few different commits, but I'd make sure to use
expect(dead_code) here so we are sure to remove it when everything is
used.

> +/// Virtual memory (VM) address space data for use in MMU operations.
> +#[pin_data]
> +pub(crate) struct VmAsData<'bound> {
> +    /// The address space seat tracks this VM's binding to a hardware address space slot.
> +    /// Uses [`LockedBy`] to ensure safe concurrent access to the slot assignment state,
> +    /// protected by the [`AsSlotManager`] lock.
> +    as_seat: LockedBy<Seat, AsSlotManager<'bound>>,

Why not use the LockedSeat type alias here?

> +        let lockaddr_val = LOCKADDR::zeroed()
> +            .try_with_size(lockaddr_size)?
> +            .try_with_base(lockaddr_base)?
> +            .into_raw();

Should this be:

	let lockaddr_val = LOCKADDR::zeroed()
	    .try_with_size(lockaddr_size)?
	    .try_with_base(lockaddr_base >> 12)?
	    .into_raw();

or even just

	let lockaddr_val = lockaddr_size | lockaddr_base;

?

Alice