64-bit data integrity field support

The nvme spec allows protection information formats with metadata extending beyond the pi field.

The nvme spec allows protection information formats with metadata extending beyond the pi field. Use the actual size of the metadata field for incrementing the protection buffer.

Signed-off-by Keith Busch <[email protected]>

---
  block/bio-integrity.c         | 1 +
  block/t10-pi.c                | 4 ++--
  include/linux/blk-integrity.h | 1 +
  3 files changed, 4 insertions(+), 2 deletions(-)

The block integrity subsystem knows how to construct protection information buffers with metadata beyond the protection information fields. Remove the driver restriction.

Note, this can only work if the PI field appears first in the metadata, as the integrity subsystem doesn't calculate guard tags on preceding metadata.

The block integrity subsystem knows how to construct protection information buffers with metadata beyond the protection information fields. Remove the driver restriction.

Note, this can only work if the PI field appears first in the metadata, as the integrity subsystem doesn't calculate guard tags on preceding metadata.

Signed-off-by Keith Busch <[email protected]>

---
  drivers/nvme/host/core.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)

The NVM Express specification extended data integrity fields to 64 bits using the Rocksoft^TM parameters. Add the poly to the crc64 table generation, and provide a library routine implementing the algorithm.

The NVM Express specification extended data integrity fields to 64 bits using the Rocksoft^TM parameters. Add the poly to the crc64 table generation, and provide a library routine implementing the algorithm.

The Rocksoft 64-bit CRC model parameters are as follows:

     Poly: 0xAD93D23594C93659
     Initial value: 0xFFFFFFFFFFFFFFFF
     Reflected Input: True
     Reflected Output: True
     Xor Final: 0xFFFFFFFFFFFFFFFF

Since this model used reflected bits, the implementation generates the reflected table so the result is ordered consistently.

Cc Eric Biggers <[email protected]>

Signed-off-by Keith Busch <[email protected]>

---
v1->v2:

Provide a module to test the rocksoft crc64 calculations with well known inputs and exepected values.

Provide a module to test the rocksoft crc64 calculations with well known inputs and exepected values.

Signed-off-by Keith Busch <[email protected]>

---
v1->v2:

Fixed Kconfig dependency

  lib/Kconfig.debug |  4 +++
  lib/Makefile      |  1 +
  lib/test_crc64.c  | 68 +++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 73 insertions(+)

create mode 100644 lib/test_crc64.c

The NVMe protocol extended the data integrity fields with unaligned 48-bit reference tags. Provide some helper accessors in preparation for these.

The NVMe protocol extended the data integrity fields with unaligned 48-bit reference tags. Provide some helper accessors in preparation for these.

Acked-by Arnd Bergmann <[email protected]>

Signed-off-by Keith Busch <[email protected]>

---
  include/asm-generic/unaligned.h | 26 ++++++++++++++++++++++++++
  1 file changed, 26 insertions(+)

The NVMe specification defines larger data integrity formats beyond the t10 tuple. Add support for the specification defined CRC64 formats, assuming the reference tag does not need to be split with the "storage tag".

Cc "Martin K. Petersen" <[email protected]>

Signed-off-by Keith Busch <[email protected]>

---
  block/Kconfig          |   1 +
  block/t10-pi.c         | 194 +++++++++++++++++++++++++++++++++++++++++
  include/linux/t10-pi.h |  20 +++++
  3 files changed, 215 insertions(+)

NVM Express ratified TP 4069 defines new protection information formats. Implement support for the CRC64 guard tags.

NVM Express ratified TP 4069 defines new protection information formats. Implement support for the CRC64 guard tags.

Since the block layer doesn't support variable length reference tags, driver support for the Storage Tag space is not supported at this time.

Signed-off-by Keith Busch <[email protected]>

---
v1->v2:

Added support for PRACT

Fixed endian conversion

  drivers/nvme/host/core.c | 164 +++++++++++++++++++++++++++++++++------
  drivers/nvme/host/nvme.h |   4 +-
  include/linux/nvme.h     |  53 +++++++++++--
  3 files changed, 190 insertions(+), 31 deletions(-)

drivers/nvme/host/core.c +45 -11

index b3eabf6a08b9..0f2ea2a4c718 100644
@@ -882,6 +882,30 @@ static blk_status_t nvme_setup_discard(struct nvme_ns *ns, struct request *req,
  	return BLK_STS_OK;
  }
 
+static inline void nvme_set_ref_tag(struct nvme_ns *ns, struct nvme_command *cmnd,
+				    struct request *req)
+{
+	u32 upper, lower;
+	u64 ref48;
+
+	/* both rw and write zeroes share the same reftag format */
+	switch (ns->guard_type) {
+	case NVME_NVM_NS_16B_GUARD:
+		cmnd->rw.reftag = cpu_to_le32(t10_pi_ref_tag(req));
+		break;
+	case NVME_NVM_NS_64B_GUARD:
+		ref48 = nvme_pi_extended_ref_tag(req);
+		lower = lower_32_bits(ref48);
+		upper = upper_32_bits(ref48);
+
+		cmnd->rw.reftag = cpu_to_le32(lower);
+		cmnd->rw.cdw3 = cpu_to_le32(upper);
+		break;
+	default:
+		break;
+	}
+}
+
  static inline blk_status_t nvme_setup_write_zeroes(struct nvme_ns *ns,
  		struct request *req, struct nvme_command *cmnd)
  {

@@ -903,8 +927,7 @@ static inline blk_status_t nvme_setup_write_zeroes(struct nvme_ns *ns,
  		switch (ns->pi_type) {
  		case NVME_NS_DPS_PI_TYPE1:
  		case NVME_NS_DPS_PI_TYPE2:
-			cmnd->write_zeroes.reftag =
-				cpu_to_le32(t10_pi_ref_tag(req));
+			nvme_set_ref_tag(ns, cmnd, req);
  			break;
  		}
  	}

@@ -931,7 +954,8 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
  	cmnd->rw.opcode = op;
  	cmnd->rw.flags = 0;
  	cmnd->rw.nsid = cpu_to_le32(ns->head->ns_id);
-	cmnd->rw.rsvd2 = 0;
+	cmnd->rw.cdw2 = 0;
+	cmnd->rw.cdw3 = 0;
  	cmnd->rw.metadata = 0;
  	cmnd->rw.slba = cpu_to_le64(nvme_sect_to_lba(ns, blk_rq_pos(req)));
  	cmnd->rw.length = cpu_to_le16((blk_rq_bytes(req) >> ns->lba_shift) - 1);

@@ -965,7 +989,7 @@ static inline blk_status_t nvme_setup_rw(struct nvme_ns *ns,
  					NVME_RW_PRINFO_PRCHK_REF;
  			if (op == nvme_cmd_zone_append)
  				control |= NVME_RW_APPEND_PIREMAP;
-			cmnd->rw.reftag = cpu_to_le32(t10_pi_ref_tag(req));
+			nvme_set_ref_tag(ns, cmnd, req);
  			break;
  		}
  	}

@@ -1619,33 +1643,58 @@ int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo)
  }
 
  #ifdef CONFIG_BLK_DEV_INTEGRITY
-static void nvme_init_integrity(struct gendisk *disk, u16 ms, u8 pi_type,
+static void nvme_init_integrity(struct gendisk *disk, struct nvme_ns *ns,
  				u32 max_integrity_segments)
  {
  	struct blk_integrity integrity = { };
 
-	switch (pi_type) {
+	switch (ns->pi_type) {
  	case NVME_NS_DPS_PI_TYPE3:
-		integrity.profile = &t10_pi_type3_crc;
-		integrity.tag_size = sizeof(u16) + sizeof(u32);
-		integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+		switch (ns->guard_type) {
+		case NVME_NVM_NS_16B_GUARD:
+			integrity.profile = &t10_pi_type3_crc;
+			integrity.tag_size = sizeof(u16) + sizeof(u32);
+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		case NVME_NVM_NS_64B_GUARD:
+			integrity.profile = &nvme_pi_type1_crc64;
+			integrity.tag_size = sizeof(u16) + 6;
+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		default:
+			integrity.profile = NULL;
+			break;
+		}
  		break;
  	case NVME_NS_DPS_PI_TYPE1:
  	case NVME_NS_DPS_PI_TYPE2:
-		integrity.profile = &t10_pi_type1_crc;
-		integrity.tag_size = sizeof(u16);

drivers/nvme/host/core.c +83 -11

+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		default:
+			integrity.profile = NULL;
+			break;
+		}
  		break;
  	default:
  		integrity.profile = NULL;
  		break;
  	}
-	integrity.tuple_size = ms;
+
+	integrity.tuple_size = ns->ms;
  	blk_integrity_register(disk, &integrity);
  	blk_queue_max_integrity_segments(disk->queue, max_integrity_segments);
  }
  #else
-static void nvme_init_integrity(struct gendisk *disk, u16 ms, u8 pi_type,
+static void nvme_init_integrity(struct gendisk *disk, struct nvme_ns *ns,
  				u32 max_integrity_segments)
  {
  }

@@ -1722,17 +1771,75 @@ static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
  	return 0;
  }
 
-static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
+static int nvme_init_ms(struct nvme_ns *ns, struct nvme_id_ns *id)
  {
+	bool first = id->dps & NVME_NS_DPS_PI_FIRST;
+	unsigned lbaf = nvme_lbaf_index(id->flbas);
  	struct nvme_ctrl *ctrl = ns->ctrl;
+	struct nvme_command c = { };
+	struct nvme_id_ns_nvm *nvm;
+	int ret = 0;
+	u32 elbaf;
+
+	ns->pi_size = 0;
+	ns->ms = le16_to_cpu(id->lbaf[lbaf].ms);
+	if (!(ctrl->ctratt & NVME_CTRL_ATTR_ELBAS)) {
+		ns->pi_size = sizeof(struct t10_pi_tuple);
+		ns->guard_type = NVME_NVM_NS_16B_GUARD;
+		goto set_pi;
+	}
 
-	ns->ms = le16_to_cpu(id->lbaf[id->flbas & NVME_NS_FLBAS_LBA_MASK].ms);
-	if (id->dps & NVME_NS_DPS_PI_FIRST ||
-	    ns->ms == sizeof(struct t10_pi_tuple))
+	nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
+	if (!nvm)
+		return -ENOMEM;
+
+	c.identify.opcode = nvme_admin_identify;
+	c.identify.nsid = cpu_to_le32(ns->head->ns_id);
+	c.identify.cns = NVME_ID_CNS_CS_NS;
+	c.identify.csi = NVME_CSI_NVM;
+
+	ret = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, nvm, sizeof(*nvm));
+	if (ret)
+		goto free_data;
+
+	elbaf = le32_to_cpu(nvm->elbaf[lbaf]);
+
+	/* no support for storage tag formats right now */
+	if (nvme_elbaf_sts(elbaf))
+		goto free_data;
+
+	ns->guard_type = nvme_elbaf_guard_type(elbaf);
+	switch (ns->guard_type) {
+	case NVME_NVM_NS_64B_GUARD:
+		ns->pi_size = sizeof(struct nvme_crc64_pi_tuple);
+		break;
+	case NVME_NVM_NS_16B_GUARD:
+		ns->pi_size = sizeof(struct t10_pi_tuple);
+		break;
+	default:
+		break;
+	}
+
+free_data:
+	kfree(nvm);
+set_pi:
+	if (ns->pi_size && (first || ns->ms == ns->pi_size))
  		ns->pi_type = id->dps & NVME_NS_DPS_PI_MASK;
  	else
  		ns->pi_type = 0;
 
+	return ret;
+}
+
+static int nvme_configure_metadata(struct nvme_ns *ns, struct nvme_id_ns *id)
+{
+	struct nvme_ctrl *ctrl = ns->ctrl;
+	int ret;
+
+	ret = nvme_init_ms(ns, id);
+	if (ret)
+		return ret;
+
  	ns->features &= ~(NVME_NS_METADATA_SUPPORTED | NVME_NS_EXT_LBAS);
  	if (!ns->ms || !(ctrl->ops->flags & NVME_F_METADATA_SUPPORTED))
  		return 0;

@@ -1850,7 +1957,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
  	if (ns->ms) {
  		if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
  		    (ns->features & NVME_NS_METADATA_SUPPORTED))
-			nvme_init_integrity(disk, ns->ms, ns->pi_type,
+			nvme_init_integrity(disk, ns,
  					    ns->ctrl->max_integrity_segments);
  		else if (!nvme_ns_has_pi(ns))
  			capacity = 0;

@@ -1905,7 +2012,7 @@ static void nvme_set_chunk_sectors(struct nvme_ns *ns, struct nvme_id_ns *id)
 
  static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
  {
-	unsigned lbaf = id->flbas & NVME_NS_FLBAS_LBA_MASK;
+	unsigned lbaf = nvme_lbaf_index(id->flbas);
  	int ret;
 
  	blk_mq_freeze_queue(ns->disk->queue);

@@ -2252,20 +2359,27 @@ static int nvme_configure_timestamp(struct nvme_ctrl *ctrl)
  	return ret;
  }
 
-static int nvme_configure_acre(struct nvme_ctrl *ctrl)
+static int nvme_configure_host_options(struct nvme_ctrl *ctrl)
  {
  	struct nvme_feat_host_behavior *host;
+	u8 acre = 0, lbafee = 0;
  	int ret;
 
  	/* Don't bother enabling the feature if retry delay is not reported */
-	if (!ctrl->crdt[0])
+	if (ctrl->crdt[0])
+		acre = NVME_ENABLE_ACRE;
+	if (ctrl->ctratt & NVME_CTRL_ATTR_ELBAS)
+		lbafee = NVME_ENABLE_LBAFEE;
+
+	if (!acre && !lbafee)
  		return 0;
 
  	host = kzalloc(sizeof(*host), GFP_KERNEL);
  	if (!host)
  		return 0;
 
-	host->acre = NVME_ENABLE_ACRE;
+	host->acre = acre;
+	host->lbafee = lbafee;
  	ret = nvme_set_features(ctrl, NVME_FEAT_HOST_BEHAVIOR, 0,
  				host, sizeof(*host), NULL);
  	kfree(host);

drivers/nvme/host/core.c int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo)

Quoted 4 lines · Keith Busch

drivers/nvme/host/core.c +4 -0

+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		case NVME_NVM_NS_64B_GUARD:
+			integrity.profile = &nvme_pi_type1_crc64

Quoted 16 lines

drivers/nvme/host/core.c +10 -3

+			integrity.tag_size = sizeof(u16) + 6;
+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		default:
+			integrity.profile = NULL;
+			break;
+		}
  		break;
  	case NVME_NS_DPS_PI_TYPE1:
  	case NVME_NS_DPS_PI_TYPE2:
-		integrity.profile = &t10_pi_type1_crc;
-		integrity.tag_size = sizeof(u16);
-		integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+		switch (ns->guard_type) {
+		case NVME_NVM_NS_16B_GUARD:
+			integrity.profile = &t10_pi_type1_crc;

drivers/nvme/host/core.c int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo) · line 1679

Quoted 6 lines

drivers/nvme/host/core.c +6 -0

+			integrity.tag_size = sizeof(u16);
+			integrity.flags |= BLK_INTEGRITY_DEVICE_CAPABLE;
+			break;
+		case NVME_NVM_NS_64B_GUARD:
+			integrity.profile = &nvme_pi_type1_crc64;
+			integrity.tag_size = sizeof(u16);

Is that correct? Shouldn't it be '8' like in the above case?

drivers/nvme/host/core.c int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl) · line 3222

Quoted 7 lines

drivers/nvme/host/core.c +1 -1

@@ -3104,7 +3218,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
  	if (ret < 0)
  		return ret;
-	ret = nvme_configure_acre(ctrl);
+	ret = nvme_configure_host_options(ctrl);
  	if (ret < 0)
  		return ret;

This could be made into a separate patch, is it's not directly related to PI support.

For type1 and 2, I believe tag_size refers to the "application" tag, which is 2 bytes here.

The reason it is 8 bytes for type3 is because there is no ref tag, so that portion of the metadata becomes part of the opaque application tag_size.

Quoted 1 line · Hannes Reinecke

On 2/1/22 20:01, Keith Busch wrote:

Quoted 10 lines

drivers/nvme/host/core.c int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl) · line 3222

Quoted 7 lines

drivers/nvme/host/core.c +1 -1

@@ -3104,7 +3218,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl)
   	if (ret < 0)
   		return ret;
-	ret = nvme_configure_acre(ctrl);
+	ret = nvme_configure_host_options(ctrl);
   	if (ret < 0)
   		return ret;

This could be made into a separate patch, is it's not directly related to PI support. >

Well, the driver can't read the new PI formats without enabling host supported features for it. Enabling the feature tells the controller we're going to check for it, so I don't think we could reasonably split this part into a prep patch from the part that sets up the PI formats.