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 D9CCB64A8F; Wed, 24 Sep 2025 15:53:47 +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=1758729228; cv=none; b=hdNFCFhUq0EJnYN7wcNgEPbHs7rzGWhh7nk4yxS0v0FPU2lWQil4UMXusN7KBadthuv3yYEAGOj8PMHOAhUcNw+Ce0UP/vffjHyWwvUNA1TQR7yoASJ8l2r5KKlFqDm2mgK9MR+7UffC2mY9zhS57E81xfQ+owPOrQVoXIzEE40= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758729228; c=relaxed/simple; bh=H0LzwzgZoZdJZXsbobQBWIXxDAEPRUzEukhvOfTHsTU=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=QBu5JRQ0j/sOcqojJoNjdYvsJetV5ydZQGcoDPnIDDiqk+z4Mf7WCbJlbuZlfmuUUbNqj9Fm50JfzZuzuB061i9/gKre2zSsOEsLkxOKsRhKgFWsBZQz5W1Lyo4FvmNy9Q+pxiswj6DCDkP6HtH7lBUnvUPJ9hiWdNTr2zsIDKI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tZoCn+M1; 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="tZoCn+M1" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 480A0C4CEE7; Wed, 24 Sep 2025 15:53:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758729227; bh=H0LzwzgZoZdJZXsbobQBWIXxDAEPRUzEukhvOfTHsTU=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=tZoCn+M1nDTPC4DtPmwe/lKdjF8Woieav5sRVNGyZTM2JrGjxVqN618SrmB4JTl49 g99lo+52UHv9MnBSgQB+bDJgIaZ+KYGvV6SWJe/HaEaow514yxY76/DN8mFlHNiw04 hukCM2xZ89278hrbRZn1Bbc8RpYazFxqS3KoDH6/uvVjrov9I7WatFowRR/tkrqQq8 yDZ/yF8I6GYjSRdMzZFuzTIwBczC4wqE6C74NrOpSa/mKRrlVOyIQA474nDlmrvFZz bS5mD9rzTO01BqdDsa61m3zUluT2CK5QI9E15j51yu+bWkY0qmLvhG4yC+FtMto05T gy9kfKmahym5g== 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: Wed, 24 Sep 2025 17:53:40 +0200 Message-Id: Subject: Re: [PATCH v4 1/6] nova-core: bitfield: Move bitfield-specific code from register! into new macro Cc: "Greg KH" , "Benno Lossin" , "Joel Fernandes" , , , , , "Alistair Popple" , "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , , "Andreas Hindborg" , "Alice Ryhl" , "Trevor Gross" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "John Hubbard" , "Timur Tabi" , , "Elle Rhumsaa" , "Daniel Almeida" , To: "Yury Norov" From: "Danilo Krummrich" References: <20250920182232.2095101-1-joelagnelf@nvidia.com> <20250920182232.2095101-2-joelagnelf@nvidia.com> <2025092157-pauper-snap-aad1@gregkh> <2025092125-urban-muppet-1c2f@gregkh> <2025092432-entrust-citizen-0232@gregkh> In-Reply-To: On Wed Sep 24, 2025 at 4:38 PM CEST, Yury Norov wrote: > I didn't ask explicitly, and maybe it's a good time to ask now: Joel, > Danilo and everyone, have you considered adopting this project in > kernel? > > The bitfield_struct builds everything into the structure: > > use bitfield_struct::bitfield; > =20 > #[bitfield(u8, order =3D Msb)] > struct MyMsbByte { > /// The first field occupies the *most* significant bits > #[bits(4)] > kind: usize, > system: bool, > #[bits(2)] > level: usize, > present: bool > } > let my_byte_msb =3D MyMsbByte::new() > .with_kind(10) > .with_system(false) > .with_level(2) > .with_present(true); > =20 > // .- kind > // | .- system > // | | .- level > // | | | .- present > assert_eq!(my_byte_msb.0, 0b1010_0_10_1); > > Here MSB is not BE. For BE you'd specify: > > #[bitfield(u16, repr =3D be16, from =3D be16::from_ne, into =3D b= e16::to_ne)] > struct MyBeBitfield { > #[bits(4)] > first_nibble: u8, > #[bits(12)] > other: u16, > } > > The "from =3D be16::from_ne", is seemingly the same as cpu_to_be32() her= e. > > It looks like bitfield_struct tries to resolve hw access problems > by outsourcing them to 'from' and 'to' callbacks, and that looks > similar to what regmap API does (is that correct?). > > Greg, Is that what you're asking about? > > This is another bitfield crate with the similar approach=20 > > https://crates.io/crates/bitfield > > So we're not the first, and we need to discuss what is already done. > > As far as I understand, Joel decided to go in the other direction: > bitfields are always native in terms of endianess and not designed to > be mapped on registers directly. Which means they don't specify order > of accesses, number of accesses, access timing, atomicity, alignment, > cacheability and whatever else I/O related. > > I discussed with Joel about the hw register access and he confirmed > that the idea of his bitfields is to be a simple wrapper around logical > ops, while the I/O is a matter of 'backbone', which is entirely different > thing: When I was working on initial Rust driver support about a year ago, I also thought about how Rust drivers can deal with registers and added the TODO i= n [1]. This was picked up by Alex, who came up with a great implementation fo= r the register!() macro, which Joel splitted up into separate register!() and bit= field parts in the context of moving it from a nova internal implementation into = a core kernel API. As also described in [2], the idea is to have a macro, register!(), that cr= eates an abstract representation of a register, in order to remove the need for drivers to manually construct values through shift operations, masks, etc. At the same time the idea also is to get proper documentation of the hardwa= re registers in the kernel; the register!() macro encourages that, by its definition trying to come close to how registers are typically documented i= n datasheets, i.e. get rid of thousands of lines of auto-generated #defines f= or base addresses, shift and masks with cryptic names and provide something li= ke register!(NV_PMC_BOOT_0 @ 0x00000000, "Basic revision information about th= e GPU" { 28:24 architecture_0 as u8, "Lower bits of the architecture"; 23:20 implementation as u8, "Implementation version of the architect= ure"; 8:8 architecture_1 as u8, "MSB of the architecture"; 7:4 major_revision as u8, "Major revision of the chip"; 3:0 minor_revision as u8, "Minor revision of the chip"; }); instead. (It has quite some more features that also allow you to directly derive com= plex types from primitives and define arrays of registers, such as in register!(NV_PFALCON_FBIF_TRANSCFG @ PFalconBase[0x00000600[8]] { 1:0 target as u8 ?=3D> FalconFbifTarget; 2:2 mem_type as bool =3D> FalconFbifMemType; }); which makes dealing with such registers in drivers way less error prone. Here's one example of how it looks like to alter a single field within a register: // `bar` is the `pci::Bar` I/O backend. regs::NV_PFALCON_FALCON_ENGINE::alter(bar, |v| v.set_reset(true)); Of course you could also alter multiple fields at once by doing more change= s within the closure.) It intentionally avoids encoding hardware bus specific endianness, because otherwise we'd need to define this for every single register definition, wh= ich also falls apart when the device can sit on top of multiple different busse= s. Instead, the only thing that matters is that there is a contract between th= e abstract register representation and the I/O backends, such that the data c= an be correctly layed out by the I/O backend, which has to be aware of the actual hardware bus instead. As mentioned in another thread, one option for that is to use regmap within= the I/O backends, but that still needs to be addressed. So, for the register!() macro, I think we should keep it an abstract representation and deal with endianness in the I/O backend. However, that's or course orthogonal to the actual feature set of the bitfi= eld macro itself. - Danilo [1] https://docs.kernel.org/gpu/nova/core/todo.html#generic-register-abstra= ction-rega [2] https://lore.kernel.org/lkml/DD0ZTZM8S84H.1YDWSY7DF14LM@kernel.org/