All of lore.kernel.org
 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: 3+ 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]

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 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.