Skip to content

libnvme/ioctl: limit to use NVME_IOCTL_IO64_CMD only for block dev#3121

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:ioctl-io64-cmd
Mar 9, 2026
Merged

libnvme/ioctl: limit to use NVME_IOCTL_IO64_CMD only for block dev#3121
igaw merged 1 commit intolinux-nvme:masterfrom
ikegami-t:ioctl-io64-cmd

Conversation

@ikegami-t
Copy link
Copy Markdown
Contributor

Since currently NVME_IOCTL_IO64_CMD not supported for the char dev. Currently the char dev nvmeX not able be used for the io-passthru command. Also previously NVME_IOCTL_IO_CMD used for the io-passthru command. Therefore for the char dev nvmeX use NVME_IOCTL_IO_CMD instead.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 2, 2026

nvme_ns_chr_ioctl forwards the IOCTL to nvme_ns_ioctl or nvme_ctrl_ioctl and bot function support both the 64bit interface. Do I miss something?

@ikegami-t
Copy link
Copy Markdown
Contributor Author

For the nvmeX char dev nvme_dev_ioctl called by the ioctl but not nvme_ns_chr_ioctl mentioned I think. (Also seems for the nvmeXnY block dev nvme_ns_chr_ioctl called and for the ngXnY char dev nvme_ns_head_chr_ioctl. I do not know why the nvme_ns_chr_ioctl name includes chr but seems for the block dev.)

@ikegami-t
Copy link
Copy Markdown
Contributor Author

Rebased and changed the patch to introduce ioctl_io64 flag instead.

Since currently only NVME_IOCTL_ADMIN64_CMD checked for ioctrl64.
But NVME_IOCTL_IO64_CMD not checked as same.
Also currently NVME_IOCTL_IO64_CMD not supported for the char dev.
Therefore introduce the ioctl_io64 flag to use NVME_IOCTL_IO64_CMD.

Signed-off-by: Tokunori Ikegami <[email protected]>
[wagi: renamed ioctl64 to ioctl_admin64]
Signed-off-by: Daniel Wagner <[email protected]>
@igaw igaw force-pushed the ioctl-io64-cmd branch from 387688b to b944e3b Compare March 9, 2026 08:56
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 9, 2026

Just documenting what I've done to figure out what's the problem.

The commit 65e68edce0db ("nvme: allow 64-bit results in passthru commands") introduced the 64bit version of admin and the io ioctl.

  • /dev/ng0n1:
# strace -o /tmp/trace nvme read -s 4096 -z 0x1000 /dev/ng0n1
# root@localhost:~# grep IOCTL /tmp/trace
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff87090700) = 16641
ioctl(3, NVME_IOCTL_ID, 0)              = 1
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff87090880) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff87090880) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff87090740) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff87090740) = 0
ioctl(3, NVME_IOCTL_IO64_CMD, 0x7fff87090880) = 0
  • /dev/nvme0 without namespace argument:
    Against /dev/nvme0 it fails because submit_io tries to figure out the namespace:
# root@localhost:~# strace -o /tmp/trace nvme read -s 4096 -z 0x1000 /dev/nvme0
get-namespace-id: Inappropriate ioctl for device

I think this behaves correctly, but the error message could be better.

  • /dev/nvme0 with namespace argument
# root@localhost:~# strace -o /tmp/trace nvme read -s 4096 -n 1  -z 0x1000 /dev/nvme0
submit-io: Inappropriate ioctl for device

#root@localhost:~# grep ioctl /tmp/trace
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff54712120) = 16641
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff547122a0) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff547122a0) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff54712160) = 0
ioctl(3, NVME_IOCTL_ADMIN64_CMD, 0x7fff54712160) = 0
ioctl(3, NVME_IOCTL_IO64_CMD, 0x7fff547122a0) = -1 ENOTTY (Inappropriate ioctl for device)
write(2, "submit-io: Inappropriate ioctl f"..., 41) = 41

Looking at the current implementation in Linux nvme_dev_ioctl supports

long nvme_dev_ioctl(struct file *file, unsigned int cmd,
		unsigned long arg)
{
	[...]
	switch (cmd) {
	case NVME_IOCTL_ADMIN_CMD:
		return nvme_user_cmd(ctrl, NULL, argp, 0, open_for_write);
	case NVME_IOCTL_ADMIN64_CMD:
		return nvme_user_cmd64(ctrl, NULL, argp, 0, open_for_write);
	case NVME_IOCTL_IO_CMD:
		return nvme_dev_user_cmd(ctrl, argp, open_for_write);
	[...]	
}

That means we can't rely on checking NVME_IOCTL_ADMIN64_CMD that NVME_IOCTL_IO64_CMD is supported.

@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 9, 2026

The kernel support seems to be an oversight. I don't see an obvious reason why it should not work. Thus we should fix this in the kernel as well.

@igaw igaw merged commit a800b6a into linux-nvme:master Mar 9, 2026
21 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Mar 9, 2026

Thanks!

@ikegami-t
Copy link
Copy Markdown
Contributor Author

I tried to fix the kernel also by the patch below but seems unfortunately difficult to be applied it. Thank you.
https://lore.kernel.org/linux-nvme/[email protected]/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants