Re: [RFC 05/13] rust: add a bit operation binding for deposit64

[Zhao Liu]

On Thu, Dec 05, 2024 at 05:09:42PM +0100, Paolo Bonzini wrote:
> Date: Thu, 5 Dec 2024 17:09:42 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: Re: [RFC 05/13] rust: add a bit operation binding for deposit64
> 
> On 12/5/24 07:07, Zhao Liu wrote:
> > +pub fn deposit64(value: u64, start: usize, length: usize, fieldval: u64) 
> > -> u64 {
> > +    /* FIXME: Implement a more elegant check with error handling support? 
> > */
> > +    assert!(length > 0 && length <= 64 - start);
> > +
> > +    let mask = (u64::MAX >> (64 - length)) << start;
> > +    (value & !mask) | ((fieldval << start) & mask)
> > +}
> 
> This should be more generic and implemented as a trait that is
> implemented by u8/u16/u32/u64.

Yes, I agree!

> It's okay to rewrite these utility
> functions in Rust instead of relying on bindgen, because the way
> you'd like to use them is likely different from C.  Something like:
> 
> pub trait IntegerExt
> {
>     fn deposit(self, start: u32, length: u32, fieldval: U) -> Self;
> }
> 
> impl IntegerExt for u64
> {
>     fn deposit(self, start: usize, length: usize, fieldval: u64) -> u64 {
>         /* FIXME: Implement a more elegant check with error handling support? 
> */
>         assert!(length > 0 && length <= 64 - start);
> 
>         let mask = (u64::MAX >> (64 - length)) << start;
>         (value & !mask) | ((fieldval << start) & mask)
>     }
> }

Then C and Rust would be using completely different bitops library, is
it necessary to implement the C interface directly in Rust instead of
keeping the C implementation (when Rust is enabled)?

> And we can add a "prelude" module so that you can do
> 
> use qemu_api::prelude::*;
> 
> and get all these useful traits at once.  I will send a patch after
> fleshing the idea out a bit more.

Thanks! Cross fingers.

Regards,
Zhao