From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CC6E8CA100C for ; Tue, 2 Sep 2025 19:53:21 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B41DF10E82C; Tue, 2 Sep 2025 19:53:20 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="CCW3q19f"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2C2F210E82C; Tue, 2 Sep 2025 19:53:19 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id B18BF41812; Tue, 2 Sep 2025 19:53:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4DAF6C4CEED; Tue, 2 Sep 2025 19:53:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1756842798; bh=EES9E4p0II9pKq1UKkB0kITaMd3S6hMDx8aHEP9YBYM=; h=Date:Cc:To:From:Subject:References:In-Reply-To:From; b=CCW3q19f1yy6u796iT5OMmZJMQLz+bWmn4U6nlUY7VzI2NWzyHxVfjszhxKJ93DmH ci8C/CpnQ5DDiJLgd0Q82xyrht1moay6Tn+b29iioYmVATBhO3UiDJ+yRVSBkmhSf/ aNcG9zBu6bQpx7D4GL+hFKD50p6ue0RUxM8Rb6uTuLTDRT2adLnXhJQqRTIyl+md46 ybLvLczL+ZeDR5e0Nb3f/1QyIQVX6Ow4VY5+gMGwOpbCvMS98XakCbuSiB4zsFDg+m kQFCbyjmGMUl+xAnJ/J4IBXXqtBKmBXqZn8emdVArub4WIhnBs/xk5elo0IQ/kz/+u lmO2IXDyaNg2Q== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 02 Sep 2025 21:53:12 +0200 Message-Id: Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Benno Lossin" , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "John Hubbard" , "Alistair Popple" , "Joel Fernandes" , "Timur Tabi" , , , , To: "Alexandre Courbot" From: "Danilo Krummrich" Subject: Re: [PATCH v3 02/11] gpu: nova-core: move GSP boot code out of `Gpu` constructor References: <20250902-nova_firmware-v3-0-56854d9c5398@nvidia.com> <20250902-nova_firmware-v3-2-56854d9c5398@nvidia.com> In-Reply-To: <20250902-nova_firmware-v3-2-56854d9c5398@nvidia.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue Sep 2, 2025 at 4:31 PM CEST, Alexandre Courbot wrote: > diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driv= er.rs > index 274989ea1fb4a5e3e6678a08920ddc76d2809ab2..1062014c0a488e959379f009c= 2e8029ffaa1e2f8 100644 > --- a/drivers/gpu/nova-core/driver.rs > +++ b/drivers/gpu/nova-core/driver.rs > @@ -6,6 +6,8 @@ > =20 > #[pin_data] > pub(crate) struct NovaCore { > + // Placeholder for the real `Gsp` object once it is built. > + pub(crate) gsp: (), > #[pin] > pub(crate) gpu: Gpu, > _reg: auxiliary::Registration, > @@ -40,8 +42,14 @@ fn probe(pdev: &pci::Device, _info: &Self::IdInf= o) -> Result )?; > =20 > let this =3D KBox::pin_init( > - try_pin_init!(Self { > + try_pin_init!(&this in Self { > gpu <- Gpu::new(pdev, bar)?, > + gsp <- { > + // SAFETY: `this.gpu` is initialized to a valid valu= e. > + let gpu =3D unsafe { &(*this.as_ptr()).gpu }; > + > + gpu.start_gsp(pdev)? > + }, Please use pin_chain() [1] for this. More in general, unsafe code should be the absolute last resort. If we add = new unsafe code I'd love to see a comment justifying why there's no other way t= han using unsafe code for this, as we agreed in [2]. I did a quick grep on this series and I see 21 occurrences of "unsafe", if = I substract the ones for annotations and for FromBytes impls, it's still 9 ne= w ones. :( Do we really need all of them? Otherwise, I really like this, it's a great improvement over initializing everything into the Gpu struct -- thanks for the refactoring! [1] https://rust.docs.kernel.org/kernel/prelude/trait.PinInit.html#method.p= in_chain [2] https://docs.kernel.org/gpu/nova/guidelines.html#language