linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Joel Granados <j.granados@samsung.com>
To: <kbusch@kernel.org>, <sagi@grimberg.me>, <hch@lst.de>
Cc: <linux-nvme@lists.infradead.org>, <gost.dev@samsung.com>,
	<joshi.k@samsung.com>, <javier.gonz@samsung.com>,
	<p.raghav@samsung.com>, Joel Granados <j.granados@samsung.com>
Subject: [RFC 0/3] nvme : Add ioctl to query nvme device attributes
Date: Thu, 27 Oct 2022 17:57:21 +0200	[thread overview]
Message-ID: <20221027155724.1161670-1-j.granados@samsung.com> (raw)
In-Reply-To: CGME20221027160102eucas1p1bb39a9a2b8ed6b54f854849532359332@eucas1p1.samsung.com

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



       reply	other threads:[~2022-10-27 16:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20221027160102eucas1p1bb39a9a2b8ed6b54f854849532359332@eucas1p1.samsung.com>
2022-10-27 15:57 ` Joel Granados [this message]
     [not found]   ` <CGME20221027160108eucas1p223383fc27ba0a27a38f7b768c562a1ed@eucas1p2.samsung.com>
2022-10-27 15:57     ` [RFC 1/3] nvme: Return -ENOMEM when kzalloc fails Joel Granados
2022-10-27 16:26       ` Keith Busch
2022-10-28  9:07         ` Joel Granados
2022-10-30  8:02         ` Christoph Hellwig
2022-10-31 12:36           ` Joel Granados
     [not found]   ` <CGME20221027160108eucas1p1cc1351fd8bf35ef6e5220e0cdc865ff4@eucas1p1.samsung.com>
2022-10-27 15:57     ` [RFC 2/3] nvme: Move nvme identify CS to separate function Joel Granados
     [not found]   ` <CGME20221027160108eucas1p2722b30a1be27a855e2b0f2495fed15ab@eucas1p2.samsung.com>
2022-10-27 15:57     ` [RFC 3/3] nvme : Add ioctl to query nvme attributes Joel Granados
2022-10-27 16:55       ` Keith Busch
2022-10-28 10:38         ` Joel Granados
2022-10-28 14:52           ` Keith Busch
2022-10-28 15:52             ` Joel Granados
2022-10-28 16:22               ` Keith Busch
2022-10-31 12:24                 ` Joel Granados

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221027155724.1161670-1-j.granados@samsung.com \
    --to=j.granados@samsung.com \
    --cc=gost.dev@samsung.com \
    --cc=hch@lst.de \
    --cc=javier.gonz@samsung.com \
    --cc=joshi.k@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=p.raghav@samsung.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).