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
next parent 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).