From f5b444361435bfc9eeaaae3dd2e9b4f18dae3888 Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Sun, 16 Mar 2025 06:16:44 -0500 Subject: [PATCH 01/47] gpu: nova-core: remove completed Vec extentions from task list The requested Vec methods have been implemented thus, remove the completed item from the nova task list. Link: https://lore.kernel.org/r/20250316111644.154602-4-andrewjballance@gmail.com Signed-off-by: Andrew Ballance Signed-off-by: Danilo Krummrich --- Documentation/gpu/nova/core/todo.rst | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst index ca08377d3b73..234d753d3eac 100644 --- a/Documentation/gpu/nova/core/todo.rst +++ b/Documentation/gpu/nova/core/todo.rst @@ -190,16 +190,6 @@ Rust abstraction for debugfs APIs. | Reference: Export GSP log buffers | Complexity: Intermediate -Vec extensions --------------- - -Implement ``Vec::truncate`` and ``Vec::resize``. - -Currently this is used for some experimental code to parse the vBIOS. - -| Reference vBIOS support -| Complexity: Beginner - GPU (general) ============= From cb271c2edfd0ab7204d5ef3c9d5ae9a0710f5bf2 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:56 +0200 Subject: [PATCH 02/47] rust: device: implement impl_device_context_deref! The Deref hierarchy for device context generics is the same for every (bus specific) device. Implement those with a generic macro to avoid duplicated boiler plate code and ensure the correct Deref hierarchy for every device implementation. Co-developed-by: Benno Lossin Signed-off-by: Benno Lossin Reviewed-by: Christian Schrefl Link: https://lore.kernel.org/r/20250413173758.12068-2-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 44 +++++++++++++++++++++++++++++++++++++++++ rust/kernel/pci.rs | 16 +++------------ rust/kernel/platform.rs | 17 +++------------- 3 files changed, 50 insertions(+), 27 deletions(-) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 21b343a1dc4d..9520e054996d 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -235,6 +235,50 @@ impl Sealed for super::Normal {} impl DeviceContext for Core {} impl DeviceContext for Normal {} +/// # Safety +/// +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the +/// generic argument of `$device`. +#[doc(hidden)] +#[macro_export] +macro_rules! __impl_device_context_deref { + (unsafe { $device:ident, $src:ty => $dst:ty }) => { + impl ::core::ops::Deref for $device<$src> { + type Target = $device<$dst>; + + fn deref(&self) -> &Self::Target { + let ptr: *const Self = self; + + // CAST: `$device<$src>` and `$device<$dst>` transparently wrap the same type by the + // safety requirement of the macro. + let ptr = ptr.cast::(); + + // SAFETY: `ptr` was derived from `&self`. + unsafe { &*ptr } + } + } + }; +} + +/// Implement [`core::ops::Deref`] traits for allowed [`DeviceContext`] conversions of a (bus +/// specific) device. +/// +/// # Safety +/// +/// The type given as `$device` must be a transparent wrapper of a type that doesn't depend on the +/// generic argument of `$device`. +#[macro_export] +macro_rules! impl_device_context_deref { + (unsafe { $device:ident }) => { + // SAFETY: This macro has the exact same safety requirement as + // `__impl_device_context_deref!`. + ::kernel::__impl_device_context_deref!(unsafe { + $device, + $crate::device::Core => $crate::device::Normal + }); + }; +} + #[doc(hidden)] #[macro_export] macro_rules! dev_printk { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index c97d6d470b28..8474608e7a90 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -422,19 +422,9 @@ pub fn set_master(&self) { } } -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 } - } -} +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); impl From<&Device> for ARef { fn from(dev: &Device) -> Self { diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 4917cb34e2fe..22590bdff7bb 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -16,7 +16,6 @@ use core::{ marker::PhantomData, - ops::Deref, ptr::{addr_of_mut, NonNull}, }; @@ -190,19 +189,9 @@ fn as_raw(&self) -> *mut bindings::platform_device { } } -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 } - } -} +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); impl From<&Device> for ARef { fn from(dev: &Device) -> Self { From fbb92b6a534081cabd75861ac9c7a8d29d8effda Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:57 +0200 Subject: [PATCH 03/47] rust: device: implement impl_device_context_into_aref! Implement a macro to implement all From conversions of a certain device to ARef. This avoids unnecessary boiler plate code for every device implementation. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-3-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 21 +++++++++++++++++++++ rust/kernel/pci.rs | 7 +------ rust/kernel/platform.rs | 9 ++------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 9520e054996d..7b96c99879a6 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -279,6 +279,27 @@ macro_rules! impl_device_context_deref { }; } +#[doc(hidden)] +#[macro_export] +macro_rules! __impl_device_context_into_aref { + ($src:ty, $device:tt) => { + impl ::core::convert::From<&$device<$src>> for $crate::types::ARef<$device> { + fn from(dev: &$device<$src>) -> Self { + (&**dev).into() + } + } + }; +} + +/// Implement [`core::convert::From`], such that all `&Device` can be converted to an +/// `ARef`. +#[macro_export] +macro_rules! impl_device_context_into_aref { + ($device:tt) => { + ::kernel::__impl_device_context_into_aref!($crate::device::Core, $device); + }; +} + #[doc(hidden)] #[macro_export] macro_rules! dev_printk { diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 8474608e7a90..7c6ec05383e0 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -425,12 +425,7 @@ pub fn set_master(&self) { // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); - -impl From<&Device> for ARef { - fn from(dev: &Device) -> Self { - (&**dev).into() - } -} +kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 22590bdff7bb..9476b717c425 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -10,7 +10,7 @@ of, prelude::*, str::CStr, - types::{ARef, ForeignOwnable, Opaque}, + types::{ForeignOwnable, Opaque}, ThisModule, }; @@ -192,12 +192,7 @@ fn as_raw(&self) -> *mut bindings::platform_device { // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); - -impl From<&Device> for ARef { - fn from(dev: &Device) -> Self { - (&**dev).into() - } -} +kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { From d32e4c24a7fe01195ba427c67fc15864279a3c0d Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:58 +0200 Subject: [PATCH 04/47] rust: device: implement device context for Device Analogous to bus specific device, implement the DeviceContext generic for generic devices. This is used for APIs that work with generic devices (such as Devres) to evaluate the device's context. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 7b96c99879a6..53a13dd4d2d4 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -9,7 +9,7 @@ str::CStr, types::{ARef, Opaque}, }; -use core::{fmt, ptr}; +use core::{fmt, marker::PhantomData, ptr}; #[cfg(CONFIG_PRINTK)] use crate::c_str; @@ -42,7 +42,7 @@ /// `bindings::device::release` is valid to be called from any thread, hence `ARef` can be /// dropped from any thread. #[repr(transparent)] -pub struct Device(Opaque); +pub struct Device(Opaque, PhantomData); impl Device { /// Creates a new reference-counted abstraction instance of an existing `struct device` pointer. @@ -59,7 +59,9 @@ pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef { // SAFETY: By the safety requirements ptr is valid unsafe { Self::as_ref(ptr) }.into() } +} +impl Device { /// Obtain the raw `struct device *`. pub(crate) fn as_raw(&self) -> *mut bindings::device { self.0.get() @@ -189,6 +191,11 @@ pub fn property_present(&self, name: &CStr) -> bool { } } +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); +kernel::impl_device_context_into_aref!(Device); + // SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate::types::AlwaysRefCounted for Device { fn inc_ref(&self) { From da6c47c6cb45ffb392e016631c08098ad2a6d418 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:36:59 +0200 Subject: [PATCH 05/47] rust: platform: preserve device context in AsRef Since device::Device has a generic over its context, preserve this device context in AsRef. For instance, when calling platform::Device the new AsRef implementation returns device::Device. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/platform.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 9476b717c425..b1c48cd95cd6 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -183,7 +183,7 @@ pub struct Device( PhantomData, ); -impl Device { +impl Device { fn as_raw(&self) -> *mut bindings::platform_device { self.0.get() } @@ -207,8 +207,8 @@ unsafe fn dec_ref(obj: NonNull) { } } -impl AsRef for Device { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { // 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) }; From 3edaefbf2b1beb9ae1cb2a842f455157b951e9f1 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:00 +0200 Subject: [PATCH 06/47] rust: pci: preserve device context in AsRef Since device::Device has a generic over its context, preserve this device context in AsRef. For instance, when calling pci::Device the new AsRef implementation returns device::Device. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-6-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 7c6ec05383e0..1234b0c4a403 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -360,11 +360,13 @@ fn deref(&self) -> &Self::Target { } } -impl Device { +impl Device { fn as_raw(&self) -> *mut bindings::pci_dev { self.0.get() } +} +impl Device { /// Returns the PCI vendor ID. pub fn vendor_id(&self) -> u16 { // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`. @@ -440,8 +442,8 @@ unsafe fn dec_ref(obj: NonNull) { } } -impl AsRef for Device { - fn as_ref(&self) -> &device::Device { +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { // 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) }; From f933b7489ffca25c9a33b65441679ee3d2943024 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:01 +0200 Subject: [PATCH 07/47] rust: device: implement Bound device context The Bound device context indicates that a device is bound to a driver. It must be used for APIs that require the device to be bound, such as Devres or dma::CoherentAllocation. Implement Bound and add the corresponding Deref hierarchy, as well as the corresponding ARef conversion for this device context. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-7-dakr@kernel.org [ Add missing `::` prefix in macros. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 53a13dd4d2d4..0353c5552769 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -232,13 +232,19 @@ pub trait DeviceContext: private::Sealed {} /// any of the bus callbacks, such as `probe()`. pub struct Core; +/// The [`Bound`] context is the context of a bus specific device reference when it is guaranteed to +/// be bound for the duration of its lifetime. +pub struct Bound; + mod private { pub trait Sealed {} + impl Sealed for super::Bound {} impl Sealed for super::Core {} impl Sealed for super::Normal {} } +impl DeviceContext for Bound {} impl DeviceContext for Core {} impl DeviceContext for Normal {} @@ -281,7 +287,14 @@ macro_rules! impl_device_context_deref { // `__impl_device_context_deref!`. ::kernel::__impl_device_context_deref!(unsafe { $device, - $crate::device::Core => $crate::device::Normal + $crate::device::Core => $crate::device::Bound + }); + + // SAFETY: This macro has the exact same safety requirement as + // `__impl_device_context_deref!`. + ::kernel::__impl_device_context_deref!(unsafe { + $device, + $crate::device::Bound => $crate::device::Normal }); }; } @@ -304,6 +317,7 @@ fn from(dev: &$device<$src>) -> Self { macro_rules! impl_device_context_into_aref { ($device:tt) => { ::kernel::__impl_device_context_into_aref!($crate::device::Core, $device); + ::kernel::__impl_device_context_into_aref!($crate::device::Bound, $device); }; } From f2a399d7b67c4a6fc0f8e59d1a9eb484efc71b5c Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:02 +0200 Subject: [PATCH 08/47] rust: pci: move iomap_region() to impl Device Require the Bound device context to be able to call iomap_region() and iomap_region_sized(). Creating I/O mapping requires the device to be bound. Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250413173758.12068-8-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/pci.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 1234b0c4a403..3664d35b8e79 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -390,7 +390,9 @@ pub fn resource_len(&self, bar: u32) -> Result { // - by its type invariant `self.as_raw` is always a valid pointer to a `struct pci_dev`. Ok(unsafe { bindings::pci_resource_len(self.as_raw(), bar.try_into()?) }) } +} +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 type size) < SIZE. pub fn iomap_region_sized( From f720efda2db5e609b32100c25d9cf383f082d945 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:03 +0200 Subject: [PATCH 09/47] rust: devres: require a bound device Require the Bound device context to be able to a new Devres container. This ensures that we can't register devres callbacks for unbound devices. Link: https://lore.kernel.org/r/20250413173758.12068-9-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index ddb1ce4a78d9..1e58f5d22044 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -8,7 +8,7 @@ use crate::{ alloc::Flags, bindings, - device::Device, + device::{Bound, Device}, error::{Error, Result}, ffi::c_void, prelude::*, @@ -45,7 +45,7 @@ struct DevresInner { /// # Example /// /// ```no_run -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}}; +/// # use kernel::{bindings, c_str, device::{Bound, Device}, devres::Devres, io::{Io, IoRaw}}; /// # use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. @@ -83,13 +83,10 @@ struct DevresInner { /// unsafe { Io::from_raw(&self.0) } /// } /// } -/// # fn no_run() -> Result<(), Error> { -/// # // SAFETY: Invalid usage; just for the example to get an `ARef` instance. -/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) }; -/// +/// # fn no_run(dev: &Device) -> Result<(), Error> { /// // SAFETY: Invalid usage for example purposes. /// let iomem = unsafe { IoMem::<{ core::mem::size_of::() }>::new(0xBAAAAAAD)? }; -/// let devres = Devres::new(&dev, iomem, GFP_KERNEL)?; +/// let devres = Devres::new(dev, iomem, GFP_KERNEL)?; /// /// let res = devres.try_access().ok_or(ENXIO)?; /// res.write8(0x42, 0x0); @@ -99,7 +96,7 @@ struct DevresInner { pub struct Devres(Arc>); impl DevresInner { - fn new(dev: &Device, data: T, flags: Flags) -> Result>> { + fn new(dev: &Device, data: T, flags: Flags) -> Result>> { let inner = Arc::pin_init( pin_init!( DevresInner { dev: dev.into(), @@ -171,7 +168,7 @@ fn remove_action(this: &Arc) { 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 { + pub fn new(dev: &Device, data: T, flags: Flags) -> Result { let inner = DevresInner::new(dev, data, flags)?; Ok(Devres(inner)) @@ -179,7 +176,7 @@ pub fn new(dev: &Device, data: T, flags: Flags) -> Result { /// 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 { + pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result { let _ = DevresInner::new(dev, data, flags)?; Ok(()) From 7bd1710aac0571d4722f1429a9332c57b3a8feaf Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sun, 13 Apr 2025 19:37:04 +0200 Subject: [PATCH 10/47] rust: dma: require a bound device Require the Bound device context to be able to create new dma::CoherentAllocation instances. DMA memory allocations are only valid to be created for bound devices. Link: https://lore.kernel.org/r/20250413173758.12068-10-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/dma.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 8cdc76043ee7..605e01e35715 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -6,7 +6,7 @@ use crate::{ bindings, build_assert, - device::Device, + device::{Bound, Device}, error::code::*, error::Result, transmute::{AsBytes, FromBytes}, @@ -22,10 +22,10 @@ /// # Examples /// /// ``` -/// use kernel::device::Device; +/// # use kernel::device::{Bound, Device}; /// use kernel::dma::{attrs::*, CoherentAllocation}; /// -/// # fn test(dev: &Device) -> Result { +/// # fn test(dev: &Device) -> Result { /// let attribs = DMA_ATTR_FORCE_CONTIGUOUS | DMA_ATTR_NO_WARN; /// let c: CoherentAllocation = /// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, attribs)?; @@ -143,16 +143,16 @@ impl CoherentAllocation { /// # Examples /// /// ``` - /// use kernel::device::Device; + /// # use kernel::device::{Bound, Device}; /// use kernel::dma::{attrs::*, CoherentAllocation}; /// - /// # fn test(dev: &Device) -> Result { + /// # fn test(dev: &Device) -> Result { /// let c: CoherentAllocation = /// CoherentAllocation::alloc_attrs(dev, 4, GFP_KERNEL, DMA_ATTR_NO_WARN)?; /// # Ok::<(), Error>(()) } /// ``` pub fn alloc_attrs( - dev: &Device, + dev: &Device, count: usize, gfp_flags: kernel::alloc::Flags, dma_attrs: Attrs, @@ -194,7 +194,7 @@ pub fn alloc_attrs( /// Performs the same functionality as [`CoherentAllocation::alloc_attrs`], except the /// `dma_attrs` is 0 by default. pub fn alloc_coherent( - dev: &Device, + dev: &Device, count: usize, gfp_flags: kernel::alloc::Flags, ) -> Result> { From a095d0d1e4849ee25c28d43d713a8f433d7f1f27 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 21 Mar 2025 22:47:54 +0100 Subject: [PATCH 11/47] rust: pci: impl TryFrom<&Device> for &pci::Device Implement TryFrom<&device::Device> for &Device. This allows us to get a &pci::Device from a generic &Device in a safe way; the conversion fails if the device' bus type does not match with the PCI bus type. Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250321214826.140946-3-dakr@kernel.org [ Support device context types, use dev_is_pci() helper. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/helpers/pci.c | 5 +++++ rust/kernel/pci.rs | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c index 8ba22f911459..cd0e6bf2cc4d 100644 --- a/rust/helpers/pci.c +++ b/rust/helpers/pci.c @@ -16,3 +16,8 @@ resource_size_t rust_helper_pci_resource_len(struct pci_dev *pdev, int bar) { return pci_resource_len(pdev, bar); } + +bool rust_helper_dev_is_pci(const struct device *dev) +{ + return dev_is_pci(dev); +} diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 3664d35b8e79..38fc8d5ffbf9 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -6,7 +6,7 @@ use crate::{ alloc::flags::*, - bindings, device, + bindings, container_of, device, device_id::RawDeviceId, devres::Devres, driver, @@ -455,6 +455,26 @@ fn as_ref(&self) -> &device::Device { } } +impl TryFrom<&device::Device> for &Device { + type Error = kernel::error::Error; + + fn try_from(dev: &device::Device) -> Result { + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a + // `struct device`. + if !unsafe { bindings::dev_is_pci(dev.as_raw()) } { + return Err(EINVAL); + } + + // SAFETY: We've just verified that the bus type of `dev` equals `bindings::pci_bus_type`, + // hence `dev` must be embedded in a valid `struct pci_dev` as guaranteed by the + // corresponding C code. + let pdev = unsafe { container_of!(dev.as_raw(), bindings::pci_dev, dev) }; + + // SAFETY: `pdev` is a valid pointer to a `struct pci_dev`. + Ok(unsafe { &*pdev.cast() }) + } +} + // SAFETY: A `Device` is always reference-counted and can be released from any thread. unsafe impl Send for Device {} From a38dfd60fe53a46a93517f02a061bb34c47946dc Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 21 Mar 2025 22:47:55 +0100 Subject: [PATCH 12/47] rust: platform: impl TryFrom<&Device> for &platform::Device Implement TryFrom<&device::Device> for &Device. This allows us to get a &platform::Device from a generic &Device in a safe way; the conversion fails if the device' bus type does not match with the platform bus type. Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250321214826.140946-4-dakr@kernel.org [ Support device context types, use dev_is_platform() helper. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/helpers/platform.c | 5 +++++ rust/kernel/platform.rs | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/rust/helpers/platform.c b/rust/helpers/platform.c index ab9b9f317301..82171233d12f 100644 --- a/rust/helpers/platform.c +++ b/rust/helpers/platform.c @@ -11,3 +11,8 @@ void rust_helper_platform_set_drvdata(struct platform_device *pdev, void *data) { platform_set_drvdata(pdev, data); } + +bool rust_helper_dev_is_platform(const struct device *dev) +{ + return dev_is_platform(dev); +} diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index b1c48cd95cd6..08849d92c074 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, device, driver, + bindings, container_of, device, driver, error::{to_result, Result}, of, prelude::*, @@ -218,6 +218,26 @@ fn as_ref(&self) -> &device::Device { } } +impl TryFrom<&device::Device> for &Device { + type Error = kernel::error::Error; + + fn try_from(dev: &device::Device) -> Result { + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a + // `struct device`. + if !unsafe { bindings::dev_is_platform(dev.as_raw()) } { + return Err(EINVAL); + } + + // SAFETY: We've just verified that the bus type of `dev` equals + // `bindings::platform_bus_type`, hence `dev` must be embedded in a valid + // `struct platform_device` as guaranteed by the corresponding C code. + let pdev = unsafe { container_of!(dev.as_raw(), bindings::platform_device, dev) }; + + // SAFETY: `pdev` is a valid pointer to a `struct platform_device`. + Ok(unsafe { &*pdev.cast() }) + } +} + // SAFETY: A `Device` is always reference-counted and can be released from any thread. unsafe impl Send for Device {} From 9647b6c5095aa54701df009a7335d07013cd13c4 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:04 +0200 Subject: [PATCH 13/47] rust: types: add `Opaque::zeroed` Analogous to `Opaque::uninit` add `Opaque::zeroed`, which sets the corresponding memory to zero. In contrast to `Opaque::uninit`, the corresponding value, depending on its type, may be initialized. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250414131934.28418-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/types.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 9d0471afc964..eee387727d1a 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -329,6 +329,14 @@ pub const fn uninit() -> Self { } } + /// Creates a new zeroed opaque value. + pub const fn zeroed() -> Self { + Self { + value: UnsafeCell::new(MaybeUninit::zeroed()), + _pin: PhantomPinned, + } + } + /// Create an opaque pin-initializer from the given pin-initializer. pub fn pin_init(slot: impl PinInit) -> impl PinInit { Self::ffi_init(|ptr: *mut T| { From a4c9f71e3440dc6e944fa05bb7fcb3949fe5bcd4 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:05 +0200 Subject: [PATCH 14/47] rust: device: implement Device::parent() Device::parent() returns a reference to the device' parent device, if any. Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 0353c5552769..0d237cebd65e 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -67,6 +67,25 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { self.0.get() } + /// Returns a reference to the parent device, if any. + #[expect(unused)] + pub(crate) fn parent(&self) -> Option<&Self> { + // SAFETY: + // - By the type invariant `self.as_raw()` is always valid. + // - The parent device is only ever set at device creation. + let parent = unsafe { (*self.as_raw()).parent }; + + if parent.is_null() { + None + } else { + // SAFETY: + // - Since `parent` is not NULL, it must be a valid pointer to a `struct device`. + // - `parent` is valid for the lifetime of `self`, since a `struct device` holds a + // reference count of its parent. + Some(unsafe { Self::as_ref(parent) }) + } + } + /// Convert a raw C `struct device` pointer to a `&'a Device`. /// /// # Safety From ce735e73dd59b169b877cedd0753297c81c2a091 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:06 +0200 Subject: [PATCH 15/47] rust: auxiliary: add auxiliary device / driver abstractions Implement the basic auxiliary abstractions required to implement a driver matching an auxiliary device. The design and implementation is analogous to PCI and platform and is based on the generic device / driver abstractions. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-4-dakr@kernel.org [ Fix typos, `let _ =` => `drop()`, use `kernel::ffi`. - Danilo ] Signed-off-by: Danilo Krummrich --- MAINTAINERS | 2 + rust/bindings/bindings_helper.h | 1 + rust/helpers/auxiliary.c | 23 +++ rust/helpers/helpers.c | 1 + rust/kernel/auxiliary.rs | 274 ++++++++++++++++++++++++++++++++ rust/kernel/device.rs | 1 - rust/kernel/lib.rs | 2 + 7 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 rust/helpers/auxiliary.c create mode 100644 rust/kernel/auxiliary.rs diff --git a/MAINTAINERS b/MAINTAINERS index 96b827049501..5a1952293119 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3872,6 +3872,8 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git F: Documentation/driver-api/auxiliary_bus.rst F: drivers/base/auxiliary.c F: include/linux/auxiliary_bus.h +F: rust/helpers/auxiliary.c +F: rust/kernel/auxiliary.rs AUXILIARY DISPLAY DRIVERS M: Andy Shevchenko diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index ab37e1d35c70..8a2add69e5d6 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -7,6 +7,7 @@ */ #include +#include #include #include #include diff --git a/rust/helpers/auxiliary.c b/rust/helpers/auxiliary.c new file mode 100644 index 000000000000..0db3860d774e --- /dev/null +++ b/rust/helpers/auxiliary.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include + +void rust_helper_auxiliary_set_drvdata(struct auxiliary_device *adev, void *data) +{ + auxiliary_set_drvdata(adev, data); +} + +void *rust_helper_auxiliary_get_drvdata(struct auxiliary_device *adev) +{ + return auxiliary_get_drvdata(adev); +} + +void rust_helper_auxiliary_device_uninit(struct auxiliary_device *adev) +{ + return auxiliary_device_uninit(adev); +} + +void rust_helper_auxiliary_device_delete(struct auxiliary_device *adev) +{ + return auxiliary_device_delete(adev); +} diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index e1c21eba9b15..6b279279cb12 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -7,6 +7,7 @@ * Sorted alphabetically. */ +#include "auxiliary.c" #include "blk.c" #include "bug.c" #include "build_assert.c" diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs new file mode 100644 index 000000000000..e15596263c22 --- /dev/null +++ b/rust/kernel/auxiliary.rs @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Abstractions for the auxiliary bus. +//! +//! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) + +use crate::{ + bindings, device, + device_id::RawDeviceId, + driver, + error::{to_result, Result}, + prelude::*, + str::CStr, + types::{ForeignOwnable, Opaque}, + ThisModule, +}; +use core::{ + marker::PhantomData, + ptr::{addr_of_mut, NonNull}, +}; + +/// An adapter for the registration of auxiliary drivers. +pub struct Adapter(T); + +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if +// a preceding call to `register` has been successful. +unsafe impl driver::RegistrationOps for Adapter { + type RegType = bindings::auxiliary_driver; + + unsafe fn register( + adrv: &Opaque, + name: &'static CStr, + module: &'static ThisModule, + ) -> Result { + // SAFETY: It's safe to set the fields of `struct auxiliary_driver` on initialization. + unsafe { + (*adrv.get()).name = name.as_char_ptr(); + (*adrv.get()).probe = Some(Self::probe_callback); + (*adrv.get()).remove = Some(Self::remove_callback); + (*adrv.get()).id_table = T::ID_TABLE.as_ptr(); + } + + // SAFETY: `adrv` is guaranteed to be a valid `RegType`. + to_result(unsafe { + bindings::__auxiliary_driver_register(adrv.get(), module.0, name.as_char_ptr()) + }) + } + + unsafe fn unregister(adrv: &Opaque) { + // SAFETY: `adrv` is guaranteed to be a valid `RegType`. + unsafe { bindings::auxiliary_driver_unregister(adrv.get()) } + } +} + +impl Adapter { + extern "C" fn probe_callback( + adev: *mut bindings::auxiliary_device, + id: *const bindings::auxiliary_device_id, + ) -> kernel::ffi::c_int { + // SAFETY: The auxiliary bus only ever calls the probe callback with a valid pointer to a + // `struct auxiliary_device`. + // + // INVARIANT: `adev` is valid for the duration of `probe_callback()`. + let adev = unsafe { &*adev.cast::>() }; + + // SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_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(adev, info) { + Ok(data) => { + // Let the `struct auxiliary_device` own a reference of the driver's private data. + // SAFETY: By the type invariant `adev.as_raw` returns a valid pointer to a + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_set_drvdata(adev.as_raw(), data.into_foreign()) }; + } + Err(err) => return Error::to_errno(err), + } + + 0 + } + + extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) { + // SAFETY: The auxiliary bus only ever calls the remove callback with a valid pointer to a + // `struct auxiliary_device`. + let ptr = unsafe { bindings::auxiliary_get_drvdata(adev) }; + + // SAFETY: `remove_callback` is only ever called after a successful call to + // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized + // `KBox` pointer created through `KBox::into_foreign`. + drop(unsafe { KBox::::from_foreign(ptr) }); + } +} + +/// Declares a kernel module that exposes a single auxiliary driver. +#[macro_export] +macro_rules! module_auxiliary_driver { + ($($f:tt)*) => { + $crate::module_driver!(, $crate::auxiliary::Adapter, { $($f)* }); + }; +} + +/// Abstraction for `bindings::auxiliary_device_id`. +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::auxiliary_device_id); + +impl DeviceId { + /// Create a new [`DeviceId`] from name. + pub const fn new(modname: &'static CStr, name: &'static CStr) -> Self { + let name = name.as_bytes_with_nul(); + let modname = modname.as_bytes_with_nul(); + + // TODO: Replace with `bindings::auxiliary_device_id::default()` once stabilized for + // `const`. + // + // SAFETY: FFI type is valid to be zero-initialized. + let mut id: bindings::auxiliary_device_id = unsafe { core::mem::zeroed() }; + + let mut i = 0; + while i < modname.len() { + id.name[i] = modname[i]; + i += 1; + } + + // Reuse the space of the NULL terminator. + id.name[i - 1] = b'.'; + + let mut j = 0; + while j < name.len() { + id.name[i] = name[j]; + i += 1; + j += 1; + } + + Self(id) + } +} + +// SAFETY: +// * `DeviceId` is a `#[repr(transparent)`] wrapper of `auxiliary_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. +// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::auxiliary_device_id; + + const DRIVER_DATA_OFFSET: usize = + core::mem::offset_of!(bindings::auxiliary_device_id, driver_data); + + fn index(&self) -> usize { + self.0.driver_data + } +} + +/// IdTable type for auxiliary drivers. +pub type IdTable = &'static dyn kernel::device_id::IdTable; + +/// Create a auxiliary `IdTable` with its alias for modpost. +#[macro_export] +macro_rules! auxiliary_device_table { + ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => { + const $table_name: $crate::device_id::IdArray< + $crate::auxiliary::DeviceId, + $id_info_type, + { $table_data.len() }, + > = $crate::device_id::IdArray::new($table_data); + + $crate::module_device_table!("auxiliary", $module_table_name, $table_name); + }; +} + +/// The auxiliary driver trait. +/// +/// Drivers must implement this trait in order to get an auxiliary driver registered. +pub trait Driver { + /// The type holding information about each device id supported by the driver. + /// + /// TODO: Use associated_type_defaults once stabilized: + /// + /// type IdInfo: 'static = (); + type IdInfo: 'static; + + /// The table of device ids supported by the driver. + const ID_TABLE: IdTable; + + /// Auxiliary driver probe. + /// + /// Called when an auxiliary device is matches a corresponding driver. + fn probe(dev: &Device, id_info: &Self::IdInfo) -> Result>>; +} + +/// The auxiliary device representation. +/// +/// This structure represents the Rust abstraction for a C `struct auxiliary_device`. The +/// implementation abstracts the usage of an already existing C `struct auxiliary_device` within +/// Rust code that we get passed from the C side. +/// +/// # Invariants +/// +/// A [`Device`] instance represents a valid `struct auxiliary_device` created by the C portion of +/// the kernel. +#[repr(transparent)] +pub struct Device( + Opaque, + PhantomData, +); + +impl Device { + fn as_raw(&self) -> *mut bindings::auxiliary_device { + self.0.get() + } + + /// Returns the auxiliary device' id. + pub fn id(&self) -> u32 { + // SAFETY: By the type invariant `self.as_raw()` is a valid pointer to a + // `struct auxiliary_device`. + unsafe { (*self.as_raw()).id } + } + + /// Returns a reference to the parent [`device::Device`], if any. + pub fn parent(&self) -> Option<&device::Device> { + let ptr: *const Self = self; + // CAST: `Device` types are transparent to each other. + let ptr: *const Device = ptr.cast(); + // SAFETY: `ptr` was derived from `&self`. + let this = unsafe { &*ptr }; + + this.as_ref().parent() + } +} + +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic +// argument. +kernel::impl_device_context_deref!(unsafe { Device }); +kernel::impl_device_context_into_aref!(Device); + +// 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) { + // CAST: `Self` a transparent wrapper of `bindings::auxiliary_device`. + let adev: *mut bindings::auxiliary_device = obj.cast().as_ptr(); + + // SAFETY: By the type invariant of `Self`, `adev` is a pointer to a valid + // `struct auxiliary_device`. + let dev = unsafe { addr_of_mut!((*adev).dev) }; + + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::put_device(dev) } + } +} + +impl AsRef> for Device { + fn as_ref(&self) -> &device::Device { + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid + // `struct auxiliary_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) } + } +} + +// 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 {} diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 0d237cebd65e..40c1f549b0ba 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -68,7 +68,6 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { } /// Returns a reference to the parent device, if any. - #[expect(unused)] pub(crate) fn parent(&self) -> Option<&Self> { // SAFETY: // - By the type invariant `self.as_raw()` is always valid. diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index de07aadd1ff5..55a8dfeece0b 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -38,6 +38,8 @@ pub use ffi; pub mod alloc; +#[cfg(CONFIG_AUXILIARY_BUS)] +pub mod auxiliary; #[cfg(CONFIG_BLOCK)] pub mod block; #[doc(hidden)] From 0d1803d25f8c7586beb3843c8efbb83f24ce2539 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:07 +0200 Subject: [PATCH 16/47] rust: auxiliary: add auxiliary registration Implement the `auxiliary::Registration` type, which provides an API to create and register new auxiliary devices in the system. Acked-by: Greg Kroah-Hartman Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250414131934.28418-5-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/auxiliary.rs | 88 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index e15596263c22..5c072960dee0 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) use crate::{ - bindings, device, + bindings, container_of, device, device_id::RawDeviceId, driver, error::{to_result, Result}, @@ -230,6 +230,18 @@ pub fn parent(&self) -> Option<&device::Device> { } } +impl Device { + extern "C" fn release(dev: *mut bindings::device) { + // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` + // embedded in `struct auxiliary_device`. + let adev = unsafe { container_of!(dev, bindings::auxiliary_device, dev) }.cast_mut(); + + // SAFETY: `adev` points to the memory that has been allocated in `Registration::new`, via + // `KBox::new(Opaque::::zeroed(), GFP_KERNEL)`. + let _ = unsafe { KBox::>::from_raw(adev.cast()) }; + } +} + // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); @@ -272,3 +284,77 @@ 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 {} + +/// The registration of an auxiliary device. +/// +/// This type represents the registration of a [`struct auxiliary_device`]. When an instance of this +/// type is dropped, its respective auxiliary device will be unregistered from the system. +/// +/// # Invariants +/// +/// `self.0` always holds a valid pointer to an initialized and registered +/// [`struct auxiliary_device`]. +pub struct Registration(NonNull); + +impl Registration { + /// Create and register a new auxiliary device. + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result { + let boxed = KBox::new(Opaque::::zeroed(), GFP_KERNEL)?; + let adev = boxed.get(); + + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. + unsafe { + (*adev).dev.parent = parent.as_raw(); + (*adev).dev.release = Some(Device::release); + (*adev).name = name.as_char_ptr(); + (*adev).id = id; + } + + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has not been initialized yet. + unsafe { bindings::auxiliary_device_init(adev) }; + + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. + let _ = KBox::into_raw(boxed); + + // SAFETY: + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has + // been initialialized, + // - `modname.as_char_ptr()` is a NULL terminated string. + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; + if ret != 0 { + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has been initialialized. + unsafe { bindings::auxiliary_device_uninit(adev) }; + + return Err(Error::from_errno(ret)); + } + + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated successfully. + // + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is called, + // which happens in `Self::drop()`. + Ok(Self(unsafe { NonNull::new_unchecked(adev) })) + } +} + +impl Drop for Registration { + fn drop(&mut self) { + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) }; + + // This drops the reference we acquired through `auxiliary_device_init()`. + // + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) }; + } +} + +// SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread. +unsafe impl Send for Registration {} + +// SAFETY: `Registration` does not expose any methods or fields that need synchronization. +unsafe impl Sync for Registration {} From 96609a1969f4ade45351ec368c65580c77592e8b Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 14 Apr 2025 15:18:08 +0200 Subject: [PATCH 17/47] samples: rust: add Rust auxiliary driver sample Add a sample Rust auxiliary driver based on a PCI driver for QEMU's "pci-testdev" device. The PCI driver only registers an auxiliary device, in order to make the corresponding auxiliary driver probe. Acked-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250414131934.28418-6-dakr@kernel.org [ Use `ok_or()` when accessing auxiliary::Device::parent(). - Danilo ] Signed-off-by: Danilo Krummrich --- MAINTAINERS | 1 + samples/rust/Kconfig | 12 +++ samples/rust/Makefile | 1 + samples/rust/rust_driver_auxiliary.rs | 120 ++++++++++++++++++++++++++ 4 files changed, 134 insertions(+) create mode 100644 samples/rust/rust_driver_auxiliary.rs diff --git a/MAINTAINERS b/MAINTAINERS index 5a1952293119..34757b8641c3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3874,6 +3874,7 @@ F: drivers/base/auxiliary.c F: include/linux/auxiliary_bus.h F: rust/helpers/auxiliary.c F: rust/kernel/auxiliary.rs +F: samples/rust/rust_driver_auxiliary.rs AUXILIARY DISPLAY DRIVERS M: Andy Shevchenko diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig index cad52b7120b5..43cb72d72631 100644 --- a/samples/rust/Kconfig +++ b/samples/rust/Kconfig @@ -82,6 +82,18 @@ config SAMPLE_RUST_DRIVER_FAUX If unsure, say N. +config SAMPLE_RUST_DRIVER_AUXILIARY + tristate "Auxiliary Driver" + depends on AUXILIARY_BUS + depends on PCI + help + This option builds the Rust auxiliary driver sample. + + To compile this as a module, choose M here: + the module will be called rust_driver_auxiliary. + + If unsure, say N. + config SAMPLE_RUST_HOSTPROGS bool "Host programs" help diff --git a/samples/rust/Makefile b/samples/rust/Makefile index c6a2479f7d9c..6a466afd2a21 100644 --- a/samples/rust/Makefile +++ b/samples/rust/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_SAMPLE_RUST_DMA) += rust_dma.o obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI) += rust_driver_pci.o obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM) += rust_driver_platform.o obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX) += rust_driver_faux.o +obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY) += rust_driver_auxiliary.o rust_print-y := rust_print_main.o rust_print_events.o diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs new file mode 100644 index 000000000000..3e15e6d002bb --- /dev/null +++ b/samples/rust/rust_driver_auxiliary.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Rust auxiliary driver sample (based on a PCI driver for QEMU's `pci-testdev`). +//! +//! To make this driver probe, QEMU must be run with `-device pci-testdev`. + +use kernel::{ + auxiliary, bindings, c_str, device::Core, driver, error::Error, pci, prelude::*, str::CStr, + InPlaceModule, +}; + +use pin_init::PinInit; + +const MODULE_NAME: &CStr = ::NAME; +const AUXILIARY_NAME: &CStr = c_str!("auxiliary"); + +struct AuxiliaryDriver; + +kernel::auxiliary_device_table!( + AUX_TABLE, + MODULE_AUX_TABLE, + ::IdInfo, + [(auxiliary::DeviceId::new(MODULE_NAME, AUXILIARY_NAME), ())] +); + +impl auxiliary::Driver for AuxiliaryDriver { + type IdInfo = (); + + const ID_TABLE: auxiliary::IdTable = &AUX_TABLE; + + fn probe(adev: &auxiliary::Device, _info: &Self::IdInfo) -> Result>> { + dev_info!( + adev.as_ref(), + "Probing auxiliary driver for auxiliary device with id={}\n", + adev.id() + ); + + ParentDriver::connect(adev)?; + + let this = KBox::new(Self, GFP_KERNEL)?; + + Ok(this.into()) + } +} + +struct ParentDriver { + _reg: [auxiliary::Registration; 2], +} + +kernel::pci_device_table!( + PCI_TABLE, + MODULE_PCI_TABLE, + ::IdInfo, + [( + pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, 0x5), + () + )] +); + +impl pci::Driver for ParentDriver { + type IdInfo = (); + + const ID_TABLE: pci::IdTable = &PCI_TABLE; + + fn probe(pdev: &pci::Device, _info: &Self::IdInfo) -> Result>> { + let this = KBox::new( + Self { + _reg: [ + auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 0, MODULE_NAME)?, + auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 1, MODULE_NAME)?, + ], + }, + GFP_KERNEL, + )?; + + Ok(this.into()) + } +} + +impl ParentDriver { + fn connect(adev: &auxiliary::Device) -> Result<()> { + let parent = adev.parent().ok_or(EINVAL)?; + let pdev: &pci::Device = parent.try_into()?; + + dev_info!( + adev.as_ref(), + "Connect auxiliary {} with parent: VendorID={:#x}, DeviceID={:#x}\n", + adev.id(), + pdev.vendor_id(), + pdev.device_id() + ); + + Ok(()) + } +} + +#[pin_data] +struct SampleModule { + #[pin] + _pci_driver: driver::Registration>, + #[pin] + _aux_driver: driver::Registration>, +} + +impl InPlaceModule for SampleModule { + fn init(module: &'static kernel::ThisModule) -> impl PinInit { + try_pin_init!(Self { + _pci_driver <- driver::Registration::new(MODULE_NAME, module), + _aux_driver <- driver::Registration::new(MODULE_NAME, module), + }) + } +} + +module! { + type: SampleModule, + name: "rust_driver_auxiliary", + author: "Danilo Krummrich", + description: "Rust auxiliary driver", + license: "GPL v2", +} From 80e62fcea4f3ce8c7cc3205d7543e532b255d322 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Fri, 11 Apr 2025 21:09:38 +0900 Subject: [PATCH 18/47] rust/revocable: add try_access_with() convenience method Revocable::try_access() returns a guard through which the wrapped object can be accessed. Code that can sleep is not allowed while the guard is held; thus, it is common for the caller to explicitly drop it before running sleepable code, e.g: let b = bar.try_access()?; let reg = b.readl(...); // Don't forget this or things could go wrong! drop(b); something_that_might_sleep(); let b = bar.try_access()?; let reg2 = b.readl(...); This is arguably error-prone. try_access_with() provides an arguably safer alternative, by taking a closure that is run while the guard is held, and by dropping the guard automatically after the closure completes. This way, code can be organized more clearly around the critical sections and the risk of forgetting to release the guard when needed is considerably reduced: let reg = bar.try_access_with(|b| b.readl(...))?; something_that_might_sleep(); let reg2 = bar.try_access_with(|b| b.readl(...))?; The closure can return nothing, or any value including a Result which is then wrapped inside the Option returned by try_access_with. Error management is driver-specific, so users are encouraged to create their own macros that map and flatten the returned values to something appropriate for the code they are working on. Suggested-by: Danilo Krummrich Reviewed-by: Benno Lossin Signed-off-by: Alexandre Courbot Reviewed-by: Joel Fernandes Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250411-try_with-v4-1-f470ac79e2e2@nvidia.com [ Link `None`, `Some`, `Option` in doc-comment. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index 1e5a9d25c21b..971d0dc38d83 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -123,6 +123,22 @@ pub fn try_access_with_guard<'a>(&'a self, _guard: &'a rcu::Guard) -> Option<&'a } } + /// Tries to access the wrapped object and run a closure on it while the guard is held. + /// + /// This is a convenience method to run short non-sleepable code blocks while ensuring the + /// guard is dropped afterwards. [`Self::try_access`] carries the risk that the caller will + /// forget to explicitly drop that returned guard before calling sleepable code; this method + /// adds an extra safety to make sure it doesn't happen. + /// + /// Returns [`None`] if the object has been revoked and is therefore no longer accessible, or + /// the result of the closure wrapped in [`Some`]. If the closure returns a [`Result`] then the + /// return type becomes `Option>`, which can be inconvenient. Users are encouraged to + /// define their own macro that turns the [`Option`] into a proper error code and flattens the + /// inner result into it if it makes sense within their subsystem. + pub fn try_access_with R>(&self, f: F) -> Option { + self.try_access().map(|t| f(&*t)) + } + /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. From 0c848b3adb45acc1722f94333e2a97bf1720e431 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Fri, 11 Apr 2025 21:09:39 +0900 Subject: [PATCH 19/47] samples: rust: convert PCI rust sample driver to use try_access_with() This method limits the scope of the revocable guard and is considered safer to use for most cases, so let's showcase it here. Reviewed-by: Benno Lossin Signed-off-by: Alexandre Courbot Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250411-try_with-v4-2-f470ac79e2e2@nvidia.com Signed-off-by: Danilo Krummrich --- samples/rust/rust_driver_pci.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index 2bb260aebc9e..9ce3a7323a16 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -83,13 +83,12 @@ fn probe(pdev: &pci::Device, info: &Self::IdInfo) -> Result GFP_KERNEL, )?; - let bar = drvdata.bar.try_access().ok_or(ENXIO)?; + let res = drvdata + .bar + .try_access_with(|b| Self::testdev(info, b)) + .ok_or(ENXIO)??; - dev_info!( - pdev.as_ref(), - "pci-testdev data-match count: {}\n", - Self::testdev(info, &bar)? - ); + dev_info!(pdev.as_ref(), "pci-testdev data-match count: {}\n", res); Ok(drvdata.into()) } From 57493a145552fe8a9a926f25b14c324c441ca358 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 11 Apr 2025 01:55:20 +0200 Subject: [PATCH 20/47] drm: drv: implement __drm_dev_alloc() In the Rust DRM device abstraction we need to allocate a struct drm_device. Currently, there are two options, the deprecated drm_dev_alloc() (which does not support subclassing) and devm_drm_dev_alloc(). The latter supports subclassing, but also manages the initial reference through devres for the parent device. In Rust we want to conform with the subclassing pattern, but do not want to get the initial reference managed for us, since Rust has its own, idiomatic ways to properly deal with it. There are two options to achieve this. 1) Allocate the memory ourselves with a KBox. 2) Implement __drm_dev_alloc(), which supports subclassing, but is unmanged. While (1) would be possible, it would be cumbersome, since it would require exporting drm_dev_init() and drmm_add_final_kfree(). Hence, go with option (2) and implement __drm_dev_alloc(). Reviewed-by: Maxime Ripard Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/drm_drv.c | 58 ++++++++++++++++++++++++++++----------- include/drm/drm_drv.h | 5 ++++ 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 17fc5dc708f4..ebb648f1c7a9 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -807,6 +807,47 @@ void *__devm_drm_dev_alloc(struct device *parent, } EXPORT_SYMBOL(__devm_drm_dev_alloc); +/** + * __drm_dev_alloc - Allocation of a &drm_device instance + * @parent: Parent device object + * @driver: DRM driver + * @size: the size of the struct which contains struct drm_device + * @offset: the offset of the &drm_device within the container. + * + * This should *NOT* be by any drivers, but is a dedicated interface for the + * corresponding Rust abstraction. + * + * This is the same as devm_drm_dev_alloc(), but without the corresponding + * resource management through the parent device, but not the same as + * drm_dev_alloc(), since the latter is the deprecated version, which does not + * support subclassing. + * + * Returns: A pointer to new DRM device, or an ERR_PTR on failure. + */ +void *__drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, + size_t size, size_t offset) +{ + void *container; + struct drm_device *drm; + int ret; + + container = kzalloc(size, GFP_KERNEL); + if (!container) + return ERR_PTR(-ENOMEM); + + drm = container + offset; + ret = drm_dev_init(drm, driver, parent); + if (ret) { + kfree(container); + return ERR_PTR(ret); + } + drmm_add_final_kfree(drm, container); + + return container; +} +EXPORT_SYMBOL(__drm_dev_alloc); + /** * drm_dev_alloc - Allocate new DRM device * @driver: DRM driver to allocate device for @@ -822,22 +863,7 @@ EXPORT_SYMBOL(__devm_drm_dev_alloc); struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent) { - struct drm_device *dev; - int ret; - - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (!dev) - return ERR_PTR(-ENOMEM); - - ret = drm_dev_init(dev, driver, parent); - if (ret) { - kfree(dev); - return ERR_PTR(ret); - } - - drmm_add_final_kfree(dev, dev); - - return dev; + return __drm_dev_alloc(parent, driver, sizeof(struct drm_device), 0); } EXPORT_SYMBOL(drm_dev_alloc); diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index a43d707b5f36..63b51942d606 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -473,6 +473,11 @@ drmm_cgroup_register_region(struct drm_device *dev, struct drm_device *drm_dev_alloc(const struct drm_driver *driver, struct device *parent); + +void *__drm_dev_alloc(struct device *parent, + const struct drm_driver *driver, + size_t size, size_t offset); + int drm_dev_register(struct drm_device *dev, unsigned long flags); void drm_dev_unregister(struct drm_device *dev); From 9a69570682b1f179a8bd9439a24495e7a6246aa9 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:21 +0200 Subject: [PATCH 21/47] rust: drm: ioctl: Add DRM ioctl abstraction DRM drivers need to be able to declare which driver-specific ioctls they support. Add an abstraction implementing the required types and a helper macro to generate the ioctl definition inside the DRM driver. Note that this macro is not usable until further bits of the abstraction are in place (but it will not fail to compile on its own, if not called). Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-3-dakr@kernel.org [ MISC fixes * wrap raw_data in Opaque to avoid UB when creating a reference * fix IOCTL sample declaration * fix safety comment of IOCTL argument * original source archive: https://archive.is/LqHDQ - Danilo ] Signed-off-by: Danilo Krummrich --- rust/bindings/bindings_helper.h | 1 + rust/kernel/drm/ioctl.rs | 162 ++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 5 + rust/kernel/lib.rs | 2 + rust/uapi/uapi_helper.h | 1 + 5 files changed, 171 insertions(+) create mode 100644 rust/kernel/drm/ioctl.rs create mode 100644 rust/kernel/drm/mod.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 8a2add69e5d6..c4140c992d43 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -6,6 +6,7 @@ * Sorted alphabetically. */ +#include #include #include #include diff --git a/rust/kernel/drm/ioctl.rs b/rust/kernel/drm/ioctl.rs new file mode 100644 index 000000000000..445639404fb7 --- /dev/null +++ b/rust/kernel/drm/ioctl.rs @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM IOCTL definitions. +//! +//! C header: [`include/linux/drm/drm_ioctl.h`](srctree/include/linux/drm/drm_ioctl.h) + +use crate::ioctl; + +const BASE: u32 = uapi::DRM_IOCTL_BASE as u32; + +/// Construct a DRM ioctl number with no argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IO(nr: u32) -> u32 { + ioctl::_IO(BASE, nr) +} + +/// Construct a DRM ioctl number with a read-only argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOR(nr: u32) -> u32 { + ioctl::_IOR::(BASE, nr) +} + +/// Construct a DRM ioctl number with a write-only argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOW(nr: u32) -> u32 { + ioctl::_IOW::(BASE, nr) +} + +/// Construct a DRM ioctl number with a read-write argument. +#[allow(non_snake_case)] +#[inline(always)] +pub const fn IOWR(nr: u32) -> u32 { + ioctl::_IOWR::(BASE, nr) +} + +/// Descriptor type for DRM ioctls. Use the `declare_drm_ioctls!{}` macro to construct them. +pub type DrmIoctlDescriptor = bindings::drm_ioctl_desc; + +/// This is for ioctl which are used for rendering, and require that the file descriptor is either +/// for a render node, or if it’s a legacy/primary node, then it must be authenticated. +pub const AUTH: u32 = bindings::drm_ioctl_flags_DRM_AUTH; + +/// This must be set for any ioctl which can change the modeset or display state. Userspace must +/// call the ioctl through a primary node, while it is the active master. +/// +/// Note that read-only modeset ioctl can also be called by unauthenticated clients, or when a +/// master is not the currently active one. +pub const MASTER: u32 = bindings::drm_ioctl_flags_DRM_MASTER; + +/// Anything that could potentially wreak a master file descriptor needs to have this flag set. +/// +/// Current that’s only for the SETMASTER and DROPMASTER ioctl, which e.g. logind can call to +/// force a non-behaving master (display compositor) into compliance. +/// +/// This is equivalent to callers with the SYSADMIN capability. +pub const ROOT_ONLY: u32 = bindings::drm_ioctl_flags_DRM_ROOT_ONLY; + +/// This is used for all ioctl needed for rendering only, for drivers which support render nodes. +/// This should be all new render drivers, and hence it should be always set for any ioctl with +/// `AUTH` set. Note though that read-only query ioctl might have this set, but have not set +/// DRM_AUTH because they do not require authentication. +pub const RENDER_ALLOW: u32 = bindings::drm_ioctl_flags_DRM_RENDER_ALLOW; + +/// Internal structures used by the `declare_drm_ioctls!{}` macro. Do not use directly. +#[doc(hidden)] +pub mod internal { + pub use bindings::drm_device; + pub use bindings::drm_file; + pub use bindings::drm_ioctl_desc; +} + +/// Declare the DRM ioctls for a driver. +/// +/// Each entry in the list should have the form: +/// +/// `(ioctl_number, argument_type, flags, user_callback),` +/// +/// `argument_type` is the type name within the `bindings` crate. +/// `user_callback` should have the following prototype: +/// +/// ```ignore +/// fn foo(device: &kernel::drm::Device, +/// data: &Opaque, +/// file: &kernel::drm::File, +/// ) -> Result +/// ``` +/// where `Self` is the drm::drv::Driver implementation these ioctls are being declared within. +/// +/// # Examples +/// +/// ```ignore +/// kernel::declare_drm_ioctls! { +/// (FOO_GET_PARAM, drm_foo_get_param, ioctl::RENDER_ALLOW, my_get_param_handler), +/// } +/// ``` +/// +#[macro_export] +macro_rules! declare_drm_ioctls { + ( $(($cmd:ident, $struct:ident, $flags:expr, $func:expr)),* $(,)? ) => { + const IOCTLS: &'static [$crate::drm::ioctl::DrmIoctlDescriptor] = { + use $crate::uapi::*; + const _:() = { + let i: u32 = $crate::uapi::DRM_COMMAND_BASE; + // Assert that all the IOCTLs are in the right order and there are no gaps, + // and that the size of the specified type is correct. + $( + let cmd: u32 = $crate::macros::concat_idents!(DRM_IOCTL_, $cmd); + ::core::assert!(i == $crate::ioctl::_IOC_NR(cmd)); + ::core::assert!(core::mem::size_of::<$crate::uapi::$struct>() == + $crate::ioctl::_IOC_SIZE(cmd)); + let i: u32 = i + 1; + )* + }; + + let ioctls = &[$( + $crate::drm::ioctl::internal::drm_ioctl_desc { + cmd: $crate::macros::concat_idents!(DRM_IOCTL_, $cmd) as u32, + func: { + #[allow(non_snake_case)] + unsafe extern "C" fn $cmd( + raw_dev: *mut $crate::drm::ioctl::internal::drm_device, + raw_data: *mut ::core::ffi::c_void, + raw_file: *mut $crate::drm::ioctl::internal::drm_file, + ) -> core::ffi::c_int { + // SAFETY: + // - The DRM core ensures the device lives while callbacks are being + // called. + // - The DRM device must have been registered when we're called through + // an IOCTL. + // + // FIXME: Currently there is nothing enforcing that the types of the + // dev/file match the current driver these ioctls are being declared + // for, and it's not clear how to enforce this within the type system. + let dev = $crate::drm::device::Device::as_ref(raw_dev); + // SAFETY: The ioctl argument has size `_IOC_SIZE(cmd)`, which we + // asserted above matches the size of this type, and all bit patterns of + // UAPI structs must be valid. + let data = unsafe { + &*(raw_data as *const $crate::types::Opaque<$crate::uapi::$struct>) + }; + // SAFETY: This is just the DRM file structure + let file = unsafe { $crate::drm::File::as_ref(raw_file) }; + + match $func(dev, data, file) { + Err(e) => e.to_errno(), + Ok(i) => i.try_into() + .unwrap_or($crate::error::code::ERANGE.to_errno()), + } + } + Some($cmd) + }, + flags: $flags, + name: $crate::c_str!(::core::stringify!($cmd)).as_char_ptr(), + } + ),*]; + ioctls + }; + }; +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs new file mode 100644 index 000000000000..9ec6d7cbcaf3 --- /dev/null +++ b/rust/kernel/drm/mod.rs @@ -0,0 +1,5 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM subsystem abstractions. + +pub mod ioctl; diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 55a8dfeece0b..ab0286857061 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -50,6 +50,8 @@ pub mod devres; pub mod dma; pub mod driver; +#[cfg(CONFIG_DRM = "y")] +pub mod drm; pub mod error; pub mod faux; #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)] diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h index 76d3f103e764..19587e55e604 100644 --- a/rust/uapi/uapi_helper.h +++ b/rust/uapi/uapi_helper.h @@ -7,6 +7,7 @@ */ #include +#include #include #include #include From 07c9016085f95fe9ad90079753f156859c54f476 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:22 +0200 Subject: [PATCH 22/47] rust: drm: add driver abstractions Implement the DRM driver abstractions. The `Driver` trait provides the interface to the actual driver to fill in the driver specific data, such as the `DriverInfo`, driver features and IOCTLs. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-4-dakr@kernel.org [ MISC changes * remove unnecessary DRM features; make remaining ones crate private * add #[expect(unused)] to avoid warnings * add sealed trait * remove shmem::Object references * original source archive: https://archive.is/Pl9ys - Danilo ] Signed-off-by: Danilo Krummrich --- rust/bindings/bindings_helper.h | 1 + rust/kernel/drm/driver.rs | 113 ++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 8 +++ 3 files changed, 122 insertions(+) create mode 100644 rust/kernel/drm/driver.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index c4140c992d43..a647fcc3d4a4 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -6,6 +6,7 @@ * Sorted alphabetically. */ +#include #include #include #include diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs new file mode 100644 index 000000000000..c85cf5e4cd7c --- /dev/null +++ b/rust/kernel/drm/driver.rs @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM driver core. +//! +//! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h) + +use crate::{bindings, drm, str::CStr}; +use macros::vtable; + +/// Driver use the GEM memory manager. This should be set for all modern drivers. +#[expect(unused)] +pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM; + +/// Information data for a DRM Driver. +pub struct DriverInfo { + /// Driver major version. + pub major: i32, + /// Driver minor version. + pub minor: i32, + /// Driver patchlevel version. + pub patchlevel: i32, + /// Driver name. + pub name: &'static CStr, + /// Driver description. + pub desc: &'static CStr, +} + +/// Internal memory management operation set, normally created by memory managers (e.g. GEM). +pub struct AllocOps { + #[expect(unused)] + pub(crate) gem_create_object: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + size: usize, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) prime_handle_to_fd: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + file_priv: *mut bindings::drm_file, + handle: u32, + flags: u32, + prime_fd: *mut core::ffi::c_int, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) prime_fd_to_handle: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + file_priv: *mut bindings::drm_file, + prime_fd: core::ffi::c_int, + handle: *mut u32, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) gem_prime_import: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + dma_buf: *mut bindings::dma_buf, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) gem_prime_import_sg_table: Option< + unsafe extern "C" fn( + dev: *mut bindings::drm_device, + attach: *mut bindings::dma_buf_attachment, + sgt: *mut bindings::sg_table, + ) -> *mut bindings::drm_gem_object, + >, + #[expect(unused)] + pub(crate) dumb_create: Option< + unsafe extern "C" fn( + file_priv: *mut bindings::drm_file, + dev: *mut bindings::drm_device, + args: *mut bindings::drm_mode_create_dumb, + ) -> core::ffi::c_int, + >, + #[expect(unused)] + pub(crate) dumb_map_offset: Option< + unsafe extern "C" fn( + file_priv: *mut bindings::drm_file, + dev: *mut bindings::drm_device, + handle: u32, + offset: *mut u64, + ) -> core::ffi::c_int, + >, +} + +/// Trait for memory manager implementations. Implemented internally. +pub trait AllocImpl: super::private::Sealed { + /// The C callback operations for this memory manager. + const ALLOC_OPS: AllocOps; +} + +/// The DRM `Driver` trait. +/// +/// This trait must be implemented by drivers in order to create a `struct drm_device` and `struct +/// drm_driver` to be registered in the DRM subsystem. +#[vtable] +pub trait Driver { + /// Context data associated with the DRM driver + type Data: Sync + Send; + + /// The type used to manage memory for this driver. + type Object: AllocImpl; + + /// Driver metadata + const INFO: DriverInfo; + + /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`. + const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor]; +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 9ec6d7cbcaf3..2e3f9a8a9353 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -2,4 +2,12 @@ //! DRM subsystem abstractions. +pub mod driver; pub mod ioctl; + +pub use self::driver::Driver; +pub use self::driver::DriverInfo; + +pub(crate) mod private { + pub trait Sealed {} +} From 1e4b8896c0f3cb305a32870e1d8624f1155072d5 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:23 +0200 Subject: [PATCH 23/47] rust: drm: add device abstraction Implement the abstraction for a `struct drm_device`. A `drm::Device` creates a static const `struct drm_driver` filled with the data from the `drm::Driver` trait implementation of the actual driver creating the `drm::Device`. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-5-dakr@kernel.org [ Rewrite of drm::Device * full rewrite of the drm::Device abstraction using the subclassing pattern * original source archive: http://archive.today/5NxBo - Danilo ] Signed-off-by: Danilo Krummrich --- rust/bindings/bindings_helper.h | 1 + rust/kernel/drm/device.rs | 198 ++++++++++++++++++++++++++++++++ rust/kernel/drm/driver.rs | 8 -- rust/kernel/drm/mod.rs | 2 + 4 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 rust/kernel/drm/device.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index a647fcc3d4a4..280d3c3159c9 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -6,6 +6,7 @@ * Sorted alphabetically. */ +#include #include #include #include diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs new file mode 100644 index 000000000000..d6c6cecbdd51 --- /dev/null +++ b/rust/kernel/drm/device.rs @@ -0,0 +1,198 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM device. +//! +//! C header: [`include/linux/drm/drm_device.h`](srctree/include/linux/drm/drm_device.h) + +use crate::{ + bindings, device, drm, + drm::driver::AllocImpl, + error::from_err_ptr, + error::Result, + prelude::*, + types::{ARef, AlwaysRefCounted, Opaque}, +}; +use core::{mem, ops::Deref, ptr, ptr::NonNull}; + +#[cfg(CONFIG_DRM_LEGACY)] +macro_rules! drm_legacy_fields { + ( $($field:ident: $val:expr),* $(,)? ) => { + bindings::drm_driver { + $( $field: $val ),*, + firstopen: None, + preclose: None, + dma_ioctl: None, + dma_quiescent: None, + context_dtor: None, + irq_handler: None, + irq_preinstall: None, + irq_postinstall: None, + irq_uninstall: None, + get_vblank_counter: None, + enable_vblank: None, + disable_vblank: None, + dev_priv_size: 0, + } + } +} + +#[cfg(not(CONFIG_DRM_LEGACY))] +macro_rules! drm_legacy_fields { + ( $($field:ident: $val:expr),* $(,)? ) => { + bindings::drm_driver { + $( $field: $val ),* + } + } +} + +/// A typed DRM device with a specific `drm::Driver` implementation. +/// +/// The device is always reference-counted. +/// +/// # Invariants +/// +/// `self.dev` is a valid instance of a `struct device`. +#[repr(C)] +#[pin_data] +pub struct Device { + dev: Opaque, + #[pin] + data: T::Data, +} + +impl Device { + const VTABLE: bindings::drm_driver = drm_legacy_fields! { + load: None, + open: None, // TODO: File abstraction + postclose: None, // TODO: File abstraction + unload: None, + release: None, + master_set: None, + master_drop: None, + debugfs_init: None, + gem_create_object: T::Object::ALLOC_OPS.gem_create_object, + prime_handle_to_fd: T::Object::ALLOC_OPS.prime_handle_to_fd, + prime_fd_to_handle: T::Object::ALLOC_OPS.prime_fd_to_handle, + gem_prime_import: T::Object::ALLOC_OPS.gem_prime_import, + gem_prime_import_sg_table: T::Object::ALLOC_OPS.gem_prime_import_sg_table, + dumb_create: T::Object::ALLOC_OPS.dumb_create, + dumb_map_offset: T::Object::ALLOC_OPS.dumb_map_offset, + show_fdinfo: None, + fbdev_probe: None, + + major: T::INFO.major, + minor: T::INFO.minor, + patchlevel: T::INFO.patchlevel, + name: T::INFO.name.as_char_ptr() as *mut _, + desc: T::INFO.desc.as_char_ptr() as *mut _, + + driver_features: drm::driver::FEAT_GEM, + ioctls: T::IOCTLS.as_ptr(), + num_ioctls: T::IOCTLS.len() as i32, + fops: core::ptr::null_mut() as _, + }; + + /// Create a new `drm::Device` for a `drm::Driver`. + pub fn new(dev: &device::Device, data: impl PinInit) -> Result> { + // SAFETY: + // - `VTABLE`, as a `const` is pinned to the read-only section of the compilation, + // - `dev` is valid by its type invarants, + let raw_drm: *mut Self = unsafe { + bindings::__drm_dev_alloc( + dev.as_raw(), + &Self::VTABLE, + mem::size_of::(), + mem::offset_of!(Self, dev), + ) + } + .cast(); + let raw_drm = NonNull::new(from_err_ptr(raw_drm)?).ok_or(ENOMEM)?; + + // SAFETY: `raw_drm` is a valid pointer to `Self`. + let raw_data = unsafe { ptr::addr_of_mut!((*raw_drm.as_ptr()).data) }; + + // SAFETY: + // - `raw_data` is a valid pointer to uninitialized memory. + // - `raw_data` will not move until it is dropped. + unsafe { data.__pinned_init(raw_data) }.inspect_err(|_| { + // SAFETY: `__drm_dev_alloc()` was successful, hence `raw_drm` must be valid and the + // refcount must be non-zero. + unsafe { bindings::drm_dev_put(ptr::addr_of_mut!((*raw_drm.as_ptr()).dev).cast()) }; + })?; + + // SAFETY: The reference count is one, and now we take ownership of that reference as a + // `drm::Device`. + Ok(unsafe { ARef::from_raw(raw_drm) }) + } + + pub(crate) fn as_raw(&self) -> *mut bindings::drm_device { + self.dev.get() + } + + /// # Safety + /// + /// `ptr` must be a valid pointer to a `struct device` embedded in `Self`. + unsafe fn from_drm_device(ptr: *const bindings::drm_device) -> *mut Self { + // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a + // `struct drm_device` embedded in `Self`. + unsafe { crate::container_of!(ptr, Self, dev) }.cast_mut() + } + + /// Not intended to be called externally, except via declare_drm_ioctls!() + /// + /// # Safety + /// + /// Callers must ensure that `ptr` is valid, non-null, and has a non-zero reference count, + /// i.e. it must be ensured that the reference count of the C `struct drm_device` `ptr` points + /// to can't drop to zero, for the duration of this function call and the entire duration when + /// the returned reference exists. + /// + /// Additionally, callers must ensure that the `struct device`, `ptr` is pointing to, is + /// embedded in `Self`. + #[doc(hidden)] + pub unsafe fn as_ref<'a>(ptr: *const bindings::drm_device) -> &'a Self { + // SAFETY: By the safety requirements of this function `ptr` is a valid pointer to a + // `struct drm_device` embedded in `Self`. + let ptr = unsafe { Self::from_drm_device(ptr) }; + + // SAFETY: `ptr` is valid by the safety requirements of this function. + unsafe { &*ptr.cast() } + } +} + +impl Deref for Device { + type Target = T::Data; + + fn deref(&self) -> &Self::Target { + &self.data + } +} + +// SAFETY: DRM device objects are always reference counted and the get/put functions +// satisfy the requirements. +unsafe impl AlwaysRefCounted for Device { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_dev_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::drm_dev_put(obj.cast().as_ptr()) }; + } +} + +impl AsRef for Device { + fn as_ref(&self) -> &device::Device { + // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid, + // which is guaranteed by the type invariant. + unsafe { device::Device::as_ref((*self.as_raw()).dev) } + } +} + +// SAFETY: A `drm::Device` can be released from any thread. +unsafe impl Send for Device {} + +// SAFETY: A `drm::Device` can be shared among threads because all immutable methods are protected +// by the synchronization in `struct drm_device`. +unsafe impl Sync for Device {} diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index c85cf5e4cd7c..26a5d66cc4c5 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -8,7 +8,6 @@ use macros::vtable; /// Driver use the GEM memory manager. This should be set for all modern drivers. -#[expect(unused)] pub(crate) const FEAT_GEM: u32 = bindings::drm_driver_feature_DRIVER_GEM; /// Information data for a DRM Driver. @@ -27,14 +26,12 @@ pub struct DriverInfo { /// Internal memory management operation set, normally created by memory managers (e.g. GEM). pub struct AllocOps { - #[expect(unused)] pub(crate) gem_create_object: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, size: usize, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) prime_handle_to_fd: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -44,7 +41,6 @@ pub struct AllocOps { prime_fd: *mut core::ffi::c_int, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) prime_fd_to_handle: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -53,14 +49,12 @@ pub struct AllocOps { handle: *mut u32, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) gem_prime_import: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, dma_buf: *mut bindings::dma_buf, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) gem_prime_import_sg_table: Option< unsafe extern "C" fn( dev: *mut bindings::drm_device, @@ -68,7 +62,6 @@ pub struct AllocOps { sgt: *mut bindings::sg_table, ) -> *mut bindings::drm_gem_object, >, - #[expect(unused)] pub(crate) dumb_create: Option< unsafe extern "C" fn( file_priv: *mut bindings::drm_file, @@ -76,7 +69,6 @@ pub struct AllocOps { args: *mut bindings::drm_mode_create_dumb, ) -> core::ffi::c_int, >, - #[expect(unused)] pub(crate) dumb_map_offset: Option< unsafe extern "C" fn( file_priv: *mut bindings::drm_file, diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 2e3f9a8a9353..967854a2083e 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -2,9 +2,11 @@ //! DRM subsystem abstractions. +pub mod device; pub mod driver; pub mod ioctl; +pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; From 0600032c54b7adc309d334c109374433ce3ab243 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:24 +0200 Subject: [PATCH 24/47] rust: drm: add DRM driver registration Implement the DRM driver `Registration`. The `Registration` structure is responsible to register and unregister a DRM driver. It makes use of the `Devres` container in order to allow the `Registration` to be owned by devres, such that it is automatically dropped (and the DRM driver unregistered) once the parent device is unbound. Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-6-dakr@kernel.org [ Rework of drm::Registration * move VTABLE to drm::Device to prevent use-after-free bugs; VTABLE needs to be bound to the lifetime of drm::Device, not the drm::Registration * combine new() and register() to get rid of the registered boolean * remove file_operations * move struct drm_device creation to drm::Device * introduce Devres * original source archive: https://archive.is/Pl9ys - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/driver.rs | 60 ++++++++++++++++++++++++++++++++++++++- rust/kernel/drm/mod.rs | 1 + 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 26a5d66cc4c5..4f80bc448c09 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -4,7 +4,15 @@ //! //! C header: [`include/linux/drm/drm_drv.h`](srctree/include/linux/drm/drm_drv.h) -use crate::{bindings, drm, str::CStr}; +use crate::{ + bindings, device, + devres::Devres, + drm, + error::{to_result, Result}, + prelude::*, + str::CStr, + types::ARef, +}; use macros::vtable; /// Driver use the GEM memory manager. This should be set for all modern drivers. @@ -103,3 +111,53 @@ pub trait Driver { /// IOCTL list. See `kernel::drm::ioctl::declare_drm_ioctls!{}`. const IOCTLS: &'static [drm::ioctl::DrmIoctlDescriptor]; } + +/// The registration type of a `drm::Device`. +/// +/// Once the `Registration` structure is dropped, the device is unregistered. +pub struct Registration(ARef>); + +impl Registration { + /// Creates a new [`Registration`] and registers it. + fn new(drm: &drm::Device, flags: usize) -> Result { + // SAFETY: `drm.as_raw()` is valid by the invariants of `drm::Device`. + to_result(unsafe { bindings::drm_dev_register(drm.as_raw(), flags) })?; + + Ok(Self(drm.into())) + } + + /// Same as [`Registration::new`}, but transfers ownership of the [`Registration`] to + /// [`Devres`]. + pub fn new_foreign_owned( + drm: &drm::Device, + dev: &device::Device, + flags: usize, + ) -> Result { + if drm.as_ref().as_raw() != dev.as_raw() { + return Err(EINVAL); + } + + let reg = Registration::::new(drm, flags)?; + Devres::new_foreign_owned(dev, reg, GFP_KERNEL) + } + + /// Returns a reference to the `Device` instance for this registration. + pub fn device(&self) -> &drm::Device { + &self.0 + } +} + +// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between +// threads, hence it's safe to share it. +unsafe impl Sync for Registration {} + +// SAFETY: Registration with and unregistration from the DRM subsystem can happen from any thread. +unsafe impl Send for Registration {} + +impl Drop for Registration { + fn drop(&mut self) { + // SAFETY: Safe by the invariant of `ARef>`. The existence of this + // `Registration` also guarantees the this `drm::Device` is actually registered. + unsafe { bindings::drm_dev_unregister(self.0.as_raw()) }; + } +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 967854a2083e..2d88e70ba607 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -9,6 +9,7 @@ pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; +pub use self::driver::Registration; pub(crate) mod private { pub trait Sealed {} From a98a73be9ee9c42495690b2fe56e1ce27768289a Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:25 +0200 Subject: [PATCH 25/47] rust: drm: file: Add File abstraction A DRM File is the DRM counterpart to a kernel file structure, representing an open DRM file descriptor. Add a Rust abstraction to allow drivers to implement their own File types that implement the DriverFile trait. Reviewed-by: Maxime Ripard Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-7-dakr@kernel.org [ Rework of drm::File * switch to the Opaque type * fix (mutable) references to struct drm_file (which in this context is UB) * restructure and rename functions to align with common kernel schemes * write and fix safety and invariant comments * remove necessity for and convert 'as' casts * original source archive: https://archive.is/GH8oy - Danilo ] Signed-off-by: Danilo Krummrich --- rust/bindings/bindings_helper.h | 1 + rust/kernel/drm/device.rs | 4 +- rust/kernel/drm/driver.rs | 3 + rust/kernel/drm/file.rs | 99 +++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 2 + 5 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 rust/kernel/drm/file.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 280d3c3159c9..7b7c270db401 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index d6c6cecbdd51..520f51bb40af 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -63,8 +63,8 @@ pub struct Device { impl Device { const VTABLE: bindings::drm_driver = drm_legacy_fields! { load: None, - open: None, // TODO: File abstraction - postclose: None, // TODO: File abstraction + open: Some(drm::File::::open_callback), + postclose: Some(drm::File::::postclose_callback), unload: None, release: None, master_set: None, diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index 4f80bc448c09..f0c2936c3374 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -105,6 +105,9 @@ pub trait Driver { /// The type used to manage memory for this driver. type Object: AllocImpl; + /// The type used to represent a DRM File (client) + type File: drm::file::DriverFile; + /// Driver metadata const INFO: DriverInfo; diff --git a/rust/kernel/drm/file.rs b/rust/kernel/drm/file.rs new file mode 100644 index 000000000000..b9527705e551 --- /dev/null +++ b/rust/kernel/drm/file.rs @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM File objects. +//! +//! C header: [`include/linux/drm/drm_file.h`](srctree/include/linux/drm/drm_file.h) + +use crate::{bindings, drm, error::Result, prelude::*, types::Opaque}; +use core::marker::PhantomData; +use core::pin::Pin; + +/// Trait that must be implemented by DRM drivers to represent a DRM File (a client instance). +pub trait DriverFile { + /// The parent `Driver` implementation for this `DriverFile`. + type Driver: drm::Driver; + + /// Open a new file (called when a client opens the DRM device). + fn open(device: &drm::Device) -> Result>>; +} + +/// An open DRM File. +/// +/// # Invariants +/// +/// `self.0` is a valid instance of a `struct drm_file`. +#[repr(transparent)] +pub struct File(Opaque, PhantomData); + +impl File { + #[doc(hidden)] + /// Not intended to be called externally, except via declare_drm_ioctls!() + /// + /// # Safety + /// + /// `raw_file` must be a valid pointer to an open `struct drm_file`, opened through `T::open`. + pub unsafe fn as_ref<'a>(ptr: *mut bindings::drm_file) -> &'a File { + // SAFETY: `raw_file` is valid by the safety requirements of this function. + unsafe { &*ptr.cast() } + } + + pub(super) fn as_raw(&self) -> *mut bindings::drm_file { + self.0.get() + } + + fn driver_priv(&self) -> *mut T { + // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid. + unsafe { (*self.as_raw()).driver_priv }.cast() + } + + /// Return a pinned reference to the driver file structure. + pub fn inner(&self) -> Pin<&T> { + // SAFETY: By the type invariant the pointer `self.as_raw()` points to a valid and opened + // `struct drm_file`, hence `driver_priv` has been properly initialized by `open_callback`. + unsafe { Pin::new_unchecked(&*(self.driver_priv())) } + } + + /// The open callback of a `struct drm_file`. + pub(crate) extern "C" fn open_callback( + raw_dev: *mut bindings::drm_device, + raw_file: *mut bindings::drm_file, + ) -> core::ffi::c_int { + // SAFETY: A callback from `struct drm_driver::open` guarantees that + // - `raw_dev` is valid pointer to a `struct drm_device`, + // - the corresponding `struct drm_device` has been registered. + let drm = unsafe { drm::Device::as_ref(raw_dev) }; + + // SAFETY: `raw_file` is a valid pointer to a `struct drm_file`. + let file = unsafe { File::::as_ref(raw_file) }; + + let inner = match T::open(drm) { + Err(e) => { + return e.to_errno(); + } + Ok(i) => i, + }; + + // SAFETY: This pointer is treated as pinned, and the Drop guarantee is upheld in + // `postclose_callback()`. + let driver_priv = KBox::into_raw(unsafe { Pin::into_inner_unchecked(inner) }); + + // SAFETY: By the type invariants of `Self`, `self.as_raw()` is always valid. + unsafe { (*file.as_raw()).driver_priv = driver_priv.cast() }; + + 0 + } + + /// The postclose callback of a `struct drm_file`. + pub(crate) extern "C" fn postclose_callback( + _raw_dev: *mut bindings::drm_device, + raw_file: *mut bindings::drm_file, + ) { + // SAFETY: This reference won't escape this function + let file = unsafe { File::::as_ref(raw_file) }; + + // SAFETY: `file.driver_priv` has been created in `open_callback` through `KBox::into_raw`. + let _ = unsafe { KBox::from_raw(file.driver_priv()) }; + } +} + +impl super::private::Sealed for File {} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index 2d88e70ba607..b36223e5bd98 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -4,12 +4,14 @@ pub mod device; pub mod driver; +pub mod file; pub mod ioctl; pub use self::device::Device; pub use self::driver::Driver; pub use self::driver::DriverInfo; pub use self::driver::Registration; +pub use self::file::File; pub(crate) mod private { pub trait Sealed {} From c284d3e423382be3591d5b1e402e330e6c3f726c Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Fri, 11 Apr 2025 01:55:26 +0200 Subject: [PATCH 26/47] rust: drm: gem: Add GEM object abstraction DRM GEM is the DRM memory management subsystem used by most modern drivers; add a Rust abstraction for DRM GEM. This includes the BaseObject trait, which contains operations shared by all GEM object classes. Signed-off-by: Asahi Lina Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-8-dakr@kernel.org [ Rework of GEM object abstractions * switch to the Opaque type * fix (mutable) references to struct drm_gem_object (which in this context is UB) * drop all custom reference types in favor of AlwaysRefCounted * bunch of minor changes and simplifications (e.g. IntoGEMObject trait) * write and fix safety and invariant comments * remove necessity for and convert 'as' casts * original source archive: https://archive.is/dD5SL - Danilo ] [ Fix missing CONFIG_DRM guards in rust/helpers/drm.c. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/bindings/bindings_helper.h | 2 + rust/helpers/drm.c | 23 +++ rust/helpers/helpers.c | 1 + rust/kernel/drm/device.rs | 4 +- rust/kernel/drm/driver.rs | 2 +- rust/kernel/drm/gem/mod.rs | 320 ++++++++++++++++++++++++++++++++ rust/kernel/drm/mod.rs | 1 + 7 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 rust/helpers/drm.c create mode 100644 rust/kernel/drm/gem/mod.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index 7b7c270db401..e0676ba99d37 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -60,3 +61,4 @@ const gfp_t RUST_CONST_HELPER___GFP_ZERO = __GFP_ZERO; const gfp_t RUST_CONST_HELPER___GFP_HIGHMEM = ___GFP_HIGHMEM; const gfp_t RUST_CONST_HELPER___GFP_NOWARN = ___GFP_NOWARN; const blk_features_t RUST_CONST_HELPER_BLK_FEAT_ROTATIONAL = BLK_FEAT_ROTATIONAL; +const fop_flags_t RUST_CONST_HELPER_FOP_UNSIGNED_OFFSET = FOP_UNSIGNED_OFFSET; diff --git a/rust/helpers/drm.c b/rust/helpers/drm.c new file mode 100644 index 000000000000..450b406c6f27 --- /dev/null +++ b/rust/helpers/drm.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include +#include + +#ifdef CONFIG_DRM + +void rust_helper_drm_gem_object_get(struct drm_gem_object *obj) +{ + drm_gem_object_get(obj); +} + +void rust_helper_drm_gem_object_put(struct drm_gem_object *obj) +{ + drm_gem_object_put(obj); +} + +__u64 rust_helper_drm_vma_node_offset_addr(struct drm_vma_offset_node *node) +{ + return drm_vma_node_offset_addr(node); +} + +#endif diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index 6b279279cb12..6c205454a18e 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -15,6 +15,7 @@ #include "cpumask.c" #include "cred.c" #include "device.c" +#include "drm.c" #include "err.c" #include "fs.c" #include "io.c" diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 520f51bb40af..74c9a3dd719e 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -89,9 +89,11 @@ impl Device { driver_features: drm::driver::FEAT_GEM, ioctls: T::IOCTLS.as_ptr(), num_ioctls: T::IOCTLS.len() as i32, - fops: core::ptr::null_mut() as _, + fops: &Self::GEM_FOPS as _, }; + const GEM_FOPS: bindings::file_operations = drm::gem::create_fops(); + /// Create a new `drm::Device` for a `drm::Driver`. pub fn new(dev: &device::Device, data: impl PinInit) -> Result> { // SAFETY: diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index f0c2936c3374..acb638086131 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -88,7 +88,7 @@ pub struct AllocOps { } /// Trait for memory manager implementations. Implemented internally. -pub trait AllocImpl: super::private::Sealed { +pub trait AllocImpl: super::private::Sealed + drm::gem::IntoGEMObject { /// The C callback operations for this memory manager. const ALLOC_OPS: AllocOps; } diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs new file mode 100644 index 000000000000..0cafa4a42420 --- /dev/null +++ b/rust/kernel/drm/gem/mod.rs @@ -0,0 +1,320 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +//! DRM GEM API +//! +//! C header: [`include/linux/drm/drm_gem.h`](srctree/include/linux/drm/drm_gem.h) + +use crate::{ + alloc::flags::*, + bindings, drm, + drm::driver::{AllocImpl, AllocOps}, + error::{to_result, Result}, + prelude::*, + types::{ARef, Opaque}, +}; +use core::{mem, ops::Deref, ptr, ptr::NonNull}; + +/// GEM object functions, which must be implemented by drivers. +pub trait BaseDriverObject: Sync + Send + Sized { + /// Create a new driver data object for a GEM object of a given size. + fn new(dev: &drm::Device, size: usize) -> impl PinInit; + + /// Open a new handle to an existing object, associated with a File. + fn open( + _obj: &<::Driver as drm::Driver>::Object, + _file: &drm::File<<::Driver as drm::Driver>::File>, + ) -> Result { + Ok(()) + } + + /// Close a handle to an existing object, associated with a File. + fn close( + _obj: &<::Driver as drm::Driver>::Object, + _file: &drm::File<<::Driver as drm::Driver>::File>, + ) { + } +} + +/// Trait that represents a GEM object subtype +pub trait IntoGEMObject: Sized + super::private::Sealed { + /// Owning driver for this type + type Driver: drm::Driver; + + /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as + /// this owning object is valid. + #[allow(clippy::wrong_self_convention)] + fn into_gem_obj(&self) -> &Opaque; + + /// Converts a pointer to a `struct drm_gem_object` into a pointer to `Self`. + fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self; +} + +/// Trait which must be implemented by drivers using base GEM objects. +pub trait DriverObject: BaseDriverObject> { + /// Parent `Driver` for this object. + type Driver: drm::Driver; +} + +extern "C" fn open_callback, U: BaseObject>( + raw_obj: *mut bindings::drm_gem_object, + raw_file: *mut bindings::drm_file, +) -> core::ffi::c_int { + // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. + let file = unsafe { + drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) + }; + let obj = + <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( + raw_obj, + ); + + // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the + // `raw_obj` we got is valid. + match T::open(unsafe { &*obj }, file) { + Err(e) => e.to_errno(), + Ok(()) => 0, + } +} + +extern "C" fn close_callback, U: BaseObject>( + raw_obj: *mut bindings::drm_gem_object, + raw_file: *mut bindings::drm_file, +) { + // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. + let file = unsafe { + drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) + }; + let obj = + <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( + raw_obj, + ); + + // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the + // `raw_obj` we got is valid. + T::close(unsafe { &*obj }, file); +} + +impl IntoGEMObject for Object { + type Driver = T::Driver; + + fn into_gem_obj(&self) -> &Opaque { + &self.obj + } + + fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self { + // SAFETY: All of our objects are Object. + unsafe { crate::container_of!(obj, Object, obj).cast_mut() } + } +} + +/// Base operations shared by all GEM object classes +pub trait BaseObject +where + Self: crate::types::AlwaysRefCounted + IntoGEMObject, +{ + /// Returns the size of the object in bytes. + fn size(&self) -> usize { + // SAFETY: `self.into_gem_obj()` is guaranteed to be a pointer to a valid `struct + // drm_gem_object`. + unsafe { (*self.into_gem_obj().get()).size } + } + + /// Creates a new handle for the object associated with a given `File` + /// (or returns an existing one). + fn create_handle( + &self, + file: &drm::File<<::Driver as drm::Driver>::File>, + ) -> Result { + let mut handle: u32 = 0; + // SAFETY: The arguments are all valid per the type invariants. + to_result(unsafe { + bindings::drm_gem_handle_create( + file.as_raw().cast(), + self.into_gem_obj().get(), + &mut handle, + ) + })?; + Ok(handle) + } + + /// Looks up an object by its handle for a given `File`. + fn lookup_handle( + file: &drm::File<<::Driver as drm::Driver>::File>, + handle: u32, + ) -> Result> { + // SAFETY: The arguments are all valid per the type invariants. + let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; + let ptr = ::from_gem_obj(ptr); + let ptr = NonNull::new(ptr).ok_or(ENOENT)?; + + // SAFETY: We take ownership of the reference of `drm_gem_object_lookup()`. + Ok(unsafe { ARef::from_raw(ptr) }) + } + + /// Creates an mmap offset to map the object from userspace. + fn create_mmap_offset(&self) -> Result { + // SAFETY: The arguments are valid per the type invariant. + to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.into_gem_obj().get()) })?; + + // SAFETY: The arguments are valid per the type invariant. + Ok(unsafe { + bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!( + (*self.into_gem_obj().get()).vma_node + )) + }) + } +} + +impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObject {} + +/// A base GEM object. +/// +/// Invariants +/// +/// - `self.obj` is a valid instance of a `struct drm_gem_object`. +/// - `self.dev` is always a valid pointer to a `struct drm_device`. +#[repr(C)] +#[pin_data] +pub struct Object { + obj: Opaque, + dev: *const drm::Device, + #[pin] + data: T, +} + +impl Object { + /// The size of this object's structure. + pub const SIZE: usize = mem::size_of::(); + + const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { + free: Some(Self::free_callback), + open: Some(open_callback::>), + close: Some(close_callback::>), + print_info: None, + export: None, + pin: None, + unpin: None, + get_sg_table: None, + vmap: None, + vunmap: None, + mmap: None, + status: None, + vm_ops: core::ptr::null_mut(), + evict: None, + rss: None, + }; + + /// Create a new GEM object. + pub fn new(dev: &drm::Device, size: usize) -> Result> { + let obj: Pin> = KBox::pin_init( + try_pin_init!(Self { + obj: Opaque::new(bindings::drm_gem_object::default()), + data <- T::new(dev, size), + // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live + // as long as the GEM object lives. + dev, + }), + GFP_KERNEL, + )?; + + // SAFETY: `obj.as_raw()` is guaranteed to be valid by the initialization above. + unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS }; + + // SAFETY: The arguments are all valid per the type invariants. + to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj.obj.get(), size) })?; + + // SAFETY: We never move out of `Self`. + let ptr = KBox::into_raw(unsafe { Pin::into_inner_unchecked(obj) }); + + // SAFETY: `ptr` comes from `KBox::into_raw` and hence can't be NULL. + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + + // SAFETY: We take over the initial reference count from `drm_gem_object_init()`. + Ok(unsafe { ARef::from_raw(ptr) }) + } + + /// Returns the `Device` that owns this GEM object. + pub fn dev(&self) -> &drm::Device { + // SAFETY: The DRM subsystem guarantees that the `struct drm_device` will live as long as + // the GEM object lives, hence the pointer must be valid. + unsafe { &*self.dev } + } + + fn as_raw(&self) -> *mut bindings::drm_gem_object { + self.obj.get() + } + + extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { + // SAFETY: All of our objects are of type `Object`. + let this = unsafe { crate::container_of!(obj, Self, obj) }.cast_mut(); + + // SAFETY: The C code only ever calls this callback with a valid pointer to a `struct + // drm_gem_object`. + unsafe { bindings::drm_gem_object_release(obj) }; + + // SAFETY: All of our objects are allocated via `KBox`, and we're in the + // free callback which guarantees this object has zero remaining references, + // so we can drop it. + let _ = unsafe { KBox::from_raw(this) }; + } +} + +// SAFETY: Instances of `Object` are always reference-counted. +unsafe impl crate::types::AlwaysRefCounted for Object { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: `obj` is a valid pointer to an `Object`. + let obj = unsafe { obj.as_ref() }; + + // SAFETY: The safety requirements guarantee that the refcount is non-zero. + unsafe { bindings::drm_gem_object_put(obj.as_raw()) } + } +} + +impl super::private::Sealed for Object {} + +impl Deref for Object { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.data + } +} + +impl AllocImpl for Object { + const ALLOC_OPS: AllocOps = AllocOps { + gem_create_object: None, + prime_handle_to_fd: None, + prime_fd_to_handle: None, + gem_prime_import: None, + gem_prime_import_sg_table: None, + dumb_create: None, + dumb_map_offset: None, + }; +} + +pub(super) const fn create_fops() -> bindings::file_operations { + // SAFETY: As by the type invariant, it is safe to initialize `bindings::file_operations` + // zeroed. + let mut fops: bindings::file_operations = unsafe { core::mem::zeroed() }; + + fops.owner = core::ptr::null_mut(); + fops.open = Some(bindings::drm_open); + fops.release = Some(bindings::drm_release); + fops.unlocked_ioctl = Some(bindings::drm_ioctl); + #[cfg(CONFIG_COMPAT)] + { + fops.compat_ioctl = Some(bindings::drm_compat_ioctl); + } + fops.poll = Some(bindings::drm_poll); + fops.read = Some(bindings::drm_read); + fops.llseek = Some(bindings::noop_llseek); + fops.mmap = Some(bindings::drm_gem_mmap); + fops.fop_flags = bindings::FOP_UNSIGNED_OFFSET; + + fops +} diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs index b36223e5bd98..1b82b6945edf 100644 --- a/rust/kernel/drm/mod.rs +++ b/rust/kernel/drm/mod.rs @@ -5,6 +5,7 @@ pub mod device; pub mod driver; pub mod file; +pub mod gem; pub mod ioctl; pub use self::device::Device; From 3be746ebc1e6e32f499a65afe405df9030153a63 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Fri, 11 Apr 2025 01:55:27 +0200 Subject: [PATCH 27/47] MAINTAINERS: add DRM Rust source files to DRM DRIVERS Add the DRM Rust source files to the DRM DRIVERS maintainers entry. Reviewed-by: Alyssa Rosenzweig Reviewed-by: Lyude Paul Link: https://lore.kernel.org/r/20250410235546.43736-9-dakr@kernel.org Signed-off-by: Danilo Krummrich --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 34757b8641c3..2fe2e531f4cb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7801,6 +7801,7 @@ F: Documentation/devicetree/bindings/display/ F: Documentation/devicetree/bindings/gpu/ F: Documentation/gpu/ F: drivers/gpu/ +F: rust/kernel/drm/ F: include/drm/ F: include/linux/vga* F: include/uapi/drm/ @@ -7817,6 +7818,7 @@ F: Documentation/devicetree/bindings/gpu/ F: Documentation/gpu/ F: drivers/gpu/drm/ F: drivers/gpu/vga/ +F: rust/kernel/drm/ F: include/drm/drm F: include/linux/vga* F: include/uapi/drm/ From fc55584e00fc8409cbaef4bcd984016dca6f1b6b Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 29 Apr 2025 23:06:29 +0200 Subject: [PATCH 28/47] rust: device: conditionally expect `dead_code` for `parent()` When `CONFIG_AUXILIARY_BUS` is disabled, `parent()` is still dead code: error: method `parent` is never used --> rust/kernel/device.rs:71:19 | 64 | impl Device { | ------------------------------------ method in this implementation ... 71 | pub(crate) fn parent(&self) -> Option<&Self> { | ^^^^^^ | = note: `-D dead-code` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(dead_code)]` Thus reintroduce the `expect`, but now as a conditional one. Do so as `dead_code` since that is narrower. An `allow` would also be possible, but Danilo wants to catch new users in the future [1]. Link: https://lore.kernel.org/rust-for-linux/aBE8qQrpXOfru_K3@pollux/ [1] Fixes: ce735e73dd59 ("rust: auxiliary: add auxiliary device / driver abstractions") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250429210629.513521-1-ojeda@kernel.org [ Adjust commit subject to "rust: device: conditionally expect `dead_code` for `parent()`". - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/device.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 40c1f549b0ba..f08583fa39c9 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -68,6 +68,7 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device { } /// Returns a reference to the parent device, if any. + #[cfg_attr(not(CONFIG_AUXILIARY_BUS), expect(dead_code))] pub(crate) fn parent(&self) -> Option<&Self> { // SAFETY: // - By the type invariant `self.as_raw()` is always valid. From 46f91addfabbd4109fb64876a032ae4a4a924919 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 28 Apr 2025 16:00:27 +0200 Subject: [PATCH 29/47] rust: revocable: implement Revocable::access() Implement an unsafe direct accessor for the data stored within the Revocable. This is useful for cases where we can prove that the data stored within the Revocable is not and cannot be revoked for the duration of the lifetime of the returned reference. Reviewed-by: Christian Schrefl Reviewed-by: Benno Lossin Acked-by: Miguel Ojeda Reviewed-by: Alexandre Courbot Acked-by: Boqun Feng Reviewed-by: Joel Fernandes Link: https://lore.kernel.org/r/20250428140137.468709-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index 971d0dc38d83..db4aa46bb121 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -139,6 +139,18 @@ pub fn try_access_with R>(&self, f: F) -> Option { self.try_access().map(|t| f(&*t)) } + /// Directly access the revocable wrapped object. + /// + /// # Safety + /// + /// The caller must ensure this [`Revocable`] instance hasn't been revoked and won't be revoked + /// as long as the returned `&T` lives. + pub unsafe fn access(&self) -> &T { + // SAFETY: By the safety requirement of this function it is guaranteed that + // `self.data.get()` is a valid pointer to an instance of `T`. + unsafe { &*self.data.get() } + } + /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. From f301cb978c068faa8fcd630be2cb317a2d0ec063 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 28 Apr 2025 16:00:28 +0200 Subject: [PATCH 30/47] rust: devres: implement Devres::access() Implement a direct accessor for the data stored within the Devres for cases where we can prove that we own a reference to a Device (i.e. a bound device) of the same device that was used to create the corresponding Devres container. Usually, when accessing the data stored within a Devres container, it is not clear whether the data has been revoked already due to the device being unbound and, hence, we have to try whether the access is possible and subsequently keep holding the RCU read lock for the duration of the access. However, when we can prove that we hold a reference to Device matching the device the Devres container has been created with, we can guarantee that the device is not unbound for the duration of the lifetime of the Device reference and, hence, it is not possible for the data within the Devres container to be revoked. Therefore, in this case, we can bypass the atomic check and the RCU read lock, which is a great optimization and simplification for drivers. Reviewed-by: Christian Schrefl Reviewed-by: Alexandre Courbot Acked-by: Boqun Feng Reviewed-by: Joel Fernandes Link: https://lore.kernel.org/r/20250428140137.468709-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 1e58f5d22044..b67247db92c7 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -181,6 +181,44 @@ pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result { Ok(()) } + + /// Obtain `&'a T`, bypassing the [`Revocable`]. + /// + /// This method allows to directly obtain a `&'a T`, bypassing the [`Revocable`], by presenting + /// a `&'a Device` of the same [`Device`] this [`Devres`] instance has been created with. + /// + /// # Errors + /// + /// An error is returned if `dev` does not match the same [`Device`] this [`Devres`] instance + /// has been created with. + /// + /// # Example + /// + /// ```no_run + /// # use kernel::{device::Core, devres::Devres, pci}; + /// + /// fn from_core(dev: &pci::Device, devres: Devres>) -> Result { + /// let bar = devres.access(dev.as_ref())?; + /// + /// let _ = bar.read32(0x0); + /// + /// // might_sleep() + /// + /// bar.write32(0x42, 0x0); + /// + /// Ok(()) + /// } + /// ``` + pub fn access<'a>(&'a self, dev: &'a Device) -> Result<&'a T> { + if self.0.dev.as_raw() != dev.as_raw() { + return Err(EINVAL); + } + + // 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.deref().access() }) + } } impl Deref for Devres { From b75a99e1077b12c5631fef7ac36970a89f6021f7 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Mon, 28 Apr 2025 16:00:29 +0200 Subject: [PATCH 31/47] samples: rust: pci: take advantage of Devres::access() For the I/O operations executed from the probe() method, take advantage of Devres::access(), avoiding the atomic check and RCU read lock required otherwise entirely. Reviewed-by: Alexandre Courbot Acked-by: Boqun Feng Reviewed-by: Joel Fernandes Link: https://lore.kernel.org/r/20250428140137.468709-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- samples/rust/rust_driver_pci.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs index 9ce3a7323a16..15147e4401b2 100644 --- a/samples/rust/rust_driver_pci.rs +++ b/samples/rust/rust_driver_pci.rs @@ -83,12 +83,12 @@ fn probe(pdev: &pci::Device, info: &Self::IdInfo) -> Result GFP_KERNEL, )?; - let res = drvdata - .bar - .try_access_with(|b| Self::testdev(info, b)) - .ok_or(ENXIO)??; - - dev_info!(pdev.as_ref(), "pci-testdev data-match count: {}\n", res); + let bar = drvdata.bar.access(pdev.as_ref())?; + dev_info!( + pdev.as_ref(), + "pci-testdev data-match count: {}\n", + Self::testdev(info, bar)? + ); Ok(drvdata.into()) } From 42055939a3a4cac8afbebff85a29571c2bd3238c Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 11 May 2025 20:25:33 +0200 Subject: [PATCH 32/47] rust: devres: fix doctest build under `!CONFIG_PCI` The doctest requires `CONFIG_PCI`: error[E0432]: unresolved import `kernel::pci` --> rust/doctests_kernel_generated.rs:2689:44 | 2689 | use kernel::{device::Core, devres::Devres, pci}; | ^^^ no `pci` in the root | note: found an item that was configured out --> rust/kernel/lib.rs:96:9 note: the item is gated here --> rust/kernel/lib.rs:95:1 Thus conditionally compile it (which still checks the syntax). Fixes: f301cb978c06 ("rust: devres: implement Devres::access()") Signed-off-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250511182533.1016163-1-ojeda@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index b67247db92c7..0f79a2ec9474 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -195,6 +195,7 @@ pub fn new_foreign_owned(dev: &Device, data: T, flags: Flags) -> Result { /// # Example /// /// ```no_run + /// # #![cfg(CONFIG_PCI)] /// # use kernel::{device::Core, devres::Devres, pci}; /// /// fn from_core(dev: &pci::Device, devres: Devres>) -> Result { From e041d81a03777702249f8de16db70fdabdd1ab5f Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 24 Apr 2025 18:02:49 +0200 Subject: [PATCH 33/47] gpu: nova-core: register auxiliary device for nova-drm Register an auxiliary device for nova-drm. For now always use zero for the auxiliary device's ID; we don't use it yet anyways. However, once it lands, we should switch to XArray. Acked-by: Dave Airlie Link: https://lore.kernel.org/r/20250424160452.8070-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/Kconfig | 1 + drivers/gpu/nova-core/driver.rs | 9 ++++++++- drivers/gpu/nova-core/nova_core.rs | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig index ad0c06756516..67f99b6a023a 100644 --- a/drivers/gpu/nova-core/Kconfig +++ b/drivers/gpu/nova-core/Kconfig @@ -1,5 +1,6 @@ config NOVA_CORE tristate "Nova Core GPU driver" + depends on AUXILIARY_BUS depends on PCI depends on RUST depends on RUST_FW_LOADER_ABSTRACTIONS diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs index a08fb6599267..8c86101c26cb 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 -use kernel::{bindings, c_str, device::Core, pci, prelude::*}; +use kernel::{auxiliary, bindings, c_str, device::Core, pci, prelude::*}; use crate::gpu::Gpu; @@ -8,6 +8,7 @@ pub(crate) struct NovaCore { #[pin] pub(crate) gpu: Gpu, + _reg: auxiliary::Registration, } const BAR0_SIZE: usize = 8; @@ -38,6 +39,12 @@ fn probe(pdev: &pci::Device, _info: &Self::IdInfo) -> Result::NAME; + kernel::module_pci_driver! { type: driver::NovaCore, name: "NovaCore", From cdeaeb9dd762d7711241a62459dfb730b2cd0281 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 24 Apr 2025 18:02:50 +0200 Subject: [PATCH 34/47] drm: nova-drm: add initial driver skeleton Add the initial nova-drm driver skeleton. nova-drm is connected to nova-core through the auxiliary bus and implements the DRM parts of the nova driver stack. For now, it implements the fundamental DRM abstractions, i.e. creates a DRM device and registers it, exposing a three sample IOCTLs. DRM_IOCTL_NOVA_GETPARAM - provides the PCI bar size from the bar that maps the GPUs VRAM from nova-core DRM_IOCTL_NOVA_GEM_CREATE - creates a new dummy DRM GEM object and returns a handle DRM_IOCTL_NOVA_GEM_INFO - provides metadata for the DRM GEM object behind a given handle I implemented a small userspace test suite [1] that utilizes this interface. Link: https://gitlab.freedesktop.org/dakr/drm-test [1] Reviewed-by: Maxime Ripard Acked-by: Dave Airlie Link: https://lore.kernel.org/r/20250424160452.8070-3-dakr@kernel.org [ Kconfig: depend on DRM=y rather than just DRM. - Danilo ] Signed-off-by: Danilo Krummrich --- MAINTAINERS | 12 ++++ drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/nova/Kconfig | 14 +++++ drivers/gpu/drm/nova/Makefile | 3 + drivers/gpu/drm/nova/driver.rs | 69 ++++++++++++++++++++++ drivers/gpu/drm/nova/file.rs | 74 ++++++++++++++++++++++++ drivers/gpu/drm/nova/gem.rs | 49 ++++++++++++++++ drivers/gpu/drm/nova/nova.rs | 18 ++++++ drivers/gpu/drm/nova/uapi.rs | 61 ++++++++++++++++++++ include/uapi/drm/nova_drm.h | 101 +++++++++++++++++++++++++++++++++ rust/uapi/uapi_helper.h | 1 + 12 files changed, 405 insertions(+) create mode 100644 drivers/gpu/drm/nova/Kconfig create mode 100644 drivers/gpu/drm/nova/Makefile create mode 100644 drivers/gpu/drm/nova/driver.rs create mode 100644 drivers/gpu/drm/nova/file.rs create mode 100644 drivers/gpu/drm/nova/gem.rs create mode 100644 drivers/gpu/drm/nova/nova.rs create mode 100644 drivers/gpu/drm/nova/uapi.rs create mode 100644 include/uapi/drm/nova_drm.h diff --git a/MAINTAINERS b/MAINTAINERS index 2fe2e531f4cb..f7cffb0a5a77 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7594,6 +7594,18 @@ T: git https://gitlab.freedesktop.org/drm/nova.git nova-next F: Documentation/gpu/nova/ F: drivers/gpu/nova-core/ +DRM DRIVER FOR NVIDIA GPUS [RUST] +M: Danilo Krummrich +L: nouveau@lists.freedesktop.org +S: Supported +Q: https://patchwork.freedesktop.org/project/nouveau/ +B: https://gitlab.freedesktop.org/drm/nova/-/issues +C: irc://irc.oftc.net/nouveau +T: git https://gitlab.freedesktop.org/drm/nova.git nova-next +F: Documentation/gpu/nova/ +F: drivers/gpu/drm/nova/ +F: include/uapi/drm/nova_drm.h + DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS M: Stefan Mavrodiev S: Maintained diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 2cba2b6ebe1c..90cdaa0068bd 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -343,6 +343,8 @@ source "drivers/gpu/drm/amd/amdgpu/Kconfig" source "drivers/gpu/drm/nouveau/Kconfig" +source "drivers/gpu/drm/nova/Kconfig" + source "drivers/gpu/drm/i915/Kconfig" source "drivers/gpu/drm/xe/Kconfig" diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index ed54a546bbe2..947ed126135b 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -176,6 +176,7 @@ obj-$(CONFIG_DRM_VMWGFX)+= vmwgfx/ obj-$(CONFIG_DRM_VGEM) += vgem/ obj-$(CONFIG_DRM_VKMS) += vkms/ obj-$(CONFIG_DRM_NOUVEAU) +=nouveau/ +obj-$(CONFIG_DRM_NOVA) += nova/ obj-$(CONFIG_DRM_EXYNOS) +=exynos/ obj-$(CONFIG_DRM_ROCKCHIP) +=rockchip/ obj-$(CONFIG_DRM_GMA500) += gma500/ diff --git a/drivers/gpu/drm/nova/Kconfig b/drivers/gpu/drm/nova/Kconfig new file mode 100644 index 000000000000..123e96788484 --- /dev/null +++ b/drivers/gpu/drm/nova/Kconfig @@ -0,0 +1,14 @@ +config DRM_NOVA + tristate "Nova DRM driver" + depends on AUXILIARY_BUS + depends on DRM=y + depends on PCI + depends on RUST + default n + help + Choose this if you want to build the Nova DRM driver for Nvidia + GSP-based GPUs. + + This driver is work in progress and may not be functional. + + If M is selected, the module will be called nova. diff --git a/drivers/gpu/drm/nova/Makefile b/drivers/gpu/drm/nova/Makefile new file mode 100644 index 000000000000..42019bff3173 --- /dev/null +++ b/drivers/gpu/drm/nova/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-$(CONFIG_DRM_NOVA) += nova.o diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs new file mode 100644 index 000000000000..b28b2e05cc15 --- /dev/null +++ b/drivers/gpu/drm/nova/driver.rs @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-2.0 + +use kernel::{auxiliary, c_str, device::Core, drm, drm::gem, drm::ioctl, prelude::*, types::ARef}; + +use crate::file::File; +use crate::gem::NovaObject; + +pub(crate) struct NovaDriver { + #[expect(unused)] + drm: ARef>, +} + +/// Convienence type alias for the DRM device type for this driver +pub(crate) type NovaDevice = drm::Device; + +#[pin_data] +pub(crate) struct NovaData { + pub(crate) adev: ARef, +} + +const INFO: drm::DriverInfo = drm::DriverInfo { + major: 0, + minor: 0, + patchlevel: 0, + name: c_str!("nova"), + desc: c_str!("Nvidia Graphics"), +}; + +const NOVA_CORE_MODULE_NAME: &CStr = c_str!("NovaCore"); +const AUXILIARY_NAME: &CStr = c_str!("nova-drm"); + +kernel::auxiliary_device_table!( + AUX_TABLE, + MODULE_AUX_TABLE, + ::IdInfo, + [( + auxiliary::DeviceId::new(NOVA_CORE_MODULE_NAME, AUXILIARY_NAME), + () + )] +); + +impl auxiliary::Driver for NovaDriver { + type IdInfo = (); + const ID_TABLE: auxiliary::IdTable = &AUX_TABLE; + + fn probe(adev: &auxiliary::Device, _info: &Self::IdInfo) -> Result>> { + let data = try_pin_init!(NovaData { adev: adev.into() }); + + let drm = drm::Device::::new(adev.as_ref(), data)?; + drm::Registration::new_foreign_owned(&drm, adev.as_ref(), 0)?; + + Ok(KBox::new(Self { drm }, GFP_KERNEL)?.into()) + } +} + +#[vtable] +impl drm::Driver for NovaDriver { + type Data = NovaData; + type File = File; + type Object = gem::Object; + + const INFO: drm::DriverInfo = INFO; + + kernel::declare_drm_ioctls! { + (NOVA_GETPARAM, drm_nova_getparam, ioctl::RENDER_ALLOW, File::get_param), + (NOVA_GEM_CREATE, drm_nova_gem_create, ioctl::AUTH | ioctl::RENDER_ALLOW, File::gem_create), + (NOVA_GEM_INFO, drm_nova_gem_info, ioctl::AUTH | ioctl::RENDER_ALLOW, File::gem_info), + } +} diff --git a/drivers/gpu/drm/nova/file.rs b/drivers/gpu/drm/nova/file.rs new file mode 100644 index 000000000000..7e59a34b830d --- /dev/null +++ b/drivers/gpu/drm/nova/file.rs @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 + +use crate::driver::{NovaDevice, NovaDriver}; +use crate::gem::NovaObject; +use crate::uapi::{GemCreate, GemInfo, Getparam}; +use kernel::{ + alloc::flags::*, + drm::{self, gem::BaseObject}, + pci, + prelude::*, + types::Opaque, + uapi, +}; + +pub(crate) struct File; + +impl drm::file::DriverFile for File { + type Driver = NovaDriver; + + fn open(_dev: &NovaDevice) -> Result>> { + Ok(KBox::new(Self, GFP_KERNEL)?.into()) + } +} + +impl File { + /// IOCTL: get_param: Query GPU / driver metadata. + pub(crate) fn get_param( + dev: &NovaDevice, + getparam: &Opaque, + _file: &drm::File, + ) -> Result { + let adev = &dev.adev; + let parent = adev.parent().ok_or(ENOENT)?; + let pdev: &pci::Device = parent.try_into()?; + let getparam: &Getparam = getparam.into(); + + let value = match getparam.param() as u32 { + uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?, + _ => return Err(EINVAL), + }; + + getparam.set_value(value); + + Ok(0) + } + + /// IOCTL: gem_create: Create a new DRM GEM object. + pub(crate) fn gem_create( + dev: &NovaDevice, + req: &Opaque, + file: &drm::File, + ) -> Result { + let req: &GemCreate = req.into(); + let obj = NovaObject::new(dev, req.size().try_into()?)?; + + req.set_handle(obj.create_handle(file)?); + + Ok(0) + } + + /// IOCTL: gem_info: Query GEM metadata. + pub(crate) fn gem_info( + _dev: &NovaDevice, + req: &Opaque, + file: &drm::File, + ) -> Result { + let req: &GemInfo = req.into(); + let bo = NovaObject::lookup_handle(file, req.handle())?; + + req.set_size(bo.size().try_into()?); + + Ok(0) + } +} diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs new file mode 100644 index 000000000000..33b62d21400c --- /dev/null +++ b/drivers/gpu/drm/nova/gem.rs @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-2.0 + +use kernel::{ + drm, + drm::{gem, gem::BaseObject}, + prelude::*, + types::ARef, +}; + +use crate::{ + driver::{NovaDevice, NovaDriver}, + file::File, +}; + +/// GEM Object inner driver data +#[pin_data] +pub(crate) struct NovaObject {} + +impl gem::BaseDriverObject> for NovaObject { + fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit { + try_pin_init!(NovaObject {}) + } +} + +impl gem::DriverObject for NovaObject { + type Driver = NovaDriver; +} + +impl NovaObject { + /// Create a new DRM GEM object. + pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result>> { + let aligned_size = size.next_multiple_of(1 << 12); + + if size == 0 || size > aligned_size { + return Err(EINVAL); + } + + gem::Object::new(dev, aligned_size) + } + + /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it. + #[inline] + pub(crate) fn lookup_handle( + file: &drm::File, + handle: u32, + ) -> Result>> { + gem::Object::lookup_handle(file, handle) + } +} diff --git a/drivers/gpu/drm/nova/nova.rs b/drivers/gpu/drm/nova/nova.rs new file mode 100644 index 000000000000..902876aa14d1 --- /dev/null +++ b/drivers/gpu/drm/nova/nova.rs @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Nova DRM Driver + +mod driver; +mod file; +mod gem; +mod uapi; + +use crate::driver::NovaDriver; + +kernel::module_auxiliary_driver! { + type: NovaDriver, + name: "Nova", + author: "Danilo Krummrich", + description: "Nova GPU driver", + license: "GPL v2", +} diff --git a/drivers/gpu/drm/nova/uapi.rs b/drivers/gpu/drm/nova/uapi.rs new file mode 100644 index 000000000000..eb228a58d423 --- /dev/null +++ b/drivers/gpu/drm/nova/uapi.rs @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: GPL-2.0 + +use kernel::uapi; + +// TODO Work out some common infrastructure to avoid boilerplate code for uAPI abstractions. + +macro_rules! define_uapi_abstraction { + ($name:ident <= $inner:ty) => { + #[repr(transparent)] + pub struct $name(::kernel::types::Opaque<$inner>); + + impl ::core::convert::From<&::kernel::types::Opaque<$inner>> for &$name { + fn from(value: &::kernel::types::Opaque<$inner>) -> Self { + // SAFETY: `Self` is a transparent wrapper of `$inner`. + unsafe { ::core::mem::transmute(value) } + } + } + }; +} + +define_uapi_abstraction!(Getparam <= uapi::drm_nova_getparam); + +impl Getparam { + pub fn param(&self) -> u64 { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_getparam`. + unsafe { (*self.0.get()).param } + } + + pub fn set_value(&self, v: u64) { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_getparam`. + unsafe { (*self.0.get()).value = v }; + } +} + +define_uapi_abstraction!(GemCreate <= uapi::drm_nova_gem_create); + +impl GemCreate { + pub fn size(&self) -> u64 { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_gem_create`. + unsafe { (*self.0.get()).size } + } + + pub fn set_handle(&self, handle: u32) { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_gem_create`. + unsafe { (*self.0.get()).handle = handle }; + } +} + +define_uapi_abstraction!(GemInfo <= uapi::drm_nova_gem_info); + +impl GemInfo { + pub fn handle(&self) -> u32 { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_gem_info`. + unsafe { (*self.0.get()).handle } + } + + pub fn set_size(&self, size: u64) { + // SAFETY: `self.get()` is a valid pointer to a `struct drm_nova_gem_info`. + unsafe { (*self.0.get()).size = size }; + } +} diff --git a/include/uapi/drm/nova_drm.h b/include/uapi/drm/nova_drm.h new file mode 100644 index 000000000000..3ca90ed9d2bb --- /dev/null +++ b/include/uapi/drm/nova_drm.h @@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: MIT */ + +#ifndef __NOVA_DRM_H__ +#define __NOVA_DRM_H__ + +#include "drm.h" + +/* DISCLAIMER: Do not use, this is not a stable uAPI. + * + * This uAPI serves only testing purposes as long as this driver is still in + * development. It is required to implement and test infrastructure which is + * upstreamed in the context of this driver. See also [1]. + * + * [1] https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u + */ + +#if defined(__cplusplus) +extern "C" { +#endif + +/* + * NOVA_GETPARAM_VRAM_BAR_SIZE + * + * Query the VRAM BAR size in bytes. + */ +#define NOVA_GETPARAM_VRAM_BAR_SIZE 0x1 + +/** + * struct drm_nova_getparam - query GPU and driver metadata + */ +struct drm_nova_getparam { + /** + * @param: The identifier of the parameter to query. + */ + __u64 param; + + /** + * @value: The value for the specified parameter. + */ + __u64 value; +}; + +/** + * struct drm_nova_gem_create - create a new DRM GEM object + */ +struct drm_nova_gem_create { + /** + * @handle: The handle of the new DRM GEM object. + */ + __u32 handle; + + /** + * @pad: 32 bit padding, should be 0. + */ + __u32 pad; + + /** + * @size: The size of the new DRM GEM object. + */ + __u64 size; +}; + +/** + * struct drm_nova_gem_info - query DRM GEM object metadata + */ +struct drm_nova_gem_info { + /** + * @handle: The handle of the DRM GEM object to query. + */ + __u32 handle; + + /** + * @pad: 32 bit padding, should be 0. + */ + __u32 pad; + + /** + * @size: The size of the DRM GEM obejct. + */ + __u64 size; +}; + +#define DRM_NOVA_GETPARAM 0x00 +#define DRM_NOVA_GEM_CREATE 0x01 +#define DRM_NOVA_GEM_INFO 0x02 + +/* Note: this is an enum so that it can be resolved by Rust bindgen. */ +enum { + DRM_IOCTL_NOVA_GETPARAM = DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_GETPARAM, + struct drm_nova_getparam), + DRM_IOCTL_NOVA_GEM_CREATE = DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_GEM_CREATE, + struct drm_nova_gem_create), + DRM_IOCTL_NOVA_GEM_INFO = DRM_IOWR(DRM_COMMAND_BASE + DRM_NOVA_GEM_INFO, + struct drm_nova_gem_info), +}; + +#if defined(__cplusplus) +} +#endif + +#endif /* __NOVA_DRM_H__ */ diff --git a/rust/uapi/uapi_helper.h b/rust/uapi/uapi_helper.h index 19587e55e604..1409441359f5 100644 --- a/rust/uapi/uapi_helper.h +++ b/rust/uapi/uapi_helper.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include From 297b2cd6ba2b708d987c30360f846cfbe79ad563 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 7 May 2025 22:52:29 +0900 Subject: [PATCH 35/47] gpu: nova-core: derive useful traits for Chipset We will commonly need to compare chipset versions, so derive the ordering traits to make that possible. Also derive Copy and Clone since passing Chipset by value will be more efficient than by reference. Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-2-fcb02749754d@nvidia.com Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/gpu.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 17c9660da450..4de67a2dc163 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -13,7 +13,7 @@ macro_rules! define_chipset { ({ $($variant:ident = $value:expr),* $(,)* }) => { /// Enum representation of the GPU chipset. - #[derive(fmt::Debug)] + #[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)] pub(crate) enum Chipset { $($variant = $value),*, } From 44dda4353b9bd5dcc9cc30d97c0c3cfb00d8f987 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 7 May 2025 22:52:30 +0900 Subject: [PATCH 36/47] gpu: nova-core: add missing GA100 definition linux-firmware contains a directory for GA100, and it is a defined chipset in Nouveau. Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-3-fcb02749754d@nvidia.com Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/gpu.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 4de67a2dc163..9fe6aedaa956 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -54,6 +54,7 @@ fn try_from(value: u32) -> Result { TU117 = 0x167, TU116 = 0x168, // Ampere + GA100 = 0x170, GA102 = 0x172, GA103 = 0x173, GA104 = 0x174, @@ -73,7 +74,7 @@ pub(crate) fn arch(&self) -> Architecture { Self::TU102 | Self::TU104 | Self::TU106 | Self::TU117 | Self::TU116 => { Architecture::Turing } - Self::GA102 | Self::GA103 | Self::GA104 | Self::GA106 | Self::GA107 => { + Self::GA100 | Self::GA102 | Self::GA103 | Self::GA104 | Self::GA106 | Self::GA107 => { Architecture::Ampere } Self::AD102 | Self::AD103 | Self::AD104 | Self::AD106 | Self::AD107 => { From a2a637ffdf8692e9a376383ca82a990036424fe0 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 7 May 2025 22:52:31 +0900 Subject: [PATCH 37/47] gpu: nova-core: take bound device in Gpu::new We will need to perform things like allocating DMA memory during device creation, so make sure to take the device context that will allow us to perform these actions. This also allows us to use Devres::access to obtain the BAR without holding a RCU lock. Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-4-fcb02749754d@nvidia.com Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/gpu.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 9fe6aedaa956..a64a306e0ec8 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -134,9 +134,8 @@ pub(crate) struct Spec { } impl Spec { - fn new(bar: &Devres) -> Result { - let bar = bar.try_access().ok_or(ENXIO)?; - let boot0 = regs::Boot0::read(&bar); + fn new(bar: &Bar0) -> Result { + let boot0 = regs::Boot0::read(bar); Ok(Self { chipset: boot0.chipset().try_into()?, @@ -183,8 +182,12 @@ pub(crate) struct Gpu { } impl Gpu { - pub(crate) fn new(pdev: &pci::Device, bar: Devres) -> Result> { - let spec = Spec::new(&bar)?; + pub(crate) fn new( + pdev: &pci::Device, + devres_bar: Devres, + ) -> Result> { + let bar = devres_bar.access(pdev.as_ref())?; + let spec = Spec::new(bar)?; let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?; dev_info!( @@ -195,6 +198,10 @@ pub(crate) fn new(pdev: &pci::Device, bar: Devres) -> Result Date: Wed, 7 May 2025 22:52:32 +0900 Subject: [PATCH 38/47] gpu: nova-core: define registers layout using helper macro Add the register!() macro, which defines a given register's layout and provide bit-field accessors with a way to convert them to a given type. This macro will allow us to make clear definitions of the registers and manipulate their fields safely. The long-term goal is to eventually move it to the kernel crate so it can be used by other drivers as well, but it was agreed to first land it into nova-core and make it mature there. To illustrate its usage, use it to define the layout for the Boot0 (renamed to NV_PMC_BOOT_0 to match OpenRM's naming scheme) and take advantage of its accessors. Suggested-by: Danilo Krummrich Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-5-fcb02749754d@nvidia.com [ Fix typo in commit message. - Danilo ] Signed-off-by: Danilo Krummrich --- Documentation/gpu/nova/core/todo.rst | 6 + drivers/gpu/nova-core/gpu.rs | 10 +- drivers/gpu/nova-core/regs.rs | 61 +---- drivers/gpu/nova-core/regs/macros.rs | 380 +++++++++++++++++++++++++++ 4 files changed, 403 insertions(+), 54 deletions(-) create mode 100644 drivers/gpu/nova-core/regs/macros.rs diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst index 234d753d3eac..8a459fc08812 100644 --- a/Documentation/gpu/nova/core/todo.rst +++ b/Documentation/gpu/nova/core/todo.rst @@ -102,7 +102,13 @@ Usage: let boot0 = Boot0::read(&bar); pr_info!("Revision: {}\n", boot0.revision()); +Note: a work-in-progress implementation currently resides in +`drivers/gpu/nova-core/regs/macros.rs` and is used in nova-core. It would be +nice to improve it (possibly using proc macros) and move it to the `kernel` +crate so it can be used by other components as well. + | Complexity: Advanced +| Contact: Alexandre Courbot Delay / Sleep abstractions -------------------------- diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index a64a306e0ec8..43139b527fac 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -112,10 +112,10 @@ pub(crate) struct Revision { } impl Revision { - fn from_boot0(boot0: regs::Boot0) -> Self { + fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self { Self { - major: boot0.major_rev(), - minor: boot0.minor_rev(), + major: boot0.major_revision(), + minor: boot0.minor_revision(), } } } @@ -135,10 +135,10 @@ pub(crate) struct Spec { impl Spec { fn new(bar: &Bar0) -> Result { - let boot0 = regs::Boot0::read(bar); + let boot0 = regs::NV_PMC_BOOT_0::read(bar); Ok(Self { - chipset: boot0.chipset().try_into()?, + chipset: boot0.chipset()?, revision: Revision::from_boot0(boot0), }) } diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs index b1a25b86ef17..498fefb52f33 100644 --- a/drivers/gpu/nova-core/regs.rs +++ b/drivers/gpu/nova-core/regs.rs @@ -1,55 +1,18 @@ // SPDX-License-Identifier: GPL-2.0 -use crate::driver::Bar0; +// Required to retain the original register names used by OpenRM, which are all capital snake case +// but are mapped to types. +#![allow(non_camel_case_types)] -// TODO -// -// Create register definitions via generic macros. See task "Generic register -// abstraction" in Documentation/gpu/nova/core/todo.rst. +#[macro_use] +mod macros; -const BOOT0_OFFSET: usize = 0x00000000; +use crate::gpu::Chipset; -// 3:0 - chipset minor revision -const BOOT0_MINOR_REV_SHIFT: u8 = 0; -const BOOT0_MINOR_REV_MASK: u32 = 0x0000000f; +/* PMC */ -// 7:4 - chipset major revision -const BOOT0_MAJOR_REV_SHIFT: u8 = 4; -const BOOT0_MAJOR_REV_MASK: u32 = 0x000000f0; - -// 23:20 - chipset implementation Identifier (depends on architecture) -const BOOT0_IMPL_SHIFT: u8 = 20; -const BOOT0_IMPL_MASK: u32 = 0x00f00000; - -// 28:24 - chipset architecture identifier -const BOOT0_ARCH_MASK: u32 = 0x1f000000; - -// 28:20 - chipset identifier (virtual register field combining BOOT0_IMPL and -// BOOT0_ARCH) -const BOOT0_CHIPSET_SHIFT: u8 = BOOT0_IMPL_SHIFT; -const BOOT0_CHIPSET_MASK: u32 = BOOT0_IMPL_MASK | BOOT0_ARCH_MASK; - -#[derive(Copy, Clone)] -pub(crate) struct Boot0(u32); - -impl Boot0 { - #[inline] - pub(crate) fn read(bar: &Bar0) -> Self { - Self(bar.read32(BOOT0_OFFSET)) - } - - #[inline] - pub(crate) fn chipset(&self) -> u32 { - (self.0 & BOOT0_CHIPSET_MASK) >> BOOT0_CHIPSET_SHIFT - } - - #[inline] - pub(crate) fn minor_rev(&self) -> u8 { - ((self.0 & BOOT0_MINOR_REV_MASK) >> BOOT0_MINOR_REV_SHIFT) as u8 - } - - #[inline] - pub(crate) fn major_rev(&self) -> u8 { - ((self.0 & BOOT0_MAJOR_REV_MASK) >> BOOT0_MAJOR_REV_SHIFT) as u8 - } -} +register!(NV_PMC_BOOT_0 @ 0x00000000, "Basic revision information about the GPU" { + 3:0 minor_revision as u8, "Minor revision of the chip"; + 7:4 major_revision as u8, "Major revision of the chip"; + 28:20 chipset as u32 ?=> Chipset, "Chipset model"; +}); diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs new file mode 100644 index 000000000000..7ecc70efb3cd --- /dev/null +++ b/drivers/gpu/nova-core/regs/macros.rs @@ -0,0 +1,380 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Macro to define register layout and accessors. +//! +//! A single register typically includes several fields, which are accessed through a combination +//! of bit-shift and mask operations that introduce a class of potential mistakes, notably because +//! not all possible field values are necessarily valid. +//! +//! The macro in this module allow to define, using an intruitive and readable syntax, a dedicated +//! type for each register with its own field accessors that can return an error is a field's value +//! is invalid. + +/// Defines a dedicated type for a register with an absolute offset, alongside with getter and +/// setter methods for its fields and methods to read and write it from an `Io` region. +/// +/// Example: +/// +/// ```no_run +/// register!(BOOT_0 @ 0x00000100, "Basic revision information about the GPU" { +/// 3:0 minor_revision as u8, "Minor revision of the chip"; +/// 7:4 major_revision as u8, "Major revision of the chip"; +/// 28:20 chipset as u32 ?=> Chipset, "Chipset model"; +/// }); +/// ``` +/// +/// This defines a `BOOT_0` type which can be read or written from offset `0x100` of an `Io` +/// region. It is composed of 3 fields, for instance `minor_revision` is made of the 4 less +/// significant bits of the register. Each field can be accessed and modified using accessor +/// methods: +/// +/// ```no_run +/// // Read from the register's defined offset (0x100). +/// let boot0 = BOOT_0::read(&bar); +/// pr_info!("chip revision: {}.{}", boot0.major_revision(), boot0.minor_revision()); +/// +/// // `Chipset::try_from` will be called with the value of the field and returns an error if the +/// // value is invalid. +/// let chipset = boot0.chipset()?; +/// +/// // Update some fields and write the value back. +/// boot0.set_major_revision(3).set_minor_revision(10).write(&bar); +/// +/// // Or just read and update the register in a single step: +/// BOOT_0::alter(&bar, |r| r.set_major_revision(3).set_minor_revision(10)); +/// ``` +/// +/// Fields can be defined as follows: +/// +/// - `as ` simply returns the field value casted as the requested integer type, typically +/// `u32`, `u16`, `u8` or `bool`. Note that `bool` fields must have a range of 1 bit. +/// - `as => ` calls ``'s `From::<>` implementation and returns +/// the result. +/// - `as ?=> ` calls ``'s `TryFrom::<>` implementation +/// and returns the result. This is useful on fields for which not all values are value. +/// +/// The documentation strings are optional. If present, they will be added to the type's +/// definition, or the field getter and setter methods they are attached to. +/// +/// Putting a `+` before the address of the register makes it relative to a base: the `read` and +/// `write` methods take a `base` argument that is added to the specified address before access, +/// and `try_read` and `try_write` methods are also created, allowing access with offsets unknown +/// at compile-time: +/// +/// ```no_run +/// register!(CPU_CTL @ +0x0000010, "CPU core control" { +/// 0:0 start as bool, "Start the CPU core"; +/// }); +/// +/// // Flip the `start` switch for the CPU core which base address is at `CPU_BASE`. +/// let cpuctl = CPU_CTL::read(&bar, CPU_BASE); +/// pr_info!("CPU CTL: {:#x}", cpuctl); +/// cpuctl.set_start(true).write(&bar, CPU_BASE); +/// ``` +macro_rules! register { + // Creates a register at a fixed offset of the MMIO space. + ( + $name:ident @ $offset:literal $(, $comment:literal)? { + $($fields:tt)* + } + ) => { + register!(@common $name $(, $comment)?); + register!(@field_accessors $name { $($fields)* }); + register!(@io $name @ $offset); + }; + + // Creates a register at a relative offset from a base address. + ( + $name:ident @ + $offset:literal $(, $comment:literal)? { + $($fields:tt)* + } + ) => { + register!(@common $name $(, $comment)?); + register!(@field_accessors $name { $($fields)* }); + register!(@io$name @ + $offset); + }; + + // Defines the wrapper `$name` type, as well as its relevant implementations (`Debug`, `BitOr`, + // and conversion to regular `u32`). + (@common $name:ident $(, $comment:literal)?) => { + $( + #[doc=$comment] + )? + #[repr(transparent)] + #[derive(Clone, Copy, Default)] + pub(crate) struct $name(u32); + + // TODO: display the raw hex value, then the value of all the fields. This requires + // matching the fields, which will complexify the syntax considerably... + impl ::core::fmt::Debug for $name { + fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result { + f.debug_tuple(stringify!($name)) + .field(&format_args!("0x{0:x}", &self.0)) + .finish() + } + } + + impl core::ops::BitOr for $name { + type Output = Self; + + fn bitor(self, rhs: Self) -> Self::Output { + Self(self.0 | rhs.0) + } + } + + impl ::core::convert::From<$name> for u32 { + fn from(reg: $name) -> u32 { + reg.0 + } + } + }; + + // Defines all the field getter/methods methods for `$name`. + ( + @field_accessors $name:ident { + $($hi:tt:$lo:tt $field:ident as $type:tt + $(?=> $try_into_type:ty)? + $(=> $into_type:ty)? + $(, $comment:literal)? + ; + )* + } + ) => { + $( + register!(@check_field_bounds $hi:$lo $field as $type); + )* + + #[allow(dead_code)] + impl $name { + $( + register!(@field_accessor $name $hi:$lo $field as $type + $(?=> $try_into_type)? + $(=> $into_type)? + $(, $comment)? + ; + ); + )* + } + }; + + // Boolean fields must have `$hi == $lo`. + (@check_field_bounds $hi:tt:$lo:tt $field:ident as bool) => { + #[allow(clippy::eq_op)] + const _: () = { + kernel::build_assert!( + $hi == $lo, + concat!("boolean field `", stringify!($field), "` covers more than one bit") + ); + }; + }; + + // Non-boolean fields must have `$hi >= $lo`. + (@check_field_bounds $hi:tt:$lo:tt $field:ident as $type:tt) => { + #[allow(clippy::eq_op)] + const _: () = { + kernel::build_assert!( + $hi >= $lo, + concat!("field `", stringify!($field), "`'s MSB is smaller than its LSB") + ); + }; + }; + + // Catches fields defined as `bool` and convert them into a boolean value. + ( + @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool => $into_type:ty + $(, $comment:literal)?; + ) => { + register!( + @leaf_accessor $name $hi:$lo $field as bool + { |f| <$into_type>::from(if f != 0 { true } else { false }) } + $into_type => $into_type $(, $comment)?; + ); + }; + + // Shortcut for fields defined as `bool` without the `=>` syntax. + ( + @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as bool $(, $comment:literal)?; + ) => { + register!(@field_accessor $name $hi:$lo $field as bool => bool $(, $comment)?;); + }; + + // Catches the `?=>` syntax for non-boolean fields. + ( + @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt ?=> $try_into_type:ty + $(, $comment:literal)?; + ) => { + register!(@leaf_accessor $name $hi:$lo $field as $type + { |f| <$try_into_type>::try_from(f as $type) } $try_into_type => + ::core::result::Result< + $try_into_type, + <$try_into_type as ::core::convert::TryFrom<$type>>::Error + > + $(, $comment)?;); + }; + + // Catches the `=>` syntax for non-boolean fields. + ( + @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt => $into_type:ty + $(, $comment:literal)?; + ) => { + register!(@leaf_accessor $name $hi:$lo $field as $type + { |f| <$into_type>::from(f as $type) } $into_type => $into_type $(, $comment)?;); + }; + + // Shortcut for fields defined as non-`bool` without the `=>` or `?=>` syntax. + ( + @field_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:tt + $(, $comment:literal)?; + ) => { + register!(@field_accessor $name $hi:$lo $field as $type => $type $(, $comment)?;); + }; + + // Generates the accessor methods for a single field. + ( + @leaf_accessor $name:ident $hi:tt:$lo:tt $field:ident as $type:ty + { $process:expr } $to_type:ty => $res_type:ty $(, $comment:literal)?; + ) => { + kernel::macros::paste!( + const [<$field:upper>]: ::core::ops::RangeInclusive = $lo..=$hi; + const [<$field:upper _MASK>]: u32 = ((((1 << $hi) - 1) << 1) + 1) - ((1 << $lo) - 1); + const [<$field:upper _SHIFT>]: u32 = Self::[<$field:upper _MASK>].trailing_zeros(); + ); + + $( + #[doc="Returns the value of this field:"] + #[doc=$comment] + )? + #[inline] + pub(crate) fn $field(self) -> $res_type { + kernel::macros::paste!( + const MASK: u32 = $name::[<$field:upper _MASK>]; + const SHIFT: u32 = $name::[<$field:upper _SHIFT>]; + ); + let field = ((self.0 & MASK) >> SHIFT); + + $process(field) + } + + kernel::macros::paste!( + $( + #[doc="Sets the value of this field:"] + #[doc=$comment] + )? + #[inline] + pub(crate) fn [](mut self, value: $to_type) -> Self { + const MASK: u32 = $name::[<$field:upper _MASK>]; + const SHIFT: u32 = $name::[<$field:upper _SHIFT>]; + let value = ((value as u32) << SHIFT) & MASK; + self.0 = (self.0 & !MASK) | value; + + self + } + ); + }; + + // Creates the IO accessors for a fixed offset register. + (@io $name:ident @ $offset:literal) => { + #[allow(dead_code)] + impl $name { + #[inline] + pub(crate) fn read(io: &T) -> Self where + T: ::core::ops::Deref>, + { + Self(io.read32($offset)) + } + + #[inline] + pub(crate) fn write(self, io: &T) where + T: ::core::ops::Deref>, + { + io.write32(self.0, $offset) + } + + #[inline] + pub(crate) fn alter( + io: &T, + f: F, + ) where + T: ::core::ops::Deref>, + F: ::core::ops::FnOnce(Self) -> Self, + { + let reg = f(Self::read(io)); + reg.write(io); + } + } + }; + + // Create the IO accessors for a relative offset register. + (@io $name:ident @ + $offset:literal) => { + #[allow(dead_code)] + impl $name { + #[inline] + pub(crate) fn read( + io: &T, + base: usize, + ) -> Self where + T: ::core::ops::Deref>, + { + Self(io.read32(base + $offset)) + } + + #[inline] + pub(crate) fn write( + self, + io: &T, + base: usize, + ) where + T: ::core::ops::Deref>, + { + io.write32(self.0, base + $offset) + } + + #[inline] + pub(crate) fn alter( + io: &T, + base: usize, + f: F, + ) where + T: ::core::ops::Deref>, + F: ::core::ops::FnOnce(Self) -> Self, + { + let reg = f(Self::read(io, base)); + reg.write(io, base); + } + + #[inline] + pub(crate) fn try_read( + io: &T, + base: usize, + ) -> ::kernel::error::Result where + T: ::core::ops::Deref>, + { + io.try_read32(base + $offset).map(Self) + } + + #[inline] + pub(crate) fn try_write( + self, + io: &T, + base: usize, + ) -> ::kernel::error::Result<()> where + T: ::core::ops::Deref>, + { + io.try_write32(self.0, base + $offset) + } + + #[inline] + pub(crate) fn try_alter( + io: &T, + base: usize, + f: F, + ) -> ::kernel::error::Result<()> where + T: ::core::ops::Deref>, + F: ::core::ops::FnOnce(Self) -> Self, + { + let reg = f(Self::try_read(io, base)?); + reg.try_write(io, base) + } + } + }; +} From e4bc82af9e8b095c0f7a5aa9050b780002bd0933 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 7 May 2025 22:52:33 +0900 Subject: [PATCH 39/47] gpu: nova-core: fix layout of NV_PMC_BOOT_0 The layout of NV_PMC_BOOT_0 has two small issues: - The "chipset" field, while useful to identify a chip, is actually an aggregate of two distinct fields named "architecture" and "implementation". - The "architecture" field is split, with its MSB being at a different location than the rest of its bits. Redefine the register layout to match its actual definition as provided by OpenRM and expose the fully-constructed "architecture" field through our own "Architecture" type. The "chipset" pseudo-field is also useful to have, so keep providing it. Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-6-fcb02749754d@nvidia.com [ Use Result from kernel::prelude. - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/gpu.rs | 19 ++++++++++++++++--- drivers/gpu/nova-core/regs.rs | 25 +++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 43139b527fac..059462d7c82b 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -101,9 +101,22 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// Enum representation of the GPU generation. #[derive(fmt::Debug)] pub(crate) enum Architecture { - Turing, - Ampere, - Ada, + Turing = 0x16, + Ampere = 0x17, + Ada = 0x19, +} + +impl TryFrom for Architecture { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + 0x16 => Ok(Self::Turing), + 0x17 => Ok(Self::Ampere), + 0x19 => Ok(Self::Ada), + _ => Err(ENODEV), + } + } } pub(crate) struct Revision { diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs index 498fefb52f33..5a1273230306 100644 --- a/drivers/gpu/nova-core/regs.rs +++ b/drivers/gpu/nova-core/regs.rs @@ -7,12 +7,33 @@ #[macro_use] mod macros; -use crate::gpu::Chipset; +use crate::gpu::{Architecture, Chipset}; +use kernel::prelude::*; /* PMC */ register!(NV_PMC_BOOT_0 @ 0x00000000, "Basic revision information about the GPU" { 3:0 minor_revision as u8, "Minor revision of the chip"; 7:4 major_revision as u8, "Major revision of the chip"; - 28:20 chipset as u32 ?=> Chipset, "Chipset model"; + 8:8 architecture_1 as u8, "MSB of the architecture"; + 23:20 implementation as u8, "Implementation version of the architecture"; + 28:24 architecture_0 as u8, "Lower bits of the architecture"; }); + +impl NV_PMC_BOOT_0 { + /// Combines `architecture_0` and `architecture_1` to obtain the architecture of the chip. + pub(crate) fn architecture(self) -> Result { + Architecture::try_from( + self.architecture_0() | (self.architecture_1() << Self::ARCHITECTURE_0.len()), + ) + } + + /// Combines `architecture` and `implementation` to obtain a code unique to the chipset. + pub(crate) fn chipset(self) -> Result { + self.architecture() + .map(|arch| { + ((arch as u32) << Self::IMPLEMENTATION.len()) | self.implementation() as u32 + }) + .and_then(Chipset::try_from) + } +} From 61479ae38cb7bf6083de302598b7d491ec54168a Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Wed, 7 May 2025 22:52:34 +0900 Subject: [PATCH 40/47] gpu: nova-core: move Firmware to firmware module We will extend the firmware methods, so move it to its own module instead to keep gpu.rs focused. Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250507-nova-frts-v3-7-fcb02749754d@nvidia.com [ Don't require a bound device, remove pub visibility from Firmware fields, use FIRMWARE_VERSION consistently. - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/firmware.rs | 44 ++++++++++++++++++++++++++++--- drivers/gpu/nova-core/gpu.rs | 35 +++--------------------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs index 6e6361c59ca1..4b8a38358a4f 100644 --- a/drivers/gpu/nova-core/firmware.rs +++ b/drivers/gpu/nova-core/firmware.rs @@ -1,13 +1,49 @@ // SPDX-License-Identifier: GPL-2.0 -use crate::gpu; +//! Contains structures and functions dedicated to the parsing, building and patching of firmwares +//! to be loaded into a given execution unit. + +use kernel::device; use kernel::firmware; +use kernel::prelude::*; +use kernel::str::CString; + +use crate::gpu; +use crate::gpu::Chipset; + +pub(crate) const FIRMWARE_VERSION: &str = "535.113.01"; + +/// Structure encapsulating the firmware blobs required for the GPU to operate. +#[expect(dead_code)] +pub(crate) struct Firmware { + booter_load: firmware::Firmware, + booter_unload: firmware::Firmware, + bootloader: firmware::Firmware, + gsp: firmware::Firmware, +} + +impl Firmware { + pub(crate) fn new(dev: &device::Device, chipset: Chipset, ver: &str) -> Result { + let mut chip_name = CString::try_from_fmt(fmt!("{}", chipset))?; + chip_name.make_ascii_lowercase(); + + let request = |name_| { + CString::try_from_fmt(fmt!("nvidia/{}/gsp/{}-{}.bin", &*chip_name, name_, ver)) + .and_then(|path| firmware::Firmware::request(&path, dev)) + }; + + Ok(Firmware { + booter_load: request("booter_load")?, + booter_unload: request("booter_unload")?, + bootloader: request("bootloader")?, + gsp: request("gsp")?, + }) + } +} pub(crate) struct ModInfoBuilder(firmware::ModInfoBuilder); impl ModInfoBuilder { - const VERSION: &'static str = "535.113.01"; - const fn make_entry_file(self, chipset: &str, fw: &str) -> Self { ModInfoBuilder( self.0 @@ -17,7 +53,7 @@ const fn make_entry_file(self, chipset: &str, fw: &str) -> Self { .push("/gsp/") .push(fw) .push("-") - .push(Self::VERSION) + .push(FIRMWARE_VERSION) .push(".bin"), ) } diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index 059462d7c82b..99c6796e73e9 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -1,10 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 -use kernel::{ - device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString, -}; +use kernel::{device, devres::Devres, error::code::*, pci, prelude::*}; use crate::driver::Bar0; +use crate::firmware::{Firmware, FIRMWARE_VERSION}; use crate::regs; use crate::util; use core::fmt; @@ -157,34 +156,6 @@ fn new(bar: &Bar0) -> Result { } } -/// Structure encapsulating the firmware blobs required for the GPU to operate. -#[expect(dead_code)] -pub(crate) struct Firmware { - booter_load: firmware::Firmware, - booter_unload: firmware::Firmware, - bootloader: firmware::Firmware, - gsp: firmware::Firmware, -} - -impl Firmware { - fn new(dev: &device::Device, spec: &Spec, ver: &str) -> Result { - let mut chip_name = CString::try_from_fmt(fmt!("{}", spec.chipset))?; - chip_name.make_ascii_lowercase(); - - let request = |name_| { - CString::try_from_fmt(fmt!("nvidia/{}/gsp/{}-{}.bin", &*chip_name, name_, ver)) - .and_then(|path| firmware::Firmware::request(&path, dev)) - }; - - Ok(Firmware { - booter_load: request("booter_load")?, - booter_unload: request("booter_unload")?, - bootloader: request("bootloader")?, - gsp: request("gsp")?, - }) - } -} - /// Structure holding the resources required to operate the GPU. #[pin_data] pub(crate) struct Gpu { @@ -201,7 +172,7 @@ pub(crate) fn new( ) -> Result> { let bar = devres_bar.access(pdev.as_ref())?; let spec = Spec::new(bar)?; - let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?; + let fw = Firmware::new(pdev.as_ref(), spec.chipset, FIRMWARE_VERSION)?; dev_info!( pdev.as_ref(), From 6ee48aee8c705717051109ff889f2a1d4f409ea9 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:54 -0400 Subject: [PATCH 41/47] rust: drm: gem: Use NonNull for Object::dev There is usually not much of a reason to use a raw pointer in a data struct, so move this to NonNull instead. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-2-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 0cafa4a42420..df8f9fdae5c2 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -177,7 +177,7 @@ impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObj #[pin_data] pub struct Object { obj: Opaque, - dev: *const drm::Device, + dev: NonNull>, #[pin] data: T, } @@ -212,7 +212,7 @@ pub fn new(dev: &drm::Device, size: usize) -> Result> { data <- T::new(dev, size), // INVARIANT: The drm subsystem guarantees that the `struct drm_device` will live // as long as the GEM object lives. - dev, + dev: dev.into(), }), GFP_KERNEL, )?; @@ -237,7 +237,7 @@ pub fn new(dev: &drm::Device, size: usize) -> Result> { pub fn dev(&self) -> &drm::Device { // SAFETY: The DRM subsystem guarantees that the `struct drm_device` will live as long as // the GEM object lives, hence the pointer must be valid. - unsafe { &*self.dev } + unsafe { self.dev.as_ref() } } fn as_raw(&self) -> *mut bindings::drm_gem_object { From 36b1ccbfa0c2f45fae3e07bf66e40d7fd6704874 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:55 -0400 Subject: [PATCH 42/47] rust: drm: gem: Refactor IntoGEMObject::from_gem_obj() to as_ref() There's a few issues with this function, mainly: * This function -probably- should have been unsafe from the start. Pointers are not always necessarily valid, but you want a function that does field-projection for a pointer that can travel outside of the original struct to be unsafe, at least if I understand properly. * *mut Self is not terribly useful in this context, the majority of uses of from_gem_obj() grab a *mut Self and then immediately convert it into a &'a Self. It also goes against the ffi conventions we've set in the rest of the kernel thus far. * from_gem_obj() also doesn't follow the naming conventions in the rest of the DRM bindings at the moment, as_ref() would be a better name. So, let's: * Make from_gem_obj() unsafe * Convert it to return &'a Self * Rename it to as_ref() * Update all call locations Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-3-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 63 ++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index df8f9fdae5c2..1ea1f15d8313 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -45,8 +45,14 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { #[allow(clippy::wrong_self_convention)] fn into_gem_obj(&self) -> &Opaque; - /// Converts a pointer to a `struct drm_gem_object` into a pointer to `Self`. - fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self; + /// Converts a pointer to a `struct drm_gem_object` into a reference to `Self`. + /// + /// # Safety + /// + /// - `self_ptr` must be a valid pointer to `Self`. + /// - The caller promises that holding the immutable reference returned by this function does + /// not violate rust's data aliasing rules and remains valid throughout the lifetime of `'a`. + unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self; } /// Trait which must be implemented by drivers using base GEM objects. @@ -63,14 +69,13 @@ extern "C" fn open_callback, U: BaseObject>( let file = unsafe { drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) }; - let obj = - <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( - raw_obj, - ); + // SAFETY: `open_callback` is specified in the AllocOps structure for `Object`, ensuring that + // `raw_obj` is indeed contained within a `Object`. + let obj = unsafe { + <<::Driver as drm::Driver>::Object as IntoGEMObject>::as_ref(raw_obj) + }; - // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the - // `raw_obj` we got is valid. - match T::open(unsafe { &*obj }, file) { + match T::open(obj, file) { Err(e) => e.to_errno(), Ok(()) => 0, } @@ -84,14 +89,13 @@ extern "C" fn close_callback, U: BaseObject>( let file = unsafe { drm::File::<<::Driver as drm::Driver>::File>::as_ref(raw_file) }; - let obj = - <<::Driver as drm::Driver>::Object as IntoGEMObject>::from_gem_obj( - raw_obj, - ); + // SAFETY: `close_callback` is specified in the AllocOps structure for `Object`, ensuring + // that `raw_obj` is indeed contained within a `Object`. + let obj = unsafe { + <<::Driver as drm::Driver>::Object as IntoGEMObject>::as_ref(raw_obj) + }; - // SAFETY: `from_gem_obj()` returns a valid pointer as long as the type is correct and the - // `raw_obj` we got is valid. - T::close(unsafe { &*obj }, file); + T::close(obj, file); } impl IntoGEMObject for Object { @@ -101,9 +105,10 @@ fn into_gem_obj(&self) -> &Opaque { &self.obj } - fn from_gem_obj(obj: *mut bindings::drm_gem_object) -> *mut Self { - // SAFETY: All of our objects are Object. - unsafe { crate::container_of!(obj, Object, obj).cast_mut() } + unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { + // SAFETY: `obj` is guaranteed to be in an `Object` via the safety contract of this + // function + unsafe { &*crate::container_of!(self_ptr, Object, obj) } } } @@ -144,11 +149,23 @@ fn lookup_handle( ) -> Result> { // SAFETY: The arguments are all valid per the type invariants. let ptr = unsafe { bindings::drm_gem_object_lookup(file.as_raw().cast(), handle) }; - let ptr = ::from_gem_obj(ptr); - let ptr = NonNull::new(ptr).ok_or(ENOENT)?; + if ptr.is_null() { + return Err(ENOENT); + } - // SAFETY: We take ownership of the reference of `drm_gem_object_lookup()`. - Ok(unsafe { ARef::from_raw(ptr) }) + // SAFETY: + // - A `drm::Driver` can only have a single `File` implementation. + // - `file` uses the same `drm::Driver` as `Self`. + // - Therefore, we're guaranteed that `ptr` must be a gem object embedded within `Self`. + // - And we check if the pointer is null befoe calling as_ref(), ensuring that `ptr` is a + // valid pointer to an initialized `Self`. + let obj = unsafe { Self::as_ref(ptr) }; + + // SAFETY: + // - We take ownership of the reference of `drm_gem_object_lookup()`. + // - Our `NonNull` comes from an immutable reference, thus ensuring it is a valid pointer to + // `Self`. + Ok(unsafe { ARef::from_raw(obj.into()) }) } /// Creates an mmap offset to map the object from userspace. From b36ff40b4abd0f468564cfe9805f0d8e850cafe9 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:56 -0400 Subject: [PATCH 43/47] rust: drm: gem: s/into_gem_obj()/as_raw()/ There's a few changes here: * The rename, of course (this should also let us drop the clippy annotation here) * Return *mut bindings::drm_gem_object instead of &Opaque - the latter doesn't really have any benefit and just results in conversion from the rust type to the C type having to be more verbose than necessary. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-4-lyude@redhat.com [ Fixup s/into_gem_obj()/as_raw()/ in safety comment. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 1ea1f15d8313..e11cf7837bf5 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -12,7 +12,7 @@ prelude::*, types::{ARef, Opaque}, }; -use core::{mem, ops::Deref, ptr, ptr::NonNull}; +use core::{mem, ops::Deref, ptr::NonNull}; /// GEM object functions, which must be implemented by drivers. pub trait BaseDriverObject: Sync + Send + Sized { @@ -42,8 +42,7 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as /// this owning object is valid. - #[allow(clippy::wrong_self_convention)] - fn into_gem_obj(&self) -> &Opaque; + fn as_raw(&self) -> *mut bindings::drm_gem_object; /// Converts a pointer to a `struct drm_gem_object` into a reference to `Self`. /// @@ -101,8 +100,8 @@ extern "C" fn close_callback, U: BaseObject>( impl IntoGEMObject for Object { type Driver = T::Driver; - fn into_gem_obj(&self) -> &Opaque { - &self.obj + fn as_raw(&self) -> *mut bindings::drm_gem_object { + self.obj.get() } unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { @@ -119,9 +118,8 @@ pub trait BaseObject { /// Returns the size of the object in bytes. fn size(&self) -> usize { - // SAFETY: `self.into_gem_obj()` is guaranteed to be a pointer to a valid `struct - // drm_gem_object`. - unsafe { (*self.into_gem_obj().get()).size } + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `struct drm_gem_object`. + unsafe { (*self.as_raw()).size } } /// Creates a new handle for the object associated with a given `File` @@ -133,11 +131,7 @@ fn create_handle( let mut handle: u32 = 0; // SAFETY: The arguments are all valid per the type invariants. to_result(unsafe { - bindings::drm_gem_handle_create( - file.as_raw().cast(), - self.into_gem_obj().get(), - &mut handle, - ) + bindings::drm_gem_handle_create(file.as_raw().cast(), self.as_raw(), &mut handle) })?; Ok(handle) } @@ -171,14 +165,10 @@ fn lookup_handle( /// Creates an mmap offset to map the object from userspace. fn create_mmap_offset(&self) -> Result { // SAFETY: The arguments are valid per the type invariant. - to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.into_gem_obj().get()) })?; + to_result(unsafe { bindings::drm_gem_create_mmap_offset(self.as_raw()) })?; // SAFETY: The arguments are valid per the type invariant. - Ok(unsafe { - bindings::drm_vma_node_offset_addr(ptr::addr_of_mut!( - (*self.into_gem_obj().get()).vma_node - )) - }) + Ok(unsafe { bindings::drm_vma_node_offset_addr(&raw mut (*self.as_raw()).vma_node) }) } } From 38cb08c3fcd3f3b1d0225dcec8ae50fab5751549 Mon Sep 17 00:00:00 2001 From: Lyude Paul Date: Tue, 13 May 2025 18:09:57 -0400 Subject: [PATCH 44/47] rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically Currently we are requiring AlwaysRefCounted in most trait bounds for gem objects, and implementing it by hand for our only current type of gem object. However, all gem objects use the same functions for reference counting - and all gem objects support reference counting. We're planning on adding support for shmem gem objects, let's move this around a bit by instead making IntoGEMObject require AlwaysRefCounted as a trait bound, and then provide a blanket AlwaysRefCounted implementation for any object that implements IntoGEMObject so all gem object types can use the same AlwaysRefCounted implementation. This also makes things less verbose by making the AlwaysRefCounted trait bound implicit for any IntoGEMObject bound. Signed-off-by: Lyude Paul Reviewed-by: Daniel Almeida Link: https://lore.kernel.org/r/20250513221046.903358-5-lyude@redhat.com Signed-off-by: Danilo Krummrich --- rust/kernel/drm/gem/mod.rs | 47 +++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index e11cf7837bf5..d8765e61c6c2 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -10,7 +10,7 @@ drm::driver::{AllocImpl, AllocOps}, error::{to_result, Result}, prelude::*, - types::{ARef, Opaque}, + types::{ARef, AlwaysRefCounted, Opaque}, }; use core::{mem, ops::Deref, ptr::NonNull}; @@ -36,7 +36,7 @@ fn close( } /// Trait that represents a GEM object subtype -pub trait IntoGEMObject: Sized + super::private::Sealed { +pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { /// Owning driver for this type type Driver: drm::Driver; @@ -54,6 +54,26 @@ pub trait IntoGEMObject: Sized + super::private::Sealed { unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self; } +// SAFETY: All gem objects are refcounted. +unsafe impl AlwaysRefCounted for T { + fn inc_ref(&self) { + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. + unsafe { bindings::drm_gem_object_get(self.as_raw()) }; + } + + unsafe fn dec_ref(obj: NonNull) { + // SAFETY: We either hold the only refcount on `obj`, or one of many - meaning that no one + // else could possibly hold a mutable reference to `obj` and thus this immutable reference + // is safe. + let obj = unsafe { obj.as_ref() }.as_raw(); + + // SAFETY: + // - The safety requirements guarantee that the refcount is non-zero. + // - We hold no references to `obj` now, making it safe for us to potentially deallocate it. + unsafe { bindings::drm_gem_object_put(obj) }; + } +} + /// Trait which must be implemented by drivers using base GEM objects. pub trait DriverObject: BaseDriverObject> { /// Parent `Driver` for this object. @@ -112,10 +132,7 @@ unsafe fn as_ref<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { } /// Base operations shared by all GEM object classes -pub trait BaseObject -where - Self: crate::types::AlwaysRefCounted + IntoGEMObject, -{ +pub trait BaseObject: IntoGEMObject { /// Returns the size of the object in bytes. fn size(&self) -> usize { // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `struct drm_gem_object`. @@ -172,7 +189,7 @@ fn create_mmap_offset(&self) -> Result { } } -impl BaseObject for T where Self: crate::types::AlwaysRefCounted + IntoGEMObject {} +impl BaseObject for T {} /// A base GEM object. /// @@ -266,22 +283,6 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { } } -// SAFETY: Instances of `Object` are always reference-counted. -unsafe impl crate::types::AlwaysRefCounted for Object { - fn inc_ref(&self) { - // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. - unsafe { bindings::drm_gem_object_get(self.as_raw()) }; - } - - unsafe fn dec_ref(obj: NonNull) { - // SAFETY: `obj` is a valid pointer to an `Object`. - let obj = unsafe { obj.as_ref() }; - - // SAFETY: The safety requirements guarantee that the refcount is non-zero. - unsafe { bindings::drm_gem_object_put(obj.as_raw()) } - } -} - impl super::private::Sealed for Object {} impl Deref for Object { From 80a8bcc65290bc95c11accd875fb3dd19fc7287e Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Thu, 15 May 2025 17:23:18 +0900 Subject: [PATCH 45/47] samples: rust: select AUXILIARY_BUS instead of depending on it CONFIG_AUXILIARY_BUS cannot be enabled explicitly, and unless we select it we have no way to include it (and thus to enable the auxiliary driver sample) unless a driver happens to do it for us. Fixes: 96609a1969f4 ("samples: rust: add Rust auxiliary driver sample") Reviewed-by: Greg Kroah-Hartman Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250515-aux_bus-v2-1-47c70f96ae9b@nvidia.com Signed-off-by: Danilo Krummrich --- samples/rust/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig index 43cb72d72631..b1006ab4bc3c 100644 --- a/samples/rust/Kconfig +++ b/samples/rust/Kconfig @@ -84,8 +84,8 @@ config SAMPLE_RUST_DRIVER_FAUX config SAMPLE_RUST_DRIVER_AUXILIARY tristate "Auxiliary Driver" - depends on AUXILIARY_BUS depends on PCI + select AUXILIARY_BUS help This option builds the Rust auxiliary driver sample. From a6fdda6dfeb0b900979b3aac3a661ecec9e67711 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Thu, 15 May 2025 17:23:19 +0900 Subject: [PATCH 46/47] gpu: nova-core: select AUXILIARY_BUS instead of depending on it CONFIG_AUXILIARY_BUS cannot be enabled explicitly, and unless we select it we have no way to include it (and thus to enable NOVA_CORE) unless another driver happens to do it for us. Fixes: e041d81a0377 ("gpu: nova-core: register auxiliary device for nova-drm") Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250515-aux_bus-v2-2-47c70f96ae9b@nvidia.com Signed-off-by: Danilo Krummrich --- drivers/gpu/nova-core/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig index 67f99b6a023a..8726d80d6ba4 100644 --- a/drivers/gpu/nova-core/Kconfig +++ b/drivers/gpu/nova-core/Kconfig @@ -1,9 +1,9 @@ config NOVA_CORE tristate "Nova Core GPU driver" - depends on AUXILIARY_BUS depends on PCI depends on RUST depends on RUST_FW_LOADER_ABSTRACTIONS + select AUXILIARY_BUS default n help Choose this if you want to build the Nova Core driver for Nvidia From 276c53c66e032c8e7cc0da63555f2742eb1afd69 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Thu, 15 May 2025 17:23:20 +0900 Subject: [PATCH 47/47] gpu: drm: nova: select AUXILIARY_BUS instead of depending on it CONFIG_AUXILIARY_BUS cannot be enabled explicitly, and unless we select it we have no way to include it (and thus to enable NOVA_DRM) unless another driver happens to do it for us. Fixes: cdeaeb9dd762 ("drm: nova-drm: add initial driver skeleton") Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250515-aux_bus-v2-3-47c70f96ae9b@nvidia.com Signed-off-by: Danilo Krummrich --- drivers/gpu/drm/nova/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nova/Kconfig b/drivers/gpu/drm/nova/Kconfig index 123e96788484..cca6a3fea879 100644 --- a/drivers/gpu/drm/nova/Kconfig +++ b/drivers/gpu/drm/nova/Kconfig @@ -1,9 +1,9 @@ config DRM_NOVA tristate "Nova DRM driver" - depends on AUXILIARY_BUS depends on DRM=y depends on PCI depends on RUST + select AUXILIARY_BUS default n help Choose this if you want to build the Nova DRM driver for Nvidia