Re: [PATCH 6/7] rust: pl011: fix migration stream

[Zhao Liu]

On Thu, Dec 12, 2024 at 06:22:03PM +0100, Paolo Bonzini wrote:
> Date: Thu, 12 Dec 2024 18:22:03 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 6/7] rust: pl011: fix migration stream
> X-Mailer: git-send-email 2.47.1
> 
> The Rust vmstate macros lack the type-safety of their C equivalents (so
> safe, much abstraction), and therefore they were predictably wrong.

Yes, this makes Rust more unsafe than C code...

> The registers have already been changed to 32-bits in the previous patch,
> but read_pos/read_count/read_trigger also have to be u32 instead of usize.
> The easiest way to do so is to let the FIFO use u32 indices instead
> of usize.
> 
> My plan for making VMStateField typesafe is to have a trait to retrieve
> a basic VMStateField; for example something like vmstate_uint32 would
> become an implementation of the VMState trait on u32.  Then you'd write
> something like "vmstate_of!(Type, field).with_version_id(2)".  That is,
> vmstate_of retrieves the basic VMStateField and fills in the offset,
> and then more changes can be applied on top.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/hw/char/pl011/src/device.rs       | 38 ++++++++++++++++++++++----
>  rust/hw/char/pl011/src/device_class.rs |  8 +++---
>  rust/qemu-api/src/vmstate.rs           | 22 ---------------
>  3 files changed, 36 insertions(+), 32 deletions(-)
> 
> @@ -64,9 +64,9 @@ extern "C" fn pl011_post_load(opaque: *mut c_void, 
> version_id: c_int) -> c_int {
>          vmstate_uint32!(ibrd, PL011State),
>          vmstate_uint32!(fbrd, PL011State),
>          vmstate_uint32!(ifl, PL011State),
> -        vmstate_int32!(read_pos, PL011State),
> -        vmstate_int32!(read_count, PL011State),
> -        vmstate_int32!(read_trigger, PL011State),
> +        vmstate_uint32!(read_pos, PL011State),
> +        vmstate_uint32!(read_count, PL011State),
> +        vmstate_uint32!(read_trigger, PL011State),

uint32 and int32 types both use `qemu_put_be32s` and `qemu_get_be32s`
to save and store vmstate, so I think it's safe to convert
vmstate_int32! to vmstate_uint32! here.

>      },

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>