From patchwork Fri Mar 14 16:09:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Danilo Krummrich X-Patchwork-Id: 14017122 X-Patchwork-Delegate: bhelgaas@google.com 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 B21B7204691; Fri, 14 Mar 2025 16:09:42 +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=1741968582; cv=none; b=V98eS21AWABtH37oX9caGLSQq2fkOjdUbfR0lp9xczgSPPgjHzIhLpEQHYfJBMQK4aHyGnoCLN5j5EAvJXDzrrxWTKariUNX6WS/WZauSK4BF30xmBSEBqGLBBS/wp7E1lnG7Y8JoYWj7cvPJLwxC8J47wdr2j/ZBXcFMsbTtr0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741968582; c=relaxed/simple; bh=akJ9UD9pTqLZxu203+TWIhfr05IV0kfKDAxe/p6scoo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZQleSJQc3O//0MR/+lrgTK78DGabLhZ4Y1zlv/hm9x5TPWhz8x2J9z9RPUNElQ+hBPSVy8dRRclF/3yMF+qMGzsx3uKQtxEdE5c3HrUz9D9JskX8gU8+ksM1BE7wTlEEV7/SJWsODWHuECH1RZOwJUkxVULAYgVXNahu24cWJ6A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=oaCB7CaD; 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="oaCB7CaD" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0CC94C4CEE9; Fri, 14 Mar 2025 16:09:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741968582; bh=akJ9UD9pTqLZxu203+TWIhfr05IV0kfKDAxe/p6scoo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oaCB7CaD50UW2LPaQBWIcd+2TPa/XrqBNaoL0XepuJVj9f0NGEcMmtt1ZdfdVAPdo 734+McUOGv+aGDuuNvcIgynLqhwBdDN6kkPpISJxM2MXH4YUxsYexv0oGI12ngY2GA +NkhTCjoldBsXhqjoiZ6MwcxJDhJ9Y0ztwcjAbE6PKZQ9h3zlpk5/NmNJ4xgfoDqM/ x5a7polT8Wwaf9hiC+ALFibDmyNc2F+I0okfZ7RbkAXdP2u1X1ixB6Om6EB+9t3B/u dbSr3QRi9h51l2zPEV6la0ZgDNuA90h2u29dBPB7OfRDWTvvGnVFJfrc83T5HZvRJb UJSWWelwRbnpw== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, bhelgaas@google.com, 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: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 1/4] rust: pci: use to_result() in enable_device_mem() Date: Fri, 14 Mar 2025 17:09:04 +0100 Message-ID: <20250314160932.100165-2-dakr@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250314160932.100165-1-dakr@kernel.org> References: <20250314160932.100165-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Simplify enable_device_mem() by using to_result() to handle the return value of the corresponding FFI call. Reviewed-by: Benno Lossin Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 4c98b5b9aa1e..386484dcf36e 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -382,12 +382,7 @@ pub fn device_id(&self) -> u16 { /// Enable memory resources for this device. pub fn enable_device_mem(&self) -> Result { // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`. - let ret = unsafe { bindings::pci_enable_device_mem(self.as_raw()) }; - if ret != 0 { - Err(Error::from_errno(ret)) - } else { - Ok(()) - } + to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) }) } /// Enable bus-mastering for this device. From patchwork Fri Mar 14 16:09:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Danilo Krummrich X-Patchwork-Id: 14017123 X-Patchwork-Delegate: bhelgaas@google.com 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 43495204851; Fri, 14 Mar 2025 16:09:45 +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=1741968586; cv=none; b=jbYC4y6BaxElv+EXFrrpPKf8pIruejxOT4yFDmZbFD56QNSZK0Ep5FLOvL+lvKhX/g6v7hYUcAyHYN9TUEJykDayygg8ycZyNPqLIw2SKLX9uKYSsnOJptAhHZDXVIPom22Zlpy3pbc/F4QhxMKuxNhwYe+oilIxfesTUgWylKg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741968586; c=relaxed/simple; bh=v/gp1b7tzV12McsPQPA3dkBbRM4YjwkiFePRBVO9owg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ltjp756lY+it9XAhkJMdhOcgEG4DeF0hreUhskssAQVOWUgciGNxCmz/iouN3BOmSKqASSqnNqV+/awunCmkqUv5FHC5lva4wX+1veM9UgvCWkFBb9WfjMqKM2WN7/bLBfYqxEwXsE5bh9hqCK05r/AKPfyL/9Wj7vpFq1JQMfs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WbaTvZ4Q; 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="WbaTvZ4Q" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8E81BC4CEEE; Fri, 14 Mar 2025 16:09:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741968585; bh=v/gp1b7tzV12McsPQPA3dkBbRM4YjwkiFePRBVO9owg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WbaTvZ4QS4jp2fK63wiahyXOL1ooPvm+NIwNHRRGoOV0s/f/38lXbST5lXNBJ/eAN +Txw3pKcVwfG1D9VPuVll6T6d1vkWpwx3oGdZ5bMWEZHGCl83f/Sbe0pfTyTnzcuRV QmWhSvb9rVI6ERfHp80isrT7EOfODayZ3uiskn5saAu9GsGh1XC4uQ1qOv6zjTK8iQ Q8BrviNP8bvf5CHEL1a39Tc2nE40fluvVI6yPT/8KJNRnA+yLhTPVk7348UDpS80Cj 7POZTdnlN1mm3MJ0yDOW16Zis6sXQj423zFAt36Yyj9DrRXV5PCQS9pLniLwFAFyJJ JjbtGs4rJN+EA== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, bhelgaas@google.com, 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: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 2/4] rust: device: implement device context marker Date: Fri, 14 Mar 2025 17:09:05 +0100 Message-ID: <20250314160932.100165-3-dakr@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250314160932.100165-1-dakr@kernel.org> References: <20250314160932.100165-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Some bus device functions should only be called from bus callbacks, such as probe(), remove(), resume(), suspend(), etc. To ensure this add device context marker structs, that can be used as generics for bus device implementations. Reviewed-by: Benno Lossin Suggested-by: Benno Lossin Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index db2d9658ba47..21b343a1dc4d 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -209,6 +209,32 @@ unsafe impl Send for Device {} // synchronization in `struct device`. unsafe impl Sync for Device {} +/// Marker trait for the context of a bus specific device. +/// +/// Some functions of a bus specific device should only be called from a certain context, i.e. bus +/// callbacks, such as `probe()`. +/// +/// This is the marker trait for structures representing the context of a bus specific device. +pub trait DeviceContext: private::Sealed {} + +/// The [`Normal`] context is the context of a bus specific device when it is not an argument of +/// any bus callback. +pub struct Normal; + +/// The [`Core`] context is the context of a bus specific device when it is supplied as argument of +/// any of the bus callbacks, such as `probe()`. +pub struct Core; + +mod private { + pub trait Sealed {} + + impl Sealed for super::Core {} + impl Sealed for super::Normal {} +} + +impl DeviceContext for Core {} +impl DeviceContext for Normal {} + #[doc(hidden)] #[macro_export] macro_rules! dev_printk { From patchwork Fri Mar 14 16:09:06 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Danilo Krummrich X-Patchwork-Id: 14017124 X-Patchwork-Delegate: bhelgaas@google.com 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 B5D94204C0F; Fri, 14 Mar 2025 16:09:49 +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=1741968589; cv=none; b=XbIAexJFWrXQN/0An/bqbI7VHVN1VvCWE4owlOvcy6lauOk+aFVODxY2f9b6dDN/nEkH5EkD34VYnZyUof1q70q3XJSDt14aQGrAILwZUOGlrZLsZCMANDFCwHk8hF8KS9LGqWR5Xv3eZxwwtUrmeAM1lJos5GXxgKyXm42asUA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741968589; c=relaxed/simple; bh=PXqjksoBv5r0sMAylYy8VC+mkJmatOkt19brm7SZ2SM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=CxXsvgdv46leQF3p1K5iTSpGI9sQw2w8gcZaEBuRBDhx75AVqc9bPzc46Qglk3vYYFgVFsdTv5erLnh6tsi2jVe5QSx9BUs+uSP7eQlTq4Yo4xCxs/afD0q67aPvC+u/ah4anyKAlu/ZFQfkHjjBRxCuDTkFsk8sCr9zrjpa8W4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GXcPsePp; 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="GXcPsePp" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2110CC4CEEC; Fri, 14 Mar 2025 16:09:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741968589; bh=PXqjksoBv5r0sMAylYy8VC+mkJmatOkt19brm7SZ2SM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GXcPsePpRibJJM7R76r1gaNLoxS7sm3VOUQcgKtrEKzLbTc3M5VHYRsAXYU8XHPo/ IAFqzsEhuy8dup/DQ3IBB6D8sErianl/p5BCy2/YC/fcj7ETyjr4F/q8tXXhJ9P44/ y0mgkYFGUgerrEGoWY/+jwx6eNH4o9pygesXv/LZPDFyumcj8Z+nQMyWcQjyysWwAC Eo6+j40fxCjIsCZn7RlrPRtv2ADfxZPcvAyXmeZBNND2S+BLBht9srrYV1GzYCK+4J rE9mXgORbGKx3LdzHaTUKgknbLA6sNW/5uoc/9xAM3aQnWIhi7r+cMiIuJSPMnBAXh p/2552cYq2R9g== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, bhelgaas@google.com, 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: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 3/4] rust: pci: fix unrestricted &mut pci::Device Date: Fri, 14 Mar 2025 17:09:06 +0100 Message-ID: <20250314160932.100165-4-dakr@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250314160932.100165-1-dakr@kernel.org> References: <20250314160932.100165-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 As by now, pci::Device is implemented as: #[derive(Clone)] pub struct Device(ARef); This may be convenient, but has the implication that drivers can call device methods that require a mutable reference concurrently at any point of time. Instead define pci::Device as pub struct Device( Opaque, PhantomData, ); and manually implement the AlwaysRefCounted trait. With this we can implement methods that should only be called from bus callbacks (such as probe()) for pci::Device. Consequently, we make this type accessible in bus callbacks only. Arbitrary references taken by the driver are still of type ARef and hence don't provide access to methods that are reserved for bus callbacks. Fixes: 1bd8b6b2c5d3 ("rust: pci: add basic PCI device / driver abstractions") Reviewed-by: Benno Lossin Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 132 ++++++++++++++++++++------------ samples/rust/rust_driver_pci.rs | 8 +- 2 files changed, 89 insertions(+), 51 deletions(-) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 386484dcf36e..0ac6cef74f81 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -6,7 +6,7 @@ use crate::{ alloc::flags::*, - bindings, container_of, device, + bindings, device, device_id::RawDeviceId, devres::Devres, driver, @@ -17,7 +17,11 @@ types::{ARef, ForeignOwnable, Opaque}, ThisModule, }; -use core::{ops::Deref, ptr::addr_of_mut}; +use core::{ + marker::PhantomData, + ops::Deref, + ptr::{addr_of_mut, NonNull}, +}; use kernel::prelude::*; /// An adapter for the registration of PCI drivers. @@ -60,17 +64,16 @@ extern "C" fn probe_callback( ) -> kernel::ffi::c_int { // SAFETY: The PCI bus only ever calls the probe callback with a valid pointer to a // `struct pci_dev`. - let dev = unsafe { device::Device::get_device(addr_of_mut!((*pdev).dev)) }; - // SAFETY: `dev` is guaranteed to be embedded in a valid `struct pci_dev` by the call - // above. - let mut pdev = unsafe { Device::from_dev(dev) }; + // + // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. + let pdev = unsafe { &*pdev.cast::>() }; // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct pci_device_id` and // does not add additional invariants, so it's safe to transmute. let id = unsafe { &*id.cast::() }; let info = T::ID_TABLE.info(id.index()); - match T::probe(&mut pdev, info) { + match T::probe(pdev, info) { Ok(data) => { // Let the `struct pci_dev` own a reference of the driver's private data. // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a @@ -192,7 +195,7 @@ macro_rules! pci_device_table { /// # Example /// ///``` -/// # use kernel::{bindings, pci}; +/// # use kernel::{bindings, device::Core, pci}; /// /// struct MyDriver; /// @@ -210,7 +213,7 @@ macro_rules! pci_device_table { /// const ID_TABLE: pci::IdTable = &PCI_TABLE; /// /// fn probe( -/// _pdev: &mut pci::Device, +/// _pdev: &pci::Device, /// _id_info: &Self::IdInfo, /// ) -> Result>> { /// Err(ENODEV) @@ -234,20 +237,23 @@ pub trait Driver { /// /// Called when a new platform device is added or discovered. /// Implementers should attempt to initialize the device here. - fn probe(dev: &mut Device, id_info: &Self::IdInfo) -> Result>>; + fn probe(dev: &Device, id_info: &Self::IdInfo) -> Result>>; } /// The PCI device representation. /// -/// A PCI device is based on an always reference counted `device:Device` instance. Cloning a PCI -/// device, hence, also increments the base device' reference count. +/// This structure represents the Rust abstraction for a C `struct pci_dev`. The implementation +/// abstracts the usage of an already existing C `struct pci_dev` within Rust code that we get +/// passed from the C side. /// /// # Invariants /// -/// `Device` hold a valid reference of `ARef` whose underlying `struct device` is a -/// member of a `struct pci_dev`. -#[derive(Clone)] -pub struct Device(ARef); +/// A [`Device`] instance represents a valid `struct device` created by the C portion of the kernel. +#[repr(transparent)] +pub struct Device( + Opaque, + PhantomData, +); /// A PCI BAR to perform I/O-Operations on. /// @@ -256,13 +262,13 @@ pub trait Driver { /// `Bar` always holds an `IoRaw` inststance that holds a valid pointer to the start of the I/O /// memory mapped PCI bar and its size. pub struct Bar { - pdev: Device, + pdev: ARef, io: IoRaw, num: i32, } impl Bar { - fn new(pdev: Device, num: u32, name: &CStr) -> Result { + fn new(pdev: &Device, num: u32, name: &CStr) -> Result { let len = pdev.resource_len(num)?; if len == 0 { return Err(ENOMEM); @@ -300,12 +306,16 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result { // `pdev` is valid by the invariants of `Device`. // `ioptr` is guaranteed to be the start of a valid I/O mapped memory region. // `num` is checked for validity by a previous call to `Device::resource_len`. - unsafe { Self::do_release(&pdev, ioptr, num) }; + unsafe { Self::do_release(pdev, ioptr, num) }; return Err(err); } }; - Ok(Bar { pdev, io, num }) + Ok(Bar { + pdev: pdev.into(), + io, + num, + }) } /// # Safety @@ -351,20 +361,8 @@ fn deref(&self) -> &Self::Target { } impl Device { - /// Create a PCI Device instance from an existing `device::Device`. - /// - /// # Safety - /// - /// `dev` must be an `ARef` whose underlying `bindings::device` is a member of - /// a `bindings::pci_dev`. - pub unsafe fn from_dev(dev: ARef) -> Self { - Self(dev) - } - fn as_raw(&self) -> *mut bindings::pci_dev { - // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` - // embedded in `struct pci_dev`. - unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ } + self.0.get() } /// Returns the PCI vendor ID. @@ -379,18 +377,6 @@ pub fn device_id(&self) -> u16 { unsafe { (*self.as_raw()).device } } - /// Enable memory resources for this device. - pub fn enable_device_mem(&self) -> Result { - // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`. - to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) }) - } - - /// Enable bus-mastering for this device. - pub fn set_master(&self) { - // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`. - unsafe { bindings::pci_set_master(self.as_raw()) }; - } - /// Returns the size of the given PCI bar resource. pub fn resource_len(&self, bar: u32) -> Result { if !Bar::index_is_valid(bar) { @@ -410,7 +396,7 @@ pub fn iomap_region_sized( bar: u32, name: &CStr, ) -> Result>> { - let bar = Bar::::new(self.clone(), bar, name)?; + let bar = Bar::::new(self, bar, name)?; let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?; Ok(devres) @@ -422,8 +408,60 @@ pub fn iomap_region(&self, bar: u32, name: &CStr) -> Result> { } } +impl Device { + /// Enable memory resources for this device. + pub fn enable_device_mem(&self) -> Result { + // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`. + to_result(unsafe { bindings::pci_enable_device_mem(self.as_raw()) }) + } + + /// Enable bus-mastering for this device. + pub fn set_master(&self) { + // SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`. + unsafe { bindings::pci_set_master(self.as_raw()) }; + } +} + +impl Deref for Device { + type Target = Device; + + fn deref(&self) -> &Self::Target { + let ptr: *const Self = self; + + // CAST: `Device` is a transparent wrapper of `Opaque`. + let ptr = ptr.cast::(); + + // SAFETY: `ptr` was derived from `&self`. + unsafe { &*ptr } + } +} + +impl From<&Device> for ARef { + fn from(dev: &Device) -> Self { + (&**dev).into() + } +} + +// SAFETY: Instances of `Device` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Device { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::pci_dev_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::pci_dev_put(obj.cast().as_ptr()) } + } +} + impl AsRef for Device { fn as_ref(&self) -> &device::Device { - &self.0 + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid + // `struct pci_dev`. + let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; + + // SAFETY: `dev` points to a valid `struct device`. + unsafe { device::Device::as_ref(dev) } } } diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index 1fb6e44f3395..4f14e63f323e 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -4,7 +4,7 @@ //! //! To make this driver probe, QEMU must be run with `-device pci-testdev`. -use kernel::{bindings, c_str, devres::Devres, pci, prelude::*}; +use kernel::{bindings, c_str, device::Core, devres::Devres, pci, prelude::*, types::ARef}; struct Regs; @@ -26,7 +26,7 @@ impl TestIndex { } struct SampleDriver { - pdev: pci::Device, + pdev: ARef, bar: Devres, } @@ -62,7 +62,7 @@ impl pci::Driver for SampleDriver { const ID_TABLE: pci::IdTable = &PCI_TABLE; - fn probe(pdev: &mut pci::Device, info: &Self::IdInfo) -> Result>> { + fn probe(pdev: &pci::Device, info: &Self::IdInfo) -> Result>> { dev_dbg!( pdev.as_ref(), "Probe Rust PCI driver sample (PCI ID: 0x{:x}, 0x{:x}).\n", @@ -77,7 +77,7 @@ fn probe(pdev: &mut pci::Device, info: &Self::IdInfo) -> Result>> let drvdata = KBox::new( Self { - pdev: pdev.clone(), + pdev: pdev.into(), bar, }, GFP_KERNEL, From patchwork Fri Mar 14 16:09:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Danilo Krummrich X-Patchwork-Id: 14017125 X-Patchwork-Delegate: bhelgaas@google.com 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 4B9F5205AB5; Fri, 14 Mar 2025 16:09:52 +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=1741968593; cv=none; b=sy0GfntcOz7QLlSXJ3STLW+/sVbQXC15kr4i1CTIJg2nxKci/TboBEnsuOoTts56YkUQfvzCKgLnFIs0wmKPjJxwqMlyd5R/Xsez1VU0UfyX/QPrUwn/sx36DCbV4Ho5bukEfw89nPBium65GHhi8ALDGlyKhJLsmie+uZlz3Hw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741968593; c=relaxed/simple; bh=9bMvDFkiEn0flcv2RD7JgC1AAL7/PYhrBNtwY663bsg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=q8mOmAmo17NLXI+JU/L2Y3wXQHnkyxkTG4AJyY2IjX4/KA+eI6MoWXnSaxYDG0wmc9E0FTYdkRdjBOYddkA5Zcqz20vzCUAKJ8/Kh9bleSg9e8XODKdA3UU+whzBgJr6Cr5/JrO3ryz+kDded0x9+wKnIuKUFrn1s8aI8sgrQHM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lCzDuS5D; 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="lCzDuS5D" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A54B0C4CEEF; Fri, 14 Mar 2025 16:09:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1741968592; bh=9bMvDFkiEn0flcv2RD7JgC1AAL7/PYhrBNtwY663bsg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lCzDuS5DsdhWyhh4LM0y517nhTgA5DMESROTsLNpPQ1fiAxx94RIxs2RrcGX3K943 He8sBEWSL5luXxBTRcTQbQOmZV4imcvBifR9EUTMeuUGXyQkt8ViGqjmADqnMpLOzv fmL5orQM0Jdz5Nzllnd24huaAkPpGPet2OP7uUu5+RxSTahVJ9MdXoim2CGrID9si8 nLybnRCUXUAbuiF2fnWpnt4okZix0Op8VQNpJy+BBrntjkIkRrPSOTIlob1VZr412g md6h2DwZvwzXx6C38HuM2dmWq+4PquC1WXw7BHerkATWTe8qNIZpVHLtN4fmDXzeK2 8amjowJKKIrHg== From: Danilo Krummrich To: gregkh@linuxfoundation.org, rafael@kernel.org, bhelgaas@google.com, 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: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org, Danilo Krummrich Subject: [PATCH v2 4/4] rust: platform: fix unrestricted &mut platform::Device Date: Fri, 14 Mar 2025 17:09:07 +0100 Message-ID: <20250314160932.100165-5-dakr@kernel.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250314160932.100165-1-dakr@kernel.org> References: <20250314160932.100165-1-dakr@kernel.org> Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 As by now, platform::Device is implemented as: #[derive(Clone)] pub struct Device(ARef); This may be convenient, but has the implication that drivers can call device methods that require a mutable reference concurrently at any point of time. Instead define platform::Device as pub struct Device( Opaque, PhantomData, ); and manually implement the AlwaysRefCounted trait. With this we can implement methods that should only be called from bus callbacks (such as probe()) for platform::Device. Consequently, we make this type accessible in bus callbacks only. Arbitrary references taken by the driver are still of type ARef and hence don't provide access to methods that are reserved for bus callbacks. Fixes: 683a63befc73 ("rust: platform: add basic platform device / driver abstractions") Reviewed-by: Benno Lossin Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 95 +++++++++++++++++++--------- samples/rust/rust_driver_platform.rs | 11 ++-- 2 files changed, 72 insertions(+), 34 deletions(-) diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 50e6b0421813..c77c9f2e9aea 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/platform_device.h`](srctree/include/linux/platform_device.h) use crate::{ - bindings, container_of, device, driver, + bindings, device, driver, error::{to_result, Result}, of, prelude::*, @@ -14,7 +14,11 @@ ThisModule, }; -use core::ptr::addr_of_mut; +use core::{ + marker::PhantomData, + ops::Deref, + ptr::{addr_of_mut, NonNull}, +}; /// An adapter for the registration of platform drivers. pub struct Adapter(T); @@ -54,14 +58,14 @@ unsafe fn unregister(pdrv: &Opaque) { impl Adapter { extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ffi::c_int { - // SAFETY: The platform bus only ever calls the probe callback with a valid `pdev`. - let dev = unsafe { device::Device::get_device(addr_of_mut!((*pdev).dev)) }; - // SAFETY: `dev` is guaranteed to be embedded in a valid `struct platform_device` by the - // call above. - let mut pdev = unsafe { Device::from_dev(dev) }; + // SAFETY: The platform bus only ever calls the probe callback with a valid pointer to a + // `struct platform_device`. + // + // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. + let pdev = unsafe { &*pdev.cast::>() }; let info = ::id_info(pdev.as_ref()); - match T::probe(&mut pdev, info) { + match T::probe(pdev, info) { Ok(data) => { // Let the `struct platform_device` own a reference of the driver's private data. // SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a @@ -120,7 +124,7 @@ macro_rules! module_platform_driver { /// # Example /// ///``` -/// # use kernel::{bindings, c_str, of, platform}; +/// # use kernel::{bindings, c_str, device::Core, of, platform}; /// /// struct MyDriver; /// @@ -138,7 +142,7 @@ macro_rules! module_platform_driver { /// const OF_ID_TABLE: Option> = Some(&OF_TABLE); /// /// fn probe( -/// _pdev: &mut platform::Device, +/// _pdev: &platform::Device, /// _id_info: Option<&Self::IdInfo>, /// ) -> Result>> { /// Err(ENODEV) @@ -160,41 +164,72 @@ pub trait Driver { /// /// Called when a new platform device is added or discovered. /// Implementers should attempt to initialize the device here. - fn probe(dev: &mut Device, id_info: Option<&Self::IdInfo>) -> Result>>; + fn probe(dev: &Device, id_info: Option<&Self::IdInfo>) + -> Result>>; } /// The platform device representation. /// -/// A platform device is based on an always reference counted `device:Device` instance. Cloning a -/// platform device, hence, also increments the base device' reference count. +/// This structure represents the Rust abstraction for a C `struct platform_device`. The +/// implementation abstracts the usage of an already existing C `struct platform_device` within Rust +/// code that we get passed from the C side. /// /// # Invariants /// -/// `Device` holds a valid reference of `ARef` whose underlying `struct device` is a -/// member of a `struct platform_device`. -#[derive(Clone)] -pub struct Device(ARef); +/// A [`Device`] instance represents a valid `struct platform_device` created by the C portion of +/// the kernel. +#[repr(transparent)] +pub struct Device( + Opaque, + PhantomData, +); impl Device { - /// Convert a raw kernel device into a `Device` - /// - /// # Safety - /// - /// `dev` must be an `Aref` whose underlying `bindings::device` is a member of a - /// `bindings::platform_device`. - unsafe fn from_dev(dev: ARef) -> Self { - Self(dev) + fn as_raw(&self) -> *mut bindings::platform_device { + self.0.get() } +} - fn as_raw(&self) -> *mut bindings::platform_device { - // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` - // embedded in `struct platform_device`. - unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut() +impl Deref for Device { + type Target = Device; + + fn deref(&self) -> &Self::Target { + let ptr: *const Self = self; + + // CAST: `Device` is a transparent wrapper of `Opaque`. + let ptr = ptr.cast::(); + + // SAFETY: `ptr` was derived from `&self`. + unsafe { &*ptr } + } +} + +impl From<&Device> for ARef { + fn from(dev: &Device) -> Self { + (&**dev).into() + } +} + +// SAFETY: Instances of `Device` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Device { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::get_device(self.as_ref().as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::platform_device_put(obj.cast().as_ptr()) } } } impl AsRef for Device { fn as_ref(&self) -> &device::Device { - &self.0 + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid + // `struct platform_device`. + let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) }; + + // SAFETY: `dev` points to a valid `struct device`. + unsafe { device::Device::as_ref(dev) } } } diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs index 8120609e2940..9bb66db0a4f4 100644 --- a/samples/rust/rust_driver_platform.rs +++ b/samples/rust/rust_driver_platform.rs @@ -2,10 +2,10 @@ //! Rust Platform driver sample. -use kernel::{c_str, of, platform, prelude::*}; +use kernel::{c_str, device::Core, of, platform, prelude::*, types::ARef}; struct SampleDriver { - pdev: platform::Device, + pdev: ARef, } struct Info(u32); @@ -21,14 +21,17 @@ impl platform::Driver for SampleDriver { type IdInfo = Info; const OF_ID_TABLE: Option> = Some(&OF_TABLE); - fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result>> { + fn probe( + pdev: &platform::Device, + info: Option<&Self::IdInfo>, + ) -> Result>> { dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n"); if let Some(info) = info { dev_info!(pdev.as_ref(), "Probed with info: '{}'.\n", info.0); } - let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?; + let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?; Ok(drvdata.into()) }