On 2026-09-01 at 00:33 +1000, Danilo Krummrich <[email protected]> wrote...
> On Fri Aug 28, 2026 at 5:35 AM CEST, Alistair Popple wrote:
> > impl NovaCoreApi<'_> {
> > + /// Returns the NUL-terminated full GPU name supplied by GSP-RM.
> > + pub fn gpu_name(&self) -> [u8; 64] {
>
> I don't really like that we have two separate accessors for this, can't we
> just
> use the one we already have, which also does all the validation already?
Yeah, I don't know how I forgot we already had an accessor for that. I've been
carrying this patch on my own tree for quite a while, so guess just enough code
moved around change that I missed it.
Anyway I agree, a new acessor is totally unnecessary.
> The constructor of GpuInfo could look like this:
>
> fn new(reg_data: &DrmRegData<'_>) -> Result<Self> {
> let mut info = uapi::drm_nova_gpu_info {
> architecture: reg_data.api.architecture(),
> implementation: reg_data.api.implementation(),
> ..Default::default()
> };
> let bytes = reg_data.api.gpu_name()?.as_bytes();
> info.gpu_name[..bytes.len()].copy_from_slice(bytes);
> Ok(Self(info))
> }
>
> Could also be infallible if we want to go with a fallback name, given that we
> consider the firmware not providing something useful as non-fatal so far.
Yeah, I think keeping this infallible makes sense.
> > + *self.gpu.gsp_static_info.gpu_name_bytes()
> > + }
> > +
> > /// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered
> > /// by nova-core.
> > pub fn of(adev: &auxiliary::Device<Bound>) ->
> > Result<Pin<&NovaCoreApi<'_>>> {
> > diff --git a/drivers/gpu/nova-core/gsp/commands.rs
> > b/drivers/gpu/nova-core/gsp/commands.rs
> > index 6453184af55b..b8dc64808620 100644
> > --- a/drivers/gpu/nova-core/gsp/commands.rs
> > +++ b/drivers/gpu/nova-core/gsp/commands.rs
> > @@ -251,6 +251,11 @@ pub(crate) enum GpuNameError {
> > }
> >
> > impl GetGspStaticInfoReply {
> > + /// Returns the full GPU name as a NUL-terminated byte string.
> > + pub(crate) fn gpu_name_bytes(&self) -> &[u8; 64] {
> > + &self.gpu_name
> > + }
>
> AFAICS there's nothing ensuring that this is actually NULL terminated? The
> existing gpu_name() method already does this.
Yep, relied on FW which is sub-optimal. The existing accessors obviously fix
that.
- Alistair
