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 <[email protected]>
> > Signed-off-by: Joel Fernandes <[email protected]>
> > ---
> > 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
