Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/rust.rst              |   4 --
 rust/hw/timer/hpet/src/fw_cfg.rs |   6 +-
 rust/qemu-api/src/zeroable.rs    | 104 +++++--------------------------
 3 files changed, 18 insertions(+), 96 deletions(-)

diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 8167ff49aa9..13a002cfe69 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -83,10 +83,6 @@ are missing:
 * "Return position ``impl Trait`` in Traits" (1.75.0, blocker for including
   the pinned-init create).
 
-* ``MaybeUninit::zeroed()`` as a ``const`` function (1.75.0).  QEMU's
-  ``Zeroable`` trait can be implemented without ``MaybeUninit::zeroed()``,
-  so this would be just a cleanup.
-
 * ``c"" literals`` (stable in 1.77.0).  QEMU provides a ``c_str!()`` macro
   to define ``CStr`` constants easily
 
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index bef03727ea3..aa08d283519 100644
--- a/rust/hw/timer/hpet/src/fw_cfg.rs
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -4,7 +4,7 @@
 
 use std::ptr::addr_of_mut;
 
-use qemu_api::{cell::bql_locked, impl_zeroable, zeroable::Zeroable};
+use qemu_api::{cell::bql_locked, zeroable::Zeroable};
 
 /// Each `HPETState` represents a Event Timer Block. The v1 spec supports
 /// up to 8 blocks. QEMU only uses 1 block (in PC machine).
@@ -18,7 +18,7 @@ pub struct HPETFwEntry {
     pub min_tick: u16,
     pub page_prot: u8,
 }
-impl_zeroable!(HPETFwEntry);
+unsafe impl Zeroable for HPETFwEntry {}
 
 #[repr(C, packed)]
 #[derive(Copy, Clone, Default)]
@@ -26,7 +26,7 @@ pub struct HPETFwConfig {
     pub count: u8,
     pub hpet: [HPETFwEntry; HPET_MAX_NUM_EVENT_TIMER_BLOCK],
 }
-impl_zeroable!(HPETFwConfig);
+unsafe impl Zeroable for HPETFwConfig {}
 
 #[allow(non_upper_case_globals)]
 #[no_mangle]
diff --git a/rust/qemu-api/src/zeroable.rs b/rust/qemu-api/src/zeroable.rs
index a3415a2ebcc..57d802db69b 100644
--- a/rust/qemu-api/src/zeroable.rs
+++ b/rust/qemu-api/src/zeroable.rs
@@ -4,89 +4,15 @@
 
 /// Encapsulates the requirement that
 /// `MaybeUninit::<Self>::zeroed().assume_init()` does not cause undefined
-/// behavior.  This trait in principle could be implemented as just:
-///
-/// ```
-/// pub unsafe trait Zeroable: Default {
-///     const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() };
-/// }
-/// ```
-///
-/// The need for a manual implementation is only because `zeroed()` cannot
-/// be used as a `const fn` prior to Rust 1.75.0. Once we can assume a new
-/// enough version of the compiler, we could provide a `#[derive(Zeroable)]`
-/// macro to check at compile-time that all struct fields are Zeroable, and
-/// use the above blanket implementation of the `ZERO` constant.
+/// behavior.
 ///
 /// # Safety
 ///
-/// Because the implementation of `ZERO` is manual, it does not make
-/// any assumption on the safety of `zeroed()`.  However, other users of the
-/// trait could use it that way.  Do not add this trait to a type unless
-/// all-zeroes is a valid value for the type.  In particular, remember that
-/// raw pointers can be zero, but references and `NonNull<T>` cannot
+/// Do not add this trait to a type unless all-zeroes is a valid value for the
+/// type.  In particular, raw pointers can be zero, but references and `NonNull<T>`
+/// cannot.
 pub unsafe trait Zeroable: Default {
-    const ZERO: Self;
-}
-
-/// A macro that acts similarly to [`core::mem::zeroed()`], only is const
-///
-/// ## Safety
-///
-/// Similar to `core::mem::zeroed()`, except this zeroes padding bits. Zeroed
-/// padding usually isn't relevant to safety, but might be if a C union is used.
-///
-/// Just like for `core::mem::zeroed()`, an all zero byte pattern might not
-/// be a valid value for a type, as is the case for references `&T` and `&mut
-/// T`. Reference types trigger a (denied by default) lint and cause immediate
-/// undefined behavior if the lint is ignored
-///
-/// ```rust compile_fail
-/// use const_zero::const_zero;
-/// // error: any use of this value will cause an error
-/// // note: `#[deny(const_err)]` on by default
-/// const STR: &str = unsafe{const_zero!(&'static str)};
-/// ```
-///
-/// `const_zero` does not work on unsized types:
-///
-/// ```rust compile_fail
-/// use const_zero::const_zero;
-/// // error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
-/// const BYTES: [u8] = unsafe{const_zero!([u8])};
-/// ```
-/// ## Differences with `core::mem::zeroed`
-///
-/// `const_zero` zeroes padding bits, while `core::mem::zeroed` doesn't
-#[macro_export]
-macro_rules! const_zero {
-    // This macro to produce a type-generic zero constant is taken from the
-    // const_zero crate (v0.1.1):
-    //
-    //     https://docs.rs/const-zero/latest/src/const_zero/lib.rs.html
-    //
-    // and used under MIT license
-    ($type_:ty) => {{
-        const TYPE_SIZE: ::core::primitive::usize = ::core::mem::size_of::<$type_>();
-        union TypeAsBytes {
-            bytes: [::core::primitive::u8; TYPE_SIZE],
-            inner: ::core::mem::ManuallyDrop<$type_>,
-        }
-        const ZERO_BYTES: TypeAsBytes = TypeAsBytes {
-            bytes: [0; TYPE_SIZE],
-        };
-        ::core::mem::ManuallyDrop::<$type_>::into_inner(ZERO_BYTES.inner)
-    }};
-}
-
-/// A wrapper to implement the `Zeroable` trait through the `const_zero` macro.
-#[macro_export]
-macro_rules! impl_zeroable {
-    ($type:ty) => {
-        unsafe impl $crate::zeroable::Zeroable for $type {
-            const ZERO: Self = unsafe { $crate::const_zero!($type) };
-        }
-    };
+    const ZERO: Self = unsafe { ::core::mem::MaybeUninit::<Self>::zeroed().assume_init() };
 }
 
 // bindgen does not derive Default here
@@ -97,13 +23,13 @@ fn default() -> Self {
     }
 }
 
-impl_zeroable!(crate::bindings::Property__bindgen_ty_1);
-impl_zeroable!(crate::bindings::Property);
-impl_zeroable!(crate::bindings::VMStateFlags);
-impl_zeroable!(crate::bindings::VMStateField);
-impl_zeroable!(crate::bindings::VMStateDescription);
-impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_1);
-impl_zeroable!(crate::bindings::MemoryRegionOps__bindgen_ty_2);
-impl_zeroable!(crate::bindings::MemoryRegionOps);
-impl_zeroable!(crate::bindings::MemTxAttrs);
-impl_zeroable!(crate::bindings::CharBackend);
+unsafe impl Zeroable for crate::bindings::Property__bindgen_ty_1 {}
+unsafe impl Zeroable for crate::bindings::Property {}
+unsafe impl Zeroable for crate::bindings::VMStateFlags {}
+unsafe impl Zeroable for crate::bindings::VMStateField {}
+unsafe impl Zeroable for crate::bindings::VMStateDescription {}
+unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_1 {}
+unsafe impl Zeroable for crate::bindings::MemoryRegionOps__bindgen_ty_2 {}
+unsafe impl Zeroable for crate::bindings::MemoryRegionOps {}
+unsafe impl Zeroable for crate::bindings::MemTxAttrs {}
+unsafe impl Zeroable for crate::bindings::CharBackend {}
-- 
2.49.0

On Fri, 02 May 2025 13:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> docs/devel/rust.rst              |   4 --
> rust/hw/timer/hpet/src/fw_cfg.rs |   6 +-
> rust/qemu-api/src/zeroable.rs    | 104 +++++--------------------------
> 3 files changed, 18 insertions(+), 96 deletions(-)

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

BTW There's this TODO in qom.rs, ObjectImpl trait

>   /// `&mut T`.  TODO: add a bound of 
>   //[`Zeroable`](crate::zeroable::Zeroable)
>   /// to T; this is more easily done once Zeroable does not require a manual
>   /// implementation (Rust 1.75.0).
>   const CLASS_INIT: fn(&mut Self::Class);

On 5/2/25 13:01, Manos Pitsidianakis wrote:
> On Fri, 02 May 2025 13:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> docs/devel/rust.rst              |   4 --
>> rust/hw/timer/hpet/src/fw_cfg.rs |   6 +-
>> rust/qemu-api/src/zeroable.rs    | 104 +++++--------------------------
>> 3 files changed, 18 insertions(+), 96 deletions(-)
> 
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> 
> BTW There's this TODO in qom.rs, ObjectImpl trait
> 
>>    /// `&mut T`.  TODO: add a bound of
>>    //[`Zeroable`](crate::zeroable::Zeroable)
>>    /// to T; this is more easily done once Zeroable does not require a manual
>>    /// implementation (Rust 1.75.0).
>>    const CLASS_INIT: fn(&mut Self::Class);

Yes, good point.  When I wrote the TODO, my idea here was to have some 
kind of

	#[derive(Zeroable)]

macro so that the compiler can "confirm" the safety of implementing 
Zeroable by hand.

However, most of the time the class will be just a C-defined class 
(DeviceClass or SysBusDeviceClass), and then it's not even possible to 
add the derive attribute to the declaration.  So adding the bound to 
ObjectType::Class is feasible now that one can just add

     unsafe impl Zeroable for bindings::ObjectClass {}
     unsafe impl Zeroable for bindings::DeviceClass {}
     unsafe impl Zeroable for bindings::SysBusDeviceClass {}
     unsafe impl Zeroable for bindings::ResettableClass {}

etc.; in fact it was already possible when Zhao added the impl_zeroed! 
macro in commit aaf3778baaa ("rust/zeroable: Implement Zeroable with 
const_zero macro", 2025-01-28).

Paolo

On Fri, May 2, 2025 at 3:23 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/2/25 13:01, Manos Pitsidianakis wrote:
> > On Fri, 02 May 2025 13:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >> docs/devel/rust.rst              |   4 --
> >> rust/hw/timer/hpet/src/fw_cfg.rs |   6 +-
> >> rust/qemu-api/src/zeroable.rs    | 104 +++++--------------------------
> >> 3 files changed, 18 insertions(+), 96 deletions(-)
> >
> > Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> >
> > BTW There's this TODO in qom.rs, ObjectImpl trait
> >
> >>    /// `&mut T`.  TODO: add a bound of
> >>    //[`Zeroable`](crate::zeroable::Zeroable)
> >>    /// to T; this is more easily done once Zeroable does not require a manual
> >>    /// implementation (Rust 1.75.0).
> >>    const CLASS_INIT: fn(&mut Self::Class);
>
> Yes, good point.  When I wrote the TODO, my idea here was to have some
> kind of
>
>         #[derive(Zeroable)]
>
> macro so that the compiler can "confirm" the safety of implementing
> Zeroable by hand.
>
> However, most of the time the class will be just a C-defined class
> (DeviceClass or SysBusDeviceClass), and then it's not even possible to
> add the derive attribute to the declaration.

Yes this makes sense. A benefit of having `#[derive(Zeroable)]` is
avoiding blanket unsafe impls of the trait, since the derive macro
would work only if all fields implement Zeroable (so in a sense, it's
safe as long as the individual field type impls are safe,
transitively). And we can still do manual unsafe impls for everything
from the FFI boundary.

>  So adding the bound to
> ObjectType::Class is feasible now that one can just add
>
>      unsafe impl Zeroable for bindings::ObjectClass {}
>      unsafe impl Zeroable for bindings::DeviceClass {}
>      unsafe impl Zeroable for bindings::SysBusDeviceClass {}
>      unsafe impl Zeroable for bindings::ResettableClass {}
>
> etc.; in fact it was already possible when Zhao added the impl_zeroed!
> macro in commit aaf3778baaa ("rust/zeroable: Implement Zeroable with
> const_zero macro", 2025-01-28).
>
> Paolo
>