On Mon, Dec 09, 2024 at 01:36:57PM +0100, Paolo Bonzini wrote:
> Date: Mon, 9 Dec 2024 13:36:57 +0100
> From: Paolo Bonzini <pbonz...@redhat.com>
> Subject: [PATCH 06/26] rust: add a bit operation module
> X-Mailer: git-send-email 2.47.1
>
> The bindgen supports `static inline` function binding since v0.64.0 as
> an experimental feature (`--wrap-static-fns`), and stabilizes it after
> v0.70.0.
>
> But the oldest version of bindgen supported by QEMU is v0.60.1, so
> there's no way to generate the binding for deposit64() which is `static
> inline` (in include/qemu/bitops.h).
>
> Instead, implement it by hand in Rust and make it available for all
> unsigned types through an IntegerExt trait. Since it only involves bit
> operations, the Rust version of the code is almost identical to the
> original C version, but it applies to more types than just u64.
>
> Signed-off-by: Zhao Liu <zhao1....@intel.com>
> Co-authored-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> rust/qemu-api/meson.build | 1 +
> rust/qemu-api/src/bitops.rs | 119 +++++++++++++++++++++++++++++++++++
> rust/qemu-api/src/lib.rs | 1 +
> rust/qemu-api/src/prelude.rs | 2 +
> 4 files changed, 123 insertions(+)
> create mode 100644 rust/qemu-api/src/bitops.rs
>
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index b927eb58c8e..adcee661150 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -16,6 +16,7 @@ _qemu_api_rs = static_library(
> [
> 'src/lib.rs',
> 'src/bindings.rs',
> + 'src/bitops.rs',
> 'src/cell.rs',
> 'src/c_str.rs',
> 'src/definitions.rs',
> diff --git a/rust/qemu-api/src/bitops.rs b/rust/qemu-api/src/bitops.rs
> new file mode 100644
> index 00000000000..5acd6642d1a
> --- /dev/null
> +++ b/rust/qemu-api/src/bitops.rs
> @@ -0,0 +1,119 @@
> +// Copyright (C) 2024 Intel Corporation.
> +// Author(s): Zhao Liu <zhai1....@intel.com>
You deserve to be the author!
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +//! This module provides bit operation extensions to integer types.
> +//! It is usually included via the `qemu_api` prelude.
> +
> +use std::ops::{
> + Add, AddAssign, BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor,
> BitXorAssign, Div, DivAssign,
> + Mul, MulAssign, Not, Rem, RemAssign, Shl, ShlAssign, Shr, ShrAssign,
> +};
> +
> +/// Trait for extensions to integer types
> +pub trait IntegerExt:
> + Add<Self, Output = Self> + AddAssign<Self> +
> + BitAnd<Self, Output = Self> + BitAndAssign<Self> +
> + BitOr<Self, Output = Self> + BitOrAssign<Self> +
> + BitXor<Self, Output = Self> + BitXorAssign<Self> +
> + Copy +
> + Div<Self, Output = Self> + DivAssign<Self> +
> + Eq +
> + Mul<Self, Output = Self> + MulAssign<Self> +
> + Not<Output = Self> + Ord + PartialOrd +
> + Rem<Self, Output = Self> + RemAssign<Self> +
> + Shl<Self, Output = Self> + ShlAssign<Self> +
> + Shl<u32, Output = Self> + ShlAssign<u32> + // add more as needed
Ah, I used to define shift bits as usize. I can change the bit shift
type back to u32 in HPET.
> + Shr<Self, Output = Self> + ShrAssign<Self> +
> + Shr<u32, Output = Self> + ShrAssign<u32> // add more as needed
> +{
> + const BITS: u32;
> + const MAX: Self;
> + const MIN: Self;
> + const ONE: Self;
> + const ZERO: Self;
> +
> + #[inline]
> + #[must_use]
> + fn bit(start: u32) -> Self
> + {
> + assert!(start <= Self::BITS);
> +
> + Self::ONE << start
> + }
I think with this helper, I can add activating_bit() and deactivating_bit()
bindings, as they are also commonly used operations.
I will rename them to activate_bit and deactivate_bit, if no one has any
objections.
> + #[inline]
> + #[must_use]
> + fn mask(start: u32, length: u32) -> Self
> + {
> + /* FIXME: Implement a more elegant check with error handling
> support? */
I think current design is elegant enough, and this FIXME is not needed.
> + assert!(length > 0 && length <= Self::BITS - start);
> +
> + (Self::MAX >> (Self::BITS - length)) << start
> + }
> +
> + #[inline]
> + #[must_use]
> + fn deposit<U: IntegerExt>(self, start: u32, length: u32,
> + fieldval: U) -> Self
> + where Self: From<U>
> + {
> + debug_assert!(length <= U::BITS);
assert? as you've already replaced debug_assert with assert in BqlCell.
> + let mask = Self::mask(start, length);
> + (self & !mask) | ((Self::from(fieldval) << start) & mask)
> + }
> +
Very useful for HPET! Thanks.
Reviewed-by: Zhao Liu <zhao1....@intel.com>
