Re: [PATCH 02/13] gpu: nova-core: mm: add VramBlock and Bar1Map

[Alistair Popple]

On 2026-09-05 at 18:11 +1000, Zhi Wang <zhiw@xxxxxxxxxx> wrote...
> GPU page table setup and VRAM-backed control structures require the
> driver to allocate physical VRAM and map it into the BAR1 aperture for
> CPU access. These operations are common to both the base driver and
> vGPU paths.
> 
> VramBlock owns a buddy allocator allocation. Shared VramRegion views
> keep that allocation alive while callers select byte ranges within a
> larger preallocated block. Bar1Map retains one such region while mapping
> the containing pages and bounds all CPU accesses to the requested view.
> The mapping must be explicitly destroyed to release GPU VA resources and
> invalidate PTEs.
> 
> Keep BarUser inline in Gpu and let short-lived BarUserAccess objects
> borrow it. Bar1Map owns its mapped VA range and borrows the driver-owned
> BAR1 mapping; explicit destruction returns the VA through BarUser and
> GpuMm. Its MMIO accessors remain runtime checked because BAR1 and the
> logical mapping have runtime sizes.

This patch seems to be doing three different things that build on top of each
other, so it would be a bit easier to review if it was split up into three
patches. One patch adding the VRAM allocator, one dealing with the shared
VramRegion views and another to do the actual BAR1 mappings.

> Signed-off-by: Zhi Wang <zhiw@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gpu.rs         |  20 ++-
>  drivers/gpu/nova-core/mm.rs          |   6 +-
>  drivers/gpu/nova-core/mm/bar_user.rs | 143 ++++++++++++++++++--
>  drivers/gpu/nova-core/mm/vram.rs     | 187 +++++++++++++++++++++++++++
>  4 files changed, 329 insertions(+), 27 deletions(-)
>  create mode 100644 drivers/gpu/nova-core/mm/vram.rs

[...]

> +/// Allocate an exact VRAM range relative to a usable region's buddy base.
> +pub(crate) fn alloc_vram_range(
> +    mm: &GpuMm<'_>,

Given this operates on GpuMm I think it might be better if this was a method
implemented on GpuMm.

> +    range: Range<u64>,
> +    align: u64,
> +) -> Result<Arc<VramBlock>> {
> +    let page_size = u64::try_from(PAGE_SIZE).map_err(|_| EOVERFLOW)?;
> +    let size = range
> +        .end
> +        .checked_sub(range.start)
> +        .filter(|size| *size != 0)
> +        .ok_or(EINVAL)?;
> +    if !range.start.is_multiple_of(page_size) || !size.is_multiple_of(page_size) {
> +        return Err(EINVAL);
> +    }

I think these should already be checked in gpu_buddy_alloc_blocks() so no need
to repeat the same checks here.

> +
> +    let align = align.max(page_size);
> +    let align_usize = usize::try_from(align).map_err(|_| EOVERFLOW)?;
> +    let min_block_size = Alignment::new_checked(align_usize).ok_or(EINVAL)?;
> +    let buddy = mm.buddy();
> +    if range.end > buddy.size() {
> +        return Err(ENOSPC);
> +    }

Ditto.

> +    let blocks = KBox::pin_init(
> +        buddy.alloc_blocks(
> +            GpuBuddyAllocMode::Range(range.clone()),
> +            size,
> +            min_block_size,
> +            GpuBuddyAllocFlags::default(),
> +        ),
> +        GFP_KERNEL,
> +    )?;
> +
> +    let mut address = None;
> +    let mut allocation_end = None;
> +    let mut covered = 0u64;
> +    for block in blocks.as_ref().iter() {
> +        let block_address = block.offset();
> +        let block_size = block.size();
> +        let block_end = block_address.checked_add(block_size).ok_or(EOVERFLOW)?;
> +        address = Some(address.map_or(block_address, |start: u64| start.min(block_address)));
> +        allocation_end = Some(allocation_end.map_or(block_end, |end: u64| end.max(block_end)));
> +        covered = covered.checked_add(block_size).ok_or(EOVERFLOW)?;
> +    }
> +
> +    let address = address.ok_or(ENOMEM)?;
> +    let allocation_end = allocation_end.ok_or(ENOMEM)?;
> +    let expected_address = buddy
> +        .base_offset()
> +        .checked_add(range.start)
> +        .ok_or(EOVERFLOW)?;
> +    if address != expected_address
> +        || covered != size
> +        || allocation_end.checked_sub(address).ok_or(EIO)? != size
> +        || !address.is_multiple_of(align)
> +    {
> +        return Err(EIO);
> +    }

What is the purpose of these checks? Do we ever expect to hit any of these
errors?

Thanks.

> +    Ok(Arc::new(
> +        VramBlock {
> +            _blocks: blocks,
> +            address,
> +            size,
> +        },
> +        GFP_KERNEL,
> +    )?)
> +}
> -- 
> 2.53.0
>