linux-kernel - Re: [PATCH net-next v10 1/8] hinic3: Async Event Queue interfaces

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250724134551.30168-1-gongfan1@huawei.com>
Date: Thu, 24 Jul 2025 21:45:51 +0800
From: Fan Gong <gongfan1@...wei.com>
To: <horms@...nel.org>
CC: <andrew+netdev@...n.ch>, <christophe.jaillet@...adoo.fr>,
	<corbet@....net>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<fuguiming@...artners.com>, <gongfan1@...wei.com>, <guoxin09@...wei.com>,
	<gur.stavi@...wei.com>, <helgaas@...nel.org>, <jdamato@...tly.com>,
	<kuba@...nel.org>, <lee@...ger.us>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <luosifu@...wei.com>,
	<meny.yossefi@...wei.com>, <mpe@...erman.id.au>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>, <przemyslaw.kitszel@...el.com>,
	<shenchenyang1@...ilicon.com>, <shijing34@...wei.com>, <sumang@...vell.com>,
	<vadim.fedorenko@...ux.dev>, <wulike1@...wei.com>, <zhoushuai28@...wei.com>,
	<zhuyikai1@...artners.com>
Subject: Re: [PATCH net-next v10 1/8] hinic3: Async Event Queue interfaces

> > +
> > +/* Data provided to/by cmdq is arranged in structs with little endian fields but
> > + * every dword (32bits) should be swapped since HW swaps it again when it
> > + * copies it from/to host memory.
> > + */
>
> This scheme may work on little endian hosts.
> But if so it seems unlikely to work on big endian hosts.
>
> I expect you want be32_to_cpu_array() for data coming from hw,
> with a source buffer as an array of __be32 while
> the destination buffer is an array of u32.
>
> And cpu_to_be32_array() for data going to the hw,
> with the types of the source and destination buffers reversed.
>
> If those types don't match your data, then we have
> a framework to have that discussion.
>
>
> That said, it is more usual for drivers to keep structures in the byte
> order they are received. Stored in structures with members with types, in
> this case it seems that would be __be32, and accessed using a combination
> of BIT/GENMASK, FIELD_PREP/FIELD_GET, and cpu_to_be*/be*_to_cpu (in this
> case cpu_to_be32/be32_to_cpu).
> 
> An advantage of this approach is that the byte order of
> data is only changed when needed. Another is that it is clear
> what the byte order of data is.

There is a simplified example:

Here is a 64 bit little endian that may appear in cmdq:
__le64 x
After the swap it will become:
__be32 x_lo
__be32 x_hi
This is NOT __be64.
__be64 looks like this:
__be32 x_hi
__be32 x_lo

So the swapped data by HW is neither BE or LE. In this case, we should use
swab32 to obtain the correct LE data because our driver currently supports LE.
This is for compensating for bad HW decisions.

> > +void hinic3_cmdq_buf_swab32(void *data, int len)
> > +{
> > +	u32 *mem = data;
> > +	u32 i;
> > +
> > +	for (i = 0; i < len / sizeof(u32); i++)
> > +		mem[i] = swab32(mem[i]);
> > +}
>
> This seems to open code swab32_array().

We will use swab32_array in next patch.
Besides, we will use LE for cmdq structs to avoid confusion and enhance
readability.