From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8731D2C11C3; Fri, 3 Oct 2025 16:34:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759509259; cv=none; b=jwA+ZSQ31pFzK+CjDDnTjvBDpKVbuWkoocAW3/uxvY5x9GY3h0YWKV7J57bQyOmx1qXl1deIg3Iyfa6f5IphN0UqsTWyjIfEH8/AtQ53QzJoHLKCAgNJLDuf4PLa6mfELRs77ejGcUrShy8b3ADeo3+z3Rupf/bgwCCwtQw7aHs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1759509259; c=relaxed/simple; bh=ozG5IP2yJLNMxQLolYxbuES4sVM7rtrK+UQzCETiaCs=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=l28TFvVmpx38f6HLgS20XpoSLWNITCGf+pXsCqhPhyJajZlL4K8dDbTifB03qO26YqKdU6byG0ZH5wLVPfeTgF5e4Xht81PVTbgaWPnaIlkQn5SX3LFIiQEOphT1znNm3zvFZYzXn27+xNhc2kPSx5g8Q6rKVhP+uDsy7ojAdIk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Ryh7Z5K0; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Ryh7Z5K0" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B803BC4CEF5; Fri, 3 Oct 2025 16:34:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1759509259; bh=ozG5IP2yJLNMxQLolYxbuES4sVM7rtrK+UQzCETiaCs=; h=Date:Cc:Subject:From:To:References:In-Reply-To:From; b=Ryh7Z5K0/iw8iGFMMvEPGqI7LA/C+E7thDMQtF9lNLT47PZSMMukMxGOsDi862J3u 5ZIM8kv6mMYYbz79NgtbyCb7HpVEF52XyEFBs39+wqM5S/gzNGPnElOn16aZoewC7O 8d7CK8dFfFn5ZojeqstI7hSswtf2ayOoB9K+TWIX8QkIEvkpUHoaYlXwMi2SRiGHvM sIBx7uSI2KlKz1ViBiZv3bit4VDE79UoSFtEqXJwfa1PCQSF80czN7sJlyiDNml5vx u0Awg6y7xurEOLHW+IYPasjHpez1v0Jg2nCdL3PDRssCwZ5JlF82WVOPTewgAM/YCV QKkLqY9WaO3CQ== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Fri, 03 Oct 2025 18:34:12 +0200 Message-Id: Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "John Hubbard" , "Joel Fernandes" , "Timur Tabi" , , Subject: Re: [PATCH v3 08/13] gpu: nova-core: Add bindings and accessors for GspSystemInfo From: "Benno Lossin" To: "Alexandre Courbot" , "Alistair Popple" , , , X-Mailer: aerc 0.21.0 References: <20250930131648.411720-1-apopple@nvidia.com> <20250930131648.411720-9-apopple@nvidia.com> In-Reply-To: On Thu Oct 2, 2025 at 3:49 PM CEST, Alexandre Courbot wrote: > Hi Alistair, (+Benno as this concerns the `init!` macros) > > On Tue Sep 30, 2025 at 10:16 PM JST, Alistair Popple wrote: >> Adds bindings and an in-place initialiser for the GspSystemInfo struct. >> >> Signed-off-by: Alistair Popple >> >> --- >> >> It would be good to move to using the `init!` macros at some point, but >> I couldn't figure out how to make that work to initialise an enum rather >> than a struct as is required for the transparent representation. > > Indeed we have to jump through a few (minor) hoops. > > First the `init!` macros do not seem to support tuple structs. They > match a `{` after the type name, which is not present in > `GspSystemInfo`. By turning it into a regular struct with a single > field, we can overcome this, and it doesn't affect the layout the > `#[repr(transparent)]` can still be used. Yeah that's the correct workaround at the moment. I'm tracking support for tuple structs in [1]. Essentially the problem is that it requires lots of effort to parse tuple structs using declarative macros. We will get `syn` this cycle, which will enable me to support several things, including tuple structs. [1]: https://github.com/Rust-for-Linux/pin-init/issues/85 > Then, due to a limitation with declarative macros, `init!` interprets > `::` as a separator for generic arguments, so `bindings::GspSystemInfo` > also doesn't parse. Here the trick is to use a local type alias. This one will also be solved when we switch to syn. > After overcoming these two, I have been able to make > `GspSystemInfo::init` return an in-place initializer. It is then a > matter of changing `send_gsp_command` to accept it - please apply the > following patch on top of your series for an illustration of how it can > be done. > > Note that I have added a new generic argument for the error returned by > the `Init` - this is to let us also use infallible initializers > transparently. The cool thing is that callers don't need to specify any > generic argument since they can be inferred automatically. > > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gs= p/cmdq.rs > index 5580eaf52f7b..0709153f9dc9 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -247,12 +247,20 @@ fn notify_gsp(bar: &Bar0) { > NV_PGSP_QUEUE_HEAD::default().set_address(0).write(bar); > } > > - pub(crate) fn send_gsp_command( > + pub(crate) fn send_gsp_command( > &mut self, > bar: &Bar0, > payload_size: usize, > - init: impl FnOnce(&mut M, SBuffer>) -> Result, > - ) -> Result { > + init: impl Init, > + init_sbuffer: impl FnOnce(SBuffer>) -> Result, > + ) -> Result > + where > + M: CommandToGsp, > + // This allows all error types, including `Infallible`, to be us= ed with `init`. Without > + // this we cannot use regular stack objects as `init` since thei= r `Init` implementation > + // does not return any error. > + Error: From, > + { > // TODO: a method that extracts the regions for a given command? > // ... and another that reduces the region to a given number of = bytes! > let driver_area =3D self.gsp_mem.driver_write_area(); > @@ -264,7 +272,7 @@ pub(crate) fn send_gsp_command( > return Err(EAGAIN); > } > > - let (msg_header, cmd, payload_1, payload_2) =3D { > + let (msg_header, cmd_ptr, payload_1, payload_2) =3D { > #[allow(clippy::incompatible_msrv)] > let (msg_header_slice, slice_1) =3D driver_area > .0 > @@ -272,7 +280,6 @@ pub(crate) fn send_gsp_command( > .split_at_mut(size_of::()); > let msg_header =3D GspMsgElement::from_bytes_mut(msg_header_= slice).ok_or(EINVAL)?; > let (cmd_slice, payload_1) =3D slice_1.split_at_mut(size_of:= :()); > - let cmd =3D M::from_bytes_mut(cmd_slice).ok_or(EINVAL)?; > #[allow(clippy::incompatible_msrv)] > let payload_2 =3D driver_area.1.as_flattened_mut(); > // TODO: Replace this workaround to cut the payload size. > @@ -283,11 +290,22 @@ pub(crate) fn send_gsp_command( > None =3D> (&mut payload_1[..payload_size], payload_2), > }; > > - (msg_header, cmd, payload_1, payload_2) > + ( > + msg_header, > + cmd_slice.as_mut_ptr().cast(), > + payload_1, > + payload_2, > + ) > + }; > + > + let cmd =3D unsafe { > + init.__init(cmd_ptr)?; This is missing a safety comment. I haven't looked at this locally, so I don't know what is happening in the 10-20 lines that aren't shown, so I don't know if this is correct (if you're only assuming its initialized after this line completes then it's fine). The rest of the patch looks normal. Hope this helps! --- Cheers, Benno > + // Convert the pointer backto a reference for checksum. > + &mut *cmd_ptr > };