rust/kernel/impl_flags.rs | 260 ++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 2 + rust/kernel/prelude.rs | 1 + 3 files changed, 263 insertions(+)
We have seen a proliferation of mod_whatever::foo::Flags
being defined with essentially the same implementation
for BitAnd, BitOr, contains and etc.
This macro aims to bring a solution for this,
allowing to generate these methods for user-defined structs.
With some use cases in KMS and upcoming GPU drivers.
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
Suggested-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
---
Changes in v8:
- Doc: Fix some terms and improving the macro documentation.
- Link to v7: https://lore.kernel.org/r/20260101-feat-add-bitmask-macro-v7-1-e58a2ef69f0b@gmail.com
Changes in v7:
- Mask invalid bits: add all_bits mask to preserve only valid bits in Not and BitXor ops.
- Link to v6: https://lore.kernel.org/r/20251205-feat-add-bitmask-macro-v6-1-31fbbf14b15a@gmail.com
Changes in v6:
- New methods: add new methods contains_any and contains_all.
- Link to v5: https://lore.kernel.org/r/20251109-feat-add-bitmask-macro-v5-1-9d911b207ef4@gmail.com
Changes in v5:
- Docs: improve macro documentation.
- Link to v4: https://lore.kernel.org/r/20251026-feat-add-bitmask-macro-v4-1-e1b59b4762bc@gmail.com
Changes in v4:
- Use enum: changed flag type from struct to enum.
- Minor fix: airect casting (flag as $ty) instead of field access (.0).
- Link to v3: https://lore.kernel.org/r/20250411-feat-add-bitmask-macro-v3-1-187bd3e4a03e@gmail.com
Changes in v3:
- New Feat: added support to declare flags inside macro use.
- Minor fixes: used absolute paths to refer to items, fix rustdoc and fix example cases.
- Link to v2: https://lore.kernel.org/r/20250325-feat-add-bitmask-macro-v2-1-d3beabdad90f@gmail.com
Changes in v2:
- rename: change macro and file name to impl_flags.
- negation sign: change char for negation to `!`.
- transpose docs: add support to transpose user provided docs.
- visibility: add support to use user defined visibility.
- operations: add new operations for flag,
to support use between bit and bitmap, eg: flag & flags.
- code style: small fixes to remove warnings.
- Link to v1: https://lore.kernel.org/r/20250304-feat-add-bitmask-macro-v1-1-1c2d2bcb476b@gmail.com
---
rust/kernel/impl_flags.rs | 260 ++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
rust/kernel/prelude.rs | 1 +
3 files changed, 263 insertions(+)
diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
new file mode 100644
index 0000000000000000000000000000000000000000..0c50b8404fa795926b4c12c21350b611a2a3d149
--- /dev/null
+++ b/rust/kernel/impl_flags.rs
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Common helper for declaring bitflag and bitmask types.
+///
+/// This macro takes as input:
+/// - A struct declaration representing a bitmask type
+/// (e.g., `pub struct Permissions(u32)`).
+/// - An enumeration declaration representing individual bit flags
+/// (e.g., `pub enum Permission { ... }`).
+///
+/// And generates:
+/// - The struct and enum types with appropriate `#[repr]` attributes.
+/// - Implementations of common bitflag operators
+/// ([`::core::ops::BitOr`], [`::core::ops::BitAnd`], etc.).
+/// - Utility methods such as `.contains()` to check flags.
+///
+/// # Examples
+///
+/// Defining and using [`impl_flags!`]:
+///
+/// ```
+/// use kernel::impl_flags;
+///
+/// impl_flags!(
+/// /// Represents multiple permissions.
+/// #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
+/// pub struct Permissions(u32);
+///
+/// /// Represents a single permission.
+/// #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+/// pub enum Permission {
+/// Read = 1 << 0,
+/// Write = 1 << 1,
+/// Execute = 1 << 2,
+/// }
+/// );
+///
+/// // Combine multiple permissions using the bitwise OR (`|`) operator.
+/// let mut read_write: Permissions = Permission::Read | Permission::Write;
+/// assert!(read_write.contains(Permission::Read));
+/// assert!(read_write.contains(Permission::Write));
+/// assert!(!read_write.contains(Permission::Execute));
+/// assert!(read_write.contains_any(Permission::Read | Permission::Execute));
+/// assert!(read_write.contains_all(Permission::Read | Permission::Write));
+///
+/// // Using the bitwise OR assignment (`|=`) operator.
+/// read_write |= Permission::Execute;
+/// assert!(read_write.contains(Permission::Execute));
+///
+/// // Masking a permission with the bitwise AND (`&`) operator.
+/// let read_only: Permissions = read_write & Permission::Read;
+/// assert!(read_only.contains(Permission::Read));
+/// assert!(!read_only.contains(Permission::Write));
+///
+/// // Toggling permissions with the bitwise XOR (`^`) operator.
+/// let toggled: Permissions = read_only ^ Permission::Read;
+/// assert!(!toggled.contains(Permission::Read));
+///
+/// // Inverting permissions with the bitwise NOT (`!`) operator.
+/// let negated = !read_only;
+/// assert!(negated.contains(Permission::Write));
+/// assert!(!negated.contains(Permission::Read));
+/// ```
+#[macro_export]
+macro_rules! impl_flags {
+ (
+ $(#[$outer_flags:meta])*
+ $vis_flags:vis struct $flags:ident($ty:ty);
+
+ $(#[$outer_flag:meta])*
+ $vis_flag:vis enum $flag:ident {
+ $(
+ $(#[$inner_flag:meta])*
+ $name:ident = $value:expr
+ ),+ $( , )?
+ }
+ ) => {
+ $(#[$outer_flags])*
+ #[repr(transparent)]
+ $vis_flags struct $flags($ty);
+
+ $(#[$outer_flag])*
+ #[repr($ty)]
+ $vis_flag enum $flag {
+ $(
+ $(#[$inner_flag])*
+ $name = $value
+ ),+
+ }
+
+ impl ::core::convert::From<$flag> for $flags {
+ #[inline]
+ fn from(value: $flag) -> Self {
+ Self(value as $ty)
+ }
+ }
+
+ impl ::core::convert::From<$flags> for $ty {
+ #[inline]
+ fn from(value: $flags) -> Self {
+ value.0
+ }
+ }
+
+ impl ::core::ops::BitOr for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitor(self, rhs: Self) -> Self::Output {
+ Self(self.0 | rhs.0)
+ }
+ }
+
+ impl ::core::ops::BitOrAssign for $flags {
+ #[inline]
+ fn bitor_assign(&mut self, rhs: Self) {
+ *self = *self | rhs;
+ }
+ }
+
+ impl ::core::ops::BitAnd for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitand(self, rhs: Self) -> Self::Output {
+ Self(self.0 & rhs.0)
+ }
+ }
+
+ impl ::core::ops::BitAndAssign for $flags {
+ #[inline]
+ fn bitand_assign(&mut self, rhs: Self) {
+ *self = *self & rhs;
+ }
+ }
+
+ impl ::core::ops::BitOr<$flag> for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitor(self, rhs: $flag) -> Self::Output {
+ self | Self::from(rhs)
+ }
+ }
+
+ impl ::core::ops::BitOrAssign<$flag> for $flags {
+ #[inline]
+ fn bitor_assign(&mut self, rhs: $flag) {
+ *self = *self | rhs;
+ }
+ }
+
+ impl ::core::ops::BitAnd<$flag> for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitand(self, rhs: $flag) -> Self::Output {
+ self & Self::from(rhs)
+ }
+ }
+
+ impl ::core::ops::BitAndAssign<$flag> for $flags {
+ #[inline]
+ fn bitand_assign(&mut self, rhs: $flag) {
+ *self = *self & rhs;
+ }
+ }
+
+ impl ::core::ops::BitXor for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitxor(self, rhs: Self) -> Self::Output {
+ Self((self.0 ^ rhs.0) & Self::all_bits())
+ }
+ }
+
+ impl ::core::ops::BitXorAssign for $flags {
+ #[inline]
+ fn bitxor_assign(&mut self, rhs: Self) {
+ *self = *self ^ rhs;
+ }
+ }
+
+ impl ::core::ops::Not for $flags {
+ type Output = Self;
+ #[inline]
+ fn not(self) -> Self::Output {
+ Self((!self.0) & Self::all_bits())
+ }
+ }
+
+ impl ::core::ops::BitOr for $flag {
+ type Output = $flags;
+ #[inline]
+ fn bitor(self, rhs: Self) -> Self::Output {
+ $flags(self as $ty | rhs as $ty)
+ }
+ }
+
+ impl ::core::ops::BitAnd for $flag {
+ type Output = $flags;
+ #[inline]
+ fn bitand(self, rhs: Self) -> Self::Output {
+ $flags(self as $ty & rhs as $ty)
+ }
+ }
+
+ impl ::core::ops::BitXor for $flag {
+ type Output = $flags;
+ #[inline]
+ fn bitxor(self, rhs: Self) -> Self::Output {
+ $flags((self as $ty ^ rhs as $ty) & $flags::all_bits())
+ }
+ }
+
+ impl ::core::ops::Not for $flag {
+ type Output = $flags;
+ #[inline]
+ fn not(self) -> Self::Output {
+ $flags((!(self as $ty)) & $flags::all_bits())
+ }
+ }
+
+ impl ::core::ops::BitXor<$flag> for $flags {
+ type Output = Self;
+ #[inline]
+ fn bitxor(self, rhs: $flag) -> Self::Output {
+ self ^ Self::from(rhs)
+ }
+ }
+
+ impl $flags {
+ /// Returns an empty instance of `type` where no flags are set.
+ #[inline]
+ pub const fn empty() -> Self {
+ Self(0)
+ }
+
+ /// Returns a mask containing all valid flag bits.
+ #[inline]
+ pub const fn all_bits() -> $ty {
+ 0 $( | $value )+
+ }
+
+ /// Checks if a specific flag is set.
+ #[inline]
+ pub fn contains(self, flag: $flag) -> bool {
+ (self.0 & flag as $ty) == flag as $ty
+ }
+
+ /// Checks if at least one of the provided flags is set.
+ #[inline]
+ pub fn contains_any(self, flags: $flags) -> bool {
+ (self.0 & flags.0) != 0
+ }
+
+ /// Checks if all of the provided flags are set.
+ #[inline]
+ pub fn contains_all(self, flags: $flags) -> bool {
+ (self.0 & flags.0) == flags.0
+ }
+ }
+ };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 28007be98fbad0e875d7e5345e164e2af2c5da32..57b789315deda3d2fa8961af79cad4462fff7fa8 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -67,6 +67,8 @@
#[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
pub mod firmware;
pub mod fs;
+#[doc(hidden)]
+pub mod impl_flags;
pub mod init;
pub mod io;
pub mod ioctl;
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index baa774a351ceeb995a2a647f78a27b408d9f3834..085f23502544c80c2202eacd18ba190186a74f30 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -27,6 +27,7 @@
#[doc(no_inline)]
pub use super::dbg;
pub use super::fmt;
+pub use super::impl_flags;
pub use super::{dev_alert, dev_crit, dev_dbg, dev_emerg, dev_err, dev_info, dev_notice, dev_warn};
pub use super::{pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
---
base-commit: edc5e6e019c99b529b3d1f2801d5cce9924ae79b
change-id: 20250304-feat-add-bitmask-macro-6424b1c317e2
Best regards,
--
Filipe Xavier <felipeaggger@gmail.com>"Filipe Xavier" <felipeaggger@gmail.com> writes:
> We have seen a proliferation of mod_whatever::foo::Flags
> being defined with essentially the same implementation
> for BitAnd, BitOr, contains and etc.
>
> This macro aims to bring a solution for this,
> allowing to generate these methods for user-defined structs.
> With some use cases in KMS and upcoming GPU drivers.
>
> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Suggested-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
I think it would be useful to add:
impl Flags {
unsafe from_raw(value: Repr) -> {
...
}
}
impl TryFrom<Repr> for Flag {
// Succeed if `value` is a valid `Flag` enum bit pattern.
}
impl TryFrom<Repr> for Flags {
// Succeed if `value` is the logical OR of valid enum bit patterns.
}
Best regards,
Andreas Hindborg
> On 15 Jan 2026, at 09:09, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>
>> We have seen a proliferation of mod_whatever::foo::Flags
>> being defined with essentially the same implementation
>> for BitAnd, BitOr, contains and etc.
>>
>> This macro aims to bring a solution for this,
>> allowing to generate these methods for user-defined structs.
>> With some use cases in KMS and upcoming GPU drivers.
>>
>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Suggested-by: Lyude Paul <lyude@redhat.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>
> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>
> I think it would be useful to add:
>
> impl Flags {
> unsafe from_raw(value: Repr) -> {
> ...
> }
> }
>
> impl TryFrom<Repr> for Flag {
> // Succeed if `value` is a valid `Flag` enum bit pattern.
> }
>
> impl TryFrom<Repr> for Flags {
> // Succeed if `value` is the logical OR of valid enum bit patterns.
> }
>
>
>
> Best regards,
> Andreas Hindborg
Ah, true. There is a use-case for that in Tyr as well where we ingest the raw
values from userspace and need to validate and build valid flags from. This may
be somewhat common for others too.
Andreas Hindborg <a.hindborg@kernel.org> writes:
> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>
>> We have seen a proliferation of mod_whatever::foo::Flags
>> being defined with essentially the same implementation
>> for BitAnd, BitOr, contains and etc.
>>
>> This macro aims to bring a solution for this,
>> allowing to generate these methods for user-defined structs.
>> With some use cases in KMS and upcoming GPU drivers.
>>
>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Suggested-by: Lyude Paul <lyude@redhat.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>
> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>
> I think it would be useful to add:
>
> impl Flags {
> unsafe from_raw(value: Repr) -> {
> ...
> }
> }
>
> impl TryFrom<Repr> for Flag {
> // Succeed if `value` is a valid `Flag` enum bit pattern.
> }
>
> impl TryFrom<Repr> for Flags {
> // Succeed if `value` is the logical OR of valid enum bit patterns.
> }
Also bitwise operations on the underlying type such as
impl BitOrAssign<Flag> for u32 {
...
}
Not sure if that is possible with the orphan rule, but it would be nice.
I was trying to
lim.features |= request::Feature::Rotational;
where `lim: bindings::queue_limits`.
Best regards,
Andreas Hindborg
On Thu Jan 15, 2026 at 12:14 PM GMT, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>>
>>> We have seen a proliferation of mod_whatever::foo::Flags
>>> being defined with essentially the same implementation
>>> for BitAnd, BitOr, contains and etc.
>>>
>>> This macro aims to bring a solution for this,
>>> allowing to generate these methods for user-defined structs.
>>> With some use cases in KMS and upcoming GPU drivers.
>>>
>>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Suggested-by: Lyude Paul <lyude@redhat.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>>
>> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>>
>> I think it would be useful to add:
>>
>> impl Flags {
>> unsafe from_raw(value: Repr) -> {
>> ...
>> }
>> }
>>
>> impl TryFrom<Repr> for Flag {
>> // Succeed if `value` is a valid `Flag` enum bit pattern.
>> }
>>
>> impl TryFrom<Repr> for Flags {
>> // Succeed if `value` is the logical OR of valid enum bit patterns.
>> }
>
> Also bitwise operations on the underlying type such as
>
> impl BitOrAssign<Flag> for u32 {
> ...
> }
>
> Not sure if that is possible with the orphan rule, but it would be nice.
> I was trying to
>
> lim.features |= request::Feature::Rotational;
This is allowed by orphan rules, in a similar way that you can implement
`From<Local> for Foreign`.
However I don't think this is desirable feature? Why can't a conversion be used
if you're using raw reprs?
I think we should discourage the use of raw integers when bitflags can be used,
adding that (or even `TryFrom`) can be a negative incentive.
Best,
Gary
>
> where `lim: bindings::queue_limits`.
>
>
> Best regards,
> Andreas Hindborg
> On 15 Jan 2026, at 10:19, Gary Guo <gary@garyguo.net> wrote:
>
> On Thu Jan 15, 2026 at 12:14 PM GMT, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>>> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>>>
>>>> We have seen a proliferation of mod_whatever::foo::Flags
>>>> being defined with essentially the same implementation
>>>> for BitAnd, BitOr, contains and etc.
>>>>
>>>> This macro aims to bring a solution for this,
>>>> allowing to generate these methods for user-defined structs.
>>>> With some use cases in KMS and upcoming GPU drivers.
>>>>
>>>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>>>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Suggested-by: Lyude Paul <lyude@redhat.com>
>>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>>>
>>> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>
>>>
>>> I think it would be useful to add:
>>>
>>> impl Flags {
>>> unsafe from_raw(value: Repr) -> {
>>> ...
>>> }
>>> }
>>>
>>> impl TryFrom<Repr> for Flag {
>>> // Succeed if `value` is a valid `Flag` enum bit pattern.
>>> }
>>>
>>> impl TryFrom<Repr> for Flags {
>>> // Succeed if `value` is the logical OR of valid enum bit patterns.
>>> }
>>
>> Also bitwise operations on the underlying type such as
>>
>> impl BitOrAssign<Flag> for u32 {
>> ...
>> }
>>
>> Not sure if that is possible with the orphan rule, but it would be nice.
>> I was trying to
>>
>> lim.features |= request::Feature::Rotational;
>
> This is allowed by orphan rules, in a similar way that you can implement
> `From<Local> for Foreign`.
>
> However I don't think this is desirable feature? Why can't a conversion be used
> if you're using raw reprs?
>
> I think we should discourage the use of raw integers when bitflags can be used,
> adding that (or even `TryFrom`) can be a negative incentive.
The flags may be in your uapi or they may otherwise come from C. In that case
you’re stuck with a TryFrom or a manual conversion.
I think a generated TryFrom shouldn't hurt. Just my 0.02c.
>
> Best,
> Gary
>
>>
>> where `lim: bindings::queue_limits`.
>>
>>
>> Best regards,
>> Andreas Hindborg
On Thu, Jan 15, 2026 at 2:45 PM Daniel Almeida <daniel.almeida@collabora.com> wrote: > > The flags may be in your uapi or they may otherwise come from C. In that case > you’re stuck with a TryFrom or a manual conversion. > > I think a generated TryFrom shouldn't hurt. Just my 0.02c. I would say let's land this patch as-is (with the doc bit I mentioned) and then we can discuss additions on top independently. Cheers, Miguel
> On 15 Jan 2026, at 16:03, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Thu, Jan 15, 2026 at 2:45 PM Daniel Almeida > <daniel.almeida@collabora.com> wrote: >> >> The flags may be in your uapi or they may otherwise come from C. In that case >> you’re stuck with a TryFrom or a manual conversion. >> >> I think a generated TryFrom shouldn't hurt. Just my 0.02c. > > I would say let's land this patch as-is (with the doc bit I mentioned) > and then we can discuss additions on top independently. Sure, that is also fine. Thanks for all the work, Filipe :) — Daniel
"Daniel Almeida" <daniel.almeida@collabora.com> writes: >> On 15 Jan 2026, at 16:03, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: >> >> On Thu, Jan 15, 2026 at 2:45 PM Daniel Almeida >> <daniel.almeida@collabora.com> wrote: >>> >>> The flags may be in your uapi or they may otherwise come from C. In that case >>> you’re stuck with a TryFrom or a manual conversion. >>> >>> I think a generated TryFrom shouldn't hurt. Just my 0.02c. >> >> I would say let's land this patch as-is (with the doc bit I mentioned) >> and then we can discuss additions on top independently. > > Sure, that is also fine. > > Thanks for all the work, Filipe :) Absolutely, I did not intend to stall this series. Just suggestions for future work. Best regards, Andreas Hindborg
"Gary Guo" <gary@garyguo.net> writes:
> On Thu Jan 15, 2026 at 12:14 PM GMT, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>>> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>>>
>>>> We have seen a proliferation of mod_whatever::foo::Flags
>>>> being defined with essentially the same implementation
>>>> for BitAnd, BitOr, contains and etc.
>>>>
>>>> This macro aims to bring a solution for this,
>>>> allowing to generate these methods for user-defined structs.
>>>> With some use cases in KMS and upcoming GPU drivers.
>>>>
>>>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>>>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Suggested-by: Lyude Paul <lyude@redhat.com>
>>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>>>
>>> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>>
>>>
>>> I think it would be useful to add:
>>>
>>> impl Flags {
>>> unsafe from_raw(value: Repr) -> {
>>> ...
>>> }
>>> }
>>>
>>> impl TryFrom<Repr> for Flag {
>>> // Succeed if `value` is a valid `Flag` enum bit pattern.
>>> }
>>>
>>> impl TryFrom<Repr> for Flags {
>>> // Succeed if `value` is the logical OR of valid enum bit patterns.
>>> }
>>
>> Also bitwise operations on the underlying type such as
>>
>> impl BitOrAssign<Flag> for u32 {
>> ...
>> }
>>
>> Not sure if that is possible with the orphan rule, but it would be nice.
>> I was trying to
>>
>> lim.features |= request::Feature::Rotational;
>
> This is allowed by orphan rules, in a similar way that you can implement
> `From<Local> for Foreign`.
>
> However I don't think this is desirable feature? Why can't a conversion be used
> if you're using raw reprs?
>
> I think we should discourage the use of raw integers when bitflags can be used,
> adding that (or even `TryFrom`) can be a negative incentive.
In this case I have a C struct with a flag field. It would not be
ergonomic to read out the field, convert to `Flag` type, do the
operation, and store back again.
This assignment is in abstraction code. I could use the raw bindings
constant for the operation, but I would rather use the Rust type.
I do not think there is any risk by allowing operators between flags and
integers. What situation do you imagine where there would be a problem?
Best regards
Andreas Hindborg
Andreas Hindborg <a.hindborg@kernel.org> writes:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Filipe Xavier" <felipeaggger@gmail.com> writes:
>>
>>> We have seen a proliferation of mod_whatever::foo::Flags
>>> being defined with essentially the same implementation
>>> for BitAnd, BitOr, contains and etc.
>>>
>>> This macro aims to bring a solution for this,
>>> allowing to generate these methods for user-defined structs.
>>> With some use cases in KMS and upcoming GPU drivers.
>>>
>>> Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/We.20really.20need.20a.20common.20.60Flags.60.20type
>>> Suggested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Suggested-by: Lyude Paul <lyude@redhat.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Lyude Paul <lyude@redhat.com>
>>> Signed-off-by: Filipe Xavier <felipeaggger@gmail.com>
>>
>> Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>>
>> I think it would be useful to add:
>>
>> impl Flags {
>> unsafe from_raw(value: Repr) -> {
>> ...
>> }
>> }
>>
>> impl TryFrom<Repr> for Flag {
>> // Succeed if `value` is a valid `Flag` enum bit pattern.
>> }
>>
>> impl TryFrom<Repr> for Flags {
>> // Succeed if `value` is the logical OR of valid enum bit patterns.
>> }
>
> Also bitwise operations on the underlying type such as
>
> impl BitOrAssign<Flag> for u32 {
> ...
> }
>
> Not sure if that is possible with the orphan rule, but it would be nice.
> I was trying to
>
> lim.features |= request::Feature::Rotational;
>
> where `lim: bindings::queue_limits`.
Sorry for the spam. This works for me:
diff --git a/rust/kernel/impl_flags.rs b/rust/kernel/impl_flags.rs
index 0c50b8404fa79..e1143e6381d47 100644
--- a/rust/kernel/impl_flags.rs
+++ b/rust/kernel/impl_flags.rs
@@ -102,6 +102,25 @@ fn from(value: $flags) -> Self {
}
}
+ impl ::core::convert::From<$flag> for $ty {
+ #[inline]
+ fn from(value: $flag) -> Self {
+ value as _
+ }
+ }
+
+ impl ::core::convert::TryFrom<$ty> for $flags {
+ type Error = ::kernel::error::Error;
+
+ fn try_from(value: $ty) -> Result<Self, Self::Error> {
+ match value & !$flags::all_bits() == 0 {
+ // SAFETY: We checked above that `value` is a valid bit pattern for `$flags`.
+ true => Ok(unsafe {::core::mem::transmute(value)}),
+ false => Err(::kernel::error::code::EINVAL),
+ }
+ }
+ }
+
impl ::core::ops::BitOr for $flags {
type Output = Self;
#[inline]
@@ -225,6 +244,22 @@ fn bitxor(self, rhs: $flag) -> 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 of `type` where no flags are set.
#[inline]
Best regards,
Andreas Hindborg
On Sun, Jan 11, 2026 at 2:33 PM Filipe Xavier <felipeaggger@gmail.com> wrote: > > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Common helper for declaring bitflag and bitmask types. > +/// > +/// This macro takes as input: So I didn't mean to convert the first line to `//!`, but to add documentation to the module between the SPDX line and the documentation for the item. Please see how other files do it, e.g. `rust/kernel/slice.rs` and `rust/kernel/irq.rs` are similar cases (no `use` blocks between `//!` and the first item). But please wait a few days in case there is any other feedback -- hopefully we are getting finally there. (By the way, in general, it is good to reply to the feedback messages you get, to confirm or not whether you applied a particular suggestion). Thanks for updating the docs and being persistent! Cheers, Miguel
On 1/11/26 5:06 PM, Miguel Ojeda wrote: > On Sun, Jan 11, 2026 at 2:33 PM Filipe Xavier <felipeaggger@gmail.com> wrote: >> +// SPDX-License-Identifier: GPL-2.0 >> + >> +//! Common helper for declaring bitflag and bitmask types. >> +/// >> +/// This macro takes as input: > So I didn't mean to convert the first line to `//!`, but to add > documentation to the module between the SPDX line and the > documentation for the item. Oh, sorry, I misunderstood. I thought it might be redundant; I'll adjust it in the next version. > Please see how other files do it, e.g. `rust/kernel/slice.rs` and > `rust/kernel/irq.rs` are similar cases (no `use` blocks between `//!` > and the first item). > > But please wait a few days in case there is any other feedback -- > hopefully we are getting finally there. > > (By the way, in general, it is good to reply to the feedback messages > you get, to confirm or not whether you applied a particular > suggestion). > > Thanks for updating the docs and being persistent! I am the one who is grateful for the feedback and patience. Xavier
© 2016 - 2026 Red Hat, Inc.