On 3/9/2026 10:29 PM, John Hubbard wrote:
> On 3/9/26 7:17 PM, Joel Fernandes wrote:
>> Hi John,
>>
>>> On Mar 9, 2026, at 8:06 PM, John Hubbard <[email protected]> wrote:
>>>
>>> On 3/9/26 4:41 PM, Joel Fernandes wrote:
>>>>>> On Mar 9, 2026, at 5:22 PM, Joel Fernandes <[email protected]> wrote:
>>>>> On Fri, Feb 27, 2026 at 09:32:08PM +0900, Eliot Courtney wrote:
>>>>>> Expose the `hInternalClient` and `hInternalSubdevice` handles. These are
>>>>>> needed for RM control calls.
>>>>>>
>>>>>> Signed-off-by: Eliot Courtney <[email protected]>
>>>>>> ---
>>>>>> drivers/gpu/nova-core/gsp/commands.rs    | 16 ++++++++++++++++
>>>>>> drivers/gpu/nova-core/gsp/fw/commands.rs | 10 ++++++++++
>>>>>> 2 files changed, 26 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/nova-core/gsp/commands.rs 
>>>>>> b/drivers/gpu/nova-core/gsp/commands.rs
>>>>>> index 4740cda0b51c..2cadfcaf9a8a 100644
>>>>>> --- a/drivers/gpu/nova-core/gsp/commands.rs
>>>>>> +++ b/drivers/gpu/nova-core/gsp/commands.rs
>>>>>> @@ -197,6 +197,8 @@ fn init(&self) -> impl Init<Self::Command, 
>>>>>> Self::InitError> {
>>>>>> /// The reply from the GSP to the [`GetGspInfo`] command.
>>>>>> pub(crate) struct GetGspStaticInfoReply {
>>>>>>    gpu_name: [u8; 64],
>>>>>> +    h_client: u32,
>>>>>> +    h_subdevice: u32,
>>>>>
>>>>> I would rather have more descriptive names please. 'client_handle',
>>>
>>> Maybe it's better to mirror the Open RM names, which are ancient and
>>> well known in those circles. Changing them at this point is probably
>>> going to result in a slightly worse situation, because there are
>>> probably millions of lines of code out there that use the existing
>>> nomenclature.
>>
>> I have to disagree a bit here. Saying h_ in code is a bit meaningless:
>> there is no mention of the word "handle" anywhere near these fields.
>> h_ could mean "higher", "hardware", or any number of things. The only
>> reason I know it means "handle" is because of expertise with Nvidia
>> drivers. The `_handle` suffix is self-documenting; `h_` is not.
> 
> Maybe if we write h_client in the code, and "handle..." in a comment
> above it? Or the other way around. Just to make the connection to
> the Open RM client code that is out there.
> 
Sure, a descriptive name is better but if we're going with descriptive type
names instead, then perhaps a comment with a descriptive type name would be fine
with me too.

Here are also some sample comments that could be used (Eliot do confirm
accuracy, but this is from my notes/research):

A client is a handle that provides a namespace and lifetime boundary for GPU
resource access. All child objects (devices, memory, channels) are scoped to it
and freed when it's destroyed. Under a client, we have Device and Sub-Device
handles.

The Device/Sub-Device split comes from SLI (multiple GPUs acting as one), where
the Device broadcasts operations to all GPUs, while the Sub-Device targets a
single GPU. Today with single-GPU setups, the pair is still mandatory but always
created together as a 1:1 mapping to a single physical GPU.

>>> particular, is an old SLI term that no one wants to keep around
>>> either. It was an ugly hack in Open RM that took more than a decade
>>> to recover from, by moving the SLI concept out to user space.
>>>
>>> So even though we should document what we're doing now, I would like
>>> to also note that we suspect a certain amount of this will
>>> disappear, to be replaced with a somewhat simpler API, in the
>>> somewhat near future.
>>
>> Sure, but client handles are a broader GPU driver concept even if this
>> particular one is GSP-internal. We are certainly going to need a rust type
>> to represent a client right? Other GPU drivers also have concept of
>> clients. The point is not that `hInternalClient` represents a GPU user
>> today, it may well be temporary as you note, but that using
>> `#[repr(transparent)]` new types for raw u32 handles costs nothing and
>> makes the code better and more readable. This pattern is already
>> well-established in nova-core itself: see `PackedRegistryEntry` for example
>> being a repr type. IMHO, there should be little reason that we need the
>> struct to have magic u32 numbers in Rust code for concepts like "handles".
>>
> 
> We will debate this when it shows up, perhaps I should not have 
> mentioned it, other than to remind Eliot to make it easy to delete.
> 
>> All I am saying is let us think this through before just doing the shortcut
>> of using u32 for client handles, etc. Rust gives us rich types, lets use 
>> them.
>>
> 
> ohh, that's a whole other topic and idea. I wasn't going into that,
> but feel free to explore if Rust can make it better.
Oh ok, yeah it sounds like we are aligned on this with Eliot's other reply with
introducing new rich types for these, so we should be good there (with abundant
comments on the rich types). Will explore this further on my side as well.

Thanks!

--
Joel Fernandes







Reply via email to