Re: [PATCH net 1/2] net: dsa: mxl862xx: avoid unaligned 16-bit access in api_wrap

[David Laight <david.laight.linux@gmail.com>]

On Wed, 24 Jun 2026 17:52:39 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 19 Jun 2026 10:01:54 +0100 David Laight wrote:
> > > The MXL862XX_API_* macros pass the address of a stack-allocated, __packed
> > > firmware-ABI struct to mxl862xx_api_wrap() as a void *. The struct has an
> > > alignment of 1, so the compiler is free to place it at an odd address.
> > > 
> > > mxl862xx_api_wrap() reinterprets that buffer as a __le16 * and accesses it
> > > with data[i], for which the compiler assumes the natural 2-byte alignment
> > > of __le16 and emits aligned 16-bit loads/stores (e.g. lhu/sh on MIPS).
> > > When the buffer lands on an odd address these fault on architectures that
> > > do not support unaligned access, such as MIPS32.    
> > 
> > Isn't the correct fix to not pack the structure?
> > (or probably any of the associated structures??)  
> 
> Agreed, this is very silly:
> 
> struct mxl862xx_register_mod {
> 	__le16 addr;
> 	__le16 data;
> 	__le16 mask;
> } __packed;
> 
> But some structs won't get aligned:
> 
> struct mxl862xx_mac_table_clear {
> 	u8 type;
> 	u8 port_id;
> } __packed;

Does that one need an aligned(2) ?

> So I guess the "just don't pack" will have some corner cases, too.

The main problem is the original 32bit arm abi which 32bit aligns
all structures.
But that is pretty much dead and would want a packed_if_arm_oabi
define.
Unlikely to be relevant for this code.

	David