On 2026-08-17 at 18:20 +1000, Eliot Courtney <[email protected]> wrote...
> On Mon Aug 17, 2026 at 3:00 PM JST, Alistair Popple wrote:
> > On 2026-08-10 at 23:55 +1000, Eliot Courtney <[email protected]> wrote...
> >> From: Joel Fernandes <[email protected]>
> >> 
> >> Add the `VramAddress` type representing a physical address in VRAM. Also
> >> add an arithmetic helper, comparison, and operator overloads which are
> >> required in later patches for address arithmetic.
> >> 
> >> Signed-off-by: Joel Fernandes <[email protected]>
> >> [ecourtney: create mm.rs here, squashing in the arithmetic patch]
> >> [ecourtney: splice the two commit bodies]
> >> [ecourtney: drop the Pfn fields, open-coding what bitfield! generated]
> >> [ecourtney: drop align_down and the IntoVramOffset/IntoVramRange traits]
> >> [ecourtney: make checked_add() const over a plain u64, derive the ordering]
> >> [ecourtney: doc wording, header, import, and signature cleanups]
> >> Signed-off-by: Eliot Courtney <[email protected]>
> >> ---
> >>  drivers/gpu/nova-core/mm.rs        | 60 
> >> ++++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/nova-core/nova_core.rs |  1 +
> >>  2 files changed, 61 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/nova-core/mm.rs b/drivers/gpu/nova-core/mm.rs
> >> new file mode 100644
> >> index 000000000000..dcd5e5e919bf
> >> --- /dev/null
> >> +++ b/drivers/gpu/nova-core/mm.rs
> >> @@ -0,0 +1,60 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & 
> >> AFFILIATES. All rights reserved.
> >> +
> >> +//! Memory management subsystems.
> >> +
> >> +#![expect(dead_code)]
> >> +
> >> +use core::{
> >> +    fmt::LowerHex,
> >> +    ops, //
> >> +};
> >> +
> >> +use kernel::fmt;
> >> +
> >> +/// Physical VRAM address in GPU video memory.
> >> +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
> >> +#[repr(transparent)]
> >> +pub(crate) struct VramAddress(u64);
> >
> > Is this the best type here, or should this be bounded via runtime checking 
> > to
> > ensure it refers to a valid address?
> >
> > I was going to ask if it should be bounded to reflect the physical 
> > addressing
> > capabilities of the GPU in question, but that would actually need to be a
> > different type as GPU physical addresses might not even refer to local 
> > vidmem.
> >
> > Although I think that's a concern for future series - AFAIK PRAMIN can only
> > access local VRAM.
> 
> Yeah good question. I am looking at memory management stuff after this,
> and e.g. how many bits the hardware actually accept for physical
> addresses (e.g. page table structures) is different depending on the
> hardware. So it's not trivial to determine if the address is valid
> without a bunch of extra info.
> 
> So the approach here IMO is just to tag it as a general VRAM address at
> the type level and gradually apply runtime checks to narrow the type
> into code that is more specific. Since our HALs use dynamic dispatch
> based on the chipid we can't avoid some kind of runtime checks here
> anyway.
> 
> I think it makes sense to say VramAddress is a local vidmem address, and
> later we could e.g. add an enum to distinguish local vidmem, sysmem,
> peer memory etc. This appears to be essentially what UVM does
> (uvm_gpu_phys_address_t), AFAICT. What do you think?

Yeah, after reading the rest of the series I think this is where I landed as
well - VramAddress as a local vidmem address that we use as one component when
building up the more complicated physical address. As much as anything though
just wanted to check we weren't accidentally conflating this with physical
addressing which is more complicated than just VramAddress.

Also seeing as you've obviously been looking at UVM source it's worth noting
that UVM's physical address type is not exhaustive either as it currently only
deals with some aspects of physical memory addressing and leaves the rest to RM.

> And yeah I believe PRAMIN can only access local VRAM and sysmem (on pre
> hopper).
> 
> >
> >> +
> >> +impl VramAddress {
> >> +    /// Creates an address from a raw value.
> >> +    pub(crate) const fn from_raw(addr: u64) -> Self {
> >> +        Self(addr)
> >> +    }
> >> +
> >> +    /// Returns the address as a raw value.
> >> +    pub(crate) const fn into_raw(self) -> u64 {
> >> +        self.0
> >> +    }
> >> +
> >> +    /// Adds `rhs` to this address, returning [`None`] on overflow.
> >> +    pub(crate) const fn checked_add(self, rhs: u64) -> Option<Self> {
> >> +        match self.into_raw().checked_add(rhs) {
> >> +            Some(addr) => Some(Self::from_raw(addr)),
> >> +            None => None,
> >> +        }
> >> +    }
> >> +}
> >> +
> >> +impl LowerHex for VramAddress {
> >> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> >> +        LowerHex::fmt(&self.into_raw(), f)
> >> +    }
> >> +}
> >> +
> >> +impl ops::Add<u64> for VramAddress {
> >> +    type Output = Self;
> >> +
> >> +    fn add(self, rhs: u64) -> Self::Output {
> >> +        Self::from_raw(self.into_raw() + rhs)
> >> +    }
> >> +}
> >> +
> >> +impl ops::Sub for VramAddress {
> >> +    type Output = u64;
> >> +
> >> +    fn sub(self, rhs: Self) -> Self::Output {
> >> +        self.into_raw() - rhs.into_raw()
> >> +    }
> >> +}
> >> diff --git a/drivers/gpu/nova-core/nova_core.rs 
> >> b/drivers/gpu/nova-core/nova_core.rs
> >> index 35a8b1214b0e..8f59cfa97017 100644
> >> --- a/drivers/gpu/nova-core/nova_core.rs
> >> +++ b/drivers/gpu/nova-core/nova_core.rs
> >> @@ -18,6 +18,7 @@
> >>  mod gpu;
> >>  mod gsp;
> >>  mod mctp;
> >> +mod mm;
> >>  #[macro_use]
> >>  mod num;
> >>  mod regs;
> >> 
> >> -- 
> >> 2.55.0
> >> 
> 

Reply via email to