On Fri, Jul 30, 2021 at 06:58:30PM +0200, Arnd Bergmann wrote: > On Fri, Jul 30, 2021 at 6:43 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote: > > > > Hi, > > > > The background is that I'm reviewing Wedson's PR on IoMem for > > Rust-for-Linux project: > > > > https://github.com/Rust-for-Linux/linux/pull/462 > > > > readX() and writeX() are used to provide Rust code to read/write IO > > memory. And I want to find whether we need to check the alignment of the > > pointer. I wonder whether the addresses passed to readX() and writeX() > > need to be aligned to the size of the accesses (e.g. the parameter of > > readl() has to be a 4-byte aligned pointer). > > > > The only related information I get so far is the following quote in > > Documentation/driver-io/device-io.rst: > > > > On many platforms, I/O accesses must be aligned with respect to > > the access size; failure to do so will result in an exception or > > unpredictable results. > > > > Does it mean all readX() and writeX() need to use aligned addresses? > > Or the alignment requirement is arch-dependent, i.e. if the architecture > > supports and has enabled misalignment load and store, no alignment > > requirement on readX() and writeX(), otherwise still need to use aligned > > addresses. > > > > I know different archs have their own alignment requirement on memory > > accesses, just want to make sure the requirement of the readX() and > > writeX() APIs. > > I am not aware of any driver that requires unaligned access on __iomem > pointers, and since it definitely doesn't work on most architectures, I think > having an unconditional alignment check makes sense. > > What would the alignment check look like? Is there a way to annotate > a pointer that is 'void __iomem *' in C as having a minimum alignment > when it gets passed into a function that uses readl()/writel() on it? > If we want to check, I'd expect we do the checks inside readX()/writeX(), for example, readl() could be implemented as: #define readl(c) \ ({ \ u32 __v; \ \ /* alignment checking */ \ BUG_ON(c & (sizeof(__v) - 1)); \ __v = readl_relaxed(c); \ __iormb(__v); \ __v; \ }) It's a runtime check, so if anyone hates it I can understand ;-) Regards, Boqun > Arnd
