From: Zhao Liu <zhao1.liu@intel.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-rust@nongnu.org
Subject: Re: [PATCH 04/12] rust: timer: wrap QEMUTimer with Opaque<> and express pinning requirements
Date: Mon, 3 Mar 2025 21:48:55 +0800 [thread overview]
Message-ID: <Z8WzRyPbt6d7CHWM@intel.com> (raw)
In-Reply-To: <20250227142219.812270-5-pbonzini@redhat.com>
On Thu, Feb 27, 2025 at 03:22:11PM +0100, Paolo Bonzini wrote:
> Date: Thu, 27 Feb 2025 15:22:11 +0100
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 04/12] rust: timer: wrap QEMUTimer with Opaque<> and
> express pinning requirements
> X-Mailer: git-send-email 2.48.1
>
> Timers must be pinned in memory, because modify() stores a pointer to them
> in the TimerList. To ensure this is the case, replace the separate new()
> and init_full() with a single function that returns a pinned box. Because
> the only way to obtain a Timer is through Timer::new_full(), modify()
> knows that the timer it got is also pinned. In the future the pinning
> requirement will be expressed through the pin_init crate instead.
>
> Note that Timer is a bit different from other users of Opaque, in that
> it is created in Rust code rather than C code. This is why it has to
> use the unsafe Opaque::new() function.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> meson.build | 7 -----
> rust/hw/timer/hpet/src/hpet.rs | 23 ++++++++---------
> rust/qemu-api/src/timer.rs | 47 ++++++++++++++++++++++------------
> 3 files changed, 41 insertions(+), 36 deletions(-)
Great! LGTM,
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
(But pls wait, I have a question below...)
> @@ -156,7 +157,7 @@ pub struct HPETTimer {
> /// timer N index within the timer block (`HPETState`)
> #[doc(alias = "tn")]
> index: usize,
> - qemu_timer: Option<Box<Timer>>,
> + qemu_timer: Option<Pin<Box<Timer>>>,
I'm removing this Option<> wrapper in migration series. This is because
Option<> can't be treated as pointer as you mentioned in [*].
So for this reason, does this mean that VMStateField cannot accept
Option<>? I realize that all the current VMStateFlags don't seem
compatible with Option<> unless a new flag is introduced.
[*]: https://lore.kernel.org/qemu-devel/9a0389fa-765c-443b-ac2f-7c99ed862982@redhat.com/
Thanks,
Zhao
next prev parent reply other threads:[~2025-03-03 13:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-27 14:22 [PATCH v2 00/12] rust: wrap all C types exposed through qemu_api Paolo Bonzini
2025-02-27 14:22 ` [PATCH 01/12] rust: cell: add wrapper for FFI types Paolo Bonzini
2025-02-27 14:22 ` [PATCH 02/12] rust: qemu_api_macros: add Wrapper derive macro Paolo Bonzini
2025-02-27 14:22 ` [PATCH 03/12] rust: vmstate: add std::pin::Pin as transparent wrapper Paolo Bonzini
2025-03-03 13:25 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 04/12] rust: timer: wrap QEMUTimer with Opaque<> and express pinning requirements Paolo Bonzini
2025-03-03 13:48 ` Zhao Liu [this message]
2025-03-03 15:58 ` Paolo Bonzini
2025-03-04 9:13 ` Zhao Liu
2025-03-06 10:45 ` Paolo Bonzini
2025-03-06 11:35 ` Zhao Liu
2025-03-03 14:28 ` Zhao Liu
2025-03-03 14:51 ` Paolo Bonzini
2025-03-03 16:15 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 05/12] rust: irq: wrap IRQState with Opaque<> Paolo Bonzini
2025-03-03 15:07 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 06/12] rust: qom: wrap Object " Paolo Bonzini
2025-02-27 14:22 ` [PATCH 07/12] rust: qdev: wrap Clock and DeviceState " Paolo Bonzini
2025-02-27 14:22 ` [PATCH 08/12] rust: hpet: do not access fields of SysBusDevice Paolo Bonzini
2025-03-03 15:09 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 09/12] rust: sysbus: wrap SysBusDevice with Opaque<> Paolo Bonzini
2025-03-03 15:19 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 10/12] rust: memory: wrap MemoryRegion " Paolo Bonzini
2025-03-03 15:25 ` Zhao Liu
2025-03-05 7:09 ` Zhao Liu
2025-02-27 14:22 ` [PATCH 11/12] rust: chardev: wrap Chardev " Paolo Bonzini
2025-02-27 14:22 ` [PATCH 12/12] rust: bindings: remove more unnecessary Send/Sync impls Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z8WzRyPbt6d7CHWM@intel.com \
--to=zhao1.liu@intel.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-rust@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.