Re: [PATCH 1/1] drm: nova: Align GEM memory allocation to system page size

[Brendan Shephard <bshephar@bne-home.net>]

On Tue, Nov 25, 2025 at 11:55:08PM +0900, Alexandre Courbot wrote:
> On Tue Nov 25, 2025 at 11:41 PM JST, Alice Ryhl wrote:
> >> > @@ -27,12 +31,13 @@ fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> {
> >> >  impl NovaObject {
> >> >      /// Create a new DRM GEM object.
> >> >      pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
> >> > -        let aligned_size = size.next_multiple_of(1 << 12);
> >> > -
> >> > -        if size == 0 || size > aligned_size {
> >> > +        // Check for 0 size or potential usize overflow before calling page_align
> >> > +        if size == 0 || size > usize::MAX - PAGE_SIZE + 1 {
> >>
> >> `PAGE_SIZE` here is no more correct than the hardcoded `1 << 12` - well,
> >> I'll admit it looks better as a placeholder. :) But the actual alignment
> >> will eventually be provided elsewhere.
> >
> > What about kernels with 16k pages?
> 
> The actual alignment should IIUC be a mix of the GPU and kernel's
> requirements (GPU can also use a different page size). So no matter what
> we pick right now, it won't be great but you are right that PAGE_SIZE
> will at least accomodate the kernel.
> 

So, maybe what we realistically should be doing is aligning to the
larger page size when comparing system and GPU page sizes?

> >
> >> >              return Err(EINVAL);
> >> >          }
> >> >
> >> > +        let aligned_size = page_align(size);
> >>
> >> `page_align` won't panic on overflow, but it will still return an
> >> invalid size. This is a job for `kernel::ptr::Alignment`, which let's
> >> you return an error when an overflow occurs.
> >
> > The Rust implementation of page_align() is implemented as (addr +
> > (PAGE_SIZE - 1)) & PAGE_MASK, which definitely will panic on overflow
> > if the appropriate config options are enabled.
> 
> That's right, I skimmed its code too fast. ^_^; All the more reason to
> use `Alignment`.

We would still need to ensure the value is a multiple of PAGE_SIZE
though right? Like if the user requests a size that is _not_ a multiple
of 2, then we would want to align the value to a PAGE_SIZE. Which is
what the existing logic does, it's just always rounding to the next
multiple of 4096. Maybe I'm missing something about Alignment and I need
to spend some more time looking at it as an alternative here.