Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Tlb::flush unconditionally uses TlbAckMode::None, which fails to wait 
for global TLB invalidation and creates a use-after-free risk during unmaps.
--

commit d4de9556e03b0600a4f0e5f45fee95f566102c9d
Author: Joel Fernandes <[email protected]>

gpu: nova-core: mm: Add buddy allocator and TLB to GpuMm

Extend GpuMm with the buddy allocator for VRAM allocation and the TLB
manager for translation buffer operations. This completes the centralized
ownership model for memory management components.

> diff --git a/drivers/gpu/nova-core/mm/tlb.rs b/drivers/gpu/nova-core/mm/tlb.rs
> new file mode 100644
> index 0000000000000..cc862e8159a19
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/tlb.rs

[ ... ]

> +    /// Flush the GPU TLB for a specific page directory base.
> +    ///
> +    /// This invalidates all TLB entries associated with the given PDB 
> address.
> +    /// Must be called after modifying page table entries to ensure the GPU 
> sees
> +    /// the updated mappings.
> +    pub(super) fn flush(&self, pdb_addr: VramAddress) -> Result {
> +        let _guard = self.lock.lock();
> +
> +        // Write PDB address.
> +        self.bar.write_reg(regs::NV_TLB_FLUSH_PDB_LO::from_pdb_addr(
> +            pdb_addr.into_raw(),
> +        ));
> +        self.bar.write_reg(regs::NV_TLB_FLUSH_PDB_HI::from_pdb_addr(
> +            pdb_addr.into_raw(),
> +        ));
> +
> +        // Trigger flush.
> +        self.bar.write_reg(
> +            regs::NV_TLB_FLUSH_CTRL::zeroed()
> +                .with_all_va(true)
> +                .with_ack(TlbAckMode::None)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does hardcoding the fire-and-forget acknowledgment mode here create a
use-after-free risk?

Since TLB flushes are used during unmap operations, a fire-and-forget mode
fails to wait for global TLB invalidation. The GPU might retain and use stale
TLB entries to access freed physical memory before the invalidation fully
propagates.

The documentation for TlbAckMode::Globally notes that it is strictly
required during unmap or permission tightening to prevent this exact issue.
Should Tlb::flush accept the acknowledgment mode as a parameter, or use
TlbAckMode::Globally by default?

> +                .with_trigger(true),
> +        );
> +
> +        // Poll for completion.
> +        read_poll_timeout(
> +            || Ok(self.bar.read(regs::NV_TLB_FLUSH_CTRL)),
> +            |ctrl: &regs::NV_TLB_FLUSH_CTRL| !ctrl.trigger(),
> +            Delta::ZERO,
> +            Delta::from_secs(2),
> +        )?;
> +
> +        Ok(())
> +    }
> +}

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to