Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls
@ 2019-11-05  8:10 Marta Rybczynska
  2019-11-05 13:45 ` Keith Busch
  0 siblings, 1 reply; 5+ messages in thread
From: Marta Rybczynska @ 2019-11-05  8:10 UTC (permalink / raw)
  To: Keith Busch, linux-nvme

The existing ioctl passthru commands had a limit of 32-bit result.
Some commands (like get property for the CAP field) require 64
bits. A new added ioctl in the kernel allows this operation.

This patch adds usage of the 64-bit version for the get-property
command, falling back to 32-bit if necessary.

Signed-off-by: Marta Rybczynska <marta.rybczynska@kalray.eu>
---
 nvme-ioctl.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/nvme-ioctl.c b/nvme-ioctl.c
index 35df04d..e9fc158 100644
--- a/nvme-ioctl.c
+++ b/nvme-ioctl.c
@@ -93,6 +93,24 @@ static int nvme_submit_admin_passthru(int fd, struct nvme_passthru_cmd *cmd)
 	return ioctl(fd, NVME_IOCTL_ADMIN_CMD, cmd);
 }
 
+static int nvme_submit_admin_passthru64(int fd, struct nvme_passthru_cmd64 *cmd)
+{
+	int res;
+
+	res = ioctl(fd, NVME_IOCTL_ADMIN64_CMD, cmd);
+	if (res && (errno == EINVAL)) {
+		/* If the 64-bit command is not implemented in the system,
+		 * fallback to the 32-bit one. The structures differ only
+		 * in the result that is at the end.
+		 */
+		struct nvme_passthru_cmd cmd32;
+
+		memcpy(&cmd32, cmd, sizeof(cmd32));
+		res = ioctl(fd, NVME_IOCTL_ADMIN_CMD, cmd32);
+	}
+	return res;
+}
+
 static int nvme_submit_io_passthru(int fd, struct nvme_passthru_cmd *cmd)
 {
 	return ioctl(fd, NVME_IOCTL_IO_CMD, cmd);
@@ -600,9 +618,30 @@ static void nvme_to_passthru_cmd(struct nvme_passthru_cmd *pcmd,
 	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
 }
 
+static void nvme_to_passthru_cmd64(struct nvme_passthru_cmd64 *pcmd,
+				   const struct nvme_command *ncmd)
+{
+	assert(sizeof(*ncmd) < sizeof(*pcmd));
+	memset(pcmd, 0, sizeof(*pcmd));
+	pcmd->opcode = ncmd->common.opcode;
+	pcmd->flags = ncmd->common.flags;
+	pcmd->rsvd1 = ncmd->common.command_id;
+	pcmd->nsid = le32_to_cpu(ncmd->common.nsid);
+	pcmd->cdw2 = le32_to_cpu(ncmd->common.cdw2[0]);
+	pcmd->cdw3 = le32_to_cpu(ncmd->common.cdw2[1]);
+	/* Skip metadata and addr */
+	pcmd->cdw10 = le32_to_cpu(ncmd->common.cdw10[0]);
+	pcmd->cdw11 = le32_to_cpu(ncmd->common.cdw10[1]);
+	pcmd->cdw12 = le32_to_cpu(ncmd->common.cdw10[2]);
+	pcmd->cdw13 = le32_to_cpu(ncmd->common.cdw10[3]);
+	pcmd->cdw14 = le32_to_cpu(ncmd->common.cdw10[4]);
+	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
+}
+
+
 int nvme_get_property(int fd, int offset, uint64_t *value)
 {
-	struct nvme_passthru_cmd pcmd;
+	struct nvme_passthru_cmd64 pcmd;
 	struct nvmf_property_get_command pg = {
 		.opcode	= nvme_fabrics_command,
 		.fctype	= nvme_fabrics_type_property_get,
@@ -613,12 +652,14 @@ int nvme_get_property(int fd, int offset, uint64_t *value)
 	int err;
 
 	gcmd.prop_get = pg;
-	nvme_to_passthru_cmd(&pcmd, &gcmd);
-	err = nvme_submit_admin_passthru(fd, &pcmd);
+	nvme_to_passthru_cmd64(&pcmd, &gcmd);
+	err = nvme_submit_admin_passthru64(fd, &pcmd);
 	if (!err) {
 		/*
-		 * nvme_submit_admin_passthru() stores the lower 32 bits
-		 * of the property value in pcmd.result using CPU endianness.
+		 * If we have the 64-bit ioctl version, we got the
+		 * complete result. Otherwise nvme_submit_admin_passthru()
+		 * stores the lower 32 bits of the property value in
+		 * pcmd.result using CPU endianness.
 		 */
 		*value = pcmd.result;
 	}
-- 
1.8.3.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls
  2019-11-05  8:10 [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls Marta Rybczynska
@ 2019-11-05 13:45 ` Keith Busch
  2019-11-05 14:05   ` Marta Rybczynska
  0 siblings, 1 reply; 5+ messages in thread
From: Keith Busch @ 2019-11-05 13:45 UTC (permalink / raw)
  To: Marta Rybczynska; +Cc: Keith Busch, linux-nvme

On Tue, Nov 05, 2019 at 09:10:57AM +0100, Marta Rybczynska wrote:
> @@ -600,9 +618,30 @@ static void nvme_to_passthru_cmd(struct nvme_passthru_cmd *pcmd,
>  	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
>  }

You're working off of some non-public fork, and a broken one at that.
See below.

> +static void nvme_to_passthru_cmd64(struct nvme_passthru_cmd64 *pcmd,
> +				   const struct nvme_command *ncmd)
> +{

User space has no business using a 'struct nvme_command'. That's for
kernel use only.

> +	assert(sizeof(*ncmd) < sizeof(*pcmd));
> +	memset(pcmd, 0, sizeof(*pcmd));
> +	pcmd->opcode = ncmd->common.opcode;
> +	pcmd->flags = ncmd->common.flags;
> +	pcmd->rsvd1 = ncmd->common.command_id;
> +	pcmd->nsid = le32_to_cpu(ncmd->common.nsid);
> +	pcmd->cdw2 = le32_to_cpu(ncmd->common.cdw2[0]);
> +	pcmd->cdw3 = le32_to_cpu(ncmd->common.cdw2[1]);
> +	/* Skip metadata and addr */
> +	pcmd->cdw10 = le32_to_cpu(ncmd->common.cdw10[0]);
> +	pcmd->cdw11 = le32_to_cpu(ncmd->common.cdw10[1]);
> +	pcmd->cdw12 = le32_to_cpu(ncmd->common.cdw10[2]);
> +	pcmd->cdw13 = le32_to_cpu(ncmd->common.cdw10[3]);
> +	pcmd->cdw14 = le32_to_cpu(ncmd->common.cdw10[4]);
> +	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
> +}

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls
  2019-11-05 13:45 ` Keith Busch
@ 2019-11-05 14:05   ` Marta Rybczynska
  2019-11-05 14:58     ` Keith Busch
  0 siblings, 1 reply; 5+ messages in thread
From: Marta Rybczynska @ 2019-11-05 14:05 UTC (permalink / raw)
  To: kbusch; +Cc: Keith Busch, linux-nvme



----- On 5 Nov, 2019, at 14:45, kbusch kbusch@kernel.org wrote:

> On Tue, Nov 05, 2019 at 09:10:57AM +0100, Marta Rybczynska wrote:
>> @@ -600,9 +618,30 @@ static void nvme_to_passthru_cmd(struct nvme_passthru_cmd
>> *pcmd,
>>  	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
>>  }
> 
> You're working off of some non-public fork, and a broken one at that.
> See below.
>

It's based on https://github.com/linux-nvme/nvme-cli.git and applies to the
current master of that (66652af38042fc9624a8fbf25a325a788ccd3c82)

If there's a better one to use, please let me know.
 
>> +static void nvme_to_passthru_cmd64(struct nvme_passthru_cmd64 *pcmd,
>> +				   const struct nvme_command *ncmd)
>> +{
> 
> User space has no business using a 'struct nvme_command'. That's for
> kernel use only.
> 
>> +	assert(sizeof(*ncmd) < sizeof(*pcmd));
>> +	memset(pcmd, 0, sizeof(*pcmd));
>> +	pcmd->opcode = ncmd->common.opcode;
>> +	pcmd->flags = ncmd->common.flags;
>> +	pcmd->rsvd1 = ncmd->common.command_id;
>> +	pcmd->nsid = le32_to_cpu(ncmd->common.nsid);
>> +	pcmd->cdw2 = le32_to_cpu(ncmd->common.cdw2[0]);
>> +	pcmd->cdw3 = le32_to_cpu(ncmd->common.cdw2[1]);
>> +	/* Skip metadata and addr */
>> +	pcmd->cdw10 = le32_to_cpu(ncmd->common.cdw10[0]);
>> +	pcmd->cdw11 = le32_to_cpu(ncmd->common.cdw10[1]);
>> +	pcmd->cdw12 = le32_to_cpu(ncmd->common.cdw10[2]);
>> +	pcmd->cdw13 = le32_to_cpu(ncmd->common.cdw10[3]);
>> +	pcmd->cdw14 = le32_to_cpu(ncmd->common.cdw10[4]);
>> +	pcmd->cdw15 = le32_to_cpu(ncmd->common.cdw10[5]);
> > +}

The code was a copy of nvme_to_passthru that was introduced by 
Bart Van Assche in 301e263c.

There is only a couple of values set to nvme_command, so we can
code that directly in nvme_get_property if that's the better way.
This will require changing the older code too.

Regards,
Marta

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls
  2019-11-05 14:05   ` Marta Rybczynska
@ 2019-11-05 14:58     ` Keith Busch
  2019-11-05 15:13       ` Keith Busch
  0 siblings, 1 reply; 5+ messages in thread
From: Keith Busch @ 2019-11-05 14:58 UTC (permalink / raw)
  To: Marta Rybczynska; +Cc: Keith Busch, linux-nvme

On Tue, Nov 05, 2019 at 03:05:58PM +0100, Marta Rybczynska wrote:
> It's based on https://github.com/linux-nvme/nvme-cli.git and applies to the
> current master of that (66652af38042fc9624a8fbf25a325a788ccd3c82)
> 
> If there's a better one to use, please let me know.

That's the right one to use. That nvme_commmand usage just got past me.
I'll fix it up, we should not be using them from user space.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls
  2019-11-05 14:58     ` Keith Busch
@ 2019-11-05 15:13       ` Keith Busch
  0 siblings, 0 replies; 5+ messages in thread
From: Keith Busch @ 2019-11-05 15:13 UTC (permalink / raw)
  To: Marta Rybczynska; +Cc: Keith Busch, linux-nvme

On Tue, Nov 05, 2019 at 11:58:24PM +0900, Keith Busch wrote:
> On Tue, Nov 05, 2019 at 03:05:58PM +0100, Marta Rybczynska wrote:
> > It's based on https://github.com/linux-nvme/nvme-cli.git and applies to the
> > current master of that (66652af38042fc9624a8fbf25a325a788ccd3c82)
> > 
> > If there's a better one to use, please let me know.
> 
> That's the right one to use. That nvme_commmand usage just got past me.
> I'll fix it up, we should not be using them from user space.

Fixed now, sorry for the confusion.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  8:10 [PATCH 4/4] nvme-cli: ioctl: support 64-bit ioctls Marta Rybczynska
2019-11-05 13:45 ` Keith Busch
2019-11-05 14:05   ` Marta Rybczynska
2019-11-05 14:58     ` Keith Busch
2019-11-05 15:13       ` Keith Busch

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git