Re: [PATCH] libnvme: do not byte swap command dwords

[Keith Busch <kbusch@kernel.org>]

On Thu, Apr 03, 2025 at 07:25:13PM +0200, Daniel Wagner wrote:
> On Thu, Apr 03, 2025 at 07:43:15AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > The dwords always need to be set in the cpu native format. The driver
> > will handle setting up the SQE in the nvme little-endian order, and big
> > endian is never used, so this byte swapping was wrong.
> > 
> > Link: https://github.com/linux-nvme/nvme-cli/issues/2761
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> >  src/nvme/ioctl.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/nvme/ioctl.c b/src/nvme/ioctl.c
> > index 18b228d9..86c2b702 100644
> > --- a/src/nvme/ioctl.c
> > +++ b/src/nvme/ioctl.c
> > @@ -1920,9 +1920,9 @@ static int nvme_set_var_size_tags(__u32 *cmd_dw2, __u32 *cmd_dw3, __u32 *cmd_dw1
> >  		return -1;
> >  	}
> >  
> > -	*cmd_dw2 = cpu_to_be32(cdw2);
> > -	*cmd_dw3 = cpu_to_be32(cdw3);
> > -	*cmd_dw14 = cpu_to_be32(cdw14);
> > +	*cmd_dw2 = cdw2;
> > +	*cmd_dw3 = cdw3;
> > +	*cmd_dw14 = cdw14;
> 
> in this case shouldn't we use cpu_to_le32?

No, user space was only uses cpu native endian when constructing the
command. The driver handles the endian conversion if necessary.

That conversion is just for the command control side. The driver doesn't
do any such manipulation on the data plane, so endianness in payloads
for commands like "copy", "dsm", or PI meatada need to be handled by the
application.