mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] rust: impl_flags: add convenience functions
@ 2026-11-05 15:42 Andreas Hindborg
  2026-11-05 15:42 ` [PATCH v3 1/2] rust: impl_flags: add conversion functions Andreas Hindborg
  2026-11-05 15:42 ` [PATCH v3 2/2] rust: impl_flags: add bitwise operations with the underlying type Andreas Hindborg
  0 siblings, 2 replies; 4+ messages in thread
From: Andreas Hindborg @ 2026-11-05 15:42 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, David Airlie, Simona Vetter, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: rust-for-linux, linux-kernel, dri-devel, 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>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] rust: impl_flags: add conversion functions
  2026-11-05 15:42 [PATCH v3 0/2] rust: impl_flags: add convenience functions Andreas Hindborg
@ 2026-11-05 15:42 ` Andreas Hindborg
  2026-11-09 15:26   ` Gary Guo
  2026-11-05 15:42 ` [PATCH v3 2/2] rust: impl_flags: add bitwise operations with the underlying type Andreas Hindborg
  1 sibling, 1 reply; 4+ messages in thread
From: Andreas Hindborg @ 2026-11-05 15:42 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, David Airlie, Simona Vetter, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: rust-for-linux, linux-kernel, dri-devel, 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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] rust: impl_flags: add bitwise operations with the underlying type
  2026-11-05 15:42 [PATCH v3 0/2] rust: impl_flags: add convenience functions Andreas Hindborg
  2026-11-05 15:42 ` [PATCH v3 1/2] rust: impl_flags: add conversion functions Andreas Hindborg
@ 2026-11-05 15:42 ` Andreas Hindborg
  1 sibling, 0 replies; 4+ messages in thread
From: Andreas Hindborg @ 2026-11-05 15:42 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Daniel Almeida, David Airlie, Simona Vetter, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: rust-for-linux, linux-kernel, dri-devel, 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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] rust: impl_flags: add conversion functions
  2026-11-05 15:42 ` [PATCH v3 1/2] rust: impl_flags: add conversion functions Andreas Hindborg
@ 2026-11-09 15:26   ` Gary Guo
  0 siblings, 0 replies; 4+ messages in thread
From: Gary Guo @ 2026-11-09 15:26 UTC (permalink / raw)
  To: Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, David Airlie, Simona Vetter,
	Tamir Duberstein, Alexandre Courbot, Onur Özkan
  Cc: rust-for-linux, linux-kernel, dri-devel

On Thu Nov 5, 2026 at 3:42 PM GMT, Andreas Hindborg wrote:
> 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);

IMO this should be a new type, perhaps something like
`kernel::bitflags::UnknownFlags`.

Best,
Gary

> +                }
> +
> +                Ok(Self(value))
> +            }
> +        }
> +
>          impl ::core::ops::BitOr for $flags {
>              type Output = Self;
>              #[inline]



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-09 15:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-05 15:42 [PATCH v3 0/2] rust: impl_flags: add convenience functions Andreas Hindborg
2026-11-05 15:42 ` [PATCH v3 1/2] rust: impl_flags: add conversion functions Andreas Hindborg
2026-11-09 15:26   ` Gary Guo
2026-11-05 15:42 ` [PATCH v3 2/2] rust: impl_flags: add bitwise operations with the underlying type Andreas Hindborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®