[PATCH v2] rust: kernel: add support for bits/genmask macros

[PATCH v2] rust: kernel: add support for bits/genmask macros

[Daniel Almeida]

These macros are converted from their kernel equivalent.

---
- Added ticks around `BIT`, and `h >=l` (Thanks, Benno).
- Decided to keep the arguments as `expr`, as I see no issues with that
- Added a proper example, with an assert_eq!() (Thanks, Benno)
- Fixed the condition h <= l, which should be h >= l.
- Checked that the assert for the condition above is described in the
  docs.

Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
 rust/kernel/bits.rs | 36 ++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs  |  1 +
 2 files changed, 37 insertions(+)

diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs
new file mode 100644
index 0000000000000000000000000000000000000000..479883984f995f6b44272fa4566a08f1c1e6b143
--- /dev/null
+++ b/rust/kernel/bits.rs
@@ -0,0 +1,36 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Bit manipulation macros.
+//!
+//! C header: [`include/linux/bits.h`](srctree/include/linux/bits.h)
+
+/// Produces a literal where bit `n` is set.
+///
+/// Equivalent to the kernel's `BIT` macro.
+///
+#[macro_export]
+macro_rules! bit {
+    ($n:expr) => {
+        (1 << $n)
+    };
+}
+
+/// Create a contiguous bitmask starting at bit position `l` and ending at
+/// position `h`, where `h >= l`.
+///
+/// # Examples
+/// ```
+///     use kernel::genmask;
+///     let mask = genmask!(39, 21);
+///     assert_eq!(mask, 0x000000ffffe00000);
+/// ```
+///
+#[macro_export]
+macro_rules! genmask {
+    ($h:expr, $l:expr) => {{
+        const _: () = {
+            assert!($h >= $l);
+        };
+        ((!0u64 - (1u64 << $l) + 1) & (!0u64 >> (64 - 1 - $h)))
+    }};
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b5f4b3ce6b48203507f89bcc4b0bf7b076be6247..4f256941ac8fbd1263d5c014a0ce2f5ffb9d1849 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -27,6 +27,7 @@
 extern crate self as kernel;
 
 pub mod alloc;
+pub mod bits;
 #[cfg(CONFIG_BLOCK)]
 pub mod block;
 mod build_assert;

---
base-commit: 91e21479c81dd4e9e22a78d7446f92f6b96a7284
change-id: 20241023-topic-panthor-rs-genmask-fabc573fef43

Best regards,
-- 
Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v2] rust: kernel: add support for bits/genmask macros

[Daniel Almeida]

Sorry, I switched to b4 yesterday and I am still getting used to the workflow with it.


> On 24 Oct 2024, at 11:17, Daniel Almeida <daniel.almeida@collabora.com> wrote:
> 
> These macros are converted from their kernel equivalent.
> 
> ---
> - Added ticks around `BIT`, and `h >=l` (Thanks, Benno).
> - Decided to keep the arguments as `expr`, as I see no issues with that
> - Added a proper example, with an assert_eq!() (Thanks, Benno)
> - Fixed the condition h <= l, which should be h >= l.
> - Checked that the assert for the condition above is described in the
>  docs.
> 
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>


This should read “Changes from v1”

Re: [PATCH v2] rust: kernel: add support for bits/genmask macros

[Miguel Ojeda]

On Thu, Oct 24, 2024 at 5:07 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> Sorry, I switched to b4 yesterday and I am still getting used to the workflow with it.

No worries at all!

> This should read “Changes from v1”

It should also go below the `---` line (i.e. currently the SoB would
be dropped).

Thanks!

Cheers,
Miguel

Re: [PATCH v2] rust: kernel: add support for bits/genmask macros

[kernel test robot]

Hi Daniel,

kernel test robot noticed the following build errors:

[auto build test ERROR on 91e21479c81dd4e9e22a78d7446f92f6b96a7284]

url:    https://github.com/intel-lab-lkp/linux/commits/Daniel-Almeida/rust-kernel-add-support-for-bits-genmask-macros/20241024-222614
base:   91e21479c81dd4e9e22a78d7446f92f6b96a7284
patch link:    https://lore.kernel.org/r/20241024-topic-panthor-rs-genmask-v2-1-85237c1f0cea%40collabora.com
patch subject: [PATCH v2] rust: kernel: add support for bits/genmask macros
config: x86_64-buildonly-randconfig-003-20241025 (https://download.01.org/0day-ci/archive/20241025/202410252126.lJKyvEk1-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241025/202410252126.lJKyvEk1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410252126.lJKyvEk1-lkp@intel.com/

All errors (new ones prefixed by >>):

>> error[E0015]: cannot call non-const fn `kunit_get_current_test` in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error: `core::ptr::mut_ptr::<impl *mut T>::is_null` is not yet stable as a const fn
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = help: add `#![feature(const_ptr_is_null)]` to the crate attributes to enable
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0015]: cannot call non-const fn `Arguments::<'_>::new_v1` in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `format_args` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0015]: cannot call non-const fn `err` in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error: `Arguments::<'a>::new_const` is not yet stable as a const fn
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = help: add `#![feature(const_fmt_arguments_new)]` to the crate attributes to enable
   = note: this error originates in the macro `format_args` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0658]: referencing statics in constants is unstable
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: see issue #119618 <https://github.com/rust-lang/rust/issues/119618> for more information
   = help: add `#![feature(const_refs_to_static)]` to the crate attributes to enable
   = note: this compiler was built on 2024-04-29; consider upgrading it if it is out of date
   = note: `static` and `const` variables can refer to other `const` variables. A `const` variable, however, cannot refer to a `static` variable.
   = help: to fix this, the value can be extracted to a `const` and then used.
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0015]: cannot call non-const fn `__kunit_do_failed_assertion` in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0015]: cannot call non-const fn `__kunit_abort` in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `kernel::kunit_assert` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)
--
>> error[E0015]: cannot call non-const formatting macro in constants
   --> rust/doctests_kernel_generated.rs:317:16
   |
   317 |     let mask = genmask!(39, 21);
   |                ^^^^^^^^^^^^^^^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `format_args` which comes from the expansion of the macro `genmask` (in Nightly builds, run with -Z macro-backtrace for more info)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] rust: kernel: add support for bits/genmask macros

[Alice Ryhl]

On Thu, Oct 24, 2024 at 4:18 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
> These macros are converted from their kernel equivalent.
>
> ---
> - Added ticks around `BIT`, and `h >=l` (Thanks, Benno).
> - Decided to keep the arguments as `expr`, as I see no issues with that
> - Added a proper example, with an assert_eq!() (Thanks, Benno)
> - Fixed the condition h <= l, which should be h >= l.
> - Checked that the assert for the condition above is described in the
>   docs.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>

My main question here is whether this should be a macro_rules! or const fn.

One factor in favor of a macro is that if you need it to work in const
context, then you can't use generics, which means that you must
restrict it so it only works with a single integer type. On the other
hand, genmask! already only works for the u64 type, so that argument
doesn't work for genmask!.

Either way, I'd like to see the reason for your choice in the commit message.

>  rust/kernel/bits.rs | 36 ++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs  |  1 +
>  2 files changed, 37 insertions(+)
>
> diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..479883984f995f6b44272fa4566a08f1c1e6b143
> --- /dev/null
> +++ b/rust/kernel/bits.rs
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Bit manipulation macros.
> +//!
> +//! C header: [`include/linux/bits.h`](srctree/include/linux/bits.h)
> +
> +/// Produces a literal where bit `n` is set.
> +///
> +/// Equivalent to the kernel's `BIT` macro.
> +///
> +#[macro_export]
> +macro_rules! bit {
> +    ($n:expr) => {
> +        (1 << $n)
> +    };
> +}
> +
> +/// Create a contiguous bitmask starting at bit position `l` and ending at
> +/// position `h`, where `h >= l`.
> +///
> +/// # Examples
> +/// ```
> +///     use kernel::genmask;
> +///     let mask = genmask!(39, 21);
> +///     assert_eq!(mask, 0x000000ffffe00000);
> +/// ```
> +///
> +#[macro_export]
> +macro_rules! genmask {
> +    ($h:expr, $l:expr) => {{
> +        const _: () = {
> +            assert!($h >= $l);
> +        };

Based on the error reported by the bot, you probably need to use
`$crate::build_assert!` here instead of the stdlib `assert!` macro. I
don't think the stdlib macro works in const context.

> +        ((!0u64 - (1u64 << $l) + 1) & (!0u64 >> (64 - 1 - $h)))

The implementation looks okay. I tried it:
fn main() {
    println!("{:03b}", genmask!(0,0));
    println!("{:03b}", genmask!(1,0));
    println!("{:03b}", genmask!(2,0));
    println!("{:03b}", genmask!(2,1));
}
prints:
001
011
111
110

Alice