diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs index 84b0e1703150..8fe484d357f6 100644 --- a/drivers/gpu/nova-core/driver.rs +++ b/drivers/gpu/nova-core/driver.rs @@ -32,8 +32,7 @@ pub(crate) struct NovaCore { #[pin] pub(crate) gpu: Gpu, - #[pin] - _reg: Devres, + _reg: Devres>, } const BAR0_SIZE: usize = SZ_16M; @@ -96,14 +95,15 @@ fn probe(pdev: &pci::Device, _info: &Self::IdInfo) -> impl PinInit &device::Device { // SAFETY: A bound auxiliary device always has a bound parent device. unsafe { parent.as_bound() } } + + /// Returns a pinned reference to the registration data set by the registering (parent) driver. + /// + /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling + /// [`Registration::new()`]. + /// + /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was + /// registered by a C driver. + pub fn registration_data(&self) -> Result> { + // SAFETY: By the type invariant, `self.as_raw()` is a valid `struct auxiliary_device`. + let ptr = unsafe { (*self.as_raw()).registration_data_rust }; + if ptr.is_null() { + dev_warn!( + self.as_ref(), + "No registration data set; parent is not a Rust driver.\n" + ); + return Err(ENOENT); + } + + // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`; + // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId` + // at the start of the allocation is valid regardless of `T`. + let type_id = unsafe { ptr.cast::().read() }; + if type_id != TypeId::of::() { + return Err(EINVAL); + } + + // SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains + // valid until `Registration::drop()` calls `from_foreign()`. + let wrapper = unsafe { Pin::>>::borrow(ptr) }; + + // SAFETY: `data` is a structurally pinned field of `RegistrationData`. + Ok(unsafe { wrapper.map_unchecked(|w| &w.data) }) + } } impl Device { @@ -326,87 +365,132 @@ unsafe impl Send for Device {} // (i.e. `Device) are thread safe. unsafe impl Sync for Device {} +/// Wrapper that stores a [`TypeId`] alongside the registration data for runtime type checking. +#[repr(C)] +#[pin_data] +struct RegistrationData { + type_id: TypeId, + #[pin] + data: T, +} + /// The registration of an auxiliary device. /// /// This type represents the registration of a [`struct auxiliary_device`]. When its parent device /// is unbound, the corresponding auxiliary device will be unregistered from the system. /// +/// The type parameter `T` is the type of the registration data owned by the registering (parent) +/// driver. It can be accessed by the auxiliary driver through +/// [`Device::registration_data()`]. +/// /// # Invariants /// -/// `self.0` always holds a valid pointer to an initialized and registered -/// [`struct auxiliary_device`]. -pub struct Registration(NonNull); +/// `self.adev` always holds a valid pointer to an initialized and registered +/// [`struct auxiliary_device`] whose `registration_data_rust` field points to a +/// valid `Pin>>`. +pub struct Registration { + adev: NonNull, + _data: PhantomData, +} -impl Registration { - /// Create and register a new auxiliary device. - pub fn new<'a>( - parent: &'a device::Device, - name: &'a CStr, +impl Registration { + /// Create and register a new auxiliary device with the given registration data. + /// + /// The `data` is owned by the registration and can be accessed through the auxiliary device + /// via [`Device::registration_data()`]. + pub fn new( + parent: &device::Device, + name: &CStr, id: u32, - modname: &'a CStr, - ) -> impl PinInit, Error> + 'a { - pin_init::pin_init_scope(move || { - let boxed = KBox::new(Opaque::::zeroed(), GFP_KERNEL)?; - let adev = boxed.get(); + modname: &CStr, + data: impl PinInit, + ) -> Result> + where + Error: From, + { + let data = KBox::pin_init::( + try_pin_init!(RegistrationData { + type_id: TypeId::of::(), + data <- data, + }), + GFP_KERNEL, + )?; - // 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; - } + let boxed: KBox> = KBox::zeroed(GFP_KERNEL)?; + let adev = boxed.get(); - // 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) }; + // 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; + (*adev).registration_data_rust = data.into_foreign(); + } - // 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 not been initialized yet. + unsafe { bindings::auxiliary_device_init(adev) }; - // SAFETY: - // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which - // has been initialized, - // - `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 initialized. - unsafe { bindings::auxiliary_device_uninit(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); - return Err(Error::from_errno(ret)); - } + // SAFETY: + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which + // has been initialized, + // - `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: `registration_data` was set above via `into_foreign()`. + drop(unsafe { + Pin::>>::from_foreign((*adev).registration_data_rust) + }); - // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is - // called, which happens in `Self::drop()`. - Ok(Devres::new( - parent, - // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated - // successfully. - Self(unsafe { NonNull::new_unchecked(adev) }), - )) - }) + // SAFETY: `adev` is guaranteed to be a valid pointer to a + // `struct auxiliary_device`, which has been initialized. + unsafe { bindings::auxiliary_device_uninit(adev) }; + + return Err(Error::from_errno(ret)); + } + + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is + // called, which happens in `Self::drop()`. + let reg = Self { + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated + // successfully. + adev: unsafe { NonNull::new_unchecked(adev) }, + _data: PhantomData, + }; + + Devres::new::(parent, reg) } } -impl Drop for Registration { +impl Drop for Registration { fn drop(&mut self) { - // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // SAFETY: By the type invariant of `Self`, `self.adev.as_ptr()` is a valid registered // `struct auxiliary_device`. - unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) }; + unsafe { bindings::auxiliary_device_delete(self.adev.as_ptr()) }; + + // SAFETY: `registration_data` was set in `new()` via `into_foreign()`. + drop(unsafe { + Pin::>>::from_foreign( + (*self.adev.as_ptr()).registration_data_rust, + ) + }); // 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 + // SAFETY: By the type invariant of `Self`, `self.adev.as_ptr()` is a valid registered // `struct auxiliary_device`. - unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) }; + unsafe { bindings::auxiliary_device_uninit(self.adev.as_ptr()) }; } } // SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread. -unsafe impl Send for Registration {} +unsafe impl Send for Registration {} // SAFETY: `Registration` does not expose any methods or fields that need synchronization. -unsafe impl Sync for Registration {} +unsafe impl Sync for Registration {} diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs index 5c5a5105a3ff..319ef734c02b 100644 --- a/samples/rust/rust_driver_auxiliary.rs +++ b/samples/rust/rust_driver_auxiliary.rs @@ -17,8 +17,6 @@ InPlaceModule, // }; -use core::any::TypeId; - const MODULE_NAME: &CStr = ::NAME; const AUXILIARY_NAME: &CStr = c"auxiliary"; @@ -49,13 +47,13 @@ fn probe(adev: &auxiliary::Device, _info: &Self::IdInfo) -> impl PinInit, - #[pin] - _reg1: Devres, + _reg0: Devres>, + _reg1: Devres>, } kernel::pci_device_table!( @@ -71,10 +69,21 @@ impl pci::Driver for ParentDriver { const ID_TABLE: pci::IdTable = &PCI_TABLE; fn probe(pdev: &pci::Device, _info: &Self::IdInfo) -> impl PinInit { - try_pin_init!(Self { - private: TypeId::of::(), - _reg0 <- auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 0, MODULE_NAME), - _reg1 <- auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 1, MODULE_NAME), + Ok(Self { + _reg0: auxiliary::Registration::new( + pdev.as_ref(), + AUXILIARY_NAME, + 0, + MODULE_NAME, + Data { index: 0 }, + )?, + _reg1: auxiliary::Registration::new( + pdev.as_ref(), + AUXILIARY_NAME, + 1, + MODULE_NAME, + Data { index: 1 }, + )?, }) } } @@ -83,7 +92,8 @@ impl ParentDriver { fn connect(adev: &auxiliary::Device) -> Result { let dev = adev.parent(); let pdev: &pci::Device = dev.try_into()?; - let drvdata = dev.drvdata::()?; + + let data = adev.registration_data::()?; dev_info!( dev, @@ -95,8 +105,8 @@ fn connect(adev: &auxiliary::Device) -> Result { dev_info!( dev, - "We have access to the private data of {:?}.\n", - drvdata.private + "Connected to auxiliary device with index {}.\n", + data.index ); Ok(())