[RFC 0/3] nvme : Add ioctl to query nvme device attributes

Joel Granados j.granados at samsung.com
Thu Oct 27 08:57:21 PDT 2022


What?
In this patch I add an ioctl that provides nvme controller attributes to
the user that can open the char device file. We also provide them for the
block devices for convenience.

Why?
Applications with write permissions should not need to be privileged to
write to the device. With Kanchan's latest patch
(https://lore.kernel.org/linux-nvme/20221020070205.57366-1-joshi.k@samsung.com/)
the nvme IO and identify commands in passthru now follow device
permissions; however there are still some controller attributes like
minimal data transfer size (MDTS) which need a privileged user to be
queried.

How?
Added an ioctl (NVME_IOCTL_GET_ATTR) that fetches attributes from the nvme
driver. It sends both nvme_identify_ctrl and nvme_identify_cs_ctrl commands
to query the attributes that are not contained in nvme_ctrl already.

I have rebased this on top of Kanchans "nvme: identify-namespace without
CAP_SYS_ADMIN" (https://lore.kernel.org/linux-nvme/20221020070205.57366-3-joshi.k@samsung.com)
because this only makes sense if we can also do IO as unprivileged.

I have made the call extensible by embedding the struct size as the first
member and using it as a version. As more members get added, the switch
statement gets populated with more checks. For this I followed in the steps
of the openat2 system call [1] and the extensible ioctl for vfio [2].
Another interesting reference for this is here https://lwn.net/Articles/830666/

The first patch in the series is a fix to a potential bug in a return
value. This was in a section of the code that was getting changed so I
decided to include it in this series.

Questions:
1. I started by providing members that have relevant attributes like mdts,
   and mpsmin for regular writes as well as wzsl, wusl for more specialized
   write commands like write zeroes and also dmsl, dmrsl, dmrl for dataset
   management commands.  Have I missed an attribute that should definitely
   be there? Or have a included one which should be removed?

2. Naming is always important. We called it "GET_ATTR" because it "gets"
   nvme specific attributes. This is fairly general for whatever we want to
   add in the future, but do we need to be more specific here? Alternatives
   are  "NVME_IOCTL_DEV_ATTR", "NVME_IOCTL_DRV_ATTR" or
   "NVME_IOCTL_CTRL_ATTR". Thoughts?

3. I have named the communication structure "nvme_get_attr" to be
   consistent with the ioctl name. Are there alternatives?

4. I have made the ioctl call extensible because I believe that this ioctl
   might grow in the future as ppl see that there might be more data needed
   to do an nvme write.

[1] https://github.com/torvalds/linux/commit/fddb5d430ad9
[2] https://github.com/torvalds/linux/blob/master/include/uapi/linux/vfio.h#L56

Joel Granados (3):
  nvme: Return -ENOMEM when kzalloc fails
  nvme: Move nvme identify CS to separate function
  nvme : Add ioctl to query nvme attributes

 drivers/nvme/host/core.c        | 35 ++++++++++------
 drivers/nvme/host/ioctl.c       | 58 ++++++++++++++++++++++++++
 drivers/nvme/host/nvme.h        | 10 +++++
 include/uapi/linux/nvme_ioctl.h | 74 +++++++++++++++++++++++++++++++++
 4 files changed, 164 insertions(+), 13 deletions(-)

-- 
2.30.2




More information about the Linux-nvme mailing list