Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: "Onur Özkan" <work@onurozkan.dev>
To: leixiang <leenollei@gmail.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, pbonzini@redhat.com,
	seanjc@google.com, ojeda@kernel.org
Subject: Re: [RFC PATCH] KVM: rust: add Rust reimplementation of eventfd
Date: Mon, 29 Jun 2026 11:29:05 +0300	[thread overview]
Message-ID: <20260629082930.23182-1-work@onurozkan.dev> (raw)
In-Reply-To: <20260626083212.39611-1-leixiang@kylinos.cn>

On Fri, 26 Jun 2026 16:29:34 +0800
leixiang <leenollei@gmail.com> wrote:

> From: ninol <leenollei@gmail.com>
> 
> Introduce a Rust reimplementation of virt/kvm/eventfd.c, providing
> irqfd (interrupt injection via eventfd) and ioeventfd (MMIO/PIO write
> to eventfd signal) functionality with full C ABI compatibility.
> 
> The Rust implementation leverages RAII guards for SRCU, mutex, and
> spinlock management, reducing the risk of resource leaks on error
> paths. It is selectable via CONFIG_RUST_KVM_EVENTFD and replaces
> the C eventfd.o with a shim providing only weak default functions.
> 
> Signed-off-by: ninol <leenollei@gmail.com>
> ---
> 
> Hi all,
> 
> This is an experimental/exploratory RFC for a Rust reimplementation of
> virt/kvm/eventfd.c. It is intended as a learning exercise and a proof
> of concept to explore whether Rust can be practically applied to KVM
> subsystem components.
> 
> I fully understand this may not be the direction the KVM community
> wants to take. If you feel this is not worth your time, please ignore
> it entirely. If, however, you find this approach interesting or have
> any feedback on the implementation, I would greatly appreciate
> your comments.
> 
> The patch provides functionally equivalent irqfd and ioeventfd support,
> gated behind CONFIG_RUST_KVM_EVENTFD. It uses RAII guards for lock and
> resource management and maintains full ABI compatibility with the C
> implementation.
> 
> Testing: Built and boot-tested on x86_64 with KVM enabled.
> 
> Thanks for your time.
> 
>  rust/bindgen_parameters         |    2 +
>  rust/bindings/bindings_helper.h |   18 +
>  rust/helpers/eventfd.c          |   34 +
>  rust/helpers/helpers.c          |    7 +
>  rust/helpers/kvm.c              |  221 +++++
>  rust/helpers/seqcount.c         |   30 +
>  rust/kernel/kvm/eventfd.rs      | 1419 +++++++++++++++++++++++++++++++
>  rust/kernel/kvm/mod.rs          |    8 +
>  rust/kernel/lib.rs              |    2 +
>  virt/kvm/Kconfig                |   15 +
>  virt/kvm/Makefile.kvm           |    7 +-
>  virt/kvm/eventfd_shim.c         |   52 ++
>  12 files changed, 1814 insertions(+), 1 deletion(-)
>  create mode 100644 rust/helpers/eventfd.c
>  create mode 100644 rust/helpers/kvm.c
>  create mode 100644 rust/helpers/seqcount.c
>  create mode 100644 rust/kernel/kvm/eventfd.rs
>  create mode 100644 rust/kernel/kvm/mod.rs
>  create mode 100644 virt/kvm/eventfd_shim.c
> 
> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> index 6f02d9720ad2..c433a6ca5336 100644
> --- a/rust/bindgen_parameters
> +++ b/rust/bindgen_parameters
> @@ -14,6 +14,8 @@
>  --opaque-type alt_instr
>  --opaque-type x86_msi_data
>  --opaque-type x86_msi_addr_lo
> +--opaque-type hv_.*
> +--opaque-type IO_APIC_route_entry
>  
>  # If SMP is disabled, `arch_spinlock_t` is defined as a ZST which triggers a Rust
>  # warning. We don't need to peek into it anyway.
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 446dbeaf0866..2d65f2b1672d 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -65,8 +65,12 @@
>  #include <linux/io-pgtable.h>
>  #include <linux/ioport.h>
>  #include <linux/iosys-map.h>
> +#include <linux/irqbypass.h>
>  #include <linux/jiffies.h>
>  #include <linux/jump_label.h>
> +#include <kvm/iodev.h>
> +#include <linux/kvm_host.h>
> +#include <linux/kvm_irqfd.h>
>  #include <linux/mdio.h>
>  #include <linux/mm.h>
>  #include <linux/miscdevice.h>
> @@ -94,6 +98,20 @@
>  #include <linux/xarray.h>
>  #include <trace/events/rust_sample.h>
>  
> +#ifdef CONFIG_RUST_KVM_EVENTFD
> +/* Custom helpers for eventfd.rs */
> +unsigned long rust_helper_spin_lock_irqsave(spinlock_t *lock);
> +void rust_helper_spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags);
> +void rust_helper_hlist_add_head_rcu(struct hlist_node *n, struct hlist_head *h);
> +void rust_helper_hlist_del_init_rcu(struct hlist_node *n);

[...]

> +#[cfg(CONFIG_LOCKDEP)]
> +#[inline(always)]
> +unsafe fn spin_acquire(
> +    map: *mut bindings::lockdep_map,
> +    subclass: c_int,
> +    trylock: c_int,
> +    ip: core::ffi::c_ulong,
> +) {
> +    // SAFETY: Caller holds the corresponding lock; this is a lockdep annotation only.
> +    unsafe { bindings::rust_helper_spin_acquire(map, subclass, trylock, ip) }
> +}
> +
> +// RAII Guards
> +
> +/// RAII Guard for SRCU read lock.
> +struct SrcuGuard {
> +    srcu: *mut bindings::srcu_struct,
> +    idx: core::ffi::c_int,
> +    _not_thread_safe: crate::types::NotThreadSafe,
> +}
> +

> +impl SrcuGuard {
> +    /// # Safety
> +    /// Caller must ensure `srcu` is a valid pointer.
> +    unsafe fn new(srcu: *mut bindings::srcu_struct) -> Self {
> +        // SAFETY: Caller guarantees `srcu` is valid.
> +        let idx = unsafe { bindings::srcu_read_lock(srcu) };
> +        Self {
> +            srcu,
> +            idx,
> +            _not_thread_safe: crate::types::NotThreadSafe,
> +        }
> +    }
> +}
> +
> +impl Drop for SrcuGuard {
> +    fn drop(&mut self) {
> +        // SAFETY: We hold the lock `idx` on `srcu`.
> +        unsafe { bindings::srcu_read_unlock(self.srcu, self.idx) };
> +    }
> +}

You are adding things that are already being implemented in much better way. For
example, for SRCU we already have v10 [1]. But even before that I think this
patch has even more serious problems. It has multiple fundamental issues and
things seem to be placed randomly all around.

Regards,
Onur

[1]: https://lore.kernel.org/all/20260613065348.96750-1-work@onurozkan.dev

> +
> +/// RAII Guard for mutex lock.
> +struct MutexGuard {
> +    lock: *mut bindings::mutex,
> +}
> +
> +impl MutexGuard {
> +    /// # Safety
> +    /// Caller must ensure `lock` is a valid pointer.
> +    unsafe fn new(lock: *mut bindings::mutex) -> Self {
> +        // SAFETY: Caller guarantees `lock` is valid.
> +        unsafe { bindings::mutex_lock(lock) };
> +        Self { lock }
> +    }
> +}
> +
> +impl Drop for MutexGuard {
> +    fn drop(&mut self) {
> +        // SAFETY: We hold the lock.
> +        unsafe { bindings::mutex_unlock(self.lock) };
> +    }

[...]

> +#include <linux/kvm_irqfd.h>
> +#include <linux/irqbypass.h>
> +
> +bool __weak kvm_arch_irqfd_allowed(struct kvm *kvm, struct kvm_irqfd *args)
> +{
> +	return true;
> +}
> +
> +int __weak kvm_arch_set_irq_inatomic(
> +				struct kvm_kernel_irq_routing_entry *irq,
> +				struct kvm *kvm, int irq_source_id,
> +				int level,
> +				bool line_status)
> +{
> +	return -EWOULDBLOCK;
> +}
> +
> +#if IS_ENABLED(CONFIG_HAVE_KVM_IRQ_BYPASS)
> +void __weak kvm_arch_irq_bypass_stop(
> +				struct irq_bypass_consumer *cons)
> +{
> +}
> +
> +void __weak kvm_arch_irq_bypass_start(
> +				struct irq_bypass_consumer *cons)
> +{
> +}
> +
> +void __weak kvm_arch_update_irqfd_routing(struct kvm_kernel_irqfd *irqfd,
> +					  struct kvm_kernel_irq_routing_entry *old,
> +					  struct kvm_kernel_irq_routing_entry *new)
> +{
> +}
> +#endif
> +
> +/*
> + * The Rust implementation provides kvm_irq_has_notifier via #[no_mangle].
> + * Symbol export for KVM-internal use must be done from C.
> + */
> +extern bool kvm_irq_has_notifier(struct kvm *kvm, unsigned int irqchip,
> +				 unsigned int pin);
> +EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_irq_has_notifier);
> -- 
> 2.43.0
> 

  parent reply	other threads:[~2026-06-29  8:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26  8:29 [RFC PATCH] KVM: rust: add Rust reimplementation of eventfd leixiang
2026-06-26  8:48 ` sashiko-bot
2026-06-29  8:29 ` Onur Özkan [this message]
2026-06-30  7:16   ` ninol

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=20260629082930.23182-1-work@onurozkan.dev \
    --to=work@onurozkan.dev \
    --cc=kvm@vger.kernel.org \
    --cc=leenollei@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=seanjc@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox