Alexandre Courbot
2025-Oct-07 10:36 UTC
[PATCH v6 0/5] Introduce bitfield and move register macro to rust/kernel/
Hi Yuri, On Tue Oct 7, 2025 at 7:29 AM JST, Yury Norov wrote: <snip>> Regardless, I don't think that this is the right path to move the > bitfields into the core. The natural path for a feature that has > been originally developed on driver side is to mature in there and > get merged to core libraries after a while. Resctrl from Intel is one > recent example. > > With that said, I'm OK if you move the bitfields as a whole, like you > do in v5, and I'm also OK if you split out the part essential for nova > and take it into the driver. In that case the bitfields will stay in > drivers and you'll be able to focus on the features that _you_ need, > not on generic considerations. > > I'm not OK to move bitfields in their current (v6) incomplete form in > rust/kernel. We still have no solid understanding on the API and > implementation that we've been all agreed on.Initially the plan was indeed to give this code some more time to mature in nova-core before moving it out. The reason for the early move is that we have another driver (Tyr) who wants to start using the register macro. Without it, they would be left with the option of either reinventing the wheel, or poking at registers the old-fashioned way, which I think we can agree is not going to be any safer than the current macro. :) IIUC your remaining concern is with the possible loss of data when setting a field that is smaller than its primitive type? That should be addressed by [0], but as it introduces a new core feature I expect some discussion to take place before it can be merged. In the meantime, it would be great if we can make the register macro available. Because letting it fully mature within nova-core also has the drawback that we might miss the perspective of other potential users, which may make us draw ourselves into a corner that will make the macro less useful generally speaking. We are at a stage where we can still make design changes if needed, but we need to hear from other users, and these won't come as long as the macro is in nova-core. [0] https://lore.kernel.org/rust-for-linux/20251002-bounded_ints-v1-0-dd60f5804ea4 at nvidia.com/> On maintenance: no core functionality can be merged unmaintained - it's > a strict rule. While in drivers, bitfields are maintained by the nova > maintainers as part of the driver. If you make it a generic library, > you need to define a responsible person(s) in advance. It's also a good > practice to add a core maintainer as a reviewer or co-maintainer. Alice > and Burak added me for bitmap/rust because I already look after bitmaps > in C. You can do the same for bitfields, for the same reason.We can assume maintainership of this of course, but is there a problem if this falls under the core Rust umbrella? As this is a pretty core functionality. Miguel and other core folks, WDYT?
Danilo Krummrich
2025-Oct-07 13:16 UTC
[PATCH v6 0/5] Introduce bitfield and move register macro to rust/kernel/
On Tue Oct 7, 2025 at 12:36 PM CEST, Alexandre Courbot wrote:> Because letting it fully mature within nova-core also has the drawback > that we might miss the perspective of other potential users, which may > make us draw ourselves into a corner that will make the macro less > useful generally speaking. We are at a stage where we can still make > design changes if needed, but we need to hear from other users, and > these won't come as long as the macro is in nova-core.There are two different things here that are getting mixed up a bit. (1) Moving the register!() code out of nova-core to make it accessible for other drivers. (2) Generalize the bitfield implementation that so far is baked into the register!() code. Both of those make sense, but they don't have to happen at the same time necessarily. Now, I'm not saying that we necessarily have to change the approach here. The current merge window isn't even closed, so we have plently of time left, i.e. there's no rush with with patch series. However, if it helps, I'm perfectly fine to take the register!() implementation into the I/O entry in a first step and in a second step generalize the bitfield implementation and move it out of the register!() code. Again, there's no rush as far as I'm concerned, yet the latter approach might add a bit more structure and hence run a bit smoother. - Danilo