From nobody Thu Oct 9 00:37:01 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 928B32AE8E; Sun, 22 Jun 2025 16:41:25 +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=1750610485; cv=none; b=IzVWXVWAmtMMSicw5PewERddKbcyrWKQG9NCDlh3O36/ebXb/0EvYbDUepopPA1h2IwAgsTUyo5koORk4gfZ3WNSYks+rri8kq8t+02JEASNP6nafqvrPSODm3NNHmdt9o4j/xzXyv8NdVbmdwPyW/Lam7Cw/TjKREsCcfuqH68= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610485; c=relaxed/simple; bh=U2lNMOoZ5HlWXq74SYihka3ZDd0webYnAlwntlr3428=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=gPFrd0Pd2HI3L5Px4JwTR+y0jd0mliHjVVkVN+CZa+f0ZeGk+sLfJpcaWDfrlwqwXam37Ter4pzFQURfbRj8lUYtAQP3kRU7tQZPsg/uNQnx4ysHleThL6nhm9/alnlTPUzXhYMzZBHe0VFAOrkYlMTVyOIoz275PTCtnDx3C6s= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=USPwYkms; 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="USPwYkms" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 44C03C4CEF0; Sun, 22 Jun 2025 16:41:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750610485; bh=U2lNMOoZ5HlWXq74SYihka3ZDd0webYnAlwntlr3428=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=USPwYkmsvVqoai+rnv13hTNqXdpyVmFGzCSR5wWewjyWh8FGrEe98vYicD+NmUtMW QYUoiNB/LNfHKnS2z6tLwvFfIHkorXjOFCcWWyHiIbFD1qrx6/kfzuEJRt4Dn//UuZ iuZbzsIwMcGuhMT3u8K2rPy9ix4AoS/ueVJiXNwdXTio9W3tRRyYdNk5yXFVqTGIPw 7gSKzTjMP4De8Dn5SXZsuz/K5rlgaY2Ik731+DRklyM/fXIKZF6MnOhdr3M+kp7yJr Zd5xZewo1SkFZ9YA1cJm9ZyagvVnE8xzZ0CuKOS7PG0i5buDxB4uFLrqarNVGdynFd 5WLlNwXUcliyQ== 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, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 1/4] rust: revocable: support fallible PinInit types Date: Sun, 22 Jun 2025 18:40:38 +0200 Message-ID: <20250622164050.20358-2-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250622164050.20358-1-dakr@kernel.org> References: <20250622164050.20358-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 Acked-by: Miguel Ojeda Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin --- rust/kernel/devres.rs | 2 +- rust/kernel/revocable.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 57502534d985..544e50efab43 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -100,7 +100,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..46768b374656 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -82,11 +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 { + try_pin_init!(Self { is_available: AtomicBool::new(true), data <- Opaque::pin_init(data), - }) + }? E) } =20 /// Tries to access the revocable wrapped object. --=20 2.49.0 From nobody Thu Oct 9 00:37:01 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 94CDA2AE8E; Sun, 22 Jun 2025 16:41:30 +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=1750610490; cv=none; b=K+ZWBpG1CcXp25zPM/a4CYlSt8FiHGnuEHAICNwOCpDKI5qWgL/fY0RkUT3hK0Ye52gQXfqERXwgJ0NGeoCZQGeQYAyEyuESUmKITspalZmH6diRTdGgNQnWRi0uh+1ihR0tF7CxHBuszeMJGE0q69B2zfLwXBaBEd9sVip6R/I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610490; c=relaxed/simple; bh=qd4m+pRuwBT7Lu3gw5UGth4bTaVLrbANlY/hRPsaidQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=DV/MfXj7rDlGLSL/O7Hr7vPmJvySa/gxnz08zTpUpZ8A1VBsXRP/bJyhc5q933cr3qfrJ3v9uvlvOYMOVhxP1NBw5o+nJwgjI0z1bF9TrDn+3CWCRmE9eolFKEapIMGIVlI60eDGIRK9gYekI2NQHbrAcRE/0iTd3hQAFxndEV8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=W+62q8nQ; 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="W+62q8nQ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9EAFBC4CEF1; Sun, 22 Jun 2025 16:41:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750610490; bh=qd4m+pRuwBT7Lu3gw5UGth4bTaVLrbANlY/hRPsaidQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W+62q8nQUI17F5oF98Yvcj/CC+uM199E6m8Lox7oQ/CfndPAiOX+QGiZDDeimjCNj qzhp0lOjOf/TABucF8NCeWSuvzNoQw8cAro1OtrMespJNYBNmbInbFe8RLPaklTAbD D0plLnCSR+wUeEKGODnQI+2M/bMN2JfNlgU0EytUXw6CrWP8p7mw5RX1MbgwzsLo8/ btrfti0glsvg6XgrT+kbG+ETODdyFgR54IDlnWRKSFej82Ppvvm7U8viOMotHPBc1a yYjn5F6dcCZrKpX0qoQPqbUCnJGGD49uDvYAZxOZ8mPuWZ/4Z55GTDN3d+qQQpQ+Ef iAQNgb3rqXlKw== 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, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Danilo Krummrich , Dave Airlie , Simona Vetter , Viresh Kumar Subject: [PATCH v2 2/4] rust: devres: replace Devres::new_foreign_owned() Date: Sun, 22 Jun 2025 18:40:39 +0200 Message-ID: <20250622164050.20358-3-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250622164050.20358-1-dakr@kernel.org> References: <20250622164050.20358-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(). 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(), 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 Acked-by: Viresh Kumar Signed-off-by: Danilo Krummrich Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin --- rust/helpers/device.c | 7 ++++ rust/kernel/cpufreq.rs | 11 +++--- rust/kernel/devres.rs | 70 +++++++++++++++++++++++++++++++++------ rust/kernel/drm/driver.rs | 14 ++++---- 4 files changed, 82 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 11b03e9d7e89..dd84e2b4d7ae 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -13,7 +13,7 @@ cpu::CpuId, 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::*, @@ -1046,10 +1046,13 @@ 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 [`devres::register`] and = will be dropped, once the /// device is detached. - pub fn new_foreign_owned(dev: &Device) -> Result { - Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL) + pub fn new_foreign_owned(dev: &Device) -> Result + where + T: 'static, + { + devres::register(dev, Self::new()?, GFP_KERNEL) } } =20 diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 544e50efab43..250073749279 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] @@ -184,14 +184,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 @@ -261,3 +253,61 @@ 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}; +/// +/// /// Registration of e.g. a class device, IRQ, etc. +/// struct Registration; +/// +/// impl Registration { +/// fn new() -> Self { +/// // register +/// +/// Self +/// } +/// } +/// +/// impl Drop for Registration { +/// fn drop(&mut self) { +/// // unregister +/// } +/// } +/// +/// fn from_bound_context(dev: &Device) -> Result { +/// devres::register(dev, Registration::new(), GFP_KERNEL) +/// } +/// ``` +pub fn register(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..f63addaf7235 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, @@ -130,18 +128,22 @@ fn new(drm: &drm::Device, flags: usize) -> Result<= Self> { } =20 /// Same as [`Registration::new`}, but transfers ownership of the [`Re= gistration`] to - /// [`Devres`]. + /// [`devres::register`]. pub fn new_foreign_owned( drm: &drm::Device, dev: &device::Device, flags: usize, - ) -> Result { + ) -> Result + where + T: 'static, + { if drm.as_ref().as_raw() !=3D dev.as_raw() { return Err(EINVAL); } =20 let reg =3D Registration::::new(drm, flags)?; - Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + + devres::register(dev, reg, GFP_KERNEL) } =20 /// Returns a reference to the `Device` instance for this registration. --=20 2.49.0 From nobody Thu Oct 9 00:37:01 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 E740C2AE8E; Sun, 22 Jun 2025 16:41:34 +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=1750610495; cv=none; b=WQ+eck3Bmw6XxyOIqBIrLpB/xyUrs/TtIvnpo1g34OASniEGoHeunDF3BY7BpRr/inqmRyaPDqF+0WneJKTC7v360gb7fBDG7RYrkndGI2Ej23chRPyy6X0fjuUtI7rP8YU2+TI0D90YXVwNtDWTk2BdIG7oPCfh4pHkp1phUNk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610495; c=relaxed/simple; bh=vll6R+1yyMdNdcYfvLHVws/nFKfFPPbIweQImlIq800=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=PP4cdYmhGa3MR+mX8I8L3k9GB8IERibodSrvjE5RUITTyjU3jV2/9FOAxaW1Z3e/r3D9VjBidHBO3P9KNZgP+K7NDQPLFJ7TSy8drec9xjHV/1g/kHwvAsUs+sVruVS6j2fX1EQv0ffhqzBAXO0AyKxMSPXQly7cLUmq1QI8p1o= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hRRcUYIG; 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="hRRcUYIG" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88D66C4CEE3; Sun, 22 Jun 2025 16:41:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750610494; bh=vll6R+1yyMdNdcYfvLHVws/nFKfFPPbIweQImlIq800=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hRRcUYIGVirrySADg0Lydl1tbeJ/joF1vZcq/05N/9Y0DaUszT3aR0KH9OhNbxBps WKut1m5Wlj9MXMaHEXheEjXpuDwb3KNhHF97Z7fAbzdv1rFnRcFXdV5fkarLfW9+Au 5sVvFdVWFFD3fQRI1JlJMXCBBY1/moan8xOXNuKyvH1Ko22pzmk3KoZjABlHwLZ1fC mSChWA1NbyJyPJsE5nG27Eb9z+htM10cty1nBir4RoarMAf0Nm89YfcT6qWKpuRW6Z OVVzG2+y6cBJJEs0Kpob+8sSgeG0tWLeKmHJ0IM3D/HD8kb3DG85gFgr2nzm6iqcQA DdEZqMZT3p83w== 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, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 3/4] rust: devres: get rid of Devres' inner Arc Date: Sun, 22 Jun 2025 18:40:40 +0200 Message-ID: <20250622164050.20358-4-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250622164050.20358-1-dakr@kernel.org> References: <20250622164050.20358-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 | 208 +++++++++++++++++++------------- rust/kernel/pci.rs | 20 +-- samples/rust/rust_driver_pci.rs | 19 +-- 5 files changed, 149 insertions(+), 111 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 250073749279..15a0a94e992b 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -13,20 +13,31 @@ ffi::c_void, prelude::*, revocable::{Revocable, RevocableGuard}, - sync::{rcu, Arc, Completion}, - types::{ARef, ForeignOwnable}, + sync::{rcu, Completion}, + types::{ARef, ForeignOwnable, Opaque}, }; =20 +use pin_init::Wrapper; + +/// [`Devres`] inner data accessed from [`Devres::callback`]. #[pin_data] -struct DevresInner { - dev: ARef, - callback: unsafe extern "C" fn(*mut c_void), +struct Inner { #[pin] data: Revocable, + /// Tracks whether [`Devres::callback`] has been completed. + #[pin] + devm: Completion, + /// Tracks whether revoking [`Self::data`] has been completed. #[pin] revoke: Completion, } =20 +impl Inner { + fn as_ptr(&self) -> *const Self { + self + } +} + /// This abstraction is meant to be used by subsystems to containerize [`D= evice`] bound resources to /// manage their lifetime. /// @@ -88,100 +99,106 @@ 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>); +#[pin_data(PinnedDrop)] +pub struct Devres { + dev: ARef, + /// Pointer to [`Self::devres_callback`]. + /// + /// Has to be stored, since Rust does not guarantee to always return t= he same address for a + /// function. However, the C API uses the address as a key. + callback: unsafe extern "C" fn(*mut c_void), + /// Contains all the fields shared with [`Self::callback`]. + // TODO: Replace with `UnsafePinned`, once available. + #[pin] + inner: Opaque>, +} =20 -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, +impl Devres { + /// Creates a new [`Devres`] instance of the given `data`. + /// + /// The `data` encapsulated within the returned `Devres` instance' `da= ta` will be + /// (revoked)[`Revocable`] 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; + + try_pin_init!(&this in Self { + inner <- Opaque::pin_init(try_pin_init!(Inner { data <- Revocable::new(data), + devm <- Completion::new(), 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(); + })), + callback, + dev: { + // SAFETY: It is valid to dereference `this` to find the a= ddress of `inner`. + let inner =3D unsafe { core::ptr::addr_of_mut!((*this.as_p= tr()).inner) }; =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 _) }; - - 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)); - } + // SAFETY: + // - `dev.as_raw()` is a pointer to a valid bound device. + // - `inner` is guaranteed to be a valid for the duration = of 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)= , inner.cast()) + })?; =20 - Ok(inner) + dev.into() + }, + }) } =20 - fn as_ptr(&self) -> *const Self { - self as _ + fn inner(&self) -> &Inner { + // SAFETY: `inner` is valid and properly initialized. + unsafe { &*self.inner.get() } } =20 - 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 + fn data(&self) -> &Revocable { + &self.inner().data } =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 `Inner` = to `devm_add_action()`, + // hence `ptr` must be a valid pointer to `Inner`. + let inner =3D unsafe { &*ptr.cast::>() }; =20 if !inner.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`. + // `data` for us. Hence we have to wait until `Devres::drop` s= ignals that it + // completed revoking `data`. inner.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 `inner`. + inner.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, + (unsafe { + bindings::devm_remove_action_nowarn( + self.dev.as_raw(), + Some(self.callback), + self.inner().as_ptr().cast_mut().cast(), + ) + } =3D=3D 0) } =20 /// Obtain `&'a T`, bypassing the [`Revocable`]. @@ -213,44 +230,61 @@ 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) { +// SAFETY: `Devres` can be send to any task, if `T: Send`. +unsafe impl Send for Devres {} + +// SAFETY: `Devres` can be shared with any task, if `T: Sync`. +unsafe impl Sync for Devres {} + +#[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.inner().revoke.complete_all(); + + // Wait for `Self::devres_callback` to be done using this = object. + self.inner().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.inner().devm.wait_for_completion(); } + + // SAFETY: `inner` is valid for dropping. + unsafe { core::ptr::drop_in_place(self.inner.get()) }; } } =20 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 Thu Oct 9 00:37:01 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 47E5E1F5423; Sun, 22 Jun 2025 16:41:38 +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=1750610499; cv=none; b=qg4nz11V77/kHG0e4kQm4+M+TRoRk2GPW9/7PfRevrHQa/J+IeEqsyweUghNB91uvSw5EkXPJn5EeKXZd15GtbM/en0k21MiJeBu9lPhsqzEZUTpOjKA1UJTahMRDaGbaU8TnQhBzu7URpA0Zjh8zi7OR2N2IxPnz9UyYbluJ+Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750610499; c=relaxed/simple; bh=YeMl5gWrowwdhB4vjOFO00ruw6lOsJ7HM1stb5uHsKY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=DyYDiriZRl8ACkNK6py8ll93QdIKoYUTTla/p+jRjWKhHW6uvGCLsSCJquEmUmRxD7DTX6Ca+QT2qF3qMXU0cZFIAN6zR3jCzLdegi1p5ucp8y/vhIEhLqw0IEEHMoLbf5k2vUN/1Htw3BMn8KHT28bibzjFaCySny2MVr2t0SA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TQTGAJX5; 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="TQTGAJX5" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E3A6CC4CEF0; Sun, 22 Jun 2025 16:41:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750610498; bh=YeMl5gWrowwdhB4vjOFO00ruw6lOsJ7HM1stb5uHsKY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TQTGAJX51ezU2zY2Qj80DiA+W8Q7M4kzfv6oGbedC2oF/PVo+NhKsyr/+oIDwfBEj TaaPPtTheYE9ynzm+UPq9d0mzgTvk1KjrDqOtyVoKypLiaiclpWN4ucrMdmvJaUjgK XRE0vEWJevAseB4suVSHxaWYFce7/NSh6UXbeK82rKQ3fO/rocVOrIluP+ic+JReKa wU/cxojOARs4QmaMC00SmD69x2lnNqf/2JI423gOgrjO211cJopZ4grJ/uwSeGPP8z /V9FZKfT3CMRwbVD5KjC4mW5n1E+DERY0zqXM07MlY1KJRHzHHCy9W8HZonf9yll6Y i37qDKnLJuXJg== 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, lossin@kernel.org, a.hindborg@kernel.org, aliceryhl@google.com, tmgross@umich.edu, david.m.ertman@intel.com, ira.weiny@intel.com, leon@kernel.org, kwilczynski@kernel.org, bhelgaas@google.com Cc: rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 4/4] rust: devres: implement register_release() Date: Sun, 22 Jun 2025 18:40:41 +0200 Message-ID: <20250622164050.20358-5-dakr@kernel.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250622164050.20358-1-dakr@kernel.org> References: <20250622164050.20358-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_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_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 Reviewed-by: Benno Lossin --- rust/kernel/devres.rs | 84 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 15a0a94e992b..4b61e94d34a0 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -16,6 +16,7 @@ sync::{rcu, Completion}, types::{ARef, ForeignOwnable, Opaque}, }; +use core::ops::Deref; =20 use pin_init::Wrapper; =20 @@ -345,3 +346,86 @@ pub fn register(dev: &Device, data: impl = PinInit, flags: Flag =20 register_foreign(dev, data) } + +/// [`Devres`]-releaseable resource. +/// +/// Register an object implementing this trait with [`register_release`]. = Its `release` +/// function will be called once the device is being unbound. +pub trait Release { + /// Called once the [`Device`] given to [`register_release`] is unboun= d. + 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}; +/// +/// /// Registration of e.g. a class device, IRQ, etc. +/// struct Registration { +/// data: T, +/// } +/// +/// impl Registration { +/// fn new(data: T) -> Result> { +/// // register +/// +/// 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_release(dev, reg.clone()) +/// } +/// ``` +pub fn register_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