[Question] Alignment requirement for readX() and writeX()

[Question] Alignment requirement for readX() and writeX()

[Boqun Feng]

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.

Thanks a lot!

Regards,
Boqun

Re: [Question] Alignment requirement for readX() and writeX()

[Arnd Bergmann]

On Fri, Jul 30, 2021 at 6:43 PM Boqun Feng <boqun.feng@gmail.com> 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?

       Arnd

Re: [Question] Alignment requirement for readX() and writeX()

[Boqun Feng]

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@gmail.com> 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

Re: [Question] Alignment requirement for readX() and writeX()

[Arnd Bergmann]

On Fri, Jul 30, 2021 at 7:31 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> On Fri, Jul 30, 2021 at 06:58:30PM +0200, Arnd Bergmann wrote:
>
> 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 ;-)

Right, I really don't think that adds any value, this just replaces one
oops message with a more different oops message, while adding
some overhead.

        Arnd

Re: [Question] Alignment requirement for readX() and writeX()

[Boqun Feng]

On Fri, Jul 30, 2021 at 10:24:53PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 30, 2021 at 7:31 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > On Fri, Jul 30, 2021 at 06:58:30PM +0200, Arnd Bergmann wrote:
> >
> > 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 ;-)
> 
> Right, I really don't think that adds any value, this just replaces one
> oops message with a more different oops message, while adding
> some overhead.
> 

Agreed. I wasn't planning to propose this kind of checks for C code.
Just want to understand better on the alignment requirement of these
APIs. Thanks  ;-)

Regards,
Boqun

>         Arnd

RE: [Question] Alignment requirement for readX() and writeX()

[David Laight]

From: Arnd Bergmann
> Sent: 30 July 2021 17:59
...
> I am not aware of any driver that requires unaligned access on __iomem
> pointers, and since it definitely doesn't work on most architectures, ...

Unaligned accesses into PCIe space can generate a TLP that requests
only some bytes of the first and last 32bit words be transferred.
The target is expected to honour such requests.

On the x86 systems where I've looked at a TLP trace misaligned
accesses are even atomic provided the target doesn't let a local
request interleave.

OTOH drivers are unlikely to make such requests.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)