From 354fd6e86fac60b7c1ce2e6c83d4e6bf8af95f59 Mon Sep 17 00:00:00 2001 From: Fiona Behrens Date: Mon, 17 Feb 2025 21:58:14 +0100 Subject: rust: io: rename `io::Io` accessors Rename the I/O accessors provided by `Io` to encode the type as number instead of letter. This is in preparation for Port I/O support to use a trait for generic accessors. Add a `c_fn` argument to the accessor generation macro to translate between rust and C names. Suggested-by: Danilo Krummrich Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/PIO.20support/near/499460541 Signed-off-by: Fiona Behrens Acked-by: Danilo Krummrich Acked-by: Daniel Almeida Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250217-io-generic-rename-v1-1-06d97a9e3179@kloenk.dev Signed-off-by: Greg Kroah-Hartman --- rust/kernel/io.rs | 66 +++++++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 33 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index d4a73e52e3ee..72d80a6f131e 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -98,9 +98,9 @@ impl IoRaw { ///# fn no_run() -> Result<(), Error> { /// // SAFETY: Invalid usage for example purposes. /// let iomem = unsafe { IoMem::<{ core::mem::size_of::() }>::new(0xBAAAAAAD)? }; -/// iomem.writel(0x42, 0x0); -/// assert!(iomem.try_writel(0x42, 0x0).is_ok()); -/// assert!(iomem.try_writel(0x42, 0x4).is_err()); +/// iomem.write32(0x42, 0x0); +/// assert!(iomem.try_write32(0x42, 0x0).is_ok()); +/// assert!(iomem.try_write32(0x42, 0x4).is_err()); /// # Ok(()) /// # } /// ``` @@ -108,7 +108,7 @@ impl IoRaw { pub struct Io(IoRaw); macro_rules! define_read { - ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => { + ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident -> $type_name:ty) => { /// Read IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile @@ -119,7 +119,7 @@ macro_rules! define_read { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(addr as _) } + unsafe { bindings::$c_fn(addr as _) } } /// Read IO data from a given offset. @@ -131,13 +131,13 @@ macro_rules! define_read { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - Ok(unsafe { bindings::$name(addr as _) }) + Ok(unsafe { bindings::$c_fn(addr as _) }) } }; } macro_rules! define_write { - ($(#[$attr:meta])* $name:ident, $try_name:ident, $type_name:ty) => { + ($(#[$attr:meta])* $name:ident, $try_name:ident, $c_fn:ident <- $type_name:ty) => { /// Write IO data from a given offset known at compile time. /// /// Bound checks are performed on compile time, hence if the offset is not known at compile @@ -148,7 +148,7 @@ macro_rules! define_write { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(value, addr as _, ) } + unsafe { bindings::$c_fn(value, addr as _, ) } } /// Write IO data from a given offset. @@ -160,7 +160,7 @@ macro_rules! define_write { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(value, addr as _) } + unsafe { bindings::$c_fn(value, addr as _) } Ok(()) } }; @@ -218,43 +218,43 @@ impl Io { self.addr() + offset } - define_read!(readb, try_readb, u8); - define_read!(readw, try_readw, u16); - define_read!(readl, try_readl, u32); + define_read!(read8, try_read8, readb -> u8); + define_read!(read16, try_read16, readw -> u16); + define_read!(read32, try_read32, readl -> u32); define_read!( #[cfg(CONFIG_64BIT)] - readq, - try_readq, - u64 + read64, + try_read64, + readq -> u64 ); - define_read!(readb_relaxed, try_readb_relaxed, u8); - define_read!(readw_relaxed, try_readw_relaxed, u16); - define_read!(readl_relaxed, try_readl_relaxed, u32); + define_read!(read8_relaxed, try_read8_relaxed, readb_relaxed -> u8); + define_read!(read16_relaxed, try_read16_relaxed, readw_relaxed -> u16); + define_read!(read32_relaxed, try_read32_relaxed, readl_relaxed -> u32); define_read!( #[cfg(CONFIG_64BIT)] - readq_relaxed, - try_readq_relaxed, - u64 + read64_relaxed, + try_read64_relaxed, + readq_relaxed -> u64 ); - define_write!(writeb, try_writeb, u8); - define_write!(writew, try_writew, u16); - define_write!(writel, try_writel, u32); + define_write!(write8, try_write8, writeb <- u8); + define_write!(write16, try_write16, writew <- u16); + define_write!(write32, try_write32, writel <- u32); define_write!( #[cfg(CONFIG_64BIT)] - writeq, - try_writeq, - u64 + write64, + try_write64, + writeq <- u64 ); - define_write!(writeb_relaxed, try_writeb_relaxed, u8); - define_write!(writew_relaxed, try_writew_relaxed, u16); - define_write!(writel_relaxed, try_writel_relaxed, u32); + define_write!(write8_relaxed, try_write8_relaxed, writeb_relaxed <- u8); + define_write!(write16_relaxed, try_write16_relaxed, writew_relaxed <- u16); + define_write!(write32_relaxed, try_write32_relaxed, writel_relaxed <- u32); define_write!( #[cfg(CONFIG_64BIT)] - writeq_relaxed, - try_writeq_relaxed, - u64 + write64_relaxed, + try_write64_relaxed, + writeq_relaxed <- u64 ); } -- cgit v1.2.3 From 040b17ae0e15bd7100432e0a20c6557d463a8c9f Mon Sep 17 00:00:00 2001 From: Fiona Behrens Date: Mon, 24 Feb 2025 19:36:43 +0100 Subject: rust: io: fix devres test with new io accessor functions Fix doctest of `Devres` which still used `writeb` instead of `write8`. Fixes: 354fd6e86fac ("rust: io: rename `io::Io` accessors") Signed-off-by: Fiona Behrens Link: https://lore.kernel.org/r/20250224-rust-iowrite-read8-fix-v1-1-c6abee346897@kloenk.dev Signed-off-by: Greg Kroah-Hartman --- rust/kernel/devres.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 942376f6f3af..ddb1ce4a78d9 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -92,7 +92,7 @@ struct DevresInner { /// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?; /// /// let res = devres.try_access().ok_or(ENXIO)?; -/// res.writel(0x42, 0x0); +/// res.write8(0x42, 0x0); /// # Ok(()) /// # } /// ``` -- cgit v1.2.3 From 6853d9d13dbe596a73ae968e6fb27ba9680b2441 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 25 Feb 2025 16:29:00 -0500 Subject: rust/faux: Drop #[repr(transparent)] from faux::Registration I think this change got missed during review, we don't need #[repr(transparent)] since Registration just holds a single NonNull. This attribute had originally been added by me when I was still figuring out how the bindings should look like but got committed by mistake. So, just drop it. Signed-off-by: Lyude Paul Cc: Greg Kroah-Hartman Acked-by: Danilo Krummrich Reviewed-by: Fiona Behrens Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250225213112.872264-2-lyude@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/faux.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 5acc0c02d451..41751403cd86 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -19,7 +19,6 @@ use core::ptr::{addr_of_mut, null, null_mut, NonNull}; /// `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`]. /// /// [`struct faux_device`]: srctree/include/linux/device/faux.h -#[repr(transparent)] pub struct Registration(NonNull); impl Registration { -- cgit v1.2.3 From 95cb0cb546c2892b7a31ff2fce6573f201a214b8 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Thu, 27 Feb 2025 14:35:06 -0500 Subject: rust/faux: Add missing parent argument to Registration::new() A little late in the review of the faux device interface, we added the ability to specify a parent device when creating new faux devices - but this never got ported over to the rust bindings. So, let's add the missing argument now so we don't have to convert other users later down the line. Signed-off-by: Lyude Paul Cc: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250227193522.198344-1-lyude@redhat.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/faux.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 41751403cd86..3277f35c3f79 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -23,11 +23,20 @@ pub struct Registration(NonNull); impl Registration { /// Create and register a new faux device with the given name. - pub fn new(name: &CStr) -> Result { + pub fn new(name: &CStr, parent: Option<&device::Device>) -> Result { // SAFETY: // - `name` is copied by this function into its own storage // - `faux_ops` is safe to leave NULL according to the C API - let dev = unsafe { bindings::faux_device_create(name.as_char_ptr(), null_mut(), null()) }; + // - `parent` can be either NULL or a pointer to a `struct device`, and `faux_device_create` + // will take a reference to `parent` using `device_add` - ensuring that it remains valid + // for the lifetime of the faux device. + let dev = unsafe { + bindings::faux_device_create( + name.as_char_ptr(), + parent.map_or(null_mut(), |p| p.as_raw()), + null(), + ) + }; // The above function will return either a valid device, or NULL on failure // INVARIANT: The device will remain registered until faux_device_destroy() is called, which -- cgit v1.2.3 From 74fc34937d72d04e89e4f75ea66147cdc9b785f5 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 27 Feb 2025 13:22:31 +0000 Subject: rust: miscdevice: change how f_ops vtable is constructed I was helping someone with writing a new Rust abstraction, and we were using the miscdevice abstraction as an example. While doing this, it became clear to me that the way I implemented the f_ops vtable is confusing to new Rust users, and that the approach used by the block abstractions is less confusing. Thus, update the miscdevice abstractions to use the same approach as rust/kernel/block/mq/operations.rs. Sorry about the large diff. This changes the indentation of a large amount of code. Reviewed-by: Christian Schrefl Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20250227-miscdevice-fops-change-v1-1-c9e9b75d67eb@google.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 297 ++++++++++++++++++++++------------------------ 1 file changed, 143 insertions(+), 154 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index e14433b2ab9d..fa9ecc42602a 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -35,7 +35,7 @@ impl MiscDeviceOptions { let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() }; result.minor = bindings::MISC_DYNAMIC_MINOR as _; result.name = self.name.as_char_ptr(); - result.fops = create_vtable::(); + result.fops = MiscdeviceVTable::::build(); result } } @@ -160,171 +160,160 @@ pub trait MiscDevice: Sized { } } -const fn create_vtable() -> &'static bindings::file_operations { - const fn maybe_fn(check: bool, func: T) -> Option { - if check { - Some(func) - } else { - None +/// A vtable for the file operations of a Rust miscdevice. +struct MiscdeviceVTable(PhantomData); + +impl MiscdeviceVTable { + /// # Safety + /// + /// `file` and `inode` must be the file and inode for a file that is undergoing initialization. + /// The file must be associated with a `MiscDeviceRegistration`. + unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut bindings::file) -> c_int { + // SAFETY: The pointers are valid and for a file being opened. + let ret = unsafe { bindings::generic_file_open(inode, raw_file) }; + if ret != 0 { + return ret; } - } - struct VtableHelper { - _t: PhantomData, - } - impl VtableHelper { - const VTABLE: bindings::file_operations = bindings::file_operations { - open: Some(fops_open::), - release: Some(fops_release::), - unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::), - #[cfg(CONFIG_COMPAT)] - compat_ioctl: if T::HAS_COMPAT_IOCTL { - Some(fops_compat_ioctl::) - } else if T::HAS_IOCTL { - Some(bindings::compat_ptr_ioctl) - } else { - None - }, - show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::), - // SAFETY: All zeros is a valid value for `bindings::file_operations`. - ..unsafe { MaybeUninit::zeroed().assume_init() } - }; - } + // SAFETY: The open call of a file can access the private data. + let misc_ptr = unsafe { (*raw_file).private_data }; - &VtableHelper::::VTABLE -} + // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the + // associated `struct miscdevice` before calling into this method. Furthermore, + // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this + // call to `fops_open`. + let misc = unsafe { &*misc_ptr.cast::>() }; -/// # Safety -/// -/// `file` and `inode` must be the file and inode for a file that is undergoing initialization. -/// The file must be associated with a `MiscDeviceRegistration`. -unsafe extern "C" fn fops_open( - inode: *mut bindings::inode, - raw_file: *mut bindings::file, -) -> c_int { - // SAFETY: The pointers are valid and for a file being opened. - let ret = unsafe { bindings::generic_file_open(inode, raw_file) }; - if ret != 0 { - return ret; - } + // SAFETY: + // * This underlying file is valid for (much longer than) the duration of `T::open`. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(raw_file) }; - // SAFETY: The open call of a file can access the private data. - let misc_ptr = unsafe { (*raw_file).private_data }; - - // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the - // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()` - // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`. - let misc = unsafe { &*misc_ptr.cast::>() }; + let ptr = match T::open(file, misc) { + Ok(ptr) => ptr, + Err(err) => return err.to_errno(), + }; - // SAFETY: - // * This underlying file is valid for (much longer than) the duration of `T::open`. - // * There is no active fdget_pos region on the file on this thread. - let file = unsafe { File::from_raw_file(raw_file) }; + // This overwrites the private data with the value specified by the user, changing the type + // of this file's private data. All future accesses to the private data is performed by + // other fops_* methods in this file, which all correctly cast the private data to the new + // type. + // + // SAFETY: The open call of a file can access the private data. + unsafe { (*raw_file).private_data = ptr.into_foreign() }; - let ptr = match T::open(file, misc) { - Ok(ptr) => ptr, - Err(err) => return err.to_errno(), - }; - - // This overwrites the private data with the value specified by the user, changing the type of - // this file's private data. All future accesses to the private data is performed by other - // fops_* methods in this file, which all correctly cast the private data to the new type. - // - // SAFETY: The open call of a file can access the private data. - unsafe { (*raw_file).private_data = ptr.into_foreign() }; + 0 + } - 0 -} + /// # Safety + /// + /// `file` and `inode` must be the file and inode for a file that is being released. The file + /// must be associated with a `MiscDeviceRegistration`. + unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int { + // SAFETY: The release call of a file owns the private data. + let private = unsafe { (*file).private_data }; + // SAFETY: The release call of a file owns the private data. + let ptr = unsafe { ::from_foreign(private) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + T::release(ptr, unsafe { File::from_raw_file(file) }); + + 0 + } -/// # Safety -/// -/// `file` and `inode` must be the file and inode for a file that is being released. The file must -/// be associated with a `MiscDeviceRegistration`. -unsafe extern "C" fn fops_release( - _inode: *mut bindings::inode, - file: *mut bindings::file, -) -> c_int { - // SAFETY: The release call of a file owns the private data. - let private = unsafe { (*file).private_data }; - // SAFETY: The release call of a file owns the private data. - let ptr = unsafe { ::from_foreign(private) }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - T::release(ptr, unsafe { File::from_raw_file(file) }); - - 0 -} + /// # Safety + /// + /// `file` must be a valid file that is associated with a `MiscDeviceRegistration`. + unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long { + // SAFETY: The ioctl call of a file can access the private data. + let private = unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device = unsafe { ::borrow(private) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + + match T::ioctl(device, file, cmd, arg) { + Ok(ret) => ret as c_long, + Err(err) => err.to_errno() as c_long, + } + } -/// # Safety -/// -/// `file` must be a valid file that is associated with a `MiscDeviceRegistration`. -unsafe extern "C" fn fops_ioctl( - file: *mut bindings::file, - cmd: c_uint, - arg: c_ulong, -) -> c_long { - // SAFETY: The ioctl call of a file can access the private data. - let private = unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file = unsafe { File::from_raw_file(file) }; - - match T::ioctl(device, file, cmd, arg) { - Ok(ret) => ret as c_long, - Err(err) => err.to_errno() as c_long, + /// # Safety + /// + /// `file` must be a valid file that is associated with a `MiscDeviceRegistration`. + #[cfg(CONFIG_COMPAT)] + unsafe extern "C" fn compat_ioctl( + file: *mut bindings::file, + cmd: c_uint, + arg: c_ulong, + ) -> c_long { + // SAFETY: The compat ioctl call of a file can access the private data. + let private = unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device = unsafe { ::borrow(private) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + + match T::compat_ioctl(device, file, cmd, arg) { + Ok(ret) => ret as c_long, + Err(err) => err.to_errno() as c_long, + } } -} -/// # Safety -/// -/// `file` must be a valid file that is associated with a `MiscDeviceRegistration`. -#[cfg(CONFIG_COMPAT)] -unsafe extern "C" fn fops_compat_ioctl( - file: *mut bindings::file, - cmd: c_uint, - arg: c_ulong, -) -> c_long { - // SAFETY: The compat ioctl call of a file can access the private data. - let private = unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file = unsafe { File::from_raw_file(file) }; - - match T::compat_ioctl(device, file, cmd, arg) { - Ok(ret) => ret as c_long, - Err(err) => err.to_errno() as c_long, + /// # Safety + /// + /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration`. + /// - `seq_file` must be a valid `struct seq_file` that we can write to. + unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) { + // SAFETY: The release call of a file owns the private data. + let private = unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device = unsafe { ::borrow(private) }; + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this thread. + let file = unsafe { File::from_raw_file(file) }; + // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in + // which this method is called. + let m = unsafe { SeqFile::from_raw(seq_file) }; + + T::show_fdinfo(device, m, file); } -} -/// # Safety -/// -/// - `file` must be a valid file that is associated with a `MiscDeviceRegistration`. -/// - `seq_file` must be a valid `struct seq_file` that we can write to. -unsafe extern "C" fn fops_show_fdinfo( - seq_file: *mut bindings::seq_file, - file: *mut bindings::file, -) { - // SAFETY: The release call of a file owns the private data. - let private = unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device = unsafe { ::borrow(private) }; - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file = unsafe { File::from_raw_file(file) }; - // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which - // this method is called. - let m = unsafe { SeqFile::from_raw(seq_file) }; - - T::show_fdinfo(device, m, file); + const VTABLE: bindings::file_operations = bindings::file_operations { + open: Some(Self::open), + release: Some(Self::release), + unlocked_ioctl: if T::HAS_IOCTL { + Some(Self::ioctl) + } else { + None + }, + #[cfg(CONFIG_COMPAT)] + compat_ioctl: if T::HAS_COMPAT_IOCTL { + Some(Self::compat_ioctl) + } else if T::HAS_IOCTL { + Some(bindings::compat_ptr_ioctl) + } else { + None + }, + show_fdinfo: if T::HAS_SHOW_FDINFO { + Some(Self::show_fdinfo) + } else { + None + }, + // SAFETY: All zeros is a valid value for `bindings::file_operations`. + ..unsafe { MaybeUninit::zeroed().assume_init() } + }; + + const fn build() -> &'static bindings::file_operations { + &Self::VTABLE + } } -- cgit v1.2.3 From 10b20f2d1bbeddcb6c1f6c01b6eb1b8d2828b663 Mon Sep 17 00:00:00 2001 From: Ethan Carter Edwards Date: Mon, 10 Mar 2025 15:51:38 -0400 Subject: rust/kernel/faux: mark Registration methods inline When building the kernel on Arch Linux using on x86_64 with tools: $ rustc --version rustc 1.84.0 (9fc6b4312 2025-01-07) $ clang --version clang version 19.1.7 Target: x86_64-pc-linux-gnu The following symbols are generated: $ nm vmlinux | rg ' _R' | rustfilt | rg faux ffffffff81959ae0 T ::new ffffffff81959b40 T ::drop However, these Rust symbols are wrappers around bindings in the C faux code. Inlining these functions removes the middle-man wrapper function After applying this patch, the above function signatures disappear. Link: https://github.com/Rust-for-Linux/linux/issues/1145 Signed-off-by: Ethan Carter Edwards Acked-by: Danilo Krummrich Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/jesg4yu7m6fvzmgg5tlsktrrjm36l4qsranto5mdmnucx4pvf3@nhvt4juw5es3 Signed-off-by: Greg Kroah-Hartman --- rust/kernel/faux.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/faux.rs b/rust/kernel/faux.rs index 3277f35c3f79..8a50fcd4c9bb 100644 --- a/rust/kernel/faux.rs +++ b/rust/kernel/faux.rs @@ -23,6 +23,7 @@ pub struct Registration(NonNull); impl Registration { /// Create and register a new faux device with the given name. + #[inline] pub fn new(name: &CStr, parent: Option<&device::Device>) -> Result { // SAFETY: // - `name` is copied by this function into its own storage @@ -58,6 +59,7 @@ impl AsRef for Registration { } impl Drop for Registration { + #[inline] fn drop(&mut self) { // SAFETY: `self.0` is a valid registered faux_device via our type invariants. unsafe { bindings::faux_device_destroy(self.as_raw()) } -- cgit v1.2.3 From d1f6d6c537d488b1a8f04091c7751124985a0ab9 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 14 Mar 2025 17:09:04 +0100 Subject: rust: pci: use to_result() in enable_device_mem() 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 Acked-by: Boqun Feng Link: https://lore.kernel.org/r/20250314160932.100165-2-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/pci.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'rust/kernel') 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 @@ 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`. - 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. -- cgit v1.2.3 From 4d032779ab321baa1a430ff27791e5e0c98a0c2f Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 14 Mar 2025 17:09:05 +0100 Subject: rust: device: implement device context marker 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 Acked-by: Boqun Feng Link: https://lore.kernel.org/r/20250314160932.100165-3-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/device.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'rust/kernel') 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 { -- cgit v1.2.3 From 7b948a2af6b5d64a25c14da8f63d8084ea527cd9 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 14 Mar 2025 17:09:06 +0100 Subject: rust: pci: fix unrestricted &mut pci::Device 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 Acked-by: Boqun Feng Link: https://lore.kernel.org/r/20250314160932.100165-4-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/pci.rs | 132 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 85 insertions(+), 47 deletions(-) (limited to 'rust/kernel') 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 @@ use crate::{ 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 @@ impl Adapter { ) -> 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 struct Device(ARef); /// `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 @@ impl Bar { // `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 @@ impl Deref for Bar { } 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 @@ impl Device { 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 @@ impl Device { 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 @@ impl Device { } } +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) } } } -- cgit v1.2.3 From 4d320e30ee04c25c660eca2bb33e846ebb71a79a Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 14 Mar 2025 17:09:07 +0100 Subject: rust: platform: fix unrestricted &mut platform::Device 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 Acked-by: Boqun Feng Link: https://lore.kernel.org/r/20250314160932.100165-5-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/platform.rs | 95 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 65 insertions(+), 30 deletions(-) (limited to 'rust/kernel') 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 @@ use crate::{ 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 impl driver::RegistrationOps for Adapter { 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) } } } -- cgit v1.2.3 From e2942bb4e62938fcef1481c9c1470b661087cdb7 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 18 Mar 2025 22:29:21 +0100 Subject: rust: pci: impl Send + Sync for pci::Device Commit 7b948a2af6b5 ("rust: pci: fix unrestricted &mut pci::Device") changed the definition of pci::Device and discarded the implicitly derived Send and Sync traits. This isn't required by upstream code yet, and hence did not cause any issues. However, it is relied on by upcoming drivers, hence add it back in. Signed-off-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250318212940.137577-1-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/pci.rs | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 0ac6cef74f81..0d09ae34a64d 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -465,3 +465,10 @@ impl AsRef for Device { unsafe { device::Device::as_ref(dev) } } } + +// SAFETY: A `Device` is always reference-counted and can be released from any thread. +unsafe impl Send for Device {} + +// SAFETY: `Device` can be shared among threads because all methods of `Device` +// (i.e. `Device) are thread safe. +unsafe impl Sync for Device {} -- cgit v1.2.3 From 455943aa187fd93358ccd00c894934061153ec0c Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Tue, 18 Mar 2025 22:29:22 +0100 Subject: rust: platform: impl Send + Sync for platform::Device Commit 4d320e30ee04 ("rust: platform: fix unrestricted &mut platform::Device") changed the definition of platform::Device and discarded the implicitly derived Send and Sync traits. This isn't required by upstream code yet, and hence did not cause any issues. However, it is relied on by upcoming drivers, hence add it back in. Signed-off-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250318212940.137577-2-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/platform.rs | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index c77c9f2e9aea..2811ca53d8b6 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -233,3 +233,10 @@ impl AsRef for Device { unsafe { device::Device::as_ref(dev) } } } + +// SAFETY: A `Device` is always reference-counted and can be released from any thread. +unsafe impl Send for Device {} + +// SAFETY: `Device` can be shared among threads because all methods of `Device` +// (i.e. `Device) are thread safe. +unsafe impl Sync for Device {} -- cgit v1.2.3 From 935e1d90bf6f14cd190b3a95f3cbf7e298123043 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 19 Mar 2025 15:52:55 +0100 Subject: rust: pci: require Send for Driver trait implementers The instance of Self, returned and created by Driver::probe() is dropped in the bus' remove() callback. Request implementers of the Driver trait to implement Send, since the remove() callback is not guaranteed to run from the same thread as probe(). Fixes: 1bd8b6b2c5d3 ("rust: pci: add basic PCI device / driver abstractions") Cc: stable Reported-by: Alice Ryhl Closes: https://lore.kernel.org/lkml/Z9rDxOJ2V2bPjj5i@google.com/ Signed-off-by: Danilo Krummrich Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250319145350.69543-1-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/pci.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 0d09ae34a64d..22a32172b108 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -222,7 +222,7 @@ macro_rules! pci_device_table { ///``` /// Drivers must implement this trait in order to get a PCI driver registered. Please refer to the /// `Adapter` documentation for an example. -pub trait Driver { +pub trait Driver: Send { /// The type holding information about each device id supported by the driver. /// /// TODO: Use associated_type_defaults once stabilized: -- cgit v1.2.3 From 51d0de7596a458096756c895cfed6bc4a7ecac10 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 19 Mar 2025 15:52:56 +0100 Subject: rust: platform: require Send for Driver trait implementers The instance of Self, returned and created by Driver::probe() is dropped in the bus' remove() callback. Request implementers of the Driver trait to implement Send, since the remove() callback is not guaranteed to run from the same thread as probe(). Fixes: 683a63befc73 ("rust: platform: add basic platform device / driver abstractions") Cc: stable Reported-by: Alice Ryhl Closes: https://lore.kernel.org/lkml/Z9rDxOJ2V2bPjj5i@google.com/ Signed-off-by: Danilo Krummrich Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250319145350.69543-2-dakr@kernel.org Signed-off-by: Greg Kroah-Hartman --- rust/kernel/platform.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 2811ca53d8b6..e37531bae8e9 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -149,7 +149,7 @@ macro_rules! module_platform_driver { /// } /// } ///``` -pub trait Driver { +pub trait Driver: Send { /// The type holding driver private data about each device id supported by the driver. /// /// TODO: Use associated_type_defaults once stabilized: -- cgit v1.2.3