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: <81AD31F2-D2F3-4877-BE30-C08EB1839B02@nvidia.com>
Date: Fri, 19 Dec 2025 06:42:42 +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



> On Dec 18, 2025, at 10:26 PM, Alexandre Courbot <acourbot@...dia.com> wrote:
> 
> On Fri Dec 19, 2025 at 5:52 AM JST, Joel Fernandes wrote:
>> 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.
> 
> The rules are not fuzzy. The only thing modules outside of `fw` are
> seeing is a struct named `UnloadingGuestDriver`, and the name with
> `V1F07` is not leaked.
> 
> Even if we had a different structure, we would still need to write the
> rpc_unloading_guest_driver_v1F_07 at some point, so we would need to
> refer to it in `fw` anyway. The current design is not any more lax than
> that, it just removes one step.

Ah, I missed that. This is all in FW, so I think that clarifies the rule for me now.

Thank you.