linux-kernel - Re: [PATCH 6/7] gpu: nova-core: send UNLOADING_GUEST

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47F9A9AB-42B5-4A5C-90CA-8A00DD253DA7@nvidia.com>
Date: Thu, 18 Dec 2025 20:52:59 +0000
From: Joel Fernandes <joelagnelf@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
CC: Danilo Krummrich <dakr@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
	Simona Vetter <simona@...ll.ch>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>, Miguel
 Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
	<gary@...yguo.net>, Björn Roy Baron
	<bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, Andreas
 Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, Alistair
 Popple <apopple@...dia.com>, Eliot Courtney <ecourtney@...dia.com>,
	"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"rust-for-linux@...r.kernel.org" <rust-for-linux@...r.kernel.org>, Alexandre
 Courbot <acourbot@...dia.com>
Subject: Re: [PATCH 6/7] gpu: nova-core: send UNLOADING_GUEST_DRIVER GSP
 command GSP upon unloading

Hi Alex,

>>> +    }
>>> +}
>>> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
>>> index 09549f7db52d..228464fd47a0 100644
>>> --- a/drivers/gpu/nova-core/gsp/fw.rs
>>> +++ b/drivers/gpu/nova-core/gsp/fw.rs
>>> @@ -209,6 +209,7 @@ pub(crate) enum MsgFunction {
>>>    GspInitPostObjGpu = bindings::NV_VGPU_MSG_FUNCTION_GSP_INIT_POST_OBJGPU,
>>>    GspRmControl = bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL,
>>>    GetStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO,
>>> +    UnloadingGuestDriver = bindings::NV_VGPU_MSG_FUNCTION_UNLOADING_GUEST_DRIVER,
>>> 
>>>    // Event codes
>>>    GspInitDone = bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE,
>>> @@ -249,6 +250,9 @@ fn try_from(value: u32) -> Result<MsgFunction> {
>>>            }
>>>            bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL => Ok(MsgFunction::GspRmControl),
>>>            bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO => Ok(MsgFunction::GetStaticInfo),
>>> +            bindings::NV_VGPU_MSG_FUNCTION_UNLOADING_GUEST_DRIVER => {
>>> +                Ok(MsgFunction::UnloadingGuestDriver)
>>> +            }
>>>            bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE => Ok(MsgFunction::GspInitDone),
>>>            bindings::NV_VGPU_MSG_EVENT_GSP_RUN_CPU_SEQUENCER => {
>>>                Ok(MsgFunction::GspRunCpuSequencer)
>>> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
>>> index 85465521de32..c7df4783ad21 100644
>>> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
>>> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
>>> @@ -125,3 +125,30 @@ unsafe impl AsBytes for GspStaticConfigInfo {}
>>> // SAFETY: This struct only contains integer types for which all bit patterns
>>> // are valid.
>>> unsafe impl FromBytes for GspStaticConfigInfo {}
>>> +
>>> +/// Payload of the `UnloadingGuestDriver` command and message.
>>> +#[repr(transparent)]
>>> +#[derive(Clone, Copy, Debug, Zeroable)]
>>> +pub(crate) struct UnloadingGuestDriver {
>>> +    inner: bindings::rpc_unloading_guest_driver_v1F_07,
>>> +}
>>> +
>>> +impl UnloadingGuestDriver {
>>> +    pub(crate) fn new(suspend: bool) -> Self {
>>> +        Self {
>>> +            inner: bindings::rpc_unloading_guest_driver_v1F_07 {
>> 
>> We should go through intermediate firmware representation than direct bindings access?
> 
> Only if the size of the bindings justifies it - here having an opaque
> wrapper just just well, and spares us some code down the line as we
> would have to initialize the bindings anyway.

I am not sure about that, it sounds like a layering violation. It would be good not to keep the rules fuzzy about this, because then we could do it either way in all cases.

Another reason is that we cannot anticipate in advance which specific helper functions we will need to add in the future. Down the line, we may need to add some helper functions to the struct as well.  Also having V1F07 in the name sounds very magic number-ish. It would be good to abstract that out with a better-named struct anyway.

Thanks,

- Joel



> 
>> 
>> 
>>> +                bInPMTransition: if suspend { 1 } else { 0 },
>> 
>> Then this can just be passed as a bool.
>> 
>>> +                bGc6Entering: 0,
>>> +                newLevel: if suspend { 3 } else { 0 },
> 
> Note to self to figure out these magic numbers. :)
>