On Fri, Jul 19, 2024 at 03:09:09PM -0300, Daniel Almeida wrote: > Hi Danilo, > > > > > > We can easily build abstractions that ensure that the address a driver is trying > > to access is mapped properly, such that you can't have accidential out-of-bound > > accesses. > > > > Those can be implemented by the corresponding subsystem / bus that the resource > > originates from. > > > > In fact, we already have abstractions for that on the way, a generic I/O > > abstraction [1] as base implementation and a specific abstraction for PCI bars > > [2]. > > > > Of course, if the MMIO region comes from let's say the device tree, we still > > have to assume that the information in the DT is correct, but the driver does > > not need unsafe code for this. > > > > [1] https://lore.kernel.org/rust-for-linux/20240618234025.15036-8-dakr@xxxxxxxxxx/ > > [2] https://lore.kernel.org/rust-for-linux/20240618234025.15036-11-dakr@xxxxxxxxxx/ > > > > Thanks for pointing that out. So from this: > > +impl<const SIZE: usize> Io<SIZE> { > + /// > + /// > + /// # Safety > + /// > + /// Callers must ensure that `addr` is the start of a valid I/O mapped memory region of size > + /// `maxsize`. > + pub unsafe fn new(addr: usize, maxsize: usize) -> Result<Self> { > + if maxsize < SIZE { > + return Err(EINVAL); > + } > + > + Ok(Self { addr, maxsize }) > + } > > It looks like one can do this: > > let io = unsafe { Io::new(<some address>, <some size>)? }; > let value = io.readb(<some offset>)?; > > Where <some address> has already been mapped for <some size> at an earlier point? Yes, but (at least for full Rust drivers) this shouldn't be called by the driver directly, but the corresponding subsystem / bus should provide a `Devres` wrapped `Io` instance, like the PCI abstraction in [2] does. Example: ``` // Get a `Devres` managed PCI bar mapping let bar: Devres<pci::Bar> = pdev.iomap_region(0); let reg = bar.try_readl(0x42)?; ``` You can also let the driver assert that the requested resource must have a minimum size: ``` // Only succeeds if the PCI bar has at least a size of 0x1000 let bar = pdev.iomap_region_size::<0x1000>(0); // Note: `readl` does not need to return a `Result`, since the boundary checks // can be done on compile time due to the driver specified minimal mapping size. let reg = bar.readl(0x42); ``` > > That’s fine, as I said, if an abstraction makes sense, I have nothing > against it. My point is more that we shouldn’t enact a blanket ban on > 'unsafe' in drivers because corner cases do exist. But it’s good to know that this > particular example I gave does not apply. > > > >> > >> If a driver is written partially in Rust, and partially in C, and it gets a > >> pointer to some kcalloc’d memory in C, should It be forbidden to use unsafe > >> in order to build a slice from that pointer? How can you possibly design a > >> general abstraction for something that is, essentially, a driver-internal API? > > > > That sounds perfectly valid to me. > > > > > — Daniel > >
