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

["Gary Guo" <gary@garyguo.net>]

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]