Now that we have the ability to represent the context in which a DRM device is in at compile-time, we can start carrying around this context with GEM object types in order to allow a driver to safely create GEM objects before a DRM device has registered with userspace.
Signed-off-by: Lyude Paul <[email protected]> --- drivers/gpu/drm/nova/driver.rs | 2 +- drivers/gpu/drm/nova/gem.rs | 11 +++--- drivers/gpu/drm/tyr/driver.rs | 2 +- drivers/gpu/drm/tyr/gem.rs | 3 +- rust/kernel/drm/device.rs | 14 ++++---- rust/kernel/drm/driver.rs | 2 +- rust/kernel/drm/gem/mod.rs | 64 +++++++++++++++++++++++----------- 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs index 8cea5f68c3b04..2c13261450406 100644 --- a/drivers/gpu/drm/nova/driver.rs +++ b/drivers/gpu/drm/nova/driver.rs @@ -67,7 +67,7 @@ fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<S impl drm::Driver for NovaDriver { type Data = NovaData; type File = File; - type Object = gem::Object<NovaObject>; + type Object<Ctx: drm::DeviceContext> = gem::Object<NovaObject, Ctx>; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/nova/gem.rs b/drivers/gpu/drm/nova/gem.rs index 6ccfa5da57617..f6e98b9db58d8 100644 --- a/drivers/gpu/drm/nova/gem.rs +++ b/drivers/gpu/drm/nova/gem.rs @@ -2,7 +2,7 @@ use kernel::{ drm, - drm::{gem, gem::BaseObject}, + drm::{gem, gem::BaseObject, DeviceContext}, page, prelude::*, sync::aref::ARef, @@ -20,20 +20,23 @@ pub(crate) struct NovaObject {} impl gem::DriverObject for NovaObject { type Driver = NovaDriver; - fn new(_dev: &NovaDevice, _size: usize) -> impl PinInit<Self, Error> { + fn new<Ctx: DeviceContext>(_dev: &NovaDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> { try_pin_init!(NovaObject {}) } } impl NovaObject { /// Create a new DRM GEM object. - pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> { + pub(crate) fn new<Ctx: DeviceContext>( + dev: &NovaDevice<Ctx>, + size: usize, + ) -> Result<ARef<gem::Object<Self, Ctx>>> { if size == 0 { return Err(EINVAL); } let aligned_size = page::page_align(size).ok_or(EINVAL)?; - gem::Object::new(dev, aligned_size) + gem::Object::<Self, Ctx>::new(dev, aligned_size) } /// Look up a GEM object handle for a `File` and return an `ObjectRef` for it. diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs index e73c56659ea75..03e337d95521c 100644 --- a/drivers/gpu/drm/tyr/driver.rs +++ b/drivers/gpu/drm/tyr/driver.rs @@ -177,7 +177,7 @@ fn drop(self: Pin<&mut Self>) { impl drm::Driver for TyrDriver { type Data = TyrData; type File = File; - type Object = drm::gem::Object<TyrObject>; + type Object<R: drm::DeviceContext> = drm::gem::Object<TyrObject, R>; const INFO: drm::DriverInfo = INFO; diff --git a/drivers/gpu/drm/tyr/gem.rs b/drivers/gpu/drm/tyr/gem.rs index 1273bf89dbd5d..00804f8c14bd4 100644 --- a/drivers/gpu/drm/tyr/gem.rs +++ b/drivers/gpu/drm/tyr/gem.rs @@ -3,6 +3,7 @@ use crate::driver::TyrDevice; use crate::driver::TyrDriver; use kernel::drm::gem; +use kernel::drm::DeviceContext; use kernel::prelude::*; /// GEM Object inner driver data @@ -12,7 +13,7 @@ pub(crate) struct TyrObject {} impl gem::DriverObject for TyrObject { type Driver = TyrDriver; - fn new(_dev: &TyrDevice, _size: usize) -> impl PinInit<Self, Error> { + fn new<Ctx: DeviceContext>(_dev: &TyrDevice<Ctx>, _size: usize) -> impl PinInit<Self, Error> { try_pin_init!(TyrObject {}) } } diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 0e81957cf8c28..4c03105e6a817 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -163,13 +163,13 @@ impl<T: drm::Driver> UnregisteredDevice<T> { 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, + gem_create_object: T::Object::<Uninit>::ALLOC_OPS.gem_create_object, + prime_handle_to_fd: T::Object::<Uninit>::ALLOC_OPS.prime_handle_to_fd, + prime_fd_to_handle: T::Object::<Uninit>::ALLOC_OPS.prime_fd_to_handle, + gem_prime_import: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import, + gem_prime_import_sg_table: T::Object::<Uninit>::ALLOC_OPS.gem_prime_import_sg_table, + dumb_create: T::Object::<Uninit>::ALLOC_OPS.dumb_create, + dumb_map_offset: T::Object::<Uninit>::ALLOC_OPS.dumb_map_offset, show_fdinfo: None, fbdev_probe: None, diff --git a/rust/kernel/drm/driver.rs b/rust/kernel/drm/driver.rs index a16605b407159..94ebaf19ac069 100644 --- a/rust/kernel/drm/driver.rs +++ b/rust/kernel/drm/driver.rs @@ -110,7 +110,7 @@ pub trait Driver { type Data: Sync + Send; /// The type used to manage memory for this driver. - type Object: AllocImpl; + type Object<Ctx: drm::DeviceContext>: AllocImpl; /// The type used to represent a DRM File (client) type File: drm::file::DriverFile; diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index b4199945db378..3af9f52f8eda4 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -8,6 +8,10 @@ bindings, drm::{ self, + device::{ + DeviceContext, + Registered, // + }, driver::{ AllocImpl, AllocOps, // @@ -22,6 +26,7 @@ types::Opaque, }; use core::{ + marker::PhantomData, ops::Deref, ptr::NonNull, // }; @@ -33,21 +38,30 @@ /// [`DriverFile`]: drm::file::DriverFile pub type DriverFile<T> = drm::File<<<T as DriverObject>::Driver as drm::Driver>::File>; +/// A type alias for retrieving the current [`AllocImpl`] for a given [`DriverObject`]. +/// +/// [`Driver`]: drm::Driver +pub type DriverAllocImpl<T, Ctx = Registered> = + <<T as DriverObject>::Driver as drm::Driver>::Object<Ctx>; + /// GEM object functions, which must be implemented by drivers. pub trait DriverObject: Sync + Send + Sized { /// Parent `Driver` for this object. type Driver: drm::Driver; /// Create a new driver data object for a GEM object of a given size. - fn new(dev: &drm::Device<Self::Driver>, size: usize) -> impl PinInit<Self, Error>; + fn new<Ctx: DeviceContext>( + dev: &drm::Device<Self::Driver, Ctx>, + size: usize, + ) -> impl PinInit<Self, Error>; /// Open a new handle to an existing object, associated with a File. - fn open(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) -> Result { + fn open(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) -> Result { Ok(()) } /// Close a handle to an existing object, associated with a File. - fn close(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>) {} + fn close(_obj: &DriverAllocImpl<Self>, _file: &DriverFile<Self>) {} } /// Trait that represents a GEM object subtype @@ -73,9 +87,12 @@ extern "C" fn open_callback<T: DriverObject>( // SAFETY: `open_callback` is only ever called with a valid pointer to a `struct drm_file`. let file = unsafe { DriverFile::<T>::from_raw(raw_file) }; - // SAFETY: `open_callback` is specified in the AllocOps structure for `DriverObject<T>`, - // ensuring that `raw_obj` is contained within a `DriverObject<T>` - let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) }; + // SAFETY: + // * `open_callback` is specified in the AllocOps structure for `DriverObject`, ensuring that + // `raw_obj` is contained within a `DriverAllocImpl<T>` + // * It is only possible for `open_callback` to be called after device registration, ensuring + // that the object's device is in the `Registered` state. + let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) }; match T::open(obj, file) { Err(e) => e.to_errno(), @@ -92,12 +109,12 @@ extern "C" fn close_callback<T: DriverObject>( // SAFETY: `close_callback` is specified in the AllocOps structure for `Object<T>`, ensuring // that `raw_obj` is indeed contained within a `Object<T>`. - let obj = unsafe { <<T::Driver as drm::Driver>::Object as IntoGEMObject>::from_raw(raw_obj) }; + let obj: &DriverAllocImpl<T> = unsafe { IntoGEMObject::from_raw(raw_obj) }; T::close(obj, file); } -impl<T: DriverObject> IntoGEMObject for Object<T> { +impl<T: DriverObject, Ctx: DeviceContext> IntoGEMObject for Object<T, Ctx> { fn as_raw(&self) -> *mut bindings::drm_gem_object { self.obj.get() } @@ -105,7 +122,7 @@ fn as_raw(&self) -> *mut bindings::drm_gem_object { unsafe fn from_raw<'a>(self_ptr: *mut bindings::drm_gem_object) -> &'a Self { // SAFETY: `obj` is guaranteed to be in an `Object<T>` via the safety contract of this // function - unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T>, obj) } + unsafe { &*crate::container_of!(Opaque::cast_from(self_ptr), Object<T, Ctx>, obj) } } } @@ -122,7 +139,7 @@ fn size(&self) -> usize { fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32> where Self: AllocImpl<Driver = D>, - D: drm::Driver<Object = Self, File = F>, + D: drm::Driver<Object<Registered> = Self, File = F>, F: drm::file::DriverFile<Driver = D>, { let mut handle: u32 = 0; @@ -137,7 +154,7 @@ fn create_handle<D, F>(&self, file: &drm::File<F>) -> Result<u32> fn lookup_handle<D, F>(file: &drm::File<F>, handle: u32) -> Result<ARef<Self>> where Self: AllocImpl<Driver = D>, - D: drm::Driver<Object = Self, File = F>, + D: drm::Driver<Object<Registered> = Self, File = F>, F: drm::file::DriverFile<Driver = D>, { // SAFETY: The arguments are all valid per the type invariants. @@ -177,16 +194,18 @@ impl<T: IntoGEMObject> BaseObject for T {} /// /// # Invariants /// -/// - `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * `self.obj` is a valid instance of a `struct drm_gem_object`. +/// * Any type invariants of `Ctx` apply to the parent DRM device for this GEM object. #[repr(C)] #[pin_data] -pub struct Object<T: DriverObject + Send + Sync> { +pub struct Object<T: DriverObject + Send + Sync, Ctx: DeviceContext = Registered> { obj: Opaque<bindings::drm_gem_object>, #[pin] data: T, + _ctx: PhantomData<Ctx>, } -impl<T: DriverObject> Object<T> { +impl<T: DriverObject, Ctx: DeviceContext> Object<T, Ctx> { const OBJECT_FUNCS: bindings::drm_gem_object_funcs = bindings::drm_gem_object_funcs { free: Some(Self::free_callback), open: Some(open_callback::<T>), @@ -206,11 +225,12 @@ impl<T: DriverObject> Object<T> { }; /// Create a new GEM object. - pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> { + pub fn new(dev: &drm::Device<T::Driver, Ctx>, size: usize) -> Result<ARef<Self>> { let obj: Pin<KBox<Self>> = KBox::pin_init( try_pin_init!(Self { obj: Opaque::new(bindings::drm_gem_object::default()), data <- T::new(dev, size), + _ctx: PhantomData, }), GFP_KERNEL, )?; @@ -219,6 +239,8 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> { unsafe { (*obj.as_raw()).funcs = &Self::OBJECT_FUNCS }; // SAFETY: The arguments are all valid per the type invariants. + // INVARIANT: We use `dev` for creating the GEM object, which is known to be in state `Ctx` - + // ensuring that the GEM object's pointer to the DRM device is always in the same state. to_result(unsafe { bindings::drm_gem_object_init(dev.as_raw(), obj.obj.get(), size) })?; // SAFETY: We will never move out of `Self` as `ARef<Self>` is always treated as pinned. @@ -232,13 +254,15 @@ pub fn new(dev: &drm::Device<T::Driver>, size: usize) -> Result<ARef<Self>> { } /// Returns the `Device` that owns this GEM object. - pub fn dev(&self) -> &drm::Device<T::Driver> { + pub fn dev(&self) -> &drm::Device<T::Driver, Ctx> { // SAFETY: // - `struct drm_gem_object.dev` is initialized and valid for as long as the GEM // object lives. // - The device we used for creating the gem object is passed as &drm::Device<T::Driver> to // Object::<T>::new(), so we know that `T::Driver` is the right generic parameter to use // here. + // - Any type invariants of `Ctx` are upheld by using the same `Ctx` for the `Device` we + // return. unsafe { drm::Device::from_raw((*self.as_raw()).dev) } } @@ -264,7 +288,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) { } // SAFETY: Instances of `Object<T>` are always reference-counted. -unsafe impl<T: DriverObject> crate::sync::aref::AlwaysRefCounted for Object<T> { +unsafe impl<T: DriverObject, Ctx: DeviceContext> AlwaysRefCounted for Object<T, Ctx> { 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()) }; @@ -279,9 +303,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) { } } -impl<T: DriverObject> super::private::Sealed for Object<T> {} +impl<T: DriverObject, Ctx: DeviceContext> super::private::Sealed for Object<T, Ctx> {} -impl<T: DriverObject> Deref for Object<T> { +impl<T: DriverObject, Ctx: DeviceContext> Deref for Object<T, Ctx> { type Target = T; fn deref(&self) -> &Self::Target { @@ -289,7 +313,7 @@ fn deref(&self) -> &Self::Target { } } -impl<T: DriverObject> AllocImpl for Object<T> { +impl<T: DriverObject, Ctx: DeviceContext> AllocImpl for Object<T, Ctx> { type Driver = T::Driver; const ALLOC_OPS: AllocOps = AllocOps { -- 2.52.0
