[PATCH v3 0/2] rust: impl_flags: add convenience functions

[PATCH v3 0/2] rust: impl_flags: add convenience functions

[Andreas Hindborg]

Add a few convenience functions that makes it easier to work with the
`impl_flags` module and C APIs.

These conversion functions are useful when assigning flags to C structs in
Rust abstractions:

  let mut lim = bindings::queue_limits::zeroed();

  if self.rotational {
      lim.features = Feature::Rotational.into();
  }

  if self.write_cache {
      lim.features |= Feature::WriteCache;
  }

Or when providing flags from C to Rust:

  impl<T: Operations> RequestInner<T> {
      pub fn flags(&self) -> Flags {
          // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read
          let flags = unsafe { (*self.0.get()).cmd_flags & !((1 << bindings::REQ_OP_BITS) - 1) };
          Flags::try_from(flags).expect("Request should have valid flags")
      }
  }

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v3:
- Use `Self` instead of `$flags` in the `TryFrom` implementation (Miguel).
- Drop the hand-written `TryFrom<u32> for VmMapFlags` in
`drivers/gpu/drm/tyr`, which is equivalent to, and now conflicts with, the
implementation generated by the macro. Without this, tyr fails to build
with E0119.
- Expand the commit message of patch 2 with a before-and-after
  comparison of the `gen_disk` call site, the alternatives considered
  (`as` cast, explicit `From`, container temporary), and the trade-off
  of offering operators on the underlying type (Miguel, Gary).
- Use `pin_init::zeroed()` instead of `core::mem::zeroed()` in the
  patch 2 commit message example (Miguel).
- Rebase on v7.4-rc1 and on v3 of the `bits()` prerequisite patch.
- Link to v2: https://msgid.link/20260605-impl-flags-additions-v2-0-0d27242f460b@kernel.org

Changes in v2:
- Use an `as` cast instead of `core::mem::transmute` in
  `From<$flag> for $ty` (Miguel).
- Replace the `match` in `TryFrom::try_from` with an early-return
  `if` (Miguel)
- Construct the result directly instead of via `transmute` in `try_from`
  (Miguel).
- Use `Self` in the new conversion impls (Miguel).
- Mark `TryFrom::try_from` `#[inline]`.
- Extend the rustdoc example to demonstrate both new conversion
  impls (Miguel).
- Link to v1: https://msgid.link/20260215-impl-flags-additions-v1-0-6538c8fb841c@kernel.org

---
Andreas Hindborg (2):
      rust: impl_flags: add conversion functions
      rust: impl_flags: add bitwise operations with the underlying type

 drivers/gpu/drm/tyr/vm.rs | 13 -------------
 rust/kernel/impl_flags.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 13 deletions(-)
---
base-commit: bc71431221f5ba8bb33918399552636e059c2c45
change-id: 20260215-impl-flags-additions-0340ffcba5b9
prerequisite-change-id: 20260212-impl-flags-inner-c61974b27b18:v3
prerequisite-patch-id: 4f477e1886497d4ba3898867e2765c7324b62ba4

Best regards,
--  
Andreas Hindborg <a.hindborg@kernel.org>

[PATCH v3 1/2] rust: impl_flags: add conversion functions

[Andreas Hindborg]

Add two conversion functions to the `impl_flags!` macro:

 - A `From<_>` implementation to convert from the flag value enum to
   underlying type.

 - A `TryFrom<_>` implementation to convert from the underlying
   representation to flag container type.

Drop the hand-written `TryFrom<u32> for VmMapFlags` in tyr, which now
conflicts with the implementation generated by `impl_flags!`. The two are
equivalent.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/gpu/drm/tyr/vm.rs | 13 -------------
 rust/kernel/impl_flags.rs | 31 +++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/tyr/vm.rs b/drivers/gpu/drm/tyr/vm.rs
index 1dd11b2b1253..0d6e841a31ed 100644
--- a/drivers/gpu/drm/tyr/vm.rs
+++ b/drivers/gpu/drm/tyr/vm.rs
@@ -141,19 +141,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     }
 }
 
-impl TryFrom<u32> for VmMapFlags {
-    type Error = Error;
-
-    fn try_from(value: u32) -> Result<Self, Self::Error> {
-        let valid = VmFlag::Readonly as u32 | VmFlag::Noexec as u32 | VmFlag::Uncached as u32;
-
-        if value & !valid != 0 {
-            return Err(EINVAL);
-        }
-        Ok(Self(value))
-    }
-}
-
 /// Arguments for a virtual memory map operation.
 struct VmMapArgs<'drm> {
     /// Access permissions and caching behavior for the mapping.
diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
index 6051487f6e28..052759b8038d 100644
--- a/rust/kernel/impl_flags.rs
+++ b/rust/kernel/impl_flags.rs
@@ -73,6 +73,17 @@
 /// // useful for passing the value to a C API.
 /// let raw: u32 = read_only.bits();
 /// assert_eq!(raw, Permission::Read as u32);
+///
+/// // Convert a single flag value to its underlying integer representation.
+/// let raw_read: u32 = Permission::Read.into();
+/// assert_eq!(raw_read, Permission::Read as u32);
+///
+/// // Construct a `Permissions` value from a raw integer. `TryFrom` rejects
+/// // integers whose set bits do not all correspond to declared flags.
+/// let parsed = Permissions::try_from(raw_read | Permission::Write as u32).unwrap();
+/// assert!(parsed.contains(Permission::Read));
+/// assert!(parsed.contains(Permission::Write));
+/// assert!(Permissions::try_from(1 << 7).is_err());
 /// ```
 #[macro_export]
 macro_rules! impl_flags {
@@ -115,6 +126,26 @@ fn from(value: $flags) -> Self {
             }
         }
 
+        impl ::core::convert::From<$flag> for $ty {
+            #[inline]
+            fn from(value: $flag) -> Self {
+                value as Self
+            }
+        }
+
+        impl ::core::convert::TryFrom<$ty> for $flags {
+            type Error = ::kernel::error::Error;
+
+            #[inline]
+            fn try_from(value: $ty) -> Result<Self, Self::Error> {
+                if value & !Self::all_bits() != 0 {
+                    return Err(::kernel::error::code::EINVAL);
+                }
+
+                Ok(Self(value))
+            }
+        }
+
         impl ::core::ops::BitOr for $flags {
             type Output = Self;
             #[inline]

-- 
2.54.0

[PATCH v3 2/2] rust: impl_flags: add bitwise operations with the underlying type

[Andreas Hindborg]

Add bitwise or operations between the flag value enum and the underlying
type, so a flag value can be combined directly into an integer field of a C
struct.

Rust abstractions that configure a C struct assign to a raw integer field,
often conditionally and from more than one place. The upcoming
`block::mq::gen_disk` conversion declares a `Feature` enum with
`impl_flags!` and folds individual flags into `queue_limits::features`.
With only the `From` implementation from the previous patch, that reads:

    let mut lim: bindings::queue_limits = pin_init::zeroed();

    if self.rotational {
        lim.features = Feature::Rotational.into();
    }

    #[cfg(CONFIG_BLK_DEV_ZONED)]
    if self.zoned {
        lim.features |= u32::from(Feature::Zoned);
    }

    if self.write_cache {
        lim.features |= u32::from(Feature::WriteCache);
    }

With this patch:

    let mut lim: bindings::queue_limits = pin_init::zeroed();

    if self.rotational {
        lim.features = Feature::Rotational.into();
    }

    #[cfg(CONFIG_BLK_DEV_ZONED)]
    if self.zoned {
        lim.features |= Feature::Zoned;
    }

    if self.write_cache {
        lim.features |= Feature::WriteCache;
    }

Note the asymmetry of the first assignment to the rest. The plain
assignment can be written `Feature::Rotational.into()`, but the compound
assignments cannot use `.into()`: `u32` implements both `BitOrAssign<u32>`
and `BitOrAssign<&u32>`, so the target type of `.into()` is ambiguous and
inference fails with E0283. Without these implementations, every compound
assignment has to name the underlying type explicitly.

An `as` cast is one way around that, but we try to avoid casts, and unlike
`u32::from` it would keep compiling silently if the field type were
widened.

The other option is a container temporary: accumulate into a `Features`
value using the operators the macro already generates, then assign
`features.bits()` to the field at the end. This requires a new temporary
per flag field we want to assign and clutters the code.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/impl_flags.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
index 052759b8038d..53f6ba5d5952 100644
--- a/rust/kernel/impl_flags.rs
+++ b/rust/kernel/impl_flags.rs
@@ -276,6 +276,22 @@ fn not(self) -> Self::Output {
             }
         }
 
+        impl ::core::ops::BitOr<$flag> for $ty {
+            type Output = Self;
+
+            #[inline]
+            fn bitor(self, rhs: $flag) -> Self::Output {
+                self | <$flag as Into<Self>>::into(rhs)
+            }
+        }
+
+        impl ::core::ops::BitOrAssign<$flag> for $ty {
+            #[inline]
+            fn bitor_assign(&mut self, rhs: $flag) {
+                *self = *self | <$flag as Into<Self>>::into(rhs)
+            }
+        }
+
         impl $flags {
             /// Returns an empty instance where no flags are set.
             #[inline]

-- 
2.54.0