[PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP

Joel Fernandes posted 23 patches 4 weeks ago
There is a newer version of this series
[PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP
Posted by Joel Fernandes 4 weeks ago
Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
usable FB region from GSP's fbRegionInfoParams. Usable regions are those
that are not reserved or protected.

The extracted region is stored in GetGspStaticInfoReply and exposed via
usable_fb_region() API for use by the memory subsystem.

Cc: Nikola Djukic <ndjukic@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/gsp/commands.rs    | 11 ++++++--
 drivers/gpu/nova-core/gsp/fw/commands.rs | 32 ++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 8f270eca33be..8d5780d9cace 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -4,6 +4,7 @@
     array,
     convert::Infallible,
     ffi::FromBytesUntilNulError,
+    ops::Range,
     str::Utf8Error, //
 };
 
@@ -186,22 +187,28 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
     }
 }
 
-/// The reply from the GSP to the [`GetGspInfo`] command.
+/// The reply from the GSP to the [`GetGspStaticInfo`] command.
 pub(crate) struct GetGspStaticInfoReply {
     gpu_name: [u8; 64],
+    /// Usable FB (VRAM) region for driver memory allocation.
+    #[expect(dead_code)]
+    pub(crate) usable_fb_region: Range<u64>,
 }
 
 impl MessageFromGsp for GetGspStaticInfoReply {
     const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
     type Message = GspStaticConfigInfo;
-    type InitError = Infallible;
+    type InitError = Error;
 
     fn read(
         msg: &Self::Message,
         _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
     ) -> Result<Self, Self::InitError> {
+        let (base, size) = msg.first_usable_fb_region().ok_or(ENODEV)?;
+
         Ok(GetGspStaticInfoReply {
             gpu_name: msg.gpu_name_str(),
+            usable_fb_region: base..base.saturating_add(size),
         })
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 67f44421fcc3..cef86cab8a12 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -5,6 +5,7 @@
 use kernel::{device, pci};
 
 use crate::gsp::GSP_PAGE_SIZE;
+use crate::num::IntoSafeCast;
 
 use super::bindings;
 
@@ -115,6 +116,37 @@ impl GspStaticConfigInfo {
     pub(crate) fn gpu_name_str(&self) -> [u8; 64] {
         self.0.gpuNameString
     }
+
+    /// Extract the first usable FB region from GSP firmware data.
+    ///
+    /// Returns the first region suitable for driver memory allocation as a `(base, size)` tuple.
+    /// Usable regions are those that:
+    /// - Are not reserved for firmware internal use.
+    /// - Are not protected (hardware-enforced access restrictions).
+    /// - Support compression (can use GPU memory compression for bandwidth).
+    /// - Support ISO (isochronous memory for display requiring guaranteed bandwidth).
+    pub(crate) fn first_usable_fb_region(&self) -> Option<(u64, u64)> {
+        let fb_info = &self.0.fbRegionInfoParams;
+        for i in 0..fb_info.numFBRegions.into_safe_cast() {
+            if let Some(reg) = fb_info.fbRegion.get(i) {
+                // Skip malformed regions where limit < base.
+                if reg.limit < reg.base {
+                    continue;
+                }
+
+                // Filter: not reserved, not protected, supports compression and ISO.
+                if reg.reserved == 0
+                    && reg.bProtected == 0
+                    && reg.supportCompressed != 0
+                    && reg.supportISO != 0
+                {
+                    let size = reg.limit - reg.base + 1;
+                    return Some((reg.base, size));
+                }
+            }
+        }
+        None
+    }
 }
 
 // SAFETY: Padding is explicit and will not contain uninitialized data.
-- 
2.34.1
Re: [PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP
Posted by Alexandre Courbot 3 weeks, 1 day ago
On Wed Mar 11, 2026 at 9:39 AM JST, Joel Fernandes wrote:
> Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
> usable FB region from GSP's fbRegionInfoParams. Usable regions are those
> that are not reserved or protected.
>
> The extracted region is stored in GetGspStaticInfoReply and exposed via
> usable_fb_region() API for use by the memory subsystem.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

This doesn't take into account the feedback I gave in [1]. In
particular, a TODO to handle the remaining regions looks important to
me.

[1] https://lore.kernel.org/all/DGRGDFASWXB7.3NAK8RRTCV88P@nvidia.com/
Re: [PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP
Posted by Joel Fernandes 3 weeks, 1 day ago
On Mon, 16 Mar 2026, Alexandre Courbot wrote:
> This doesn't take into account the feedback I gave in [1]. In
> particular, a TODO to handle the remaining regions looks important to
> me.

Sorry about missing the v8 feedback. Fixed in upcoming v10.

--
Joel Fernandes
Re: [PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP
Posted by Eliot Courtney 3 weeks, 4 days ago
On Wed Mar 11, 2026 at 9:39 AM JST, Joel Fernandes wrote:
> Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
> usable FB region from GSP's fbRegionInfoParams. Usable regions are those
> that are not reserved or protected.
>
> The extracted region is stored in GetGspStaticInfoReply and exposed via
> usable_fb_region() API for use by the memory subsystem.
>
> Cc: Nikola Djukic <ndjukic@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/commands.rs    | 11 ++++++--
>  drivers/gpu/nova-core/gsp/fw/commands.rs | 32 ++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 8f270eca33be..8d5780d9cace 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -4,6 +4,7 @@
>      array,
>      convert::Infallible,
>      ffi::FromBytesUntilNulError,
> +    ops::Range,
>      str::Utf8Error, //
>  };
>  
> @@ -186,22 +187,28 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
>      }
>  }
>  
> -/// The reply from the GSP to the [`GetGspInfo`] command.
> +/// The reply from the GSP to the [`GetGspStaticInfo`] command.
>  pub(crate) struct GetGspStaticInfoReply {
>      gpu_name: [u8; 64],
> +    /// Usable FB (VRAM) region for driver memory allocation.
> +    #[expect(dead_code)]
> +    pub(crate) usable_fb_region: Range<u64>,
>  }
>  
>  impl MessageFromGsp for GetGspStaticInfoReply {
>      const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
>      type Message = GspStaticConfigInfo;
> -    type InitError = Infallible;
> +    type InitError = Error;
>  
>      fn read(
>          msg: &Self::Message,
>          _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
>      ) -> Result<Self, Self::InitError> {
> +        let (base, size) = msg.first_usable_fb_region().ok_or(ENODEV)?;
> +
>          Ok(GetGspStaticInfoReply {
>              gpu_name: msg.gpu_name_str(),
> +            usable_fb_region: base..base.saturating_add(size),

We already return a Result here, so why not use checked_add?:
`base..base.checked_add(size).ok_or(EOVERFLOW)?`

>          })
>      }
>  }
> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
> index 67f44421fcc3..cef86cab8a12 100644
> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
> @@ -5,6 +5,7 @@
>  use kernel::{device, pci};
>  
>  use crate::gsp::GSP_PAGE_SIZE;
> +use crate::num::IntoSafeCast;
>  
>  use super::bindings;
>  
> @@ -115,6 +116,37 @@ impl GspStaticConfigInfo {
>      pub(crate) fn gpu_name_str(&self) -> [u8; 64] {
>          self.0.gpuNameString
>      }
> +
> +    /// Extract the first usable FB region from GSP firmware data.
> +    ///
> +    /// Returns the first region suitable for driver memory allocation as a `(base, size)` tuple.
> +    /// Usable regions are those that:
> +    /// - Are not reserved for firmware internal use.
> +    /// - Are not protected (hardware-enforced access restrictions).
> +    /// - Support compression (can use GPU memory compression for bandwidth).
> +    /// - Support ISO (isochronous memory for display requiring guaranteed bandwidth).

Are the above conditions all required (AND) or any required (OR)?
Might be worth clarifying in the doc.

> +    pub(crate) fn first_usable_fb_region(&self) -> Option<(u64, u64)> {
> +        let fb_info = &self.0.fbRegionInfoParams;
> +        for i in 0..fb_info.numFBRegions.into_safe_cast() {
> +            if let Some(reg) = fb_info.fbRegion.get(i) {
> +                // Skip malformed regions where limit < base.

Is it normal that it returns a bunch of broken regions?

> +                if reg.limit < reg.base {
> +                    continue;
> +                }
> +
> +                // Filter: not reserved, not protected, supports compression and ISO.
> +                if reg.reserved == 0
> +                    && reg.bProtected == 0
> +                    && reg.supportCompressed != 0
> +                    && reg.supportISO != 0
> +                {
> +                    let size = reg.limit - reg.base + 1;
> +                    return Some((reg.base, size));

This is identifying a range, so how about returning Option<Range<u64>>
instead? It gets immediately converted into a range anyway.

> +                }
> +            }
> +        }
> +        None
> +    }
>  }
>  
>  // SAFETY: Padding is explicit and will not contain uninitialized data.
Re: [PATCH v9 04/23] gpu: nova-core: gsp: Extract usable FB region from GSP
Posted by Joel Fernandes 6 days, 3 hours ago
On Fri, Mar 13, 2026 at 03:58:35PM +0900, Eliot Courtney wrote:
> On Wed Mar 11, 2026 at 9:39 AM JST, Joel Fernandes wrote:
> > Add first_usable_fb_region() to GspStaticConfigInfo to extract the first
> > usable FB region from GSP's fbRegionInfoParams. Usable regions are those
> > that are not reserved or protected.
> >
> > The extracted region is stored in GetGspStaticInfoReply and exposed via
> > usable_fb_region() API for use by the memory subsystem.
> >
> > Cc: Nikola Djukic <ndjukic@nvidia.com>
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> > ---
> >  drivers/gpu/nova-core/gsp/commands.rs    | 11 ++++++--
> >  drivers/gpu/nova-core/gsp/fw/commands.rs | 32 ++++++++++++++++++++++++
> >  2 files changed, 41 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> > index 8f270eca33be..8d5780d9cace 100644
> > --- a/drivers/gpu/nova-core/gsp/commands.rs
> > +++ b/drivers/gpu/nova-core/gsp/commands.rs
> > @@ -4,6 +4,7 @@
> >      array,
> >      convert::Infallible,
> >      ffi::FromBytesUntilNulError,
> > +    ops::Range,
> >      str::Utf8Error, //
> >  };
> >  
> > @@ -186,22 +187,28 @@ fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> >      }
> >  }
> >  
> > -/// The reply from the GSP to the [`GetGspInfo`] command.
> > +/// The reply from the GSP to the [`GetGspStaticInfo`] command.
> >  pub(crate) struct GetGspStaticInfoReply {
> >      gpu_name: [u8; 64],
> > +    /// Usable FB (VRAM) region for driver memory allocation.
> > +    #[expect(dead_code)]
> > +    pub(crate) usable_fb_region: Range<u64>,
> >  }
> >  
> >  impl MessageFromGsp for GetGspStaticInfoReply {
> >      const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
> >      type Message = GspStaticConfigInfo;
> > -    type InitError = Infallible;
> > +    type InitError = Error;
> >  
> >      fn read(
> >          msg: &Self::Message,
> >          _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
> >      ) -> Result<Self, Self::InitError> {
> > +        let (base, size) = msg.first_usable_fb_region().ok_or(ENODEV)?;
> > +
> >          Ok(GetGspStaticInfoReply {
> >              gpu_name: msg.gpu_name_str(),
> > +            usable_fb_region: base..base.saturating_add(size),
> 
> We already return a Result here, so why not use checked_add?:
> `base..base.checked_add(size).ok_or(EOVERFLOW)?`

Hmm, I think I was trying to play it safe and handle any situation of a
corrupted size. But granted, it may be better to error out.

> 
> >          })
> >      }
> >  }
> > diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
> > index 67f44421fcc3..cef86cab8a12 100644
> > --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> > +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
> > @@ -5,6 +5,7 @@
> >  use kernel::{device, pci};
> >  
> >  use crate::gsp::GSP_PAGE_SIZE;
> > +use crate::num::IntoSafeCast;
> >  
> >  use super::bindings;
> >  
> > @@ -115,6 +116,37 @@ impl GspStaticConfigInfo {
> >      pub(crate) fn gpu_name_str(&self) -> [u8; 64] {
> >          self.0.gpuNameString
> >      }
> > +
> > +    /// Extract the first usable FB region from GSP firmware data.
> > +    ///
> > +    /// Returns the first region suitable for driver memory allocation as a `(base, size)` tuple.
> > +    /// Usable regions are those that:
> > +    /// - Are not reserved for firmware internal use.
> > +    /// - Are not protected (hardware-enforced access restrictions).
> > +    /// - Support compression (can use GPU memory compression for bandwidth).
> > +    /// - Support ISO (isochronous memory for display requiring guaranteed bandwidth).
> 
> Are the above conditions all required (AND) or any required (OR)?
> Might be worth clarifying in the doc.

I am not sure if any clarification is needed there but I will
reword to 'Usable regions are those that satisfy all of the following:'.

> 
> > +    pub(crate) fn first_usable_fb_region(&self) -> Option<(u64, u64)> {
> > +        let fb_info = &self.0.fbRegionInfoParams;
> > +        for i in 0..fb_info.numFBRegions.into_safe_cast() {
> > +            if let Some(reg) = fb_info.fbRegion.get(i) {
> > +                // Skip malformed regions where limit < base.
> 
> Is it normal that it returns a bunch of broken regions?

Not really. The aim was to make the code robust at a time when I was studying
the FB regions. I will change the comment, and/or drop the check. Thanks for
pointing it out.

> > +                if reg.limit < reg.base {
> > +                    continue;
> > +                }
> > +
> > +                // Filter: not reserved, not protected, supports compression and ISO.
> > +                if reg.reserved == 0
> > +                    && reg.bProtected == 0
> > +                    && reg.supportCompressed != 0
> > +                    && reg.supportISO != 0
> > +                {
> > +                    let size = reg.limit - reg.base + 1;
> > +                    return Some((reg.base, size));
> 
> This is identifying a range, so how about returning Option<Range<u64>>
> instead? It gets immediately converted into a range anyway.

Sure, that works.

--
Joel Fernandes