Re: [PATCH 10/10] rust: bindings for MemoryRegionOps

[Paolo Bonzini]

On Thu, Feb 6, 2025 at 9:40 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Paolo,
>
> On 17/1/25 20:40, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   rust/hw/char/pl011/src/device.rs     |  43 +++---
> >   rust/hw/char/pl011/src/lib.rs        |   1 -
> >   rust/hw/char/pl011/src/memory_ops.rs |  36 -----
> >   rust/qemu-api/meson.build            |   1 +
> >   rust/qemu-api/src/lib.rs             |   1 +
> >   rust/qemu-api/src/memory.rs          | 191 +++++++++++++++++++++++++++
> >   rust/qemu-api/src/sysbus.rs          |   7 +-
> >   rust/qemu-api/src/zeroable.rs        |  12 ++
> >   8 files changed, 234 insertions(+), 58 deletions(-)
> >   delete mode 100644 rust/hw/char/pl011/src/memory_ops.rs
> >   create mode 100644 rust/qemu-api/src/memory.rs
> >
> > diff --git a/rust/hw/char/pl011/src/device.rs 
> > b/rust/hw/char/pl011/src/device.rs
> > index 259efacb046..294394c6e82 100644
> > --- a/rust/hw/char/pl011/src/device.rs
> > +++ b/rust/hw/char/pl011/src/device.rs
> > @@ -2,7 +2,7 @@
> >   // Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> >   // SPDX-License-Identifier: GPL-2.0-or-later
> >
> > -use core::ptr::{addr_of_mut, NonNull};
> > +use core::ptr::{addr_of, addr_of_mut, NonNull};
> >   use std::{
> >       ffi::CStr,
> >       os::raw::{c_int, c_void},
> > @@ -12,14 +12,14 @@
> >       bindings::{self, *},
> >       c_str, impl_vmstate_forward,
> >       irq::InterruptSource,
> > +    memory::{hwaddr, MemoryRegion, MemoryRegionOps, 
> > MemoryRegionOpsBuilder},
> >       prelude::*,
> > -    qdev::{Clock, ClockEvent, DeviceImpl, ResettablePhasesImpl, ResetType},
> > +    qdev::{Clock, ClockEvent, DeviceImpl, ResetType, ResettablePhasesImpl},
> >       qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
> >   };
> >
> >   use crate::{
> >       device_class,
> > -    memory_ops::PL011_OPS,
> >       registers::{self, Interrupt},
> >       RegisterOffset,
> >   };
> > @@ -490,20 +490,24 @@ impl PL011State {
> >       /// location/instance. All its fields are expected to hold unitialized
> >       /// values with the sole exception of `parent_obj`.
> >       unsafe fn init(&mut self) {
> > +        static PL011_OPS: MemoryRegionOps<PL011State> = 
> > MemoryRegionOpsBuilder::<PL011State>::new()
> > +            .read(&PL011State::read)
> > +            .write(&PL011State::write)
> > +            .native_endian()
>
> Could we always make .valid_sizes() explicit?

Yes (for example build() could even fail to compile if you don't have
impl_sizes/valid_sizes set), but why do you want that? I'm not even
sure that all cases of .valid.max_access_size=4 are correct...

Paolo