On Sun Mar 9, 2025 at 5:00 PM CET, Tamir Duberstein wrote:
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 598001157293..20159b7c9293 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -45,7 +45,7 @@ struct DevresInner<T> { /// # Example /// /// ```no_run -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}}; +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}}; /// # use core::ops::Deref; /// /// // See also [`pci::Bar`] for a real example. @@ -59,19 +59,19 @@ struct DevresInner<T> { /// unsafe fn new(paddr: usize) -> Result<Self>{ /// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is /// // valid for `ioremap`. -/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) }; +/// let addr = unsafe { bindings::ioremap(paddr as u64, SIZE) };
The argument of `ioremap` is defined as `resource_size_t` which ultimately maps to `u64` on 64 bit systems and `u32` on 32 bit ones. I don't think that we should have code like this... Is there another option?
Maybe Gary knows something here, do we have a type that represents that better?
/// if addr.is_null() { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?)) +/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
This should be `addr.addr()` (requires `strict_provenance` on Rust 1.83 & before).
(I am assuming that we're never casting the usize back to a pointer, since otherwise this change would introduce UB)
/// } /// } /// /// impl<const SIZE: usize> Drop for IoMem<SIZE> { /// fn drop(&mut self) { /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`. -/// unsafe { bindings::iounmap(self.0.addr() as _); }; +/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
Can't this be a `.cast::<c_void>()`?
/// } /// } ///
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 8654d52b0bb9..eb8fa52f08ba 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t { /// Returns the error encoded as a pointer. pub fn to_ptr<T>(self) -> *mut T { // SAFETY: `self.0` is a valid error due to its invariant.
unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
Can't this be a `.into()`?
}/// Returns a string representing the error, if one exists.
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
unsafe { bindings::$name(addr as _) }
unsafe { bindings::$name(addr as *const c_void) }
Also here, is `.cast::<c_void>()` enough? (and below)
}/// Read IO data from a given offset.
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs index 04f2d8ef29cb..40d1bd13682c 100644 --- a/rust/kernel/of.rs +++ b/rust/kernel/of.rs @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data); fn index(&self) -> usize {
self.0.data as _
self.0.data as usize
This should also be `self.0.data.addr()`.
}} @@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self { // SAFETY: FFI type is valid to be zero-initialized. let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
// TODO: Use `clone_from_slice` once the corresponding types do match.
// TODO: Use `copy_from_slice` once stabilized for `const`.
This feature has just been stabilized (5 days ago!):
https://github.com/rust-lang/rust/issues/131415
@Miguel: Do we already have a target Rust version for dropping the `RUSTC_BOOTSTRAP=1`? If not, then I think we should use this feature now, since it will be stable by the time we bump the minimum version. (not in this patch [series] though)
let mut i = 0; while i < src.len() {
of.compatible[i] = src[i] as _;
of.compatible[i] = src[i]; i += 1; }
@@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) { // `ioptr` is valid by the safety requirements. // `num` is valid by the safety requirements. unsafe {
bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
Again, probably castable.
bindings::pci_release_region(pdev.as_raw(), num); } }diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 6a1a982b946d..0b80a119d5f0 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -692,9 +692,9 @@ fn new() -> Self { pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self { // INVARIANT: The safety requirements guarantee the type invariants. Self {
beg: pos as _,pos: pos as _,end: end as _,
beg: pos as usize,pos: pos as usize,end: end as usize,
I would prefer if we use `pos.expose_provenance()` (also for `end`) here.
} }@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self { /// /// N.B. It may point to invalid memory. pub(crate) fn pos(&self) -> *mut u8 {
self.pos as _
self.pos as *mut u8
This should then also be `with_exposed_provenance(self.pos)`
--- Cheers, Benno
}/// Returns the number of bytes written to the formatter. diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 8ff54105be3f..d03f3440cb5a 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -198,7 +198,7 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput unsafe { w.__enqueue(move |work_ptr| { bindings::queue_work_on(
bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32, queue_ptr, work_ptr, )