2022-09-23 05:23:15

by Michael Kelley (LINUX)

Subject: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for nvme devices

The IOC_PR_CLEAR and IOC_PR_RELEASE ioctls are
non-functional on NVMe devices because the nvme_pr_clear()
and nvme_pr_release() functions set the IEKEY field incorrectly.
The IEKEY field should be set only when the key is zero (i.e,
not specified). The current code does it backwards.

Furthermore, the NVMe spec describes the persistent
reservation "clear" function as an option on the reservation
release command. The current implementation of nvme_pr_clear()
erroneously uses the reservation register command.

Fix these errors. Note that NVMe version 1.3 and later specify
that setting the IEKEY field will return an error of Invalid
Field in Command. The fix will set IEKEY when the key is zero,
which is appropriate as these ioctls consider a zero key to
be "unspecified", and the intention of the spec change is
to require a valid key.

Tested on a version 1.4 PCI NVMe device in an Azure VM.

Fixes: 1673f1f08c88 ("nvme: move block_device_operations and ns/ctrl freeing to common code")
Fixes: 1d277a637a71 ("NVMe: Add persistent reservation ops")
Signed-off-by: Michael Kelley <[email protected]>
---
drivers/nvme/host/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 0038283..45ef8e8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2162,14 +2162,14 @@ static int nvme_pr_preempt(struct block_device *bdev, u64 old, u64 new,

static int nvme_pr_clear(struct block_device *bdev, u64 key)
{
- u32 cdw10 = 1 | (key ? 1 << 3 : 0);
+ u32 cdw10 = 1 | (key ? 0 : 1 << 3);

- return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_register);
+ return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_release);
}

static int nvme_pr_release(struct block_device *bdev, u64 key, enum pr_type type)
{
- u32 cdw10 = nvme_pr_type(type) << 8 | (key ? 1 << 3 : 0);
+ u32 cdw10 = nvme_pr_type(type) << 8 | (key ? 0 : 1 << 3);

return nvme_pr_command(bdev, cdw10, key, 0, nvme_cmd_resv_release);
}
--
1.8.3.1

2022-09-27 07:49:08

by Christoph Hellwig

Subject: Re: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for nvme devices

Thanks,

applied to 6.0.

2022-09-30 16:10:47

by Keith Busch

Subject: Re: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for nvme devices

On Fri, Sep 30, 2022 at 03:47:48PM +0000, Michael Kelley (LINUX) wrote:
> From: Christoph Hellwig <[email protected]> Sent: Tuesday, September 27, 2022 12:21 AM
> > To: Michael Kelley (LINUX) <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected]; [email protected]; linux-
> > [email protected]; [email protected]
> > Subject: Re: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for
> > nvme devices
> >
> > Thanks,
> >
> > applied to 6.0.
>
> Christoph -- where did this get applied? I'm not seeing it in either the linux
> or linux-next trees.

The path for nvme patches to upstream goes through the nvme tree[1] -> block
tree [2], then linux mainline. The pull request from block to mainline
containing your patch was sent earlier today[3], and we typically see those
merged within a few hours.

[1] http://git.infradead.org/nvme.git
[2] https://git.kernel.dk/cgit/linux-block/
[3] https://lore.kernel.org/linux-block/[email protected]/T/#u

2022-09-30 16:26:10

by Michael Kelley (LINUX)

Subject: RE: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for nvme devices

From: Christoph Hellwig <[email protected]> Sent: Tuesday, September 27, 2022 12:21 AM
> To: Michael Kelley (LINUX) <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 1/1] nvme: Fix IOC_PR_CLEAR and IOC_PR_RELEASE ioctls for
> nvme devices
>
> Thanks,
>
> applied to 6.0.

Christoph -- where did this get applied? I'm not seeing it in either the linux
or linux-next trees.

Michael