[RFC PATCH 0/1] nvme: implement get log page offset and dwords

[RFC PATCH 0/1] nvme: implement get log page offset and dwords

[Matias Bjørling]

This patch introduces support for get log page offset and extends
the number of dwords to be 32 bits.

A function is introduced that takes the offset and implements the
support. This is needed for ocssd to retrieve the report chunk log
page, which can span a couple of megabytes.

The patch is based on Javier's report chunk implementation patch,
and is generalized to make it part of the nvme core. A follow up
patch will expose the extended function and let the lightnvm module
call it to retrieve the report chunk log page.

A couple of questions with respect to the function:

 1. Should we check the offset in the extended function to be dword
    aligned?
 2. Verify that when offset is defined, the nvme controller version
    is at least 1.2.1?
 3. Handle requests that are larger than supported by the mdts field?

My take is that the caller should handle all of the above.

Thanks!

Matias Bj?rling (1):
  nvme: implement log page low/high offset and dwords

 drivers/nvme/host/core.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

-- 
2.11.0

[PATCH 1/1] nvme: implement log page low/high offset and dwords

[Matias Bjørling]

NVMe 1.2.1 extends the get log page interface to include 64 bit
offset and increases the number of dwords to 32 bits. Implement
for future use.

Signed-off-by: Matias Bj?rling <mb at lightnvm.io>
---
 drivers/nvme/host/core.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 740ceb28067c..f59527b93444 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -80,11 +80,6 @@ static struct class *nvme_subsys_class;
 static void nvme_ns_remove(struct nvme_ns *ns);
 static int nvme_revalidate_disk(struct gendisk *disk);
 
-static __le32 nvme_get_log_dw10(u8 lid, size_t size)
-{
-	return cpu_to_le32((((size / 4) - 1) << 16) | lid);
-}
-
 int nvme_reset_ctrl(struct nvme_ctrl *ctrl)
 {
 	if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING))
@@ -2132,16 +2127,33 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 	return ret;
 }
 
+int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
+			    u8 log_page, void *log,
+			    size_t size, size_t offset)
+{
+	struct nvme_command c = { };
+	unsigned long dwlen = (size / 4) - 1;
+
+	c.get_log_page.opcode = nvme_admin_get_log_page;
+
+	if (ns)
+		c.get_log_page.nsid = cpu_to_le32(ns->head->ns_id);
+	else
+		c.get_log_page.nsid = cpu_to_le32(NVME_NSID_ALL);
+
+	c.get_log_page.lid = log_page;
+	c.get_log_page.numdl = cpu_to_le16(dwlen & ((1 << 16) - 1));
+	c.get_log_page.numdu = cpu_to_le16(dwlen >> 16);
+	c.get_log_page.lpol = cpu_to_le32(offset & ((1ULL << 32) - 1));
+	c.get_log_page.lpou = cpu_to_le32(offset >> 32);
+
+	return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
+}
+
 static int nvme_get_log(struct nvme_ctrl *ctrl, u8 log_page, void *log,
 			size_t size)
 {
-	struct nvme_command c = { };
-
-	c.common.opcode = nvme_admin_get_log_page;
-	c.common.nsid = cpu_to_le32(NVME_NSID_ALL);
-	c.common.cdw10[0] = nvme_get_log_dw10(log_page, size);
-
-	return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
+	return nvme_get_log_ext(ctrl, NULL, log_page, log, size, 0);
 }
 
 static int nvme_get_effects_log(struct nvme_ctrl *ctrl)
-- 
2.11.0

[PATCH 1/1] nvme: implement log page low/high offset and dwords

[Johannes Thumshirn]

On Tue, 2018-02-13@13:49 +0100, Matias Bj?rling wrote:
> +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> +			????u8 log_page, void *log,
> +			????size_t size, size_t offset)
> +{
> +	struct nvme_command c = { };
> +	unsigned long dwlen = (size / 4) - 1;

Consulting my long gone maths knowledge suggests the parantheses aren't
really needed here.

-- 
Johannes Thumshirn ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?Storage
jthu
mshirn at suse.de ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+49 911 74053 689
SUSE
LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane
Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38
9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH 1/1] nvme: implement log page low/high offset and dwords

[Matias Bjørling]

On 02/13/2018 02:47 PM, Johannes Thumshirn wrote:
> On Tue, 2018-02-13@13:49 +0100, Matias Bj?rling wrote:
>> +int nvme_get_log_ext(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
>> +			????u8 log_page, void *log,
>> +			????size_t size, size_t offset)
>> +{
>> +	struct nvme_command c = { };
>> +	unsigned long dwlen = (size / 4) - 1;
> 
> Consulting my long gone maths knowledge suggests the parantheses aren't
> really needed here.
> 

Heh, you're right. However, it does make it a bit more readable? The 
untrained eye would properly mess it up was my thinking for keeping them.

[PATCH 1/1] nvme: implement log page low/high offset and dwords

[Javier Gonzalez]

> On 13 Feb 2018,@13.49, Matias Bj?rling <mb@lightnvm.io> wrote:
> 
> NVMe 1.2.1 extends the get log page interface to include 64 bit
> offset and increases the number of dwords to 32 bits. Implement
> for future use.
> 
> Signed-off-by: Matias Bj?rling <mb at lightnvm.io>
> ---
> drivers/nvme/host/core.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
> 
> 

Looks good to me.

Reviewed-by: Javier Gonz?lez <javier at cnexlabs.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20180226/a93ae2e8/attachment.sig>