From nobody Fri Oct 10 21:13:59 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B965126989D; Thu, 12 Jun 2025 14:51:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739914; cv=none; b=s2Ad0OW1I7a/xMc9XjzS62OKKYpZppGkzo2WXuAPIuqntsbBjxNKTLPW8E7zFQqrEhZQs/JWBJJidPS26GXHkQFe0k93bTqWWJrzP8zlR5lxTThLa63imuwBsYYfxqhpEiStzKRuiwV6bLVBzHXUy4dkEzNNL7hOiqjingWEwt8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739914; c=relaxed/simple; bh=TciDSg9bcuZTke1QmCC3h0bSb7BA7urmikCTCcO+ODA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=bmy+8qygk6z7Wn5mQ8KWMiB4yPKr17KeUwqoX5vjOg5tnhSzmgtVNSfBqatJssJtrQzHH9pSZlrQaqCWdswlcY4X6CMvO7gN+xQPWK0/orXTl6LxmYsEUjaRQXe6nX0Ox3bXcp6cduOjJWhinSKCoMIEyohs5UUJ1/T2zdaS3DI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=npDvdYDp; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="npDvdYDp" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7A75CC4CEEB; Thu, 12 Jun 2025 14:51:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749739914; bh=TciDSg9bcuZTke1QmCC3h0bSb7BA7urmikCTCcO+ODA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=npDvdYDpZPF8qJItRNBDBsBt/8K/dj0ueU5u6GBWQ9px9kW52AY1RQ9cs/0TvhUf9 AypKQTD06JJkB2lg0vl9DD/Q+nOR4CRnQvV7nIeg1APEaloYlq/jZSb13/hTxGUMbh ysDRJFxb8Xd0y3QKi8wC0Ttsu6GELZYoubSmHFibrWUsDPbEElbskjyGZO4PSxsic/ +jqIk09HZALyhIZNBWO8abHXYHyVs75hH6pDOjFZd8kurG2wvZCeuoaPm+YTkEjPaU HYsCIrNtxfvToQLoB4yJkI6jE5XOmvcHdRx3eXjsLeh5QkzUHaeQurbbETQcKhxi8Y KaWZqiipAuSkA== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, benno.lossin@proton.me, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Danilo Krummrich Subject: [PATCH 1/4] rust: revocable: support fallible PinInit types Date: Thu, 12 Jun 2025 16:51:38 +0200 Message-ID: <20250612145145.12143-2-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250612145145.12143-1-dakr@kernel.org> References: <20250612145145.12143-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Currently, Revocable::new() only supports infallible PinInit implementations, i.e. impl PinInit. This has been sufficient so far, since users such as Devres do not support fallibility. Since this is about to change, make Revocable::new() generic over the error type E. Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 2 +- rust/kernel/revocable.rs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index d8bdf2bdb879..a7df9fbd724f 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -98,7 +98,7 @@ struct DevresInner { impl DevresInner { fn new(dev: &Device, data: T, flags: Flags) -> Result>> { let inner =3D Arc::pin_init( - pin_init!( DevresInner { + try_pin_init!( DevresInner { dev: dev.into(), callback: Self::devres_callback, data <- Revocable::new(data), diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index fa1fd70efa27..41b8fe374af6 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -82,8 +82,11 @@ unsafe impl Sync for Revocable {} =20 impl Revocable { /// Creates a new revocable instance of the given data. - pub fn new(data: impl PinInit) -> impl PinInit { - pin_init!(Self { + pub fn new(data: impl PinInit) -> impl PinInit + where + Error: From, + { + try_pin_init!(Self { is_available: AtomicBool::new(true), data <- Opaque::pin_init(data), }) --=20 2.49.0 From nobody Fri Oct 10 21:13:59 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9637226A09A; Thu, 12 Jun 2025 14:51:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739919; cv=none; b=OeJRKOAfhV3c+Pm/DMMF3pmrp2/vSTTcoNd4zdmQVgEw7PVT+w5DRsyOLU+w9Myd2T8bEKL23B0EFvDoL091tP3bcD4EXV8yzzDbFg8ZDvPaljgc6emiTp2TAZ0FJ5mCwDgDGDoqETOL+QGbkWCMd/qlhW+PzwjExgX489tDxb8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739919; c=relaxed/simple; bh=PPi+XfbHE5odGwqunvYnbbCiAfWSew9KjdZ3KwbpavE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=kaPxd6GeFUfnQPKvps/5QwwyLgezLGzz67FdFIFQX07TCkBYe793sceg4TyGdpeBiQ3gCS1IgxH1FDVxme0XfG/AB0Ui5pwLgS8fNN3LLF7xTYfHR9EogzpQQ8oVYk8OCSpsDp+LdqBK+gp5NB9tvevuSIVnD0tEH+KsQHEOKtA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=XXDh+HJG; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XXDh+HJG" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C33C3C4CEEA; Thu, 12 Jun 2025 14:51:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749739918; bh=PPi+XfbHE5odGwqunvYnbbCiAfWSew9KjdZ3KwbpavE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XXDh+HJGnmxrPduEZOkYPQJRJRkM8qPQiToJ2OT16Rt8MBmo/2jhFbcfKJ87oIY3l mawu50ForaPXdIs4EZjayFH8MketLBOtECWkwdJ4U2BL7KtIcxPvxY1vpH68r0nG+q QMBE4TYk/QgcHa2xVw4eFH1suNCxDc/zychd511bGG8FARQMSx3VBn6rWk2KdDitgs 3OE9gHS4SulG4t2KHWAhP2wJH6nX6ECrdk+WpwI08dX5ktDWo3+NTB4wWN+S6mHN2L wi69VsR6ZeoKBopju4SSu4dGvTXqv60X4rdACWIUMxrj435qfHcYpM9JG4yzWJ3dqo CIIY2omntahSw== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, benno.lossin@proton.me, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Danilo Krummrich , Dave Airlie , Simona Vetter , Viresh Kumar Subject: [PATCH 2/4] rust: devres: replace Devres::new_foreign_owned() Date: Thu, 12 Jun 2025 16:51:39 +0200 Message-ID: <20250612145145.12143-3-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250612145145.12143-1-dakr@kernel.org> References: <20250612145145.12143-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Replace Devres::new_foreign_owned() with devres::register_foreign_boxed(). The current implementation of Devres::new_foreign_owned() creates a full Devres container instance, including the internal Revocable and completion. However, none of that is necessary for the intended use of giving full ownership of an object to devres and getting it dropped once the given device is unbound. Hence, implement devres::register_foreign_boxed(), which is limited to consume the given data, wrap it in a KBox and drop the KBox once the given device is unbound, without any other synchronization. Cc: Dave Airlie Cc: Simona Vetter Cc: Viresh Kumar Signed-off-by: Danilo Krummrich Acked-by: Viresh Kumar --- rust/helpers/device.c | 7 ++++ rust/kernel/cpufreq.rs | 8 ++--- rust/kernel/devres.rs | 73 +++++++++++++++++++++++++++++++++------ rust/kernel/drm/driver.rs | 11 +++--- 4 files changed, 79 insertions(+), 20 deletions(-) diff --git a/rust/helpers/device.c b/rust/helpers/device.c index b2135c6686b0..502fef7e9ae8 100644 --- a/rust/helpers/device.c +++ b/rust/helpers/device.c @@ -8,3 +8,10 @@ int rust_helper_devm_add_action(struct device *dev, { return devm_add_action(dev, action, data); } + +int rust_helper_devm_add_action_or_reset(struct device *dev, + void (*action)(void *), + void *data) +{ + return devm_add_action_or_reset(dev, action, data); +} diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index b0a9c6182aec..f20636079f7a 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -12,7 +12,7 @@ clk::Hertz, cpumask, device::{Bound, Device}, - devres::Devres, + devres, error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_= DEFAULT_ERROR}, ffi::{c_char, c_ulong}, prelude::*, @@ -910,7 +910,7 @@ unsafe impl Sync for Registration {} /// thread. unsafe impl Send for Registration {} =20 -impl Registration { +impl Registration { const VTABLE: bindings::cpufreq_driver =3D bindings::cpufreq_driver { name: Self::copy_name(T::NAME), boost_enabled: T::BOOST_ENABLED, @@ -1044,10 +1044,10 @@ pub fn new() -> Result { =20 /// Same as [`Registration::new`], but does not return a [`Registratio= n`] instance. /// - /// Instead the [`Registration`] is owned by [`Devres`] and will be re= voked / dropped, once the + /// Instead the [`Registration`] is owned by [`kernel::devres`] and wi= ll be dropped, once the /// device is detached. pub fn new_foreign_owned(dev: &Device) -> Result { - Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL) + devres::register_foreign_boxed(dev, Self::new()?, GFP_KERNEL) } } =20 diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index a7df9fbd724f..04435e810249 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -9,12 +9,12 @@ alloc::Flags, bindings, device::{Bound, Device}, - error::{Error, Result}, + error::{to_result, Error, Result}, ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, sync::{rcu, Arc, Completion}, - types::ARef, + types::{ARef, ForeignOwnable}, }; =20 #[pin_data] @@ -182,14 +182,6 @@ pub fn new(dev: &Device, data: T, flags: Flags)= -> Result { Ok(Devres(inner)) } =20 - /// Same as [`Devres::new`], but does not return a `Devres` instance. = Instead the given `data` - /// is owned by devres and will be revoked / dropped, once the device = is detached. - pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -= > Result { - let _ =3D DevresInner::new(dev, data, flags)?; - - Ok(()) - } - /// Obtain `&'a T`, bypassing the [`Revocable`]. /// /// This method allows to directly obtain a `&'a T`, bypassing the [`R= evocable`], by presenting @@ -259,3 +251,64 @@ fn drop(&mut self) { } } } + +/// Consume `data` and [`Drop::drop`] `data` once `dev` is unbound. +fn register_foreign(dev: &Device, data: P) -> Re= sult { + let ptr =3D data.into_foreign(); + + #[allow(clippy::missing_safety_doc)] + unsafe extern "C" fn callback(ptr: *mut kernel::ffi= ::c_void) { + // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked abo= ve and hence valid. + let _ =3D unsafe { P::from_foreign(ptr.cast()) }; + } + + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid and bound device. + // - `ptr` is a valid pointer the `ForeignOwnable` devres takes owners= hip of. + to_result(unsafe { + // `devm_add_action_or_reset()` also calls `callback` on failure, = such that the + // `ForeignOwnable` is released eventually. + bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::), ptr.cast()) + }) +} + +/// Encapsulate `data` in a [`KBox`] and [`Drop::drop`] `data` once `dev` = is unbound. +/// +/// # Examples +/// +/// ```no_run +/// use kernel::{device::{Bound, Device}, devres}; +/// +/// struct Registration; +/// +/// impl Registration { +/// fn new() -> Self { +/// // register (e.g. class device, IRQ, etc.) +/// +/// Self +/// } +/// } +/// +/// impl Drop for Registration { +/// fn drop(&mut self) { +/// // unregister +/// } +/// } +/// +/// fn from_bound_context(dev: &Device) -> Result { +/// devres::register_foreign_boxed(dev, Registration::new(), GFP_KERNE= L) +/// } +/// ``` +pub fn register_foreign_boxed( + dev: &Device, + data: impl PinInit, + flags: Flags, +) -> Result +where + T: 'static, + Error: From, +{ + let data =3D KBox::pin_init(data, flags)?; + + register_foreign(dev, data) +} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index acb638086131..3b0cb80c1984 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -5,9 +5,7 @@ //! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/dr= m_drv.h) =20 use crate::{ - bindings, device, - devres::Devres, - drm, + bindings, device, devres, drm, error::{to_result, Result}, prelude::*, str::CStr, @@ -120,7 +118,7 @@ pub trait Driver { /// Once the `Registration` structure is dropped, the device is unregister= ed. pub struct Registration(ARef>); =20 -impl Registration { +impl Registration { /// Creates a new [`Registration`] and registers it. fn new(drm: &drm::Device, flags: usize) -> Result { // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Devi= ce`. @@ -130,7 +128,7 @@ fn new(drm: &drm::Device, flags: usize) -> Result { } =20 /// Same as [`Registration::new`}, but transfers ownership of the [`Re= gistration`] to - /// [`Devres`]. + /// [`devres::register_foreign_boxed`]. pub fn new_foreign_owned( drm: &drm::Device, dev: &device::Device, @@ -141,7 +139,8 @@ pub fn new_foreign_owned( } =20 let reg =3D Registration::::new(drm, flags)?; - Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + + devres::register_foreign_boxed(dev, reg, GFP_KERNEL) } =20 /// Returns a reference to the `Device` instance for this registration. --=20 2.49.0 From nobody Fri Oct 10 21:13:59 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3CD1826B970; Thu, 12 Jun 2025 14:52:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739922; cv=none; b=TK6LctXcCRQZxYGuYHZ0EI26QKUa86dIxN73N1dJ5EPu9wIlMbvTtDyyyPxX1XjxZqGcn3D/Kh2tXBK3X/Zvr0ptxg6ZsGtpFnmrW3BYwC5oPMO2lQPMmjli74PNg8RAHyKzeYtzaJCX5g3B9MIcqKS23UzVnhmg2RjjItGj0rY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739922; c=relaxed/simple; bh=XNbyQvNYccJTOv6dH/h3uYQlcHJVV26M8KvK2cPi59s=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=nt9HlfEOTeoJgwht+f1vIpAVxSCbxMqZeFMrz3BzRsaYkrpHl0ZExHGlzAnmjWonYYleiQ+4urHalKcWwBYtPbsyqmUQER9AO3ZYOkRam65bvx6gYIlPmPciKNIQee/q1Vlr5IT1B3SodAAaF/9MVn0Qgl/YiyEplPVeZR0VeMQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=sG3et+QL; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="sG3et+QL" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9B7BBC4CEEB; Thu, 12 Jun 2025 14:51:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749739921; bh=XNbyQvNYccJTOv6dH/h3uYQlcHJVV26M8KvK2cPi59s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sG3et+QL8OWtLZysgJPymkUeoxuD2ryUA0xYsu+cvmFxXp496Fv+3vkWlY2+9Tqbo qe4/7B+LZaYR8y0/HTWsNxP1Ywu6VxWPYKZhSj+ns/vcpohzfVDtzszKHOR5+I6xii cfbPySTlypO8bOUs4NSORcgY7/vX/rzfQqVzJPI1lfDfzOhVC54dzKrFRciG+aswpa gk/ULpOCqnvLQ/Zdhx+8CEUBzI+XdtcnVFZS00MTRt3IPT/YlQkl2bSBOp94LJY2IM nfi5tZVHblDXOg782hP/CVE5UbQHaLnYMlxPTfh31vrj2B/c9W4GSNO7MjeC+Vpqzf z2p7oeCm7InDw== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, benno.lossin@proton.me, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Danilo Krummrich Subject: [PATCH 3/4] rust: devres: get rid of Devres' inner Arc Date: Thu, 12 Jun 2025 16:51:40 +0200 Message-ID: <20250612145145.12143-4-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250612145145.12143-1-dakr@kernel.org> References: <20250612145145.12143-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" So far Devres uses an inner memory allocation and reference count, i.e. an inner Arc, in order to ensure that the devres callback can't run into a use-after-free in case where the Devres object is dropped while the devres callback runs concurrently. Instead, use a completion in order to avoid a potential UAF: In Devres::drop(), if we detect that we can't remove the devres action anymore, we wait for the completion that is completed from the devres callback. If, in turn, we were able to successfully remove the devres action, we can just go ahead. This, again, allows us to get rid of the internal Arc, and instead let Devres consume an `impl PinInit` in order to return an `impl PinInit, E>`, which enables us to get away with less memory allocations. Additionally, having the resulting explicit synchronization in Devres::drop() prevents potential subtle undesired side effects of the devres callback dropping the final Arc reference asynchronously within the devres callback. Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/driver.rs | 7 +- drivers/gpu/nova-core/gpu.rs | 6 +- rust/kernel/devres.rs | 187 +++++++++++++++----------------- rust/kernel/pci.rs | 20 ++-- samples/rust/rust_driver_pci.rs | 19 ++-- 5 files changed, 117 insertions(+), 122 deletions(-) diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver= .rs index 8c86101c26cb..110f2b355db4 100644 --- a/drivers/gpu/nova-core/driver.rs +++ b/drivers/gpu/nova-core/driver.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 =20 -use kernel::{auxiliary, bindings, c_str, device::Core, pci, prelude::*}; +use kernel::{auxiliary, bindings, c_str, device::Core, pci, prelude::*, sy= nc::Arc}; =20 use crate::gpu::Gpu; =20 @@ -34,7 +34,10 @@ fn probe(pdev: &pci::Device, _info: &Self::IdInfo)= -> Result(0, c_str!("nova-c= ore/bar0"))?; + let bar =3D Arc::pin_init( + pdev.iomap_region_sized::(0, c_str!("nova-core/bar0= ")), + GFP_KERNEL, + )?; =20 let this =3D KBox::pin_init( try_pin_init!(Self { diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 60b86f370284..47653c14838b 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 =20 -use kernel::{device, devres::Devres, error::code::*, pci, prelude::*}; +use kernel::{device, devres::Devres, error::code::*, pci, prelude::*, sync= ::Arc}; =20 use crate::driver::Bar0; use crate::firmware::{Firmware, FIRMWARE_VERSION}; @@ -161,14 +161,14 @@ fn new(bar: &Bar0) -> Result { pub(crate) struct Gpu { spec: Spec, /// MMIO mapping of PCI BAR 0 - bar: Devres, + bar: Arc>, fw: Firmware, } =20 impl Gpu { pub(crate) fn new( pdev: &pci::Device, - devres_bar: Devres, + devres_bar: Arc>, ) -> Result> { let bar =3D devres_bar.access(pdev.as_ref())?; let spec =3D Spec::new(bar)?; diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 04435e810249..4ee9037f4ad4 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -13,20 +13,10 @@ ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, - sync::{rcu, Arc, Completion}, + sync::{rcu, Completion}, types::{ARef, ForeignOwnable}, }; =20 -#[pin_data] -struct DevresInner { - dev: ARef, - callback: unsafe extern "C" fn(*mut c_void), - #[pin] - data: Revocable, - #[pin] - revoke: Completion, -} - /// This abstraction is meant to be used by subsystems to containerize [`D= evice`] bound resources to /// manage their lifetime. /// @@ -86,100 +76,93 @@ struct DevresInner { /// # fn no_run(dev: &Device) -> Result<(), Error> { /// // SAFETY: Invalid usage for example purposes. /// let iomem =3D unsafe { IoMem::<{ core::mem::size_of::() }>::new(0= xBAAAAAAD)? }; -/// let devres =3D Devres::new(dev, iomem, GFP_KERNEL)?; +/// let devres =3D KBox::pin_init(Devres::new(dev, iomem), GFP_KERNEL)?; /// /// let res =3D devres.try_access().ok_or(ENXIO)?; /// res.write8(0x42, 0x0); /// # Ok(()) /// # } /// ``` -pub struct Devres(Arc>); - -impl DevresInner { - fn new(dev: &Device, data: T, flags: Flags) -> Result>> { - let inner =3D Arc::pin_init( - try_pin_init!( DevresInner { - dev: dev.into(), - callback: Self::devres_callback, - data <- Revocable::new(data), - revoke <- Completion::new(), - }), - flags, - )?; - - // Convert `Arc` into a raw pointer and make devres o= wn this reference until - // `Self::devres_callback` is called. - let data =3D inner.clone().into_raw(); +#[pin_data(PinnedDrop)] +pub struct Devres { + dev: ARef, + callback: unsafe extern "C" fn(*mut c_void), + #[pin] + data: Revocable, + #[pin] + devm: Completion, + #[pin] + revoke: Completion, +} =20 - // SAFETY: `devm_add_action` guarantees to call `Self::devres_call= back` once `dev` is - // detached. - let ret =3D - unsafe { bindings::devm_add_action(dev.as_raw(), Some(inner.ca= llback), data as _) }; +impl Devres { + /// Creates a new [`Devres`] instance of the given `data`. The `data` = encapsulated within the + /// returned `Devres` instance' `data` will be revoked once the device= is detached. + pub fn new<'a, E>( + dev: &'a Device, + data: impl PinInit + 'a, + ) -> impl PinInit + 'a + where + T: 'a, + Error: From, + { + let callback =3D Self::devres_callback; =20 - if ret !=3D 0 { - // SAFETY: We just created another reference to `inner` in ord= er to pass it to - // `bindings::devm_add_action`. If `bindings::devm_add_action`= fails, we have to drop - // this reference accordingly. - let _ =3D unsafe { Arc::from_raw(data) }; - return Err(Error::from_errno(ret)); - } + try_pin_init!(&this in Self { + data <- Revocable::new(data), + devm <- Completion::new(), + revoke <- Completion::new(), + callback, + dev: { + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid bound device. + // - `this` is guaranteed to be a valid for the duration o= f the lifetime of `Self`. + // - `devm_add_action()` is guaranteed not to call `callba= ck` until `this` has been + // properly initialized, because we require `dev` (i.e.= the *bound* device) to + // live at least as long as the returned `impl PinInit<= Self, Error>`. + to_result(unsafe { + bindings::devm_add_action(dev.as_raw(), Some(callback)= , this.as_ptr().cast()) + })?; =20 - Ok(inner) + dev.into() + }, + }) } =20 fn as_ptr(&self) -> *const Self { - self as _ - } - - fn remove_action(this: &Arc) -> bool { - // SAFETY: - // - `self.inner.dev` is a valid `Device`, - // - the `action` and `data` pointers are the exact same ones as g= iven to devm_add_action() - // previously, - // - `self` is always valid, even if the action has been released = already. - let success =3D unsafe { - bindings::devm_remove_action_nowarn( - this.dev.as_raw(), - Some(this.callback), - this.as_ptr() as _, - ) - } =3D=3D 0; - - if success { - // SAFETY: We leaked an `Arc` reference to devm_add_action() i= n `DevresInner::new`; if - // devm_remove_action_nowarn() was successful we can (and have= to) claim back ownership - // of this reference. - let _ =3D unsafe { Arc::from_raw(this.as_ptr()) }; - } - - success + self } =20 #[allow(clippy::missing_safety_doc)] unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) { - let ptr =3D ptr as *mut DevresInner; - // Devres owned this memory; now that we received the callback, dr= op the `Arc` and hence the - // reference. - // SAFETY: Safe, since we leaked an `Arc` reference to devm_add_ac= tion() in - // `DevresInner::new`. - let inner =3D unsafe { Arc::from_raw(ptr) }; + // SAFETY: In `Self::new` we've passed a valid pointer to `Self` t= o `devm_add_action()`, + // hence `ptr` must be a valid pointer to `Self`. + let this =3D unsafe { &*ptr.cast::() }; =20 - if !inner.data.revoke() { + if !this.data.revoke() { // If `revoke()` returns false, it means that `Devres::drop` a= lready started revoking - // `inner.data` for us. Hence we have to wait until `Devres::d= rop()` signals that it - // completed revoking `inner.data`. - inner.revoke.wait_for_completion(); + // `data` for us. Hence we have to wait until `Devres::drop` s= ignals that it + // completed revoking `data`. + this.revoke.wait_for_completion(); } - } -} =20 -impl Devres { - /// Creates a new [`Devres`] instance of the given `data`. The `data` = encapsulated within the - /// returned `Devres` instance' `data` will be revoked once the device= is detached. - pub fn new(dev: &Device, data: T, flags: Flags) -> Result= { - let inner =3D DevresInner::new(dev, data, flags)?; + // Signal that we're done using `this`. + this.devm.complete_all(); + } =20 - Ok(Devres(inner)) + fn remove_action(&self) -> bool { + // SAFETY: + // - `self.dev` is a valid `Device`, + // - the `action` and `data` pointers are the exact same ones as g= iven to devm_add_action() + // previously, + // - `self` is always valid, even if the action has been released = already. + (unsafe { + bindings::devm_remove_action_nowarn( + self.dev.as_raw(), + Some(self.callback), + self.as_ptr().cast_mut().cast(), + ) + } =3D=3D 0) } =20 /// Obtain `&'a T`, bypassing the [`Revocable`]. @@ -211,43 +194,51 @@ pub fn new(dev: &Device, data: T, flags: Flags= ) -> Result { /// } /// ``` pub fn access<'a>(&'a self, dev: &'a Device) -> Result<&'a T> { - if self.0.dev.as_raw() !=3D dev.as_raw() { + if self.dev.as_raw() !=3D dev.as_raw() { return Err(EINVAL); } =20 // SAFETY: `dev` being the same device as the device this `Devres`= has been created for - // proves that `self.0.data` hasn't been revoked and is guaranteed= to not be revoked as - // long as `dev` lives; `dev` lives at least as long as `self`. - Ok(unsafe { self.0.data.access() }) + // proves that `self.data` hasn't been revoked and is guaranteed t= o not be revoked as long + // as `dev` lives; `dev` lives at least as long as `self`. + Ok(unsafe { self.data.access() }) } =20 /// [`Devres`] accessor for [`Revocable::try_access`]. pub fn try_access(&self) -> Option> { - self.0.data.try_access() + self.data.try_access() } =20 /// [`Devres`] accessor for [`Revocable::try_access_with`]. pub fn try_access_with R>(&self, f: F) -> Option { - self.0.data.try_access_with(f) + self.data.try_access_with(f) } =20 /// [`Devres`] accessor for [`Revocable::try_access_with_guard`]. pub fn try_access_with_guard<'a>(&'a self, guard: &'a rcu::Guard) -> O= ption<&'a T> { - self.0.data.try_access_with_guard(guard) + self.data.try_access_with_guard(guard) } } =20 -impl Drop for Devres { - fn drop(&mut self) { +#[pinned_drop] +impl PinnedDrop for Devres { + fn drop(self: Pin<&mut Self>) { // SAFETY: When `drop` runs, it is guaranteed that nobody is acces= sing the revocable data // anymore, hence it is safe not to wait for the grace period to f= inish. - if unsafe { self.0.data.revoke_nosync() } { - // We revoked `self.0.data` before the devres action did, henc= e try to remove it. - if !DevresInner::remove_action(&self.0) { + if unsafe { self.data.revoke_nosync() } { + // We revoked `self.data` before the devres action did, hence = try to remove it. + if !self.remove_action() { // We could not remove the devres action, which means that= it now runs concurrently, - // hence signal that `self.0.data` has been revoked succes= sfully. - self.0.revoke.complete_all(); + // hence signal that `self.data` has been revoked by us su= ccessfully. + self.revoke.complete_all(); + + // Wait for `Self::devres_callback` to be done using this = object. + self.devm.wait_for_completion(); } + } else { + // `Self::devres_callback` revokes `self.data` for us, hence w= ait for it to be done + // using this object. + self.devm.wait_for_completion(); } } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 8435f8132e38..db0eb7eaf9b1 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -5,7 +5,6 @@ //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h) =20 use crate::{ - alloc::flags::*, bindings, container_of, device, device_id::RawDeviceId, devres::Devres, @@ -398,19 +397,20 @@ pub fn resource_len(&self, bar: u32) -> Result { impl Device { /// Mapps an entire PCI-BAR after performing a region-request on it. I= /O operation bound checks /// can be performed on compile time for offsets (plus the requested t= ype size) < SIZE. - pub fn iomap_region_sized( - &self, + pub fn iomap_region_sized<'a, const SIZE: usize>( + &'a self, bar: u32, - name: &CStr, - ) -> Result>> { - let bar =3D Bar::::new(self, bar, name)?; - let devres =3D Devres::new(self.as_ref(), bar, GFP_KERNEL)?; - - Ok(devres) + name: &'a CStr, + ) -> impl PinInit>, Error> + 'a { + Devres::new(self.as_ref(), Bar::::new(self, bar, name)) } =20 /// Mapps an entire PCI-BAR after performing a region-request on it. - pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result> { + pub fn iomap_region<'a>( + &'a self, + bar: u32, + name: &'a CStr, + ) -> impl PinInit, Error> + 'a { self.iomap_region_sized::<0>(bar, name) } } diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci= .rs index 15147e4401b2..5c35f1414172 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -25,8 +25,10 @@ impl TestIndex { const NO_EVENTFD: Self =3D Self(0); } =20 +#[pin_data(PinnedDrop)] struct SampleDriver { pdev: ARef, + #[pin] bar: Devres, } =20 @@ -73,13 +75,11 @@ fn probe(pdev: &pci::Device, info: &Self::IdInfo)= -> Result pdev.enable_device_mem()?; pdev.set_master(); =20 - let bar =3D pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!("ru= st_driver_pci"))?; - - let drvdata =3D KBox::new( - Self { + let drvdata =3D KBox::pin_init( + try_pin_init!(Self { pdev: pdev.into(), - bar, - }, + bar <- pdev.iomap_region_sized::<{ Regs::END }>(0, c_str!(= "rust_driver_pci")), + }), GFP_KERNEL, )?; =20 @@ -90,12 +90,13 @@ fn probe(pdev: &pci::Device, info: &Self::IdInfo)= -> Result Self::testdev(info, bar)? ); =20 - Ok(drvdata.into()) + Ok(drvdata) } } =20 -impl Drop for SampleDriver { - fn drop(&mut self) { +#[pinned_drop] +impl PinnedDrop for SampleDriver { + fn drop(self: Pin<&mut Self>) { dev_dbg!(self.pdev.as_ref(), "Remove Rust PCI driver sample.\n"); } } --=20 2.49.0 From nobody Fri Oct 10 21:13:59 2025 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1622B268C49; Thu, 12 Jun 2025 14:52:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739925; cv=none; b=u+vY035+zEY+7oYfh/wl0FliSo/TXlEV9soEBt8ahWxk5PUxiHAwvvvgs4NtH7I1r0wRhGQXlRCEovx+0PKKamsSPK0qe1vRJy4IeXDUlvWbPvbTcHtQ1qaiShx4zNSgde/09cSz+pbxSms8PiUCbGvqOSeIX6n9b/XgkVAw20k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1749739925; c=relaxed/simple; bh=xsSP2aCmuKNeUb9/6TPkcxw0m7gqqcmC2Tvep0z1Mtg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=rIdQ00VSvOFxOxm90TPDFAIvUk24J8PRcr8qR/ncZDLIbqaSNEljjHB79N4gvOsa7THK6TMCt3Mdgmr24Be1t7D1nbKb/uTP/NjETu4qX++L1h8RTCPI2P8N0o7sgishPYnwGimMcljsK2mnwX966b2DtQ3xYoVgw3QK7jVSooo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=a3aMkbiX; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="a3aMkbiX" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1C0DBC4CEF1; Thu, 12 Jun 2025 14:52:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749739924; bh=xsSP2aCmuKNeUb9/6TPkcxw0m7gqqcmC2Tvep0z1Mtg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a3aMkbiXHsxMSliOnEGuZCYFTeOmtRjxDrk84ibpasOPQF05worvc0b5FJxBhMieg sUOV+syF/bNxGBEeciN74Pa8E1GLKoZhKvX3Z+cGBb4dEn9Aqi+StwGvesEYWcakmk Ao2Sb3cjAjDqCu1vEy1eSJ3TQMyOy22dqqPhmlCgbpmorOF9i4CGm0ASfr/KxXeDMV trA+YPdfjmrWvy2/nApcVsPQ78WO51LLd6o52Ck4+yJpL/1kVPr32usEbIIR66d8c3 3SBLUAZSjoB8Zwm7txIlGHinXflA9OYhCRtoRE+qZb9iLZiMbBx8JvzbvzjGsNgD++ 3gJwFjH0/kFeQ== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org, alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, benno.lossin@proton.me, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Danilo Krummrich Subject: [PATCH 4/4] rust: devres: implement register_foreign_release() Date: Thu, 12 Jun 2025 16:51:41 +0200 Message-ID: <20250612145145.12143-5-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250612145145.12143-1-dakr@kernel.org> References: <20250612145145.12143-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" register_foreign_release() is useful when a device resource has associated data, but does not require the capability of accessing it or manually releasing it. If we would want to be able to access the device resource and release the device resource manually before the device is unbound, but still keep access to the associated data, we could implement it as follows. struct Registration { inner: Devres, data: T, } However, if we never need to access the resource or release it manually, register_foreign_release() is great optimization for the above, since it does not require the synchronization of the Devres type. Suggested-by: Alice Ryhl Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 80 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 4ee9037f4ad4..495dca6240fc 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -16,6 +16,7 @@ sync::{rcu, Completion}, types::{ARef, ForeignOwnable}, }; +use core::ops::Deref; =20 /// This abstraction is meant to be used by subsystems to containerize [`D= evice`] bound resources to /// manage their lifetime. @@ -303,3 +304,82 @@ pub fn register_foreign_boxed( =20 register_foreign(dev, data) } + +/// To be implemented by an object passed to [`register_foreign_release`]. +pub trait Release { + /// Called once the [`Device`] given to [`register_foreign_release`] i= s unbound. + fn release(&self); +} + +impl Release for crate::sync::ArcBorrow<'_, T> { + fn release(&self) { + self.deref().release(); + } +} + +impl Release for Pin<&'_ T> { + fn release(&self) { + self.deref().release(); + } +} + +/// Consume the `data`, [`Release::release`] and [`Drop::drop`] `data` onc= e `dev` is unbound. +/// +/// # Examples +/// +/// ```no_run +/// use kernel::{device::{Bound, Device}, devres, devres::Release, sync::A= rc}; +/// +/// struct Registration { +/// data: T, +/// } +/// +/// impl Registration { +/// fn new(data: T) -> Result> { +/// // register (e.g. class device, IRQ, etc.) +/// +/// Ok(Arc::new(Self { data }, GFP_KERNEL)?) +/// } +/// } +/// +/// impl Release for Registration { +/// fn release(&self) { +/// // unregister +/// } +/// } +/// +/// fn from_bound_context(dev: &Device) -> Result { +/// let reg =3D Registration::new(0x42)?; +/// +/// devres::register_foreign_release(dev, reg.clone()) +/// } +/// ``` +pub fn register_foreign_release

(dev: &Device, data: P) -> Result +where + P: ForeignOwnable, + for<'a> P::Borrowed<'a>: Release, +{ + let ptr =3D data.into_foreign(); + + #[allow(clippy::missing_safety_doc)] + unsafe extern "C" fn callback

(ptr: *mut kernel::ffi::c_void) + where + P: ForeignOwnable, + for<'a> P::Borrowed<'a>: Release, + { + // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked abo= ve and hence valid. + unsafe { P::borrow(ptr.cast()) }.release(); + + // SAFETY: `ptr` is the pointer to the `ForeignOwnable` leaked abo= ve and hence valid. + let _ =3D unsafe { P::from_foreign(ptr.cast()) }; + } + + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid and bound device. + // - `ptr` is a valid pointer the `ForeignOwnable` devres takes owners= hip of. + to_result(unsafe { + // `devm_add_action_or_reset()` also calls `callback` on failure, = such that the + // `ForeignOwnable` is released eventually. + bindings::devm_add_action_or_reset(dev.as_raw(), Some(callback::), ptr.cast()) + }) +} --=20 2.49.0