Re: [PATCH v3 06/10] gpu: nova-core: mm: Add support to use PRAMIN windows to write to VRAM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Compilation fails unconditionally because `kernel::io::register::OffsetLoc` is missing from the kernel tree.
- [Low] The `io_project!` macro is invoked with unbracketed projection specifications, causing a compilation error.
- [Low] The `Drop` implementation for `PraminAccess` will fail to compile with a zero-sized type due to a const assertion failure in `cast::<u8>()`.
--

commit da741ebe3207f602c3f762361899c60fcf5605a3
Author: Joel Fernandes <joelagnelf@nvidia.com>

gpu: nova-core: mm: Add support to use PRAMIN windows to write to VRAM

This commit adds a Pramin manager to handle direct CPU read/write to VRAM
via PRAMIN apertures. It returns an MMIO view of VRAM and handles repositioning
and alignment of the window.

> diff --git a/drivers/gpu/nova-core/mm/pramin.rs b/drivers/gpu/nova-core/mm/pramin.rs
> new file mode 100644
> index 0000000000000..20be3fc471ba3
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/pramin.rs
> @@ -0,0 +1,178 @@
[ ... ]
> +use kernel::{
> +    io::{
> +        io_project,
> +        register,
> +        register::OffsetLoc,

[Severity: Low]
Does this code build? kernel::io::register::OffsetLoc appears to be missing
from the kernel tree.

> +        Io,
> +        Mmio, //
> +    },
[ ... ]
> +impl<T> Drop for PraminAccess<'_, T>
> +where
> +    T: FromBytes + IntoBytes,
> +{
> +    fn drop(&mut self) {
> +        // Insert an ordering point after previous writes through this window.
> +        self.view.cast::<u8>().read_val();

[Severity: Low]
Will this fail to compile if PraminAccess is instantiated with a zero-sized
type? Mmio::cast::<u8>() statically asserts that
size_of::<u8>() <= T::MIN_SIZE, which evaluates to 1 <= 0 for ZSTs and fails
during monomorphization.

> +    }
> +}
> +
> +impl<'gpu> Pramin<'gpu> {
[ ... ]
> +    pub(super) fn new(
> +        bar: Bar0<'gpu>,
> +        chipset: Chipset,
> +        vram_range: Range<VramAddress>,
> +    ) -> Result<Self> {
> +        let hal = hal::mm_hal(chipset);
> +        let window = io_project!(bar, build: PRAMIN);

[Severity: Low]
Is this macro syntax correct? The io_project! macro expects bracketed
syntax for array and offset projections. Should this be [build: PRAMIN]?

> +        let base = vram_range.start.align_down(Self::BASE_ALIGN);
[ ... ]
> +    pub(super) fn window_at<'a, T>(
> +        &'a mut self,
> +        vram_addr: VramAddress,
> +    ) -> Result<PraminAccess<'a, T>>
> +    where
> +        T: FromBytes + IntoBytes,
> +    {
> +        let offset = self.window_offset(vram_addr, size_of::<T>())?;
> +        let view = io_project!(self.window, try: OffsetLoc::new(offset));

[Severity: Low]
Does this also require bracketed macro syntax, like
[try: OffsetLoc::new(offset)]? Also, as noted above, OffsetLoc is missing
from the kernel tree.

> +
> +        Ok(PraminAccess { view })
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-pramin-split-v3-0-24b24d7afc52@nvidia.com?part=6