mirror of
https://github.com/torvalds/linux.git
synced 2026-03-08 06:04:44 +01:00
rust: drm: Drop the use of Opaque for ioctl arguments
With the Opaque<T>, the expectations are that Rust should not make any assumptions on the layout or invariants of the wrapped C types. That runs rather counter to ioctl arguments, which must adhere to certain data-layout constraints. By using Opaque<T>, ioctl handlers are forced to use unsafe code where none is actually needed. This adds needless complexity and maintenance overhead, brining no safety benefits. Drop the use of Opaque for ioctl arguments as that is not the best fit here. Signed-off-by: Beata Michalska <beata.michalska@arm.com> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20250626162313.2755584-1-beata.michalska@arm.com Signed-off-by: Danilo Krummrich <dakr@kernel.org>
This commit is contained in:
parent
ca2a6abdae
commit
94febfb5bc
4 changed files with 16 additions and 80 deletions
|
|
@ -2,13 +2,11 @@
|
|||
|
||||
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,
|
||||
};
|
||||
|
||||
|
|
@ -26,20 +24,19 @@ impl File {
|
|||
/// IOCTL: get_param: Query GPU / driver metadata.
|
||||
pub(crate) fn get_param(
|
||||
dev: &NovaDevice,
|
||||
getparam: &Opaque<uapi::drm_nova_getparam>,
|
||||
getparam: &mut uapi::drm_nova_getparam,
|
||||
_file: &drm::File<File>,
|
||||
) -> Result<u32> {
|
||||
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 {
|
||||
let value = match getparam.param as u32 {
|
||||
uapi::NOVA_GETPARAM_VRAM_BAR_SIZE => pdev.resource_len(1)?,
|
||||
_ => return Err(EINVAL),
|
||||
};
|
||||
|
||||
getparam.set_value(value);
|
||||
getparam.value = value;
|
||||
|
||||
Ok(0)
|
||||
}
|
||||
|
|
@ -47,13 +44,12 @@ impl File {
|
|||
/// IOCTL: gem_create: Create a new DRM GEM object.
|
||||
pub(crate) fn gem_create(
|
||||
dev: &NovaDevice,
|
||||
req: &Opaque<uapi::drm_nova_gem_create>,
|
||||
req: &mut uapi::drm_nova_gem_create,
|
||||
file: &drm::File<File>,
|
||||
) -> Result<u32> {
|
||||
let req: &GemCreate = req.into();
|
||||
let obj = NovaObject::new(dev, req.size().try_into()?)?;
|
||||
let obj = NovaObject::new(dev, req.size.try_into()?)?;
|
||||
|
||||
req.set_handle(obj.create_handle(file)?);
|
||||
req.handle = obj.create_handle(file)?;
|
||||
|
||||
Ok(0)
|
||||
}
|
||||
|
|
@ -61,13 +57,12 @@ impl File {
|
|||
/// IOCTL: gem_info: Query GEM metadata.
|
||||
pub(crate) fn gem_info(
|
||||
_dev: &NovaDevice,
|
||||
req: &Opaque<uapi::drm_nova_gem_info>,
|
||||
req: &mut uapi::drm_nova_gem_info,
|
||||
file: &drm::File<File>,
|
||||
) -> Result<u32> {
|
||||
let req: &GemInfo = req.into();
|
||||
let bo = NovaObject::lookup_handle(file, req.handle())?;
|
||||
let bo = NovaObject::lookup_handle(file, req.handle)?;
|
||||
|
||||
req.set_size(bo.size().try_into()?);
|
||||
req.size = bo.size().try_into()?;
|
||||
|
||||
Ok(0)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@
|
|||
mod driver;
|
||||
mod file;
|
||||
mod gem;
|
||||
mod uapi;
|
||||
|
||||
use crate::driver::NovaDriver;
|
||||
|
||||
|
|
|
|||
|
|
@ -1,61 +0,0 @@
|
|||
// 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 };
|
||||
}
|
||||
}
|
||||
|
|
@ -83,7 +83,7 @@ pub mod internal {
|
|||
///
|
||||
/// ```ignore
|
||||
/// fn foo(device: &kernel::drm::Device<Self>,
|
||||
/// data: &Opaque<uapi::argument_type>,
|
||||
/// data: &mut uapi::argument_type,
|
||||
/// file: &kernel::drm::File<Self::File>,
|
||||
/// ) -> Result<u32>
|
||||
/// ```
|
||||
|
|
@ -138,9 +138,12 @@ macro_rules! declare_drm_ioctls {
|
|||
// 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>)
|
||||
};
|
||||
// The `ioctl` argument is exclusively owned by the handler
|
||||
// and guaranteed by the C implementation (`drm_ioctl()`) to remain
|
||||
// valid for the entire lifetime of the reference taken here.
|
||||
// There is no concurrent access or aliasing; no other references
|
||||
// to this object exist during this call.
|
||||
let data = unsafe { &mut *(raw_data.cast::<$crate::uapi::$struct>()) };
|
||||
// SAFETY: This is just the DRM file structure
|
||||
let file = unsafe { $crate::drm::File::from_raw(raw_file) };
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue