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

["Eliot Courtney" <ecourtney@nvidia.com>]

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.