[PATCH-v2 00/17] target: Add support for DIF Type1+Type3 emulation + passthrough

[PATCH-v2 00/17] target: Add support for DIF Type1+Type3 emulation + passthrough

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi MKP & Co,

This -v2 series contains support for target mode DIF Type1+Type3
emulation within target core, FILEIO, and RAMDISK_MCP device backends,
BIP passthrough of T10 PI within IBLOCK device backends, and DIF/DIX
host support in the tcm_loop fabric driver.

DIF emulation is enabled via a new 'pi_prot_type' device attribute
within configfs for FILEIO + RAMDISK_MCP, which is set after initial
device configuration and before target fabric LUN export occurs.

For IBLOCK backends, protection support is automatically detected via
struct blk_integrity at configuration time, and no additional end-user
setup is required.

For FILEIO backends, format of protection information is exposed via
a new 'pi_prot_format' device attribute, that is used during initial
configuration to format a new $FILENAME.protection file containing
DIF Type1/Type3 style metadata.

Also, the DIF read/write verify emulation has been made generic enough
so it can be used by other backend drivers (eg: FILEIO), as well as
Type4 (16-byte PI) in the near future.

Here is the v1 -> v2 changelog:

  - Drop guard_type related definitions
  - Update target_prot_op + target_prot_ho definitions (Sagi)
  - Drop SCF_PROT + pi_prot_version flag
  - Add se_subsystem_api->format_prot() (Sagi)
  - Add hw_pi_prot_type device attribute
  - Make sbc_check_prot defined as static (Fengguang + Wei)
  - Remove unprotected READ/WRITE warning (mkp)
  - Populate cmd->prot_type + friends (Sagi)
  - Drop SCF_PROT usage
  - Select CRC_T10DIF for TARGET_CORE in Kconfig (Fengguang)
  - Drop IP checksum logic from sbc_dif_v1_verify (MKP)
  - Fix offset on app_tag = 0xffff in sbc_dif_verify_read()
  - Drop pi_guard_type + pi_prot_version related code (MKP)
  - Add pi_prot_format logic (Sagi)
  - Add ->free_prot callback in target_free_device
  - Add hw_pi_prot_type read-only attribute
  - Add target/iblock blk_integrity + BIP passthrough
  - Add target/file DIF protection init/format support
  - Add target/file DIF protection support to fd_execute_rw
  - Drop unused sg_table from rd_release_device_space (Wei)
  - Drop unused sg_table from rd_release_prot_space (Wei)
  - Drop rd_release_prot_space call from rd_free_device
  - Make rd_execute_rw() to u32 sectors count instead of sector_t

At this point the main TODO item remaining for v3.14 code is to determine
how best to propigate App/Guard/Reference Tag VERIFY failures back up
into IBLOCK device backend code.

MKP has an idea how to go about doing this that will be appearing as
a seperate SCSI-core + IBLOCK patch.

Thank you,

--nab

Nicholas Bellinger (17):
  target: Add DIF related base definitions
  target: Add DIF CHECK_CONDITION ASC/ASCQ exception cases
  target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb
  target/sbc: Add DIF TYPE1+TYPE3 read/write verify emulation
  target/spc: Add protection bit to standard INQUIRY output
  target/spc: Add protection related bits to INQUIRY EVPD=0x86
  target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
  target/spc: Expose ATO bit in control mode page
  target/configfs: Expose protection device attributes
  target: Add protection SGLs to target_submit_cmd_map_sgls
  target/iblock: Add blk_integrity + BIP passthrough support
  target/file: Add DIF protection init/format support
  target/file: Add DIF protection support to fd_execute_rw
  target/rd: Refactor rd_build_device_space + rd_release_device_space
  target/rd: Add support for protection SGL setup + release
  target/rd: Add DIF protection into rd_execute_rw
  tcm_loop: Enable DIF/DIX modes in SCSI host LLD

 drivers/target/Kconfig                 |    2 +
 drivers/target/loopback/tcm_loop.c     |   12 +-
 drivers/target/target_core_configfs.c  |   12 ++
 drivers/target/target_core_device.c    |   89 +++++++++++
 drivers/target/target_core_file.c      |  256 +++++++++++++++++++++++++++++++-
 drivers/target/target_core_file.h      |    9 ++
 drivers/target/target_core_iblock.c    |   91 +++++++++++-
 drivers/target/target_core_internal.h  |    2 +
 drivers/target/target_core_rd.c        |  252 +++++++++++++++++++++++++------
 drivers/target/target_core_rd.h        |    4 +
 drivers/target/target_core_sbc.c       |  245 ++++++++++++++++++++++++++++++
 drivers/target/target_core_spc.c       |   27 ++++
 drivers/target/target_core_transport.c |   46 +++++-
 drivers/vhost/scsi.c                   |    2 +-
 include/target/target_core_backend.h   |    7 +
 include/target/target_core_base.h      |   47 ++++++
 include/target/target_core_fabric.h    |    3 +-
 17 files changed, 1052 insertions(+), 54 deletions(-)

-- 
1.7.10.4

[PATCH-v2 01/17] target: Add DIF related base definitions

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds DIF related definitions to target_core_base.h
that includes enums for target_prot_op + target_prot_type +
target_prot_version + target_guard_type + target_pi_error.

Also included is struct se_dif_v1_tuple, along with changes
to struct se_cmd, struct se_dev_attrib, and struct se_device.

Also, add new se_subsystem_api->[init,format,free]_prot() callers
used by target core code to setup backend specific protection
information after the device has been configured.

Enums taken from Sagi Grimberg's original patch.

v2 changes:
  - Drop guard_type related definitions
  - Update target_prot_op + target_prot_ho definitions (Sagi)
  - Drop SCF_PROT + pi_prot_version flag
  - Add se_subsystem_api->format_prot() (Sagi)
  - Add hw_pi_prot_type device attribute

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 include/target/target_core_backend.h |    3 +++
 include/target/target_core_base.h    |   44 ++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index 39e0114..0dc2745 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -41,6 +41,9 @@ struct se_subsystem_api {
 	unsigned int (*get_io_opt)(struct se_device *);
 	unsigned char *(*get_sense_buffer)(struct se_cmd *);
 	bool (*get_write_cache)(struct se_device *);
+	int (*init_prot)(struct se_device *);
+	int (*format_prot)(struct se_device *);
+	void (*free_prot)(struct se_device *);
 };
 
 struct sbc_ops {
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index dd87ab4..d98048b 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -435,6 +435,34 @@ struct se_tmr_req {
 	struct list_head	tmr_list;
 };
 
+enum target_prot_op {
+	TARGET_PROT_NORMAL = 0,
+	TARGET_PROT_DIN_INSERT,
+	TARGET_PROT_DOUT_INSERT,
+	TARGET_PROT_DIN_STRIP,
+	TARGET_PROT_DOUT_STRIP,
+	TARGET_PROT_DIN_PASS,
+	TARGET_PROT_DOUT_PASS,
+};
+
+enum target_prot_ho {
+	PROT_SEPERATED,
+	PROT_INTERLEAVED,
+};
+
+enum target_prot_type {
+	TARGET_DIF_TYPE0_PROT,
+	TARGET_DIF_TYPE1_PROT,
+	TARGET_DIF_TYPE2_PROT,
+	TARGET_DIF_TYPE3_PROT,
+};
+
+struct se_dif_v1_tuple {
+	__be16			guard_tag;
+	__be16			app_tag;
+	__be32			ref_tag;
+};
+
 struct se_cmd {
 	/* SAM response code being sent to initiator */
 	u8			scsi_status;
@@ -519,6 +547,17 @@ struct se_cmd {
 
 	/* Used for lun->lun_ref counting */
 	bool			lun_ref_active;
+
+	/* DIF related members */
+	enum target_prot_op	prot_op;
+	enum target_prot_type	prot_type;
+	u32			prot_length;
+	u32			reftag_seed;
+	struct scatterlist	*t_prot_sg;
+	unsigned int		t_prot_nents;
+	enum target_prot_ho	prot_handover;
+	sense_reason_t		pi_err;
+	u32			block_num;
 };
 
 struct se_ua {
@@ -629,6 +668,9 @@ struct se_dev_attrib {
 	int		emulate_tpws;
 	int		emulate_caw;
 	int		emulate_3pc;
+	int		pi_prot_format;
+	enum target_prot_type pi_prot_type;
+	enum target_prot_type hw_pi_prot_type;
 	int		enforce_pr_isids;
 	int		is_nonrot;
 	int		emulate_rest_reord;
@@ -759,6 +801,8 @@ struct se_device {
 	/* Linked list for struct se_hba struct se_device list */
 	struct list_head	dev_list;
 	struct se_lun		xcopy_lun;
+	/* Protection Information */
+	int			prot_length;
 };
 
 struct se_hba {
-- 
1.7.10.4

[PATCH-v2 02/17] target: Add DIF CHECK_CONDITION ASC/ASCQ exception cases

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for DIF related CHECK_CONDITION ASC/ASCQ
exception cases into transport_send_check_condition_and_sense().

This includes:

  LOGICAL BLOCK GUARD CHECK FAILED
  LOGICAL BLOCK APPLICATION TAG CHECK FAILED
  LOGICAL BLOCK REFERENCE TAG CHECK FAILED

that used by DIF TYPE1 and TYPE3 failure cases.

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_transport.c |   30 ++++++++++++++++++++++++++++++
 include/target/target_core_base.h      |    3 +++
 2 files changed, 33 insertions(+)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 18c828d..fa4fc04 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2674,6 +2674,36 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd,
 		buffer[SPC_ASC_KEY_OFFSET] = 0x1d;
 		buffer[SPC_ASCQ_KEY_OFFSET] = 0x00;
 		break;
+	case TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED:
+		/* CURRENT ERROR */
+		buffer[0] = 0x70;
+		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
+		/* ILLEGAL REQUEST */
+		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
+		/* LOGICAL BLOCK GUARD CHECK FAILED */
+		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
+		buffer[SPC_ASCQ_KEY_OFFSET] = 0x01;
+		break;
+	case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED:
+		/* CURRENT ERROR */
+		buffer[0] = 0x70;
+		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
+		/* ILLEGAL REQUEST */
+		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
+		/* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */
+		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
+		buffer[SPC_ASCQ_KEY_OFFSET] = 0x02;
+		break;
+	case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED:
+		/* CURRENT ERROR */
+		buffer[0] = 0x70;
+		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
+		/* ILLEGAL REQUEST */
+		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
+		/* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */
+		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
+		buffer[SPC_ASCQ_KEY_OFFSET] = 0x03;
+		break;
 	case TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE:
 	default:
 		/* CURRENT ERROR */
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index d98048b..0336d70 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -205,6 +205,9 @@ enum tcm_sense_reason_table {
 	TCM_OUT_OF_RESOURCES			= R(0x12),
 	TCM_PARAMETER_LIST_LENGTH_ERROR		= R(0x13),
 	TCM_MISCOMPARE_VERIFY			= R(0x14),
+	TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED	= R(0x15),
+	TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED	= R(0x16),
+	TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED	= R(0x17),
 #undef R
 };
 
-- 
1.7.10.4

Re: [PATCH-v2 02/17] target: Add DIF CHECK_CONDITION ASC/ASCQ exception cases

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds support for DIF related CHECK_CONDITION ASC/ASCQ
> exception cases into transport_send_check_condition_and_sense().
>
> This includes:
>
>    LOGICAL BLOCK GUARD CHECK FAILED
>    LOGICAL BLOCK APPLICATION TAG CHECK FAILED
>    LOGICAL BLOCK REFERENCE TAG CHECK FAILED
>
> that used by DIF TYPE1 and TYPE3 failure cases.
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/target_core_transport.c |   30 ++++++++++++++++++++++++++++++
>   include/target/target_core_base.h      |    3 +++
>   2 files changed, 33 insertions(+)
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 18c828d..fa4fc04 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -2674,6 +2674,36 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd,
>   		buffer[SPC_ASC_KEY_OFFSET] = 0x1d;
>   		buffer[SPC_ASCQ_KEY_OFFSET] = 0x00;
>   		break;
> +	case TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED:
> +		/* CURRENT ERROR */
> +		buffer[0] = 0x70;
> +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> +		/* ILLEGAL REQUEST */
> +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> +		/* LOGICAL BLOCK GUARD CHECK FAILED */
> +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x01;
> +		break;
> +	case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED:
> +		/* CURRENT ERROR */
> +		buffer[0] = 0x70;
> +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> +		/* ILLEGAL REQUEST */
> +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> +		/* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */
> +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x02;
> +		break;
> +	case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED:
> +		/* CURRENT ERROR */
> +		buffer[0] = 0x70;
> +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> +		/* ILLEGAL REQUEST */
> +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> +		/* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */
> +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x03;
> +		break;

Hey Nic,

I think we missed the failed LBA here. AFAICT According to SPC-4, a DIF
error should be accompanied by Information sense-data descriptor with 
the (first) failed
sector in the information field. This means that this routine should be 
ready to accept a
u32 bad_sector or something. I'm not sure how much of a must it really is.

Let me prepare a patch...

Sagi.

>   	case TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE:
>   	default:
>   		/* CURRENT ERROR */
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index d98048b..0336d70 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -205,6 +205,9 @@ enum tcm_sense_reason_table {
>   	TCM_OUT_OF_RESOURCES			= R(0x12),
>   	TCM_PARAMETER_LIST_LENGTH_ERROR		= R(0x13),
>   	TCM_MISCOMPARE_VERIFY			= R(0x14),
> +	TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED	= R(0x15),
> +	TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED	= R(0x16),
> +	TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED	= R(0x17),
>   #undef R
>   };
>   

Re: [PATCH-v2 02/17] target: Add DIF CHECK_CONDITION ASC/ASCQ exception cases

[Nicholas A. Bellinger]

On Wed, 2014-01-22 at 18:44 +0200, Sagi Grimberg wrote:
> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > This patch adds support for DIF related CHECK_CONDITION ASC/ASCQ
> > exception cases into transport_send_check_condition_and_sense().
> >
> > This includes:
> >
> >    LOGICAL BLOCK GUARD CHECK FAILED
> >    LOGICAL BLOCK APPLICATION TAG CHECK FAILED
> >    LOGICAL BLOCK REFERENCE TAG CHECK FAILED
> >
> > that used by DIF TYPE1 and TYPE3 failure cases.
> >
> > Cc: Martin K. Petersen <martin.petersen@oracle.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Sagi Grimberg <sagig@mellanox.com>
> > Cc: Or Gerlitz <ogerlitz@mellanox.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >   drivers/target/target_core_transport.c |   30 ++++++++++++++++++++++++++++++
> >   include/target/target_core_base.h      |    3 +++
> >   2 files changed, 33 insertions(+)
> >
> > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> > index 18c828d..fa4fc04 100644
> > --- a/drivers/target/target_core_transport.c
> > +++ b/drivers/target/target_core_transport.c
> > @@ -2674,6 +2674,36 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd,
> >   		buffer[SPC_ASC_KEY_OFFSET] = 0x1d;
> >   		buffer[SPC_ASCQ_KEY_OFFSET] = 0x00;
> >   		break;
> > +	case TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED:
> > +		/* CURRENT ERROR */
> > +		buffer[0] = 0x70;
> > +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> > +		/* ILLEGAL REQUEST */
> > +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> > +		/* LOGICAL BLOCK GUARD CHECK FAILED */
> > +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> > +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x01;
> > +		break;
> > +	case TCM_LOGICAL_BLOCK_APP_TAG_CHECK_FAILED:
> > +		/* CURRENT ERROR */
> > +		buffer[0] = 0x70;
> > +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> > +		/* ILLEGAL REQUEST */
> > +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> > +		/* LOGICAL BLOCK APPLICATION TAG CHECK FAILED */
> > +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> > +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x02;
> > +		break;
> > +	case TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED:
> > +		/* CURRENT ERROR */
> > +		buffer[0] = 0x70;
> > +		buffer[SPC_ADD_SENSE_LEN_OFFSET] = 10;
> > +		/* ILLEGAL REQUEST */
> > +		buffer[SPC_SENSE_KEY_OFFSET] = ILLEGAL_REQUEST;
> > +		/* LOGICAL BLOCK REFERENCE TAG CHECK FAILED */
> > +		buffer[SPC_ASC_KEY_OFFSET] = 0x10;
> > +		buffer[SPC_ASCQ_KEY_OFFSET] = 0x03;
> > +		break;
> 
> Hey Nic,
> 
> I think we missed the failed LBA here. AFAICT According to SPC-4, a DIF
> error should be accompanied by Information sense-data descriptor with 
> the (first) failed
> sector in the information field. This means that this routine should be 
> ready to accept a
> u32 bad_sector or something. I'm not sure how much of a must it really is.
> 
> Let me prepare a patch...
> 

Ah yes, good catch.  This is what se_cmd->block_num was intended for..

Care to add these assignments to target_core_sbc.c:sbc_dif_verify_*
failures as well..?

--nab

[PATCH-v2 03/17] target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds sbc_check_prot() for performing various DIF
related CDB sanity checks, along with setting cmd->prot_type
once sanity checks have passed.

Also, add calls in sbc_parse_cdb() for READ_[10,12,16] +
WRITE_[10,12,16] to perform DIF sanity checking.

v2 changes:
  - Make sbc_check_prot defined as static (Fengguang + Wei)
  - Remove unprotected READ/WRITE warning (mkp)
  - Populate cmd->prot_type + friends (Sagi)
  - Drop SCF_PROT usage

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_sbc.c |   62 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 6863dbe..91a92f3 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -563,6 +563,44 @@ sbc_compare_and_write(struct se_cmd *cmd)
 	return TCM_NO_SENSE;
 }
 
+static bool
+sbc_check_prot(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb,
+	       u32 sectors)
+{
+	if (!cmd->t_prot_sg || !cmd->t_prot_nents)
+		return true;
+
+	switch (dev->dev_attrib.pi_prot_type) {
+	case TARGET_DIF_TYPE3_PROT:
+		if (!(cdb[1] & 0xe0))
+			return true;
+
+		cmd->reftag_seed = 0xffffffff;
+		break;
+	case TARGET_DIF_TYPE2_PROT:
+		if (cdb[1] & 0xe0)
+			return false;
+
+		cmd->reftag_seed = cmd->t_task_lba;
+		break;
+	case TARGET_DIF_TYPE1_PROT:
+		if (!(cdb[1] & 0xe0))
+			return true;
+
+		cmd->reftag_seed = cmd->t_task_lba;
+		break;
+	case TARGET_DIF_TYPE0_PROT:
+	default:
+		return true;
+	}
+
+	cmd->prot_type = dev->dev_attrib.pi_prot_type;
+	cmd->prot_length = dev->prot_length * sectors;
+	cmd->prot_handover = PROT_SEPERATED;
+
+	return true;
+}
+
 sense_reason_t
 sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 {
@@ -583,6 +621,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case READ_10:
 		sectors = transport_get_sectors_10(cdb);
 		cmd->t_task_lba = transport_lba_32(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
 		cmd->execute_rw = ops->execute_rw;
 		cmd->execute_cmd = sbc_execute_rw;
@@ -590,6 +632,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case READ_12:
 		sectors = transport_get_sectors_12(cdb);
 		cmd->t_task_lba = transport_lba_32(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
 		cmd->execute_rw = ops->execute_rw;
 		cmd->execute_cmd = sbc_execute_rw;
@@ -597,6 +643,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case READ_16:
 		sectors = transport_get_sectors_16(cdb);
 		cmd->t_task_lba = transport_lba_64(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
 		cmd->execute_rw = ops->execute_rw;
 		cmd->execute_cmd = sbc_execute_rw;
@@ -612,6 +662,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case WRITE_VERIFY:
 		sectors = transport_get_sectors_10(cdb);
 		cmd->t_task_lba = transport_lba_32(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		if (cdb[1] & 0x8)
 			cmd->se_cmd_flags |= SCF_FUA;
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
@@ -621,6 +675,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case WRITE_12:
 		sectors = transport_get_sectors_12(cdb);
 		cmd->t_task_lba = transport_lba_32(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		if (cdb[1] & 0x8)
 			cmd->se_cmd_flags |= SCF_FUA;
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
@@ -630,6 +688,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 	case WRITE_16:
 		sectors = transport_get_sectors_16(cdb);
 		cmd->t_task_lba = transport_lba_64(cdb);
+
+		if (!sbc_check_prot(dev, cmd, cdb, sectors))
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
 		if (cdb[1] & 0x8)
 			cmd->se_cmd_flags |= SCF_FUA;
 		cmd->se_cmd_flags |= SCF_SCSI_DATA_CDB;
-- 
1.7.10.4

Re: [PATCH-v2 03/17] target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds sbc_check_prot() for performing various DIF
> related CDB sanity checks, along with setting cmd->prot_type
> once sanity checks have passed.
>
> Also, add calls in sbc_parse_cdb() for READ_[10,12,16] +
> WRITE_[10,12,16] to perform DIF sanity checking.
>
> v2 changes:
>    - Make sbc_check_prot defined as static (Fengguang + Wei)
>    - Remove unprotected READ/WRITE warning (mkp)
>    - Populate cmd->prot_type + friends (Sagi)
>    - Drop SCF_PROT usage
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/target_core_sbc.c |   62 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
>
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index 6863dbe..91a92f3 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -563,6 +563,44 @@ sbc_compare_and_write(struct se_cmd *cmd)
>   	return TCM_NO_SENSE;
>   }
>   
> +static bool
> +sbc_check_prot(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb,
> +	       u32 sectors)
> +{
> +	if (!cmd->t_prot_sg || !cmd->t_prot_nents)
> +		return true;
> +
> +	switch (dev->dev_attrib.pi_prot_type) {
> +	case TARGET_DIF_TYPE3_PROT:
> +		if (!(cdb[1] & 0xe0))
> +			return true;
> +
> +		cmd->reftag_seed = 0xffffffff;
> +		break;
> +	case TARGET_DIF_TYPE2_PROT:
> +		if (cdb[1] & 0xe0)
> +			return false;
> +
> +		cmd->reftag_seed = cmd->t_task_lba;
> +		break;
> +	case TARGET_DIF_TYPE1_PROT:
> +		if (!(cdb[1] & 0xe0))
> +			return true;
> +
> +		cmd->reftag_seed = cmd->t_task_lba;
> +		break;
> +	case TARGET_DIF_TYPE0_PROT:
> +	default:
> +		return true;
> +	}
> +
> +	cmd->prot_type = dev->dev_attrib.pi_prot_type;
> +	cmd->prot_length = dev->prot_length * sectors;
> +	cmd->prot_handover = PROT_SEPERATED;

I know that we are not planning to support interleaved mode at the 
moment, But I think
that the protection handover type is the backstore preference and should 
be taken from se_dev.
But it is not that important for now...

Sagi.

Re: [PATCH-v2 03/17] target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb

[Nicholas A. Bellinger]

On Sun, 2014-01-19 at 13:43 +0200, Sagi Grimberg wrote:
> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > This patch adds sbc_check_prot() for performing various DIF
> > related CDB sanity checks, along with setting cmd->prot_type
> > once sanity checks have passed.
> >
> > Also, add calls in sbc_parse_cdb() for READ_[10,12,16] +
> > WRITE_[10,12,16] to perform DIF sanity checking.
> >
> > v2 changes:
> >    - Make sbc_check_prot defined as static (Fengguang + Wei)
> >    - Remove unprotected READ/WRITE warning (mkp)
> >    - Populate cmd->prot_type + friends (Sagi)
> >    - Drop SCF_PROT usage
> >
> > Cc: Martin K. Petersen <martin.petersen@oracle.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Sagi Grimberg <sagig@mellanox.com>
> > Cc: Or Gerlitz <ogerlitz@mellanox.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >   drivers/target/target_core_sbc.c |   62 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 62 insertions(+)
> >
> > diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> > index 6863dbe..91a92f3 100644
> > --- a/drivers/target/target_core_sbc.c
> > +++ b/drivers/target/target_core_sbc.c
> > @@ -563,6 +563,44 @@ sbc_compare_and_write(struct se_cmd *cmd)
> >   	return TCM_NO_SENSE;
> >   }
> >   
> > +static bool
> > +sbc_check_prot(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb,
> > +	       u32 sectors)
> > +{
> > +	if (!cmd->t_prot_sg || !cmd->t_prot_nents)
> > +		return true;
> > +
> > +	switch (dev->dev_attrib.pi_prot_type) {
> > +	case TARGET_DIF_TYPE3_PROT:
> > +		if (!(cdb[1] & 0xe0))
> > +			return true;
> > +
> > +		cmd->reftag_seed = 0xffffffff;
> > +		break;
> > +	case TARGET_DIF_TYPE2_PROT:
> > +		if (cdb[1] & 0xe0)
> > +			return false;
> > +
> > +		cmd->reftag_seed = cmd->t_task_lba;
> > +		break;
> > +	case TARGET_DIF_TYPE1_PROT:
> > +		if (!(cdb[1] & 0xe0))
> > +			return true;
> > +
> > +		cmd->reftag_seed = cmd->t_task_lba;
> > +		break;
> > +	case TARGET_DIF_TYPE0_PROT:
> > +	default:
> > +		return true;
> > +	}
> > +
> > +	cmd->prot_type = dev->dev_attrib.pi_prot_type;
> > +	cmd->prot_length = dev->prot_length * sectors;
> > +	cmd->prot_handover = PROT_SEPERATED;
> 
> I know that we are not planning to support interleaved mode at the 
> moment, But I think
> that the protection handover type is the backstore preference and should 
> be taken from se_dev.
> But it is not that important for now...
> 

Yeah, I figured since the RDMA pieces needed the handover type defined
in some form, it made sense to include PROT_SEPERATED hardcoded here,
but stopped short of adding se_dev->prot_handler for the first round
merge.

--nab

Re: [PATCH-v2 03/17] target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb

[Sagi Grimberg]

On 1/22/2014 12:48 AM, Nicholas A. Bellinger wrote:
> +	cmd->prot_handover = PROT_SEPERATED;
>> I know that we are not planning to support interleaved mode at the
>> moment, But I think
>> that the protection handover type is the backstore preference and should
>> be taken from se_dev.
>> But it is not that important for now...
>>
> Yeah, I figured since the RDMA pieces needed the handover type defined
> in some form, it made sense to include PROT_SEPERATED hardcoded here,
> but stopped short of adding se_dev->prot_handler for the first round
> merge.
>
> --nab
>

Actually they don't, I just added them in iSER code to demonstrate the 
HW ability.
If we are not planning to support that (although as MKP mentioned it 
might be useful in some cases),
you can remove that for now and we can add it in the future - iSER can 
ignore it for now (I'll refactor the patches).

Sagi.

Re: [PATCH-v2 03/17] target/sbc: Add DIF setup in sbc_check_prot + sbc_parse_cdb

[Nicholas A. Bellinger]

On Wed, 2014-01-22 at 20:00 +0200, Sagi Grimberg wrote:
> On 1/22/2014 12:48 AM, Nicholas A. Bellinger wrote:
> > +	cmd->prot_handover = PROT_SEPERATED;
> >> I know that we are not planning to support interleaved mode at the
> >> moment, But I think
> >> that the protection handover type is the backstore preference and should
> >> be taken from se_dev.
> >> But it is not that important for now...
> >>
> > Yeah, I figured since the RDMA pieces needed the handover type defined
> > in some form, it made sense to include PROT_SEPERATED hardcoded here,
> > but stopped short of adding se_dev->prot_handler for the first round
> > merge.
> >
> > --nab
> >
> 
> Actually they don't, I just added them in iSER code to demonstrate the 
> HW ability.
> If we are not planning to support that (although as MKP mentioned it 
> might be useful in some cases),
> you can remove that for now and we can add it in the future - iSER can 
> ignore it for now (I'll refactor the patches).
> 

<nod>

I'll likely leave this in for the initial merge to avoid rebasing
target-pending/for-next now that the merge window is open.

Let's drop this bit in a separate incremental patch.

--nab

[PATCH-v2 04/17] target/sbc: Add DIF TYPE1+TYPE3 read/write verify emulation

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for DIF read/write verify emulation
for TARGET_DIF_TYPE1_PROT + TARGET_DIF_TYPE3_PROT operation.

This includes sbc_dif_verify_write() + sbc_dif_verify_read()
calls accessable by backend drivers to perform DIF verify
for SGL based data and protection information.

Also included is sbc_dif_copy_prot() logic to copy protection
information to/from backend provided protection SGLs.

Based on scsi_debug.c DIF TYPE1+TYPE3 emulation.

v2 changes:
  - Select CRC_T10DIF for TARGET_CORE in Kconfig (Fengguang)
  - Drop IP checksum logic from sbc_dif_v1_verify (MKP)
  - Fix offset on app_tag = 0xffff in sbc_dif_verify_read()

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/Kconfig               |    1 +
 drivers/target/target_core_sbc.c     |  178 ++++++++++++++++++++++++++++++++++
 include/target/target_core_backend.h |    4 +
 3 files changed, 183 insertions(+)

diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 1830368..50aad2e 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -3,6 +3,7 @@ menuconfig TARGET_CORE
 	tristate "Generic Target Core Mod (TCM) and ConfigFS Infrastructure"
 	depends on SCSI && BLOCK
 	select CONFIGFS_FS
+	select CRC_T10DIF
 	default n
 	help
 	Say Y or M here to enable the TCM Storage Engine and ConfigFS enabled
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 91a92f3..26e8bfb 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/ratelimit.h>
+#include <linux/crc-t10dif.h>
 #include <asm/unaligned.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_tcq.h>
@@ -1024,3 +1025,180 @@ err:
 	return ret;
 }
 EXPORT_SYMBOL(sbc_execute_unmap);
+
+static sense_reason_t
+sbc_dif_v1_verify(struct se_device *dev, struct se_dif_v1_tuple *sdt,
+		  const void *p, sector_t sector, unsigned int ei_lba)
+{
+	int block_size = dev->dev_attrib.block_size;
+	__be16 csum;
+
+	csum = cpu_to_be16(crc_t10dif(p, block_size));
+
+	if (sdt->guard_tag != csum) {
+		pr_err("DIFv1 checksum failed on sector %llu guard tag 0x%04x"
+			" csum 0x%04x\n", (unsigned long long)sector,
+			be16_to_cpu(sdt->guard_tag), be16_to_cpu(csum));
+		return TCM_LOGICAL_BLOCK_GUARD_CHECK_FAILED;
+	}
+
+	if (dev->dev_attrib.pi_prot_type == TARGET_DIF_TYPE1_PROT &&
+	    be32_to_cpu(sdt->ref_tag) != (sector & 0xffffffff)) {
+		pr_err("DIFv1 Type 1 reference failed on sector: %llu tag: 0x%08x"
+		       " sector MSB: 0x%08x\n", (unsigned long long)sector,
+		       be32_to_cpu(sdt->ref_tag), (u32)(sector & 0xffffffff));
+		return TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED;
+	}
+
+	if (dev->dev_attrib.pi_prot_type == TARGET_DIF_TYPE2_PROT &&
+	    be32_to_cpu(sdt->ref_tag) != ei_lba) {
+		pr_err("DIFv1 Type 2 reference failed on sector: %llu tag: 0x%08x"
+		       " ei_lba: 0x%08x\n", (unsigned long long)sector,
+			be32_to_cpu(sdt->ref_tag), ei_lba);
+		return TCM_LOGICAL_BLOCK_REF_TAG_CHECK_FAILED;
+	}
+
+	return 0;
+}
+
+static void
+sbc_dif_copy_prot(struct se_cmd *cmd, unsigned int sectors, bool read,
+		  struct scatterlist *sg, int sg_off)
+{
+	struct se_device *dev = cmd->se_dev;
+	struct scatterlist *psg;
+	void *paddr, *addr;
+	unsigned int i, len, left;
+
+	left = sectors * dev->prot_length;
+
+	for_each_sg(cmd->t_prot_sg, psg, cmd->t_prot_nents, i) {
+
+		len = min(psg->length, left);
+		paddr = kmap_atomic(sg_page(psg)) + psg->offset;
+		addr = kmap_atomic(sg_page(sg)) + sg_off;
+
+		if (read)
+			memcpy(paddr, addr, len);
+		else
+			memcpy(addr, paddr, len);
+
+		left -= len;
+		kunmap_atomic(paddr);
+		kunmap_atomic(addr);
+	}
+}
+
+sense_reason_t
+sbc_dif_verify_write(struct se_cmd *cmd, sector_t start, unsigned int sectors,
+		     unsigned int ei_lba, struct scatterlist *sg, int sg_off)
+{
+	struct se_device *dev = cmd->se_dev;
+	struct se_dif_v1_tuple *sdt;
+	struct scatterlist *dsg, *psg = cmd->t_prot_sg;
+	sector_t sector = start;
+	void *daddr, *paddr;
+	int i, j, offset = 0;
+	sense_reason_t rc;
+
+	for_each_sg(cmd->t_data_sg, dsg, cmd->t_data_nents, i) {
+		daddr = kmap_atomic(sg_page(dsg)) + dsg->offset;
+		paddr = kmap_atomic(sg_page(psg)) + psg->offset;
+
+		for (j = 0; j < dsg->length; j += dev->dev_attrib.block_size) {
+
+			if (offset >= psg->length) {
+				kunmap_atomic(paddr);
+				psg = sg_next(psg);
+				paddr = kmap_atomic(sg_page(psg)) + psg->offset;
+				offset = 0;
+			}
+
+			sdt = paddr + offset;
+
+			pr_debug("DIF WRITE sector: %llu guard_tag: 0x%04x"
+				 " app_tag: 0x%04x ref_tag: %u\n",
+				 (unsigned long long)sector, sdt->guard_tag,
+				 sdt->app_tag, be32_to_cpu(sdt->ref_tag));
+
+			rc = sbc_dif_v1_verify(dev, sdt, daddr + j, sector,
+					       ei_lba);
+			if (rc) {
+				kunmap_atomic(paddr);
+				kunmap_atomic(daddr);
+				return rc;
+			}
+
+			sector++;
+			ei_lba++;
+			offset += sizeof(struct se_dif_v1_tuple);
+		}
+
+		kunmap_atomic(paddr);
+		kunmap_atomic(daddr);
+	}
+	sbc_dif_copy_prot(cmd, sectors, false, sg, sg_off);
+
+	return 0;
+}
+EXPORT_SYMBOL(sbc_dif_verify_write);
+
+sense_reason_t
+sbc_dif_verify_read(struct se_cmd *cmd, sector_t start, unsigned int sectors,
+		    unsigned int ei_lba, struct scatterlist *sg, int sg_off)
+{
+	struct se_device *dev = cmd->se_dev;
+	struct se_dif_v1_tuple *sdt;
+	struct scatterlist *dsg;
+	sector_t sector = start;
+	void *daddr, *paddr;
+	int i, j, offset = sg_off;
+	sense_reason_t rc;
+
+	for_each_sg(cmd->t_data_sg, dsg, cmd->t_data_nents, i) {
+		daddr = kmap_atomic(sg_page(dsg)) + dsg->offset;
+		paddr = kmap_atomic(sg_page(sg)) + sg->offset;
+
+		for (j = 0; j < dsg->length; j += dev->dev_attrib.block_size) {
+
+			if (offset >= sg->length) {
+				kunmap_atomic(paddr);
+				sg = sg_next(sg);
+				paddr = kmap_atomic(sg_page(sg)) + sg->offset;
+				offset = 0;
+			}
+
+			sdt = paddr + offset;
+
+			pr_debug("DIF READ sector: %llu guard_tag: 0x%04x"
+				 " app_tag: 0x%04x ref_tag: %u\n",
+				 (unsigned long long)sector, sdt->guard_tag,
+				 sdt->app_tag, be32_to_cpu(sdt->ref_tag));
+
+			if (sdt->app_tag == cpu_to_be16(0xffff)) {
+				sector++;
+				offset += sizeof(struct se_dif_v1_tuple);
+				continue;
+			}
+
+			rc = sbc_dif_v1_verify(dev, sdt, daddr + j, sector,
+					       ei_lba);
+			if (rc) {
+				kunmap_atomic(paddr);
+				kunmap_atomic(daddr);
+				return rc;
+			}
+
+			sector++;
+			ei_lba++;
+			offset += sizeof(struct se_dif_v1_tuple);
+		}
+
+		kunmap_atomic(paddr);
+		kunmap_atomic(daddr);
+	}
+	sbc_dif_copy_prot(cmd, sectors, true, sg, sg_off);
+
+	return 0;
+}
+EXPORT_SYMBOL(sbc_dif_verify_read);
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index 0dc2745..7020e33 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -73,6 +73,10 @@ sense_reason_t sbc_execute_unmap(struct se_cmd *cmd,
 	sense_reason_t (*do_unmap_fn)(struct se_cmd *cmd, void *priv,
 				      sector_t lba, sector_t nolb),
 	void *priv);
+sense_reason_t	sbc_dif_verify_write(struct se_cmd *, sector_t, unsigned int,
+				     unsigned int, struct scatterlist *, int);
+sense_reason_t	sbc_dif_verify_read(struct se_cmd *, sector_t, unsigned int,
+				    unsigned int, struct scatterlist *, int);
 
 void	transport_set_vpd_proto_id(struct t10_vpd *, unsigned char *);
 int	transport_set_vpd_assoc(struct t10_vpd *, unsigned char *);
-- 
1.7.10.4

[PATCH-v2 05/17] target/spc: Add protection bit to standard INQUIRY output

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates spc_emulate_inquiry_std() to set the
PROTECT bit when DIF emulation is enabled by the backend
device.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_spc.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 279d260..4178c2a 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -100,6 +100,11 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
 	 */
 	if (dev->dev_attrib.emulate_3pc)
 		buf[5] |= 0x8;
+	/*
+	 * Set Protection (PROTECT) bit when DIF has been enabled.
+	 */
+	if (dev->dev_attrib.pi_prot_type)
+		buf[5] |= 0x1;
 
 	buf[7] = 0x2; /* CmdQue=1 */
 
-- 
1.7.10.4

[PATCH-v2 06/17] target/spc: Add protection related bits to INQUIRY EVPD=0x86

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates spc_emulate_evpd_86() (extended INQUIRY) to
report GRD_CHK (Guard Check) and REF_CHK (Reference Check) bits
when DIF emulation is enabled by the backend device.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_spc.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 4178c2a..73fdff5 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -475,6 +475,15 @@ spc_emulate_evpd_86(struct se_cmd *cmd, unsigned char *buf)
 	struct se_device *dev = cmd->se_dev;
 
 	buf[3] = 0x3c;
+	/*
+	 * Set GRD_CHK + REF_CHK for TYPE1 protection, or GRD_CHK
+	 * only for TYPE3 protection.
+	 */
+	if (dev->dev_attrib.pi_prot_type == TARGET_DIF_TYPE1_PROT)
+		buf[4] = 0x5;
+	else if (dev->dev_attrib.pi_prot_type == TARGET_DIF_TYPE3_PROT)
+		buf[4] = 0x4;
+
 	/* Set HEADSUP, ORDSUP, SIMPSUP */
 	buf[5] = 0x07;
 
-- 
1.7.10.4

[PATCH-v2 07/17] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates sbc_emulate_readcapacity_16() to set
P_TYPE and PROT_EN bits when DIF emulation is enabled by
the backend device.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_sbc.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 26e8bfb..75364c7 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -106,6 +106,11 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd)
 	buf[9] = (dev->dev_attrib.block_size >> 16) & 0xff;
 	buf[10] = (dev->dev_attrib.block_size >> 8) & 0xff;
 	buf[11] = dev->dev_attrib.block_size & 0xff;
+	/*
+	 * Set P_TYPE and PROT_EN bits for DIF support
+	 */
+	if (dev->dev_attrib.pi_prot_type)
+		buf[12] = (dev->dev_attrib.pi_prot_type - 1) << 1 | 0x1;
 
 	if (dev->transport->get_lbppbe)
 		buf[13] = dev->transport->get_lbppbe(dev) & 0x0f;
-- 
1.7.10.4

[PATCH-v2 08/17] target/spc: Expose ATO bit in control mode page

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates spc_modesense_control() to set the Application
Tag Owner (ATO) bit when when DIF emulation is enabled by the
backend device.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_spc.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 73fdff5..43c5ca98 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -858,6 +858,19 @@ static int spc_modesense_control(struct se_device *dev, u8 pc, u8 *p)
 	 * status (see SAM-4).
 	 */
 	p[5] = (dev->dev_attrib.emulate_tas) ? 0x40 : 0x00;
+	/*
+	 * From spc4r30, section 7.5.7 Control mode page
+	 *
+	 * Application Tag Owner (ATO) bit set to one.
+	 *
+	 * If the ATO bit is set to one the device server shall not modify the
+	 * LOGICAL BLOCK APPLICATION TAG field and, depending on the protection
+	 * type, shall not modify the contents of the LOGICAL BLOCK REFERENCE
+	 * TAG field.
+	 */
+	if (dev->dev_attrib.pi_prot_type)
+		p[5] |= 0x80;
+
 	p[8] = 0xff;
 	p[9] = 0xff;
 	p[11] = 30;
-- 
1.7.10.4

[PATCH-v2 09/17] target/configfs: Expose protection device attributes

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for exposing DIF protection device
attributes via configfs.  This includes:

   pi_prot_type: Protection Type (0, 1, 3 currently support)
   pi_prot_format: Protection Format Operation (FILEIO only)

Within se_dev_set_pi_prot_type() it also adds the se_subsystem_api
device callbacks to setup per device protection information.

v2 changes:
  - Drop pi_guard_type + pi_prot_version related code (MKP)
  - Add pi_prot_format logic (Sagi)
  - Add ->free_prot callback in target_free_device
  - Add hw_pi_prot_type read-only attribute

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_configfs.c |   12 +++++
 drivers/target/target_core_device.c   |   89 +++++++++++++++++++++++++++++++++
 drivers/target/target_core_internal.h |    2 +
 3 files changed, 103 insertions(+)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 5cf6135..f0e85b1 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -643,6 +643,15 @@ SE_DEV_ATTR(emulate_caw, S_IRUGO | S_IWUSR);
 DEF_DEV_ATTRIB(emulate_3pc);
 SE_DEV_ATTR(emulate_3pc, S_IRUGO | S_IWUSR);
 
+DEF_DEV_ATTRIB(pi_prot_type);
+SE_DEV_ATTR(pi_prot_type, S_IRUGO | S_IWUSR);
+
+DEF_DEV_ATTRIB_RO(hw_pi_prot_type);
+SE_DEV_ATTR_RO(hw_pi_prot_type);
+
+DEF_DEV_ATTRIB(pi_prot_format);
+SE_DEV_ATTR(pi_prot_format, S_IRUGO | S_IWUSR);
+
 DEF_DEV_ATTRIB(enforce_pr_isids);
 SE_DEV_ATTR(enforce_pr_isids, S_IRUGO | S_IWUSR);
 
@@ -702,6 +711,9 @@ static struct configfs_attribute *target_core_dev_attrib_attrs[] = {
 	&target_core_dev_attrib_emulate_tpws.attr,
 	&target_core_dev_attrib_emulate_caw.attr,
 	&target_core_dev_attrib_emulate_3pc.attr,
+	&target_core_dev_attrib_pi_prot_type.attr,
+	&target_core_dev_attrib_hw_pi_prot_type.attr,
+	&target_core_dev_attrib_pi_prot_format.attr,
 	&target_core_dev_attrib_enforce_pr_isids.attr,
 	&target_core_dev_attrib_is_nonrot.attr,
 	&target_core_dev_attrib_emulate_rest_reord.attr,
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 3244058..883099e 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -918,6 +918,90 @@ int se_dev_set_emulate_3pc(struct se_device *dev, int flag)
 	return 0;
 }
 
+int se_dev_set_pi_prot_type(struct se_device *dev, int flag)
+{
+	int rc, old_prot = dev->dev_attrib.pi_prot_type;
+
+	if (flag != 0 && flag != 1 && flag != 2 && flag != 3) {
+		pr_err("Illegal value %d for pi_prot_type\n", flag);
+		return -EINVAL;
+	}
+	if (flag == 2) {
+		pr_err("DIF TYPE2 protection currently not supported\n");
+		return -ENOSYS;
+	}
+	if (dev->dev_attrib.hw_pi_prot_type) {
+		pr_warn("DIF protection enabled on underlying hardware,"
+			" ignoring\n");
+		return 0;
+	}
+	if (!dev->transport->init_prot || !dev->transport->free_prot) {
+		pr_err("DIF protection not supported by backend: %s\n",
+		       dev->transport->name);
+		return -ENOSYS;
+	}
+	if (!(dev->dev_flags & DF_CONFIGURED)) {
+		pr_err("DIF protection requires device to be configured\n");
+		return -ENODEV;
+	}
+	if (dev->export_count) {
+		pr_err("dev[%p]: Unable to change SE Device PROT type while"
+		       " export_count is %d\n", dev, dev->export_count);
+		return -EINVAL;
+	}
+
+	dev->dev_attrib.pi_prot_type = flag;
+
+	if (flag && !old_prot) {
+		rc = dev->transport->init_prot(dev);
+		if (rc) {
+			dev->dev_attrib.pi_prot_type = old_prot;
+			return rc;
+		}
+
+	} else if (!flag && old_prot) {
+		dev->transport->free_prot(dev);
+	}
+	pr_debug("dev[%p]: SE Device Protection Type: %d\n", dev, flag);
+
+	return 0;
+}
+
+int se_dev_set_pi_prot_format(struct se_device *dev, int flag)
+{
+	int rc;
+
+	if (!flag)
+		return 0;
+
+	if (flag != 1) {
+		pr_err("Illegal value %d for pi_prot_format\n", flag);
+		return -EINVAL;
+	}
+	if (!dev->transport->format_prot) {
+		pr_err("DIF protection format not supported by backend %s\n",
+		       dev->transport->name);
+		return -ENOSYS;
+	}
+	if (!(dev->dev_flags & DF_CONFIGURED)) {
+		pr_err("DIF protection format requires device to be configured\n");
+		return -ENODEV;
+	}
+	if (dev->export_count) {
+		pr_err("dev[%p]: Unable to format SE Device PROT type while"
+		       " export_count is %d\n", dev, dev->export_count);
+		return -EINVAL;
+	}
+
+	rc = dev->transport->format_prot(dev);
+	if (rc)
+		return rc;
+
+	pr_debug("dev[%p]: SE Device Protection Format complete\n", dev);
+
+	return 0;
+}
+
 int se_dev_set_enforce_pr_isids(struct se_device *dev, int flag)
 {
 	if ((flag != 0) && (flag != 1)) {
@@ -1415,6 +1499,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 	dev->dev_link_magic = SE_DEV_LINK_MAGIC;
 	dev->se_hba = hba;
 	dev->transport = hba->transport;
+	dev->prot_length = sizeof(struct se_dif_v1_tuple);
 
 	INIT_LIST_HEAD(&dev->dev_list);
 	INIT_LIST_HEAD(&dev->dev_sep_list);
@@ -1457,6 +1542,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 	dev->dev_attrib.emulate_tpws = DA_EMULATE_TPWS;
 	dev->dev_attrib.emulate_caw = DA_EMULATE_CAW;
 	dev->dev_attrib.emulate_3pc = DA_EMULATE_3PC;
+	dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE0_PROT;
 	dev->dev_attrib.enforce_pr_isids = DA_ENFORCE_PR_ISIDS;
 	dev->dev_attrib.is_nonrot = DA_IS_NONROT;
 	dev->dev_attrib.emulate_rest_reord = DA_EMULATE_REST_REORD;
@@ -1589,6 +1675,9 @@ void target_free_device(struct se_device *dev)
 	core_scsi3_free_all_registrations(dev);
 	se_release_vpd_for_dev(dev);
 
+	if (dev->transport->free_prot)
+		dev->transport->free_prot(dev);
+
 	dev->transport->free_device(dev);
 }
 
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 1e27614..de9cab7 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -35,6 +35,8 @@ int	se_dev_set_emulate_tpu(struct se_device *, int);
 int	se_dev_set_emulate_tpws(struct se_device *, int);
 int	se_dev_set_emulate_caw(struct se_device *, int);
 int	se_dev_set_emulate_3pc(struct se_device *, int);
+int	se_dev_set_pi_prot_type(struct se_device *, int);
+int	se_dev_set_pi_prot_format(struct se_device *, int);
 int	se_dev_set_enforce_pr_isids(struct se_device *, int);
 int	se_dev_set_is_nonrot(struct se_device *, int);
 int	se_dev_set_emulate_rest_reord(struct se_device *dev, int);
-- 
1.7.10.4

[PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support to target_submit_cmd_map_sgls() for
accepting 'sgl_prot' + 'sgl_prot_count' parameters for
DIF protection information.

Note the passed parameters are stored at se_cmd->t_prot_sg
and se_cmd->t_prot_nents respectively.

Also, update tcm_loop and vhost-scsi fabrics usage of
target_submit_cmd_map_sgls() to take into account the
new parameters.

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/loopback/tcm_loop.c     |    2 +-
 drivers/target/target_core_transport.c |   16 ++++++++++++++--
 drivers/vhost/scsi.c                   |    2 +-
 include/target/target_core_fabric.h    |    3 ++-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 763ee45..112b795 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -217,7 +217,7 @@ static void tcm_loop_submission_work(struct work_struct *work)
 			scsi_bufflen(sc), tcm_loop_sam_attr(sc),
 			sc->sc_data_direction, 0,
 			scsi_sglist(sc), scsi_sg_count(sc),
-			sgl_bidi, sgl_bidi_count);
+			sgl_bidi, sgl_bidi_count, NULL, 0);
 	if (rc < 0) {
 		set_host_byte(sc, DID_NO_CONNECT);
 		goto out_done;
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index fa4fc04..aebe0bb 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1310,6 +1310,8 @@ transport_generic_map_mem_to_cmd(struct se_cmd *cmd, struct scatterlist *sgl,
  * @sgl_count: scatterlist count for unidirectional mapping
  * @sgl_bidi: struct scatterlist memory for bidirectional READ mapping
  * @sgl_bidi_count: scatterlist count for bidirectional READ mapping
+ * @sgl_prot: struct scatterlist memory protection information
+ * @sgl_prot_count: scatterlist count for protection information
  *
  * Returns non zero to signal active I/O shutdown failure.  All other
  * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
@@ -1322,7 +1324,8 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
 		unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
 		u32 data_length, int task_attr, int data_dir, int flags,
 		struct scatterlist *sgl, u32 sgl_count,
-		struct scatterlist *sgl_bidi, u32 sgl_bidi_count)
+		struct scatterlist *sgl_bidi, u32 sgl_bidi_count,
+		struct scatterlist *sgl_prot, u32 sgl_prot_count)
 {
 	struct se_portal_group *se_tpg;
 	sense_reason_t rc;
@@ -1364,6 +1367,14 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
 		target_put_sess_cmd(se_sess, se_cmd);
 		return 0;
 	}
+	/*
+	 * Save pointers for SGLs containing protection information,
+	 * if present.
+	 */
+	if (sgl_prot_count) {
+		se_cmd->t_prot_sg = sgl_prot;
+		se_cmd->t_prot_nents = sgl_prot_count;
+	}
 
 	rc = target_setup_cmd_from_cdb(se_cmd, cdb);
 	if (rc != 0) {
@@ -1406,6 +1417,7 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
 			return 0;
 		}
 	}
+
 	/*
 	 * Check if we need to delay processing because of ALUA
 	 * Active/NonOptimized primary access state..
@@ -1445,7 +1457,7 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
 {
 	return target_submit_cmd_map_sgls(se_cmd, se_sess, cdb, sense,
 			unpacked_lun, data_length, task_attr, data_dir,
-			flags, NULL, 0, NULL, 0);
+			flags, NULL, 0, NULL, 0, NULL, 0);
 }
 EXPORT_SYMBOL(target_submit_cmd);
 
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index f175629..84488a8 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -889,7 +889,7 @@ static void tcm_vhost_submission_work(struct work_struct *work)
 			cmd->tvc_lun, cmd->tvc_exp_data_len,
 			cmd->tvc_task_attr, cmd->tvc_data_direction,
 			TARGET_SCF_ACK_KREF, sg_ptr, cmd->tvc_sgl_count,
-			sg_bidi_ptr, sg_no_bidi);
+			sg_bidi_ptr, sg_no_bidi, NULL, 0);
 	if (rc < 0) {
 		transport_send_check_condition_and_sense(se_cmd,
 				TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index 4cf4fda..0218d68 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -105,7 +105,8 @@ sense_reason_t transport_lookup_cmd_lun(struct se_cmd *, u32);
 sense_reason_t target_setup_cmd_from_cdb(struct se_cmd *, unsigned char *);
 int	target_submit_cmd_map_sgls(struct se_cmd *, struct se_session *,
 		unsigned char *, unsigned char *, u32, u32, int, int, int,
-		struct scatterlist *, u32, struct scatterlist *, u32);
+		struct scatterlist *, u32, struct scatterlist *, u32,
+		struct scatterlist *, u32);
 int	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
 		unsigned char *, u32, u32, int, int, int);
 int	target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
-- 
1.7.10.4

Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds support to target_submit_cmd_map_sgls() for
> accepting 'sgl_prot' + 'sgl_prot_count' parameters for
> DIF protection information.
>
> Note the passed parameters are stored at se_cmd->t_prot_sg
> and se_cmd->t_prot_nents respectively.
>
> Also, update tcm_loop and vhost-scsi fabrics usage of
> target_submit_cmd_map_sgls() to take into account the
> new parameters.

I didn't see that you added protection allocation to transports that 
does not use
target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :(

Don't you think that prot SG allocation should be added also to 
target_alloc_sgl()? by then
se_cmd should contain the protection attributes and this routine can 
know if it needs to
allocate prot_sg as well. This is how I used it...

> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/loopback/tcm_loop.c     |    2 +-
>   drivers/target/target_core_transport.c |   16 ++++++++++++++--
>   drivers/vhost/scsi.c                   |    2 +-
>   include/target/target_core_fabric.h    |    3 ++-
>   4 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
> index 763ee45..112b795 100644
> --- a/drivers/target/loopback/tcm_loop.c
> +++ b/drivers/target/loopback/tcm_loop.c
> @@ -217,7 +217,7 @@ static void tcm_loop_submission_work(struct work_struct *work)
>   			scsi_bufflen(sc), tcm_loop_sam_attr(sc),
>   			sc->sc_data_direction, 0,
>   			scsi_sglist(sc), scsi_sg_count(sc),
> -			sgl_bidi, sgl_bidi_count);
> +			sgl_bidi, sgl_bidi_count, NULL, 0);
>   	if (rc < 0) {
>   		set_host_byte(sc, DID_NO_CONNECT);
>   		goto out_done;
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index fa4fc04..aebe0bb 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -1310,6 +1310,8 @@ transport_generic_map_mem_to_cmd(struct se_cmd *cmd, struct scatterlist *sgl,
>    * @sgl_count: scatterlist count for unidirectional mapping
>    * @sgl_bidi: struct scatterlist memory for bidirectional READ mapping
>    * @sgl_bidi_count: scatterlist count for bidirectional READ mapping
> + * @sgl_prot: struct scatterlist memory protection information
> + * @sgl_prot_count: scatterlist count for protection information
>    *
>    * Returns non zero to signal active I/O shutdown failure.  All other
>    * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
> @@ -1322,7 +1324,8 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
>   		unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
>   		u32 data_length, int task_attr, int data_dir, int flags,
>   		struct scatterlist *sgl, u32 sgl_count,
> -		struct scatterlist *sgl_bidi, u32 sgl_bidi_count)
> +		struct scatterlist *sgl_bidi, u32 sgl_bidi_count,
> +		struct scatterlist *sgl_prot, u32 sgl_prot_count)
>   {
>   	struct se_portal_group *se_tpg;
>   	sense_reason_t rc;
> @@ -1364,6 +1367,14 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
>   		target_put_sess_cmd(se_sess, se_cmd);
>   		return 0;
>   	}
> +	/*
> +	 * Save pointers for SGLs containing protection information,
> +	 * if present.
> +	 */
> +	if (sgl_prot_count) {
> +		se_cmd->t_prot_sg = sgl_prot;
> +		se_cmd->t_prot_nents = sgl_prot_count;
> +	}
>   
>   	rc = target_setup_cmd_from_cdb(se_cmd, cdb);
>   	if (rc != 0) {
> @@ -1406,6 +1417,7 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess
>   			return 0;
>   		}
>   	}
> +
>   	/*
>   	 * Check if we need to delay processing because of ALUA
>   	 * Active/NonOptimized primary access state..
> @@ -1445,7 +1457,7 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
>   {
>   	return target_submit_cmd_map_sgls(se_cmd, se_sess, cdb, sense,
>   			unpacked_lun, data_length, task_attr, data_dir,
> -			flags, NULL, 0, NULL, 0);
> +			flags, NULL, 0, NULL, 0, NULL, 0);
>   }
>   EXPORT_SYMBOL(target_submit_cmd);
>   
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index f175629..84488a8 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -889,7 +889,7 @@ static void tcm_vhost_submission_work(struct work_struct *work)
>   			cmd->tvc_lun, cmd->tvc_exp_data_len,
>   			cmd->tvc_task_attr, cmd->tvc_data_direction,
>   			TARGET_SCF_ACK_KREF, sg_ptr, cmd->tvc_sgl_count,
> -			sg_bidi_ptr, sg_no_bidi);
> +			sg_bidi_ptr, sg_no_bidi, NULL, 0);
>   	if (rc < 0) {
>   		transport_send_check_condition_and_sense(se_cmd,
>   				TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
> diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
> index 4cf4fda..0218d68 100644
> --- a/include/target/target_core_fabric.h
> +++ b/include/target/target_core_fabric.h
> @@ -105,7 +105,8 @@ sense_reason_t transport_lookup_cmd_lun(struct se_cmd *, u32);
>   sense_reason_t target_setup_cmd_from_cdb(struct se_cmd *, unsigned char *);
>   int	target_submit_cmd_map_sgls(struct se_cmd *, struct se_session *,
>   		unsigned char *, unsigned char *, u32, u32, int, int, int,
> -		struct scatterlist *, u32, struct scatterlist *, u32);
> +		struct scatterlist *, u32, struct scatterlist *, u32,
> +		struct scatterlist *, u32);
>   int	target_submit_cmd(struct se_cmd *, struct se_session *, unsigned char *,
>   		unsigned char *, u32, u32, int, int, int);
>   int	target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,

Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls

[Nicholas A. Bellinger]

On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote:
> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > This patch adds support to target_submit_cmd_map_sgls() for
> > accepting 'sgl_prot' + 'sgl_prot_count' parameters for
> > DIF protection information.
> >
> > Note the passed parameters are stored at se_cmd->t_prot_sg
> > and se_cmd->t_prot_nents respectively.
> >
> > Also, update tcm_loop and vhost-scsi fabrics usage of
> > target_submit_cmd_map_sgls() to take into account the
> > new parameters.
> 
> I didn't see that you added protection allocation to transports that 
> does not use
> target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :(
> 
> Don't you think that prot SG allocation should be added also to 
> target_alloc_sgl()? by then
> se_cmd should contain the protection attributes and this routine can 
> know if it needs to
> allocate prot_sg as well. This is how I used it...

Yes, this specific bit was left out for the moment as no code in the
patch for v3.14 actually uses it..

I'm planning to add it to for-next -> v3.15 code as soon as the merge
window closes.

--nab

Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls

[Sagi Grimberg]

On 1/22/2014 12:17 AM, Nicholas A. Bellinger wrote:
> On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote:
>> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
>>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>>
>>> This patch adds support to target_submit_cmd_map_sgls() for
>>> accepting 'sgl_prot' + 'sgl_prot_count' parameters for
>>> DIF protection information.
>>>
>>> Note the passed parameters are stored at se_cmd->t_prot_sg
>>> and se_cmd->t_prot_nents respectively.
>>>
>>> Also, update tcm_loop and vhost-scsi fabrics usage of
>>> target_submit_cmd_map_sgls() to take into account the
>>> new parameters.
>> I didn't see that you added protection allocation to transports that
>> does not use
>> target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :(
>>
>> Don't you think that prot SG allocation should be added also to
>> target_alloc_sgl()? by then
>> se_cmd should contain the protection attributes and this routine can
>> know if it needs to
>> allocate prot_sg as well. This is how I used it...
> Yes, this specific bit was left out for the moment as no code in the
> patch for v3.14 actually uses it..
>
> I'm planning to add it to for-next -> v3.15 code as soon as the merge
> window closes.
>
> --nab
>

Yes, that makes sense to me.

Sagi.

[PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds blk_integrity passthrough support for block_device
backends using IBLOCK.

This includes iblock_alloc_bip() + setup of bio_integrity_payload
information that attaches to the leading struct bio once bio_list
is populated during fast-path iblock_execute_rw() I/O dispatch.

It also updates setup in iblock_configure_device() to detect modes
of protection + se dev->dev_attrib.pi_prot_type accordingly, along
with creating required bio_set integrity mempools.

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/Kconfig              |    1 +
 drivers/target/target_core_iblock.c |   91 ++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 50aad2e..dc2d84a 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -14,6 +14,7 @@ if TARGET_CORE
 
 config TCM_IBLOCK
 	tristate "TCM/IBLOCK Subsystem Plugin for Linux/BLOCK"
+	select BLK_DEV_INTEGRITY
 	help
 	Say Y here to enable the TCM/IBLOCK subsystem plugin for non-buffered
 	access to Linux/Block devices using BIO
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 15d9121..293d9b0 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -91,6 +91,7 @@ static int iblock_configure_device(struct se_device *dev)
 	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
 	struct request_queue *q;
 	struct block_device *bd = NULL;
+	struct blk_integrity *bi;
 	fmode_t mode;
 	int ret = -ENOMEM;
 
@@ -155,8 +156,40 @@ static int iblock_configure_device(struct se_device *dev)
 	if (blk_queue_nonrot(q))
 		dev->dev_attrib.is_nonrot = 1;
 
+	bi = bdev_get_integrity(bd);
+	if (bi) {
+		struct bio_set *bs = ib_dev->ibd_bio_set;
+
+		if (!strcmp(bi->name, "T10-DIF-TYPE3-IP") ||
+		    !strcmp(bi->name, "T10-DIF-TYPE1-IP")) {
+			pr_err("IBLOCK export of blk_integrity: %s not"
+			       " supported\n", bi->name);
+			ret = -ENOSYS;
+			goto out_blkdev_put;
+		}
+
+		if (!strcmp(bi->name, "T10-DIF-TYPE3-CRC")) {
+			dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE3_PROT;
+		} else if (!strcmp(bi->name, "T10-DIF-TYPE1-CRC")) {
+			dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE1_PROT;
+		}
+
+		if (dev->dev_attrib.pi_prot_type) {
+			if (bioset_integrity_create(bs, IBLOCK_BIO_POOL_SIZE) < 0) {
+				pr_err("Unable to allocate bioset for PI\n");
+				ret = -ENOMEM;
+				goto out_blkdev_put;
+			}
+			pr_debug("IBLOCK setup BIP bs->bio_integrity_pool: %p\n",
+				 bs->bio_integrity_pool);
+		}
+		dev->dev_attrib.hw_pi_prot_type = dev->dev_attrib.pi_prot_type;
+	}
+
 	return 0;
 
+out_blkdev_put:
+	blkdev_put(ib_dev->ibd_bd, FMODE_WRITE|FMODE_READ|FMODE_EXCL);
 out_free_bioset:
 	bioset_free(ib_dev->ibd_bio_set);
 	ib_dev->ibd_bio_set = NULL;
@@ -170,8 +203,10 @@ static void iblock_free_device(struct se_device *dev)
 
 	if (ib_dev->ibd_bd != NULL)
 		blkdev_put(ib_dev->ibd_bd, FMODE_WRITE|FMODE_READ|FMODE_EXCL);
-	if (ib_dev->ibd_bio_set != NULL)
+	if (ib_dev->ibd_bio_set != NULL) {
+		bioset_integrity_free(ib_dev->ibd_bio_set);
 		bioset_free(ib_dev->ibd_bio_set);
+	}
 	kfree(ib_dev);
 }
 
@@ -586,13 +621,58 @@ static ssize_t iblock_show_configfs_dev_params(struct se_device *dev, char *b)
 	return bl;
 }
 
+static int
+iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio)
+{
+	struct se_device *dev = cmd->se_dev;
+	struct blk_integrity *bi;
+	struct bio_integrity_payload *bip;
+	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
+	struct scatterlist *sg;
+	int i, rc;
+
+	bi = bdev_get_integrity(ib_dev->ibd_bd);
+	if (!bi) {
+		pr_err("Unable to locate bio_integrity\n");
+		return -ENODEV;
+	}
+
+	bip = bio_integrity_alloc(bio, GFP_NOIO, cmd->t_prot_nents);
+	if (!bip) {
+		pr_err("Unable to allocate bio_integrity_payload\n");
+		return -ENOMEM;
+	}
+
+	bip->bip_size = (cmd->data_length / dev->dev_attrib.block_size) *
+			 dev->prot_length;
+	bip->bip_sector = bio->bi_sector;
+
+	pr_debug("IBLOCK BIP Size: %u Sector: %llu\n", bip->bip_size,
+		 (unsigned long long)bip->bip_sector);
+
+	for_each_sg(cmd->t_prot_sg, sg, cmd->t_prot_nents, i) {
+
+		rc = bio_integrity_add_page(bio, sg_page(sg), sg->length,
+					    sg->offset);
+		if (rc != sg->length) {
+			pr_err("bio_integrity_add_page() failed; %d\n", rc);
+			return -ENOMEM;
+		}
+
+		pr_debug("Added bio integrity page: %p length: %d offset; %d\n",
+			 sg_page(sg), sg->length, sg->offset);
+	}
+
+	return 0;
+}
+
 static sense_reason_t
 iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 		  enum dma_data_direction data_direction)
 {
 	struct se_device *dev = cmd->se_dev;
 	struct iblock_req *ibr;
-	struct bio *bio;
+	struct bio *bio, *bio_start;
 	struct bio_list list;
 	struct scatterlist *sg;
 	u32 sg_num = sgl_nents;
@@ -655,6 +735,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	if (!bio)
 		goto fail_free_ibr;
 
+	bio_start = bio;
 	bio_list_init(&list);
 	bio_list_add(&list, bio);
 
@@ -688,6 +769,12 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 		sg_num--;
 	}
 
+	if (cmd->prot_type) {
+		int rc = iblock_alloc_bip(cmd, bio_start);
+		if (rc)
+			goto fail_put_bios;
+	}
+
 	iblock_submit_bios(&list, rw);
 	iblock_complete_cmd(cmd);
 	return 0;
-- 
1.7.10.4

Re: [PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds blk_integrity passthrough support for block_device
> backends using IBLOCK.

Nice!

> This includes iblock_alloc_bip() + setup of bio_integrity_payload
> information that attaches to the leading struct bio once bio_list
> is populated during fast-path iblock_execute_rw() I/O dispatch.
>
> It also updates setup in iblock_configure_device() to detect modes
> of protection + se dev->dev_attrib.pi_prot_type accordingly, along
> with creating required bio_set integrity mempools.
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/Kconfig              |    1 +
>   drivers/target/target_core_iblock.c |   91 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
> index 50aad2e..dc2d84a 100644
> --- a/drivers/target/Kconfig
> +++ b/drivers/target/Kconfig
> @@ -14,6 +14,7 @@ if TARGET_CORE
>   
>   config TCM_IBLOCK
>   	tristate "TCM/IBLOCK Subsystem Plugin for Linux/BLOCK"
> +	select BLK_DEV_INTEGRITY
>   	help
>   	Say Y here to enable the TCM/IBLOCK subsystem plugin for non-buffered
>   	access to Linux/Block devices using BIO
> diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
> index 15d9121..293d9b0 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
> @@ -91,6 +91,7 @@ static int iblock_configure_device(struct se_device *dev)
>   	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
>   	struct request_queue *q;
>   	struct block_device *bd = NULL;
> +	struct blk_integrity *bi;
>   	fmode_t mode;
>   	int ret = -ENOMEM;
>   
> @@ -155,8 +156,40 @@ static int iblock_configure_device(struct se_device *dev)
>   	if (blk_queue_nonrot(q))
>   		dev->dev_attrib.is_nonrot = 1;
>   
> +	bi = bdev_get_integrity(bd);
> +	if (bi) {
> +		struct bio_set *bs = ib_dev->ibd_bio_set;
> +
> +		if (!strcmp(bi->name, "T10-DIF-TYPE3-IP") ||
> +		    !strcmp(bi->name, "T10-DIF-TYPE1-IP")) {
> +			pr_err("IBLOCK export of blk_integrity: %s not"
> +			       " supported\n", bi->name);
> +			ret = -ENOSYS;
> +			goto out_blkdev_put;
> +		}

Please remind me why we ignore IP-CSUM guard type again?
MKP, will this be irrelevant for the initiator as well? if so, I don't 
see a reason to expose this in RDMA verbs.

> +
> +		if (!strcmp(bi->name, "T10-DIF-TYPE3-CRC")) {
> +			dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE3_PROT;
> +		} else if (!strcmp(bi->name, "T10-DIF-TYPE1-CRC")) {
> +			dev->dev_attrib.pi_prot_type = TARGET_DIF_TYPE1_PROT;
> +		}
> +
> +		if (dev->dev_attrib.pi_prot_type) {
> +			if (bioset_integrity_create(bs, IBLOCK_BIO_POOL_SIZE) < 0) {
> +				pr_err("Unable to allocate bioset for PI\n");
> +				ret = -ENOMEM;
> +				goto out_blkdev_put;
> +			}
> +			pr_debug("IBLOCK setup BIP bs->bio_integrity_pool: %p\n",
> +				 bs->bio_integrity_pool);
> +		}
> +		dev->dev_attrib.hw_pi_prot_type = dev->dev_attrib.pi_prot_type;
> +	}
> +
>   	return 0;
>   
> +out_blkdev_put:
> +	blkdev_put(ib_dev->ibd_bd, FMODE_WRITE|FMODE_READ|FMODE_EXCL);
>   out_free_bioset:
>   	bioset_free(ib_dev->ibd_bio_set);
>   	ib_dev->ibd_bio_set = NULL;
> @@ -170,8 +203,10 @@ static void iblock_free_device(struct se_device *dev)
>   
>   	if (ib_dev->ibd_bd != NULL)
>   		blkdev_put(ib_dev->ibd_bd, FMODE_WRITE|FMODE_READ|FMODE_EXCL);
> -	if (ib_dev->ibd_bio_set != NULL)
> +	if (ib_dev->ibd_bio_set != NULL) {
> +		bioset_integrity_free(ib_dev->ibd_bio_set);
>   		bioset_free(ib_dev->ibd_bio_set);
> +	}
>   	kfree(ib_dev);
>   }
>   
> @@ -586,13 +621,58 @@ static ssize_t iblock_show_configfs_dev_params(struct se_device *dev, char *b)
>   	return bl;
>   }
>   
> +static int
> +iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio)
> +{
> +	struct se_device *dev = cmd->se_dev;
> +	struct blk_integrity *bi;
> +	struct bio_integrity_payload *bip;
> +	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
> +	struct scatterlist *sg;
> +	int i, rc;
> +
> +	bi = bdev_get_integrity(ib_dev->ibd_bd);
> +	if (!bi) {
> +		pr_err("Unable to locate bio_integrity\n");
> +		return -ENODEV;
> +	}
> +
> +	bip = bio_integrity_alloc(bio, GFP_NOIO, cmd->t_prot_nents);
> +	if (!bip) {
> +		pr_err("Unable to allocate bio_integrity_payload\n");
> +		return -ENOMEM;
> +	}
> +
> +	bip->bip_size = (cmd->data_length / dev->dev_attrib.block_size) *
> +			 dev->prot_length;
> +	bip->bip_sector = bio->bi_sector;
> +
> +	pr_debug("IBLOCK BIP Size: %u Sector: %llu\n", bip->bip_size,
> +		 (unsigned long long)bip->bip_sector);
> +
> +	for_each_sg(cmd->t_prot_sg, sg, cmd->t_prot_nents, i) {
> +
> +		rc = bio_integrity_add_page(bio, sg_page(sg), sg->length,
> +					    sg->offset);
> +		if (rc != sg->length) {
> +			pr_err("bio_integrity_add_page() failed; %d\n", rc);
> +			return -ENOMEM;
> +		}
> +
> +		pr_debug("Added bio integrity page: %p length: %d offset; %d\n",
> +			 sg_page(sg), sg->length, sg->offset);
> +	}
> +
> +	return 0;
> +}
> +
>   static sense_reason_t
>   iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   		  enum dma_data_direction data_direction)
>   {
>   	struct se_device *dev = cmd->se_dev;
>   	struct iblock_req *ibr;
> -	struct bio *bio;
> +	struct bio *bio, *bio_start;
>   	struct bio_list list;
>   	struct scatterlist *sg;
>   	u32 sg_num = sgl_nents;
> @@ -655,6 +735,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   	if (!bio)
>   		goto fail_free_ibr;
>   
> +	bio_start = bio;
>   	bio_list_init(&list);
>   	bio_list_add(&list, bio);
>   
> @@ -688,6 +769,12 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   		sg_num--;
>   	}
>   
> +	if (cmd->prot_type) {
> +		int rc = iblock_alloc_bip(cmd, bio_start);
> +		if (rc)
> +			goto fail_put_bios;
> +	}
> +
>   	iblock_submit_bios(&list, rw);
>   	iblock_complete_cmd(cmd);
>   	return 0;

Re: [PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Nicholas A. Bellinger]

On Sun, 2014-01-19 at 14:21 +0200, Sagi Grimberg wrote:
> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > This patch adds blk_integrity passthrough support for block_device
> > backends using IBLOCK.
> 
> Nice!
> 
> > This includes iblock_alloc_bip() + setup of bio_integrity_payload
> > information that attaches to the leading struct bio once bio_list
> > is populated during fast-path iblock_execute_rw() I/O dispatch.
> >
> > It also updates setup in iblock_configure_device() to detect modes
> > of protection + se dev->dev_attrib.pi_prot_type accordingly, along
> > with creating required bio_set integrity mempools.
> >
> > Cc: Martin K. Petersen <martin.petersen@oracle.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Sagi Grimberg <sagig@mellanox.com>
> > Cc: Or Gerlitz <ogerlitz@mellanox.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >   drivers/target/Kconfig              |    1 +
> >   drivers/target/target_core_iblock.c |   91 ++++++++++++++++++++++++++++++++++-
> >   2 files changed, 90 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
> > index 50aad2e..dc2d84a 100644
> > --- a/drivers/target/Kconfig
> > +++ b/drivers/target/Kconfig
> > @@ -14,6 +14,7 @@ if TARGET_CORE
> >   
> >   config TCM_IBLOCK
> >   	tristate "TCM/IBLOCK Subsystem Plugin for Linux/BLOCK"
> > +	select BLK_DEV_INTEGRITY
> >   	help
> >   	Say Y here to enable the TCM/IBLOCK subsystem plugin for non-buffered
> >   	access to Linux/Block devices using BIO
> > diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
> > index 15d9121..293d9b0 100644
> > --- a/drivers/target/target_core_iblock.c
> > +++ b/drivers/target/target_core_iblock.c
> > @@ -91,6 +91,7 @@ static int iblock_configure_device(struct se_device *dev)
> >   	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
> >   	struct request_queue *q;
> >   	struct block_device *bd = NULL;
> > +	struct blk_integrity *bi;
> >   	fmode_t mode;
> >   	int ret = -ENOMEM;
> >   
> > @@ -155,8 +156,40 @@ static int iblock_configure_device(struct se_device *dev)
> >   	if (blk_queue_nonrot(q))
> >   		dev->dev_attrib.is_nonrot = 1;
> >   
> > +	bi = bdev_get_integrity(bd);
> > +	if (bi) {
> > +		struct bio_set *bs = ib_dev->ibd_bio_set;
> > +
> > +		if (!strcmp(bi->name, "T10-DIF-TYPE3-IP") ||
> > +		    !strcmp(bi->name, "T10-DIF-TYPE1-IP")) {
> > +			pr_err("IBLOCK export of blk_integrity: %s not"
> > +			       " supported\n", bi->name);
> > +			ret = -ENOSYS;
> > +			goto out_blkdev_put;
> > +		}
> 
> Please remind me why we ignore IP-CSUM guard type again?
> MKP, will this be irrelevant for the initiator as well? if so, I don't 
> see a reason to expose this in RDMA verbs.

My understanding is that this was used for pre-production prototyping,
and is not supported by any real backend storage hardware.

--nab

Re: [PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Martin K. Petersen]

>>>>> "nab" == Nicholas A Bellinger <nab@linux-iscsi.org> writes:

>> Please remind me why we ignore IP-CSUM guard type again?  MKP, will
>> this be irrelevant for the initiator as well? if so, I don't see a
>> reason to expose this in RDMA verbs.

nab> My understanding is that this was used for pre-production
nab> prototyping, and is not supported by any real backend storage
nab> hardware.

We are shipping several products with support for the IP checksum. But
it's between application and initiator only. From there on it's all T10
CRC as required by the spec.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Martin K. Petersen]

>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:

Sagi> Please remind me why we ignore IP-CSUM guard type again?  MKP,
Sagi> will this be irrelevant for the initiator as well? if so, I don't
Sagi> see a reason to expose this in RDMA verbs.

I don't see much use for IP checksum for the target. You are required by
SBC to use T10 CRC on the wire so there is no point in converting to IP
checksum in the backend.

My impending patches will allow you to pass through PI with T10 CRC to a
device with an IP checksum block integrity profile (i.e. the choice of
checksum is a per-bio bip flag instead of an HBA-enforced global).

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH-v2 11/17] target/iblock: Add blk_integrity + BIP passthrough support

[Sagi Grimberg]

On 1/22/2014 3:52 AM, Martin K. Petersen wrote:
>>>>>> "Sagi" == Sagi Grimberg <sagig@dev.mellanox.co.il> writes:
> Sagi> Please remind me why we ignore IP-CSUM guard type again?  MKP,
> Sagi> will this be irrelevant for the initiator as well? if so, I don't
> Sagi> see a reason to expose this in RDMA verbs.
>
> I don't see much use for IP checksum for the target. You are required by
> SBC to use T10 CRC on the wire so there is no point in converting to IP
> checksum in the backend.
>
> My impending patches will allow you to pass through PI with T10 CRC to a
> device with an IP checksum block integrity profile (i.e. the choice of
> checksum is a per-bio bip flag instead of an HBA-enforced global).
>

OK, so IP checksum support still makes sense.

Thanks!

Sagi.

[PATCH-v2 12/17] target/file: Add DIF protection init/format support

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for DIF protection init/format support into
the FILEIO backend.

It involves using a seperate $FILE.protection for storing PI that is
opened via fd_init_prot() using the common pi_prot_type attribute.
The actual formatting of the protection is done via fd_format_prot()
using the common pi_prot_format attribute, that will populate the
initial PI data based upon the currently configured pi_prot_type.

Based on original FILEIO code from Sagi.

v1 changes:
  - Fix sparse warnings in fd_init_format_buf (Fengguang)

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_file.c |  137 +++++++++++++++++++++++++++++++++++++
 drivers/target/target_core_file.h |    4 ++
 2 files changed, 141 insertions(+)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 0e34cda..119d519 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -700,6 +700,140 @@ static sector_t fd_get_blocks(struct se_device *dev)
 		       dev->dev_attrib.block_size);
 }
 
+static int fd_init_prot(struct se_device *dev)
+{
+	struct fd_dev *fd_dev = FD_DEV(dev);
+	struct file *prot_file, *file = fd_dev->fd_file;
+	struct inode *inode;
+	int ret, flags = O_RDWR | O_CREAT | O_LARGEFILE | O_DSYNC;
+	char buf[FD_MAX_DEV_PROT_NAME];
+
+	if (!file) {
+		pr_err("Unable to locate fd_dev->fd_file\n");
+		return -ENODEV;
+	}
+
+	inode = file->f_mapping->host;
+	if (S_ISBLK(inode->i_mode)) {
+		pr_err("FILEIO Protection emulation only supported on"
+		       " !S_ISBLK\n");
+		return -ENOSYS;
+	}
+
+	if (fd_dev->fbd_flags & FDBD_HAS_BUFFERED_IO_WCE)
+		flags &= ~O_DSYNC;
+
+	snprintf(buf, FD_MAX_DEV_PROT_NAME, "%s.protection",
+		 fd_dev->fd_dev_name);
+
+	prot_file = filp_open(buf, flags, 0600);
+	if (IS_ERR(prot_file)) {
+		pr_err("filp_open(%s) failed\n", buf);
+		ret = PTR_ERR(prot_file);
+		return ret;
+	}
+	fd_dev->fd_prot_file = prot_file;
+
+	return 0;
+}
+
+static void fd_init_format_buf(struct se_device *dev, unsigned char *buf,
+			       u32 unit_size, u32 *ref_tag, u16 app_tag,
+			       bool inc_reftag)
+{
+	unsigned char *p = buf;
+	int i;
+
+	for (i = 0; i < unit_size; i += dev->prot_length) {
+		*((u16 *)&p[0]) = 0xffff;
+		*((__be16 *)&p[2]) = cpu_to_be16(app_tag);
+		*((__be32 *)&p[4]) = cpu_to_be32(*ref_tag);
+
+		if (inc_reftag)
+			(*ref_tag)++;
+
+		p += dev->prot_length;
+	}
+}
+
+static int fd_format_prot(struct se_device *dev)
+{
+	struct fd_dev *fd_dev = FD_DEV(dev);
+	struct file *prot_fd = fd_dev->fd_prot_file;
+	sector_t prot_length, prot;
+	unsigned char *buf;
+	loff_t pos = 0;
+	u32 ref_tag = 0;
+	int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size;
+	int rc, ret = 0, size, len;
+	bool inc_reftag = false;
+
+	if (!dev->dev_attrib.pi_prot_type) {
+		pr_err("Unable to format_prot while pi_prot_type == 0\n");
+		return -ENODEV;
+	}
+	if (!prot_fd) {
+		pr_err("Unable to locate fd_dev->fd_prot_file\n");
+		return -ENODEV;
+	}
+
+	switch (dev->dev_attrib.pi_prot_type) {
+	case TARGET_DIF_TYPE3_PROT:
+		ref_tag = 0xffffffff;
+		break;
+	case TARGET_DIF_TYPE2_PROT:
+	case TARGET_DIF_TYPE1_PROT:
+		inc_reftag = true;
+		break;
+	default:
+		break;
+	}
+
+	buf = vzalloc(unit_size);
+	if (!buf) {
+		pr_err("Unable to allocate FILEIO prot buf\n");
+		return -ENOMEM;
+	}
+
+	prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length;
+	size = prot_length;
+
+	pr_debug("Using FILEIO prot_length: %llu\n",
+		 (unsigned long long)prot_length);
+
+	for (prot = 0; prot < prot_length; prot += unit_size) {
+
+		fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff,
+				   inc_reftag);
+
+		len = min(unit_size, size);
+
+		rc = kernel_write(prot_fd, buf, len, pos);
+		if (rc != len) {
+			pr_err("vfs_write to prot file failed: %d\n", rc);
+			ret = -ENODEV;
+			goto out;
+		}
+		pos += len;
+		size -= len;
+	}
+
+out:
+	vfree(buf);
+	return ret;
+}
+
+static void fd_free_prot(struct se_device *dev)
+{
+	struct fd_dev *fd_dev = FD_DEV(dev);
+
+	if (!fd_dev->fd_prot_file)
+		return;
+
+	filp_close(fd_dev->fd_prot_file, NULL);
+	fd_dev->fd_prot_file = NULL;
+}
+
 static struct sbc_ops fd_sbc_ops = {
 	.execute_rw		= fd_execute_rw,
 	.execute_sync_cache	= fd_execute_sync_cache,
@@ -730,6 +864,9 @@ static struct se_subsystem_api fileio_template = {
 	.show_configfs_dev_params = fd_show_configfs_dev_params,
 	.get_device_type	= sbc_get_device_type,
 	.get_blocks		= fd_get_blocks,
+	.init_prot		= fd_init_prot,
+	.format_prot		= fd_format_prot,
+	.free_prot		= fd_free_prot,
 };
 
 static int __init fileio_module_init(void)
diff --git a/drivers/target/target_core_file.h b/drivers/target/target_core_file.h
index 37ffc5b..583691e 100644
--- a/drivers/target/target_core_file.h
+++ b/drivers/target/target_core_file.h
@@ -4,6 +4,7 @@
 #define FD_VERSION		"4.0"
 
 #define FD_MAX_DEV_NAME		256
+#define FD_MAX_DEV_PROT_NAME	FD_MAX_DEV_NAME + 16
 #define FD_DEVICE_QUEUE_DEPTH	32
 #define FD_MAX_DEVICE_QUEUE_DEPTH 128
 #define FD_BLOCKSIZE		512
@@ -15,6 +16,8 @@
 #define FBDF_HAS_PATH		0x01
 #define FBDF_HAS_SIZE		0x02
 #define FDBD_HAS_BUFFERED_IO_WCE 0x04
+#define FDBD_FORMAT_UNIT_SIZE	2048
+
 
 struct fd_dev {
 	struct se_device dev;
@@ -29,6 +32,7 @@ struct fd_dev {
 	u32		fd_block_size;
 	unsigned long long fd_dev_size;
 	struct file	*fd_file;
+	struct file	*fd_prot_file;
 	/* FILEIO HBA device is connected to */
 	struct fd_host *fd_host;
 } ____cacheline_aligned;
-- 
1.7.10.4

Re: [PATCH-v2 12/17] target/file: Add DIF protection init/format support

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds support for DIF protection init/format support into
> the FILEIO backend.
>
> It involves using a seperate $FILE.protection for storing PI that is
> opened via fd_init_prot() using the common pi_prot_type attribute.
> The actual formatting of the protection is done via fd_format_prot()
> using the common pi_prot_format attribute, that will populate the
> initial PI data based upon the currently configured pi_prot_type.
>
> Based on original FILEIO code from Sagi.

Nice! see comments below...

> v1 changes:
>    - Fix sparse warnings in fd_init_format_buf (Fengguang)
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/target_core_file.c |  137 +++++++++++++++++++++++++++++++++++++
>   drivers/target/target_core_file.h |    4 ++
>   2 files changed, 141 insertions(+)
>
> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index 0e34cda..119d519 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -700,6 +700,140 @@ static sector_t fd_get_blocks(struct se_device *dev)
>   		       dev->dev_attrib.block_size);
>   }
>   
> +static int fd_init_prot(struct se_device *dev)
> +{
> +	struct fd_dev *fd_dev = FD_DEV(dev);
> +	struct file *prot_file, *file = fd_dev->fd_file;
> +	struct inode *inode;
> +	int ret, flags = O_RDWR | O_CREAT | O_LARGEFILE | O_DSYNC;
> +	char buf[FD_MAX_DEV_PROT_NAME];
> +
> +	if (!file) {
> +		pr_err("Unable to locate fd_dev->fd_file\n");
> +		return -ENODEV;
> +	}
> +
> +	inode = file->f_mapping->host;
> +	if (S_ISBLK(inode->i_mode)) {
> +		pr_err("FILEIO Protection emulation only supported on"
> +		       " !S_ISBLK\n");
> +		return -ENOSYS;
> +	}
> +
> +	if (fd_dev->fbd_flags & FDBD_HAS_BUFFERED_IO_WCE)
> +		flags &= ~O_DSYNC;
> +
> +	snprintf(buf, FD_MAX_DEV_PROT_NAME, "%s.protection",
> +		 fd_dev->fd_dev_name);
> +
> +	prot_file = filp_open(buf, flags, 0600);
> +	if (IS_ERR(prot_file)) {
> +		pr_err("filp_open(%s) failed\n", buf);
> +		ret = PTR_ERR(prot_file);
> +		return ret;
> +	}
> +	fd_dev->fd_prot_file = prot_file;
> +
> +	return 0;
> +}
> +
> +static void fd_init_format_buf(struct se_device *dev, unsigned char *buf,
> +			       u32 unit_size, u32 *ref_tag, u16 app_tag,
> +			       bool inc_reftag)
> +{
> +	unsigned char *p = buf;
> +	int i;
> +
> +	for (i = 0; i < unit_size; i += dev->prot_length) {
> +		*((u16 *)&p[0]) = 0xffff;
> +		*((__be16 *)&p[2]) = cpu_to_be16(app_tag);
> +		*((__be32 *)&p[4]) = cpu_to_be32(*ref_tag);
> +
> +		if (inc_reftag)
> +			(*ref_tag)++;
> +
> +		p += dev->prot_length;
> +	}
> +}
> +
> +static int fd_format_prot(struct se_device *dev)
> +{
> +	struct fd_dev *fd_dev = FD_DEV(dev);
> +	struct file *prot_fd = fd_dev->fd_prot_file;
> +	sector_t prot_length, prot;
> +	unsigned char *buf;
> +	loff_t pos = 0;
> +	u32 ref_tag = 0;
> +	int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size;
> +	int rc, ret = 0, size, len;
> +	bool inc_reftag = false;
> +
> +	if (!dev->dev_attrib.pi_prot_type) {
> +		pr_err("Unable to format_prot while pi_prot_type == 0\n");
> +		return -ENODEV;
> +	}
> +	if (!prot_fd) {
> +		pr_err("Unable to locate fd_dev->fd_prot_file\n");
> +		return -ENODEV;
> +	}
> +
> +	switch (dev->dev_attrib.pi_prot_type) {
redundant - see below.
> +	case TARGET_DIF_TYPE3_PROT:
> +		ref_tag = 0xffffffff;
> +		break;
> +	case TARGET_DIF_TYPE2_PROT:
> +	case TARGET_DIF_TYPE1_PROT:
> +		inc_reftag = true;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	buf = vzalloc(unit_size);
> +	if (!buf) {
> +		pr_err("Unable to allocate FILEIO prot buf\n");
> +		return -ENOMEM;
> +	}
> +
> +	prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length;
> +	size = prot_length;
> +
> +	pr_debug("Using FILEIO prot_length: %llu\n",
> +		 (unsigned long long)prot_length);
> +
> +	for (prot = 0; prot < prot_length; prot += unit_size) {
> +
> +		fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff,
> +				   inc_reftag);

I didn't send you my latest patches (my fault...).T10-PI format should 
only place
escape values throughout the protection file (fill it with 0xff). so I 
guess in this case
fd_init_formast_buf() boils down to memset(buf, 0xff, unit_size) once  
before the loop
and just loop until prot_length writing buf, no need to address 
apptag/reftag...

> +
> +		len = min(unit_size, size);
> +
> +		rc = kernel_write(prot_fd, buf, len, pos);
> +		if (rc != len) {
> +			pr_err("vfs_write to prot file failed: %d\n", rc);
> +			ret = -ENODEV;
> +			goto out;
> +		}
> +		pos += len;
> +		size -= len;
> +	}
> +
> +out:
> +	vfree(buf);
> +	return ret;
> +}
> +
> +static void fd_free_prot(struct se_device *dev)
> +{
> +	struct fd_dev *fd_dev = FD_DEV(dev);
> +
> +	if (!fd_dev->fd_prot_file)
> +		return;
> +
> +	filp_close(fd_dev->fd_prot_file, NULL);
> +	fd_dev->fd_prot_file = NULL;
> +}
> +
>   static struct sbc_ops fd_sbc_ops = {
>   	.execute_rw		= fd_execute_rw,
>   	.execute_sync_cache	= fd_execute_sync_cache,
> @@ -730,6 +864,9 @@ static struct se_subsystem_api fileio_template = {
>   	.show_configfs_dev_params = fd_show_configfs_dev_params,
>   	.get_device_type	= sbc_get_device_type,
>   	.get_blocks		= fd_get_blocks,
> +	.init_prot		= fd_init_prot,
> +	.format_prot		= fd_format_prot,
> +	.free_prot		= fd_free_prot,
>   };
>   
>   static int __init fileio_module_init(void)
> diff --git a/drivers/target/target_core_file.h b/drivers/target/target_core_file.h
> index 37ffc5b..583691e 100644
> --- a/drivers/target/target_core_file.h
> +++ b/drivers/target/target_core_file.h
> @@ -4,6 +4,7 @@
>   #define FD_VERSION		"4.0"
>   
>   #define FD_MAX_DEV_NAME		256
> +#define FD_MAX_DEV_PROT_NAME	FD_MAX_DEV_NAME + 16
>   #define FD_DEVICE_QUEUE_DEPTH	32
>   #define FD_MAX_DEVICE_QUEUE_DEPTH 128
>   #define FD_BLOCKSIZE		512
> @@ -15,6 +16,8 @@
>   #define FBDF_HAS_PATH		0x01
>   #define FBDF_HAS_SIZE		0x02
>   #define FDBD_HAS_BUFFERED_IO_WCE 0x04
> +#define FDBD_FORMAT_UNIT_SIZE	2048
> +
>   
>   struct fd_dev {
>   	struct se_device dev;
> @@ -29,6 +32,7 @@ struct fd_dev {
>   	u32		fd_block_size;
>   	unsigned long long fd_dev_size;
>   	struct file	*fd_file;
> +	struct file	*fd_prot_file;
>   	/* FILEIO HBA device is connected to */
>   	struct fd_host *fd_host;
>   } ____cacheline_aligned;

Re: [PATCH-v2 12/17] target/file: Add DIF protection init/format support

[Nicholas A. Bellinger]

On Sun, 2014-01-19 at 14:31 +0200, Sagi Grimberg wrote:
> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> >
> > This patch adds support for DIF protection init/format support into
> > the FILEIO backend.
> >
> > It involves using a seperate $FILE.protection for storing PI that is
> > opened via fd_init_prot() using the common pi_prot_type attribute.
> > The actual formatting of the protection is done via fd_format_prot()
> > using the common pi_prot_format attribute, that will populate the
> > initial PI data based upon the currently configured pi_prot_type.
> >
> > Based on original FILEIO code from Sagi.
> 
> Nice! see comments below...
> 
> > v1 changes:
> >    - Fix sparse warnings in fd_init_format_buf (Fengguang)
> >
> > Cc: Martin K. Petersen <martin.petersen@oracle.com>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Hannes Reinecke <hare@suse.de>
> > Cc: Sagi Grimberg <sagig@mellanox.com>
> > Cc: Or Gerlitz <ogerlitz@mellanox.com>
> > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> > ---
> >   drivers/target/target_core_file.c |  137 +++++++++++++++++++++++++++++++++++++
> >   drivers/target/target_core_file.h |    4 ++
> >   2 files changed, 141 insertions(+)
> >
> > diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> > index 0e34cda..119d519 100644
> > --- a/drivers/target/target_core_file.c
> > +++ b/drivers/target/target_core_file.c
> > @@ -700,6 +700,140 @@ static sector_t fd_get_blocks(struct se_device *dev)
> >   		       dev->dev_attrib.block_size);
> >   }
> >   
> > +static int fd_init_prot(struct se_device *dev)
> > +{
> > +	struct fd_dev *fd_dev = FD_DEV(dev);
> > +	struct file *prot_file, *file = fd_dev->fd_file;
> > +	struct inode *inode;
> > +	int ret, flags = O_RDWR | O_CREAT | O_LARGEFILE | O_DSYNC;
> > +	char buf[FD_MAX_DEV_PROT_NAME];
> > +
> > +	if (!file) {
> > +		pr_err("Unable to locate fd_dev->fd_file\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	inode = file->f_mapping->host;
> > +	if (S_ISBLK(inode->i_mode)) {
> > +		pr_err("FILEIO Protection emulation only supported on"
> > +		       " !S_ISBLK\n");
> > +		return -ENOSYS;
> > +	}
> > +
> > +	if (fd_dev->fbd_flags & FDBD_HAS_BUFFERED_IO_WCE)
> > +		flags &= ~O_DSYNC;
> > +
> > +	snprintf(buf, FD_MAX_DEV_PROT_NAME, "%s.protection",
> > +		 fd_dev->fd_dev_name);
> > +
> > +	prot_file = filp_open(buf, flags, 0600);
> > +	if (IS_ERR(prot_file)) {
> > +		pr_err("filp_open(%s) failed\n", buf);
> > +		ret = PTR_ERR(prot_file);
> > +		return ret;
> > +	}
> > +	fd_dev->fd_prot_file = prot_file;
> > +
> > +	return 0;
> > +}
> > +
> > +static void fd_init_format_buf(struct se_device *dev, unsigned char *buf,
> > +			       u32 unit_size, u32 *ref_tag, u16 app_tag,
> > +			       bool inc_reftag)
> > +{
> > +	unsigned char *p = buf;
> > +	int i;
> > +
> > +	for (i = 0; i < unit_size; i += dev->prot_length) {
> > +		*((u16 *)&p[0]) = 0xffff;
> > +		*((__be16 *)&p[2]) = cpu_to_be16(app_tag);
> > +		*((__be32 *)&p[4]) = cpu_to_be32(*ref_tag);
> > +
> > +		if (inc_reftag)
> > +			(*ref_tag)++;
> > +
> > +		p += dev->prot_length;
> > +	}
> > +}
> > +
> > +static int fd_format_prot(struct se_device *dev)
> > +{
> > +	struct fd_dev *fd_dev = FD_DEV(dev);
> > +	struct file *prot_fd = fd_dev->fd_prot_file;
> > +	sector_t prot_length, prot;
> > +	unsigned char *buf;
> > +	loff_t pos = 0;
> > +	u32 ref_tag = 0;
> > +	int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size;
> > +	int rc, ret = 0, size, len;
> > +	bool inc_reftag = false;
> > +
> > +	if (!dev->dev_attrib.pi_prot_type) {
> > +		pr_err("Unable to format_prot while pi_prot_type == 0\n");
> > +		return -ENODEV;
> > +	}
> > +	if (!prot_fd) {
> > +		pr_err("Unable to locate fd_dev->fd_prot_file\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	switch (dev->dev_attrib.pi_prot_type) {
> redundant - see below.
> > +	case TARGET_DIF_TYPE3_PROT:
> > +		ref_tag = 0xffffffff;
> > +		break;
> > +	case TARGET_DIF_TYPE2_PROT:
> > +	case TARGET_DIF_TYPE1_PROT:
> > +		inc_reftag = true;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	buf = vzalloc(unit_size);
> > +	if (!buf) {
> > +		pr_err("Unable to allocate FILEIO prot buf\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length;
> > +	size = prot_length;
> > +
> > +	pr_debug("Using FILEIO prot_length: %llu\n",
> > +		 (unsigned long long)prot_length);
> > +
> > +	for (prot = 0; prot < prot_length; prot += unit_size) {
> > +
> > +		fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff,
> > +				   inc_reftag);
> 
> I didn't send you my latest patches (my fault...).T10-PI format should 
> only place
> escape values throughout the protection file (fill it with 0xff). so I 
> guess in this case
> fd_init_formast_buf() boils down to memset(buf, 0xff, unit_size) once  
> before the loop
> and just loop until prot_length writing buf, no need to address 
> apptag/reftag...

Yeah, was thinking about just formatting with escape values as mentioned
above, but thought it might be useful to keep around for pre-populating
values apptag + reftag values for testing purposes.

--nab

Re: [PATCH-v2 12/17] target/file: Add DIF protection init/format support

[Sagi Grimberg]

On 1/22/2014 12:28 AM, Nicholas A. Bellinger wrote:
> On Sun, 2014-01-19 at 14:31 +0200, Sagi Grimberg wrote:
>> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
>>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>>
>>> This patch adds support for DIF protection init/format support into
>>> the FILEIO backend.
>>>
>>> It involves using a seperate $FILE.protection for storing PI that is
>>> opened via fd_init_prot() using the common pi_prot_type attribute.
>>> The actual formatting of the protection is done via fd_format_prot()
>>> using the common pi_prot_format attribute, that will populate the
>>> initial PI data based upon the currently configured pi_prot_type.
>>>
>>> Based on original FILEIO code from Sagi.
>> Nice! see comments below...
>>
>>> v1 changes:
>>>     - Fix sparse warnings in fd_init_format_buf (Fengguang)
>>>
>>> Cc: Martin K. Petersen <martin.petersen@oracle.com>
>>> Cc: Christoph Hellwig <hch@lst.de>
>>> Cc: Hannes Reinecke <hare@suse.de>
>>> Cc: Sagi Grimberg <sagig@mellanox.com>
>>> Cc: Or Gerlitz <ogerlitz@mellanox.com>
>>> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
>>> ---
>>>    drivers/target/target_core_file.c |  137 +++++++++++++++++++++++++++++++++++++
>>>    drivers/target/target_core_file.h |    4 ++
>>>    2 files changed, 141 insertions(+)
>>>
>>> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
>>> index 0e34cda..119d519 100644
>>> --- a/drivers/target/target_core_file.c
>>> +++ b/drivers/target/target_core_file.c
>>> @@ -700,6 +700,140 @@ static sector_t fd_get_blocks(struct se_device *dev)
>>>    		       dev->dev_attrib.block_size);
>>>    }
>>>    
>>> +static int fd_init_prot(struct se_device *dev)
>>> +{
>>> +	struct fd_dev *fd_dev = FD_DEV(dev);
>>> +	struct file *prot_file, *file = fd_dev->fd_file;
>>> +	struct inode *inode;
>>> +	int ret, flags = O_RDWR | O_CREAT | O_LARGEFILE | O_DSYNC;
>>> +	char buf[FD_MAX_DEV_PROT_NAME];
>>> +
>>> +	if (!file) {
>>> +		pr_err("Unable to locate fd_dev->fd_file\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	inode = file->f_mapping->host;
>>> +	if (S_ISBLK(inode->i_mode)) {
>>> +		pr_err("FILEIO Protection emulation only supported on"
>>> +		       " !S_ISBLK\n");
>>> +		return -ENOSYS;
>>> +	}
>>> +
>>> +	if (fd_dev->fbd_flags & FDBD_HAS_BUFFERED_IO_WCE)
>>> +		flags &= ~O_DSYNC;
>>> +
>>> +	snprintf(buf, FD_MAX_DEV_PROT_NAME, "%s.protection",
>>> +		 fd_dev->fd_dev_name);
>>> +
>>> +	prot_file = filp_open(buf, flags, 0600);
>>> +	if (IS_ERR(prot_file)) {
>>> +		pr_err("filp_open(%s) failed\n", buf);
>>> +		ret = PTR_ERR(prot_file);
>>> +		return ret;
>>> +	}
>>> +	fd_dev->fd_prot_file = prot_file;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void fd_init_format_buf(struct se_device *dev, unsigned char *buf,
>>> +			       u32 unit_size, u32 *ref_tag, u16 app_tag,
>>> +			       bool inc_reftag)
>>> +{
>>> +	unsigned char *p = buf;
>>> +	int i;
>>> +
>>> +	for (i = 0; i < unit_size; i += dev->prot_length) {
>>> +		*((u16 *)&p[0]) = 0xffff;
>>> +		*((__be16 *)&p[2]) = cpu_to_be16(app_tag);
>>> +		*((__be32 *)&p[4]) = cpu_to_be32(*ref_tag);
>>> +
>>> +		if (inc_reftag)
>>> +			(*ref_tag)++;
>>> +
>>> +		p += dev->prot_length;
>>> +	}
>>> +}
>>> +
>>> +static int fd_format_prot(struct se_device *dev)
>>> +{
>>> +	struct fd_dev *fd_dev = FD_DEV(dev);
>>> +	struct file *prot_fd = fd_dev->fd_prot_file;
>>> +	sector_t prot_length, prot;
>>> +	unsigned char *buf;
>>> +	loff_t pos = 0;
>>> +	u32 ref_tag = 0;
>>> +	int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size;
>>> +	int rc, ret = 0, size, len;
>>> +	bool inc_reftag = false;
>>> +
>>> +	if (!dev->dev_attrib.pi_prot_type) {
>>> +		pr_err("Unable to format_prot while pi_prot_type == 0\n");
>>> +		return -ENODEV;
>>> +	}
>>> +	if (!prot_fd) {
>>> +		pr_err("Unable to locate fd_dev->fd_prot_file\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	switch (dev->dev_attrib.pi_prot_type) {
>> redundant - see below.
>>> +	case TARGET_DIF_TYPE3_PROT:
>>> +		ref_tag = 0xffffffff;
>>> +		break;
>>> +	case TARGET_DIF_TYPE2_PROT:
>>> +	case TARGET_DIF_TYPE1_PROT:
>>> +		inc_reftag = true;
>>> +		break;
>>> +	default:
>>> +		break;
>>> +	}
>>> +
>>> +	buf = vzalloc(unit_size);
>>> +	if (!buf) {
>>> +		pr_err("Unable to allocate FILEIO prot buf\n");
>>> +		return -ENOMEM;
>>> +	}
>>> +
>>> +	prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length;
>>> +	size = prot_length;
>>> +
>>> +	pr_debug("Using FILEIO prot_length: %llu\n",
>>> +		 (unsigned long long)prot_length);
>>> +
>>> +	for (prot = 0; prot < prot_length; prot += unit_size) {
>>> +
>>> +		fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff,
>>> +				   inc_reftag);
>> I didn't send you my latest patches (my fault...).T10-PI format should
>> only place
>> escape values throughout the protection file (fill it with 0xff). so I
>> guess in this case
>> fd_init_formast_buf() boils down to memset(buf, 0xff, unit_size) once
>> before the loop
>> and just loop until prot_length writing buf, no need to address
>> apptag/reftag...
> Yeah, was thinking about just formatting with escape values as mentioned
> above, but thought it might be useful to keep around for pre-populating
> values apptag + reftag values for testing purposes.
>
> --nab
>

OK, but maybe it is better to do that under some debug configuration 
rather then always do that.

Sagi.

Re: [PATCH-v2 12/17] target/file: Add DIF protection init/format support

[Nicholas A. Bellinger]

On Wed, 2014-01-22 at 12:12 +0200, Sagi Grimberg wrote:
> On 1/22/2014 12:28 AM, Nicholas A. Bellinger wrote:
> > On Sun, 2014-01-19 at 14:31 +0200, Sagi Grimberg wrote:
> >> On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> >>> From: Nicholas Bellinger <nab@linux-iscsi.org>
> >>>
> >>> This patch adds support for DIF protection init/format support into
> >>> the FILEIO backend.
> >>>
> >>> It involves using a seperate $FILE.protection for storing PI that is
> >>> opened via fd_init_prot() using the common pi_prot_type attribute.
> >>> The actual formatting of the protection is done via fd_format_prot()
> >>> using the common pi_prot_format attribute, that will populate the
> >>> initial PI data based upon the currently configured pi_prot_type.
> >>>
> >>> Based on original FILEIO code from Sagi.
> >> Nice! see comments below...
> >>

<SNIP>

> >>> +static void fd_init_format_buf(struct se_device *dev, unsigned char *buf,
> >>> +			       u32 unit_size, u32 *ref_tag, u16 app_tag,
> >>> +			       bool inc_reftag)
> >>> +{
> >>> +	unsigned char *p = buf;
> >>> +	int i;
> >>> +
> >>> +	for (i = 0; i < unit_size; i += dev->prot_length) {
> >>> +		*((u16 *)&p[0]) = 0xffff;
> >>> +		*((__be16 *)&p[2]) = cpu_to_be16(app_tag);
> >>> +		*((__be32 *)&p[4]) = cpu_to_be32(*ref_tag);
> >>> +
> >>> +		if (inc_reftag)
> >>> +			(*ref_tag)++;
> >>> +
> >>> +		p += dev->prot_length;
> >>> +	}
> >>> +}
> >>> +
> >>> +static int fd_format_prot(struct se_device *dev)
> >>> +{
> >>> +	struct fd_dev *fd_dev = FD_DEV(dev);
> >>> +	struct file *prot_fd = fd_dev->fd_prot_file;
> >>> +	sector_t prot_length, prot;
> >>> +	unsigned char *buf;
> >>> +	loff_t pos = 0;
> >>> +	u32 ref_tag = 0;
> >>> +	int unit_size = FDBD_FORMAT_UNIT_SIZE * dev->dev_attrib.block_size;
> >>> +	int rc, ret = 0, size, len;
> >>> +	bool inc_reftag = false;
> >>> +
> >>> +	if (!dev->dev_attrib.pi_prot_type) {
> >>> +		pr_err("Unable to format_prot while pi_prot_type == 0\n");
> >>> +		return -ENODEV;
> >>> +	}
> >>> +	if (!prot_fd) {
> >>> +		pr_err("Unable to locate fd_dev->fd_prot_file\n");
> >>> +		return -ENODEV;
> >>> +	}
> >>> +
> >>> +	switch (dev->dev_attrib.pi_prot_type) {
> >> redundant - see below.
> >>> +	case TARGET_DIF_TYPE3_PROT:
> >>> +		ref_tag = 0xffffffff;
> >>> +		break;
> >>> +	case TARGET_DIF_TYPE2_PROT:
> >>> +	case TARGET_DIF_TYPE1_PROT:
> >>> +		inc_reftag = true;
> >>> +		break;
> >>> +	default:
> >>> +		break;
> >>> +	}
> >>> +
> >>> +	buf = vzalloc(unit_size);
> >>> +	if (!buf) {
> >>> +		pr_err("Unable to allocate FILEIO prot buf\n");
> >>> +		return -ENOMEM;
> >>> +	}
> >>> +
> >>> +	prot_length = (dev->transport->get_blocks(dev) + 1) * dev->prot_length;
> >>> +	size = prot_length;
> >>> +
> >>> +	pr_debug("Using FILEIO prot_length: %llu\n",
> >>> +		 (unsigned long long)prot_length);
> >>> +
> >>> +	for (prot = 0; prot < prot_length; prot += unit_size) {
> >>> +
> >>> +		fd_init_format_buf(dev, buf, unit_size, &ref_tag, 0xffff,
> >>> +				   inc_reftag);
> >> I didn't send you my latest patches (my fault...).T10-PI format should
> >> only place
> >> escape values throughout the protection file (fill it with 0xff). so I
> >> guess in this case
> >> fd_init_formast_buf() boils down to memset(buf, 0xff, unit_size) once
> >> before the loop
> >> and just loop until prot_length writing buf, no need to address
> >> apptag/reftag...
> > Yeah, was thinking about just formatting with escape values as mentioned
> > above, but thought it might be useful to keep around for pre-populating
> > values apptag + reftag values for testing purposes.
> >
> > --nab
> >
> 
> OK, but maybe it is better to do that under some debug configuration 
> rather then always do that.
> 

With the apptag escape in place from the format the host is going to
ignore the other areas, so this shouldn't really matter.

If it turns out to be a issue, we can just drop this code later.

--nab

[PATCH-v2 13/17] target/file: Add DIF protection support to fd_execute_rw

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for DIF protection into fd_execute_rw() code
for WRITE/READ I/O using sbc_dif_verify_[write,read]() logic.

It adds fd_do_prot_rw() for handling interface with FILEIO PI, and
uses a locally allocated fd_prot->prot_buf + fd_prot->prot_sg for
interacting with SBC DIF verify emulation code.

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_file.c |  119 ++++++++++++++++++++++++++++++++++++-
 drivers/target/target_core_file.h |    5 ++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 119d519..aaba7c5 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -257,6 +257,72 @@ static void fd_free_device(struct se_device *dev)
 	kfree(fd_dev);
 }
 
+static int fd_do_prot_rw(struct se_cmd *cmd, struct fd_prot *fd_prot,
+			 int is_write)
+{
+	struct se_device *se_dev = cmd->se_dev;
+	struct fd_dev *dev = FD_DEV(se_dev);
+	struct file *prot_fd = dev->fd_prot_file;
+	struct scatterlist *sg;
+	loff_t pos = (cmd->t_task_lba * se_dev->prot_length);
+	unsigned char *buf;
+	u32 prot_size, len, size;
+	int rc, ret = 1, i;
+
+	prot_size = (cmd->data_length / se_dev->dev_attrib.block_size) *
+		     se_dev->prot_length;
+
+	if (!is_write) {
+		fd_prot->prot_buf = vzalloc(prot_size);
+		if (!fd_prot->prot_buf) {
+			pr_err("Unable to allocate fd_prot->prot_buf\n");
+			return -ENOMEM;
+		}
+		buf = fd_prot->prot_buf;
+
+		fd_prot->prot_sg_nents = cmd->t_prot_nents;
+		fd_prot->prot_sg = kzalloc(sizeof(struct scatterlist) *
+					   fd_prot->prot_sg_nents, GFP_KERNEL);
+		if (!fd_prot->prot_sg) {
+			pr_err("Unable to allocate fd_prot->prot_sg\n");
+			vfree(fd_prot->prot_buf);
+			return -ENOMEM;
+		}
+		size = prot_size;
+
+		for_each_sg(fd_prot->prot_sg, sg, fd_prot->prot_sg_nents, i) {
+
+			len = min_t(u32, PAGE_SIZE, size);
+			sg_set_buf(sg, buf, len);
+			size -= len;
+			buf += len;
+		}
+	}
+
+	if (is_write) {
+		rc = kernel_write(prot_fd, fd_prot->prot_buf, prot_size, pos);
+		if (rc < 0 || prot_size != rc) {
+			pr_err("kernel_write() for fd_do_prot_rw failed:"
+			       " %d\n", rc);
+			ret = -EINVAL;
+		}
+	} else {
+		rc = kernel_read(prot_fd, pos, fd_prot->prot_buf, prot_size);
+		if (rc < 0) {
+			pr_err("kernel_read() for fd_do_prot_rw failed:"
+			       " %d\n", rc);
+			ret = -EINVAL;
+		}
+	}
+
+	if (is_write || ret < 0) {
+		kfree(fd_prot->prot_sg);
+		vfree(fd_prot->prot_buf);
+	}
+
+	return ret;
+}
+
 static int fd_do_rw(struct se_cmd *cmd, struct scatterlist *sgl,
 		u32 sgl_nents, int is_write)
 {
@@ -551,6 +617,8 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	      enum dma_data_direction data_direction)
 {
 	struct se_device *dev = cmd->se_dev;
+	struct fd_prot fd_prot;
+	sense_reason_t rc;
 	int ret = 0;
 
 	/*
@@ -558,8 +626,48 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	 * physical memory addresses to struct iovec virtual memory.
 	 */
 	if (data_direction == DMA_FROM_DEVICE) {
+		memset(&fd_prot, 0, sizeof(struct fd_prot));
+
+		if (cmd->prot_type) {
+			ret = fd_do_prot_rw(cmd, &fd_prot, false);
+			if (ret < 0)
+				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+		}
+
 		ret = fd_do_rw(cmd, sgl, sgl_nents, 0);
+
+		if (ret > 0 && cmd->prot_type) {
+			u32 sectors = cmd->data_length / dev->dev_attrib.block_size;
+
+			rc = sbc_dif_verify_read(cmd, cmd->t_task_lba, sectors,
+						 0, fd_prot.prot_sg, 0);
+			if (rc) {
+				kfree(fd_prot.prot_sg);
+				vfree(fd_prot.prot_buf);
+				return rc;
+			}
+			kfree(fd_prot.prot_sg);
+			vfree(fd_prot.prot_buf);
+		}
 	} else {
+		memset(&fd_prot, 0, sizeof(struct fd_prot));
+
+		if (cmd->prot_type) {
+			u32 sectors = cmd->data_length / dev->dev_attrib.block_size;
+
+			ret = fd_do_prot_rw(cmd, &fd_prot, false);
+			if (ret < 0)
+				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+
+			rc = sbc_dif_verify_write(cmd, cmd->t_task_lba, sectors,
+						  0, fd_prot.prot_sg, 0);
+			if (rc) {
+				kfree(fd_prot.prot_sg);
+				vfree(fd_prot.prot_buf);
+				return rc;
+			}
+		}
+
 		ret = fd_do_rw(cmd, sgl, sgl_nents, 1);
 		/*
 		 * Perform implicit vfs_fsync_range() for fd_do_writev() ops
@@ -576,10 +684,19 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 
 			vfs_fsync_range(fd_dev->fd_file, start, end, 1);
 		}
+
+		if (ret > 0 && cmd->prot_type) {
+			ret = fd_do_prot_rw(cmd, &fd_prot, true);
+			if (ret < 0)
+				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+		}
 	}
 
-	if (ret < 0)
+	if (ret < 0) {
+		kfree(fd_prot.prot_sg);
+		vfree(fd_prot.prot_buf);
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+	}
 
 	if (ret)
 		target_complete_cmd(cmd, SAM_STAT_GOOD);
diff --git a/drivers/target/target_core_file.h b/drivers/target/target_core_file.h
index 583691e..97e5e7d 100644
--- a/drivers/target/target_core_file.h
+++ b/drivers/target/target_core_file.h
@@ -18,6 +18,11 @@
 #define FDBD_HAS_BUFFERED_IO_WCE 0x04
 #define FDBD_FORMAT_UNIT_SIZE	2048
 
+struct fd_prot {
+	unsigned char	*prot_buf;
+	struct scatterlist *prot_sg;
+	u32 prot_sg_nents;
+};
 
 struct fd_dev {
 	struct se_device dev;
-- 
1.7.10.4

Re: [PATCH-v2 13/17] target/file: Add DIF protection support to fd_execute_rw

[Sagi Grimberg]

On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
>
> This patch adds support for DIF protection into fd_execute_rw() code
> for WRITE/READ I/O using sbc_dif_verify_[write,read]() logic.
>
> It adds fd_do_prot_rw() for handling interface with FILEIO PI, and
> uses a locally allocated fd_prot->prot_buf + fd_prot->prot_sg for
> interacting with SBC DIF verify emulation code.
>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>   drivers/target/target_core_file.c |  119 ++++++++++++++++++++++++++++++++++++-
>   drivers/target/target_core_file.h |    5 ++
>   2 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
> index 119d519..aaba7c5 100644
> --- a/drivers/target/target_core_file.c
> +++ b/drivers/target/target_core_file.c
> @@ -257,6 +257,72 @@ static void fd_free_device(struct se_device *dev)
>   	kfree(fd_dev);
>   }
>   
> +static int fd_do_prot_rw(struct se_cmd *cmd, struct fd_prot *fd_prot,
> +			 int is_write)
> +{
> +	struct se_device *se_dev = cmd->se_dev;
> +	struct fd_dev *dev = FD_DEV(se_dev);
> +	struct file *prot_fd = dev->fd_prot_file;
> +	struct scatterlist *sg;
> +	loff_t pos = (cmd->t_task_lba * se_dev->prot_length);
> +	unsigned char *buf;
> +	u32 prot_size, len, size;
> +	int rc, ret = 1, i;
> +
> +	prot_size = (cmd->data_length / se_dev->dev_attrib.block_size) *
> +		     se_dev->prot_length;
> +
> +	if (!is_write) {
> +		fd_prot->prot_buf = vzalloc(prot_size);
> +		if (!fd_prot->prot_buf) {
> +			pr_err("Unable to allocate fd_prot->prot_buf\n");
> +			return -ENOMEM;
> +		}
> +		buf = fd_prot->prot_buf;
> +
> +		fd_prot->prot_sg_nents = cmd->t_prot_nents;
> +		fd_prot->prot_sg = kzalloc(sizeof(struct scatterlist) *
> +					   fd_prot->prot_sg_nents, GFP_KERNEL);
> +		if (!fd_prot->prot_sg) {
> +			pr_err("Unable to allocate fd_prot->prot_sg\n");
> +			vfree(fd_prot->prot_buf);
> +			return -ENOMEM;
> +		}
> +		size = prot_size;
> +
> +		for_each_sg(fd_prot->prot_sg, sg, fd_prot->prot_sg_nents, i) {
> +
> +			len = min_t(u32, PAGE_SIZE, size);
> +			sg_set_buf(sg, buf, len);
> +			size -= len;
> +			buf += len;
> +		}
> +	}
> +
> +	if (is_write) {
> +		rc = kernel_write(prot_fd, fd_prot->prot_buf, prot_size, pos);
> +		if (rc < 0 || prot_size != rc) {
> +			pr_err("kernel_write() for fd_do_prot_rw failed:"
> +			       " %d\n", rc);
> +			ret = -EINVAL;
> +		}
> +	} else {
> +		rc = kernel_read(prot_fd, pos, fd_prot->prot_buf, prot_size);
> +		if (rc < 0) {
> +			pr_err("kernel_read() for fd_do_prot_rw failed:"
> +			       " %d\n", rc);
> +			ret = -EINVAL;
> +		}
> +	}
> +
> +	if (is_write || ret < 0) {
> +		kfree(fd_prot->prot_sg);
> +		vfree(fd_prot->prot_buf);
> +	}
> +
> +	return ret;
> +}
> +
>   static int fd_do_rw(struct se_cmd *cmd, struct scatterlist *sgl,
>   		u32 sgl_nents, int is_write)
>   {
> @@ -551,6 +617,8 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   	      enum dma_data_direction data_direction)
>   {
>   	struct se_device *dev = cmd->se_dev;
> +	struct fd_prot fd_prot;
> +	sense_reason_t rc;
>   	int ret = 0;
>   
>   	/*
> @@ -558,8 +626,48 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   	 * physical memory addresses to struct iovec virtual memory.
>   	 */
>   	if (data_direction == DMA_FROM_DEVICE) {

Maybe its better to export this one to a separate function? 
fd_execute_prot_rw()? just a nit...

> +		memset(&fd_prot, 0, sizeof(struct fd_prot));
> +
> +		if (cmd->prot_type) {
> +			ret = fd_do_prot_rw(cmd, &fd_prot, false);
> +			if (ret < 0)
> +				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +		}
> +
>   		ret = fd_do_rw(cmd, sgl, sgl_nents, 0);
> +
> +		if (ret > 0 && cmd->prot_type) {
> +			u32 sectors = cmd->data_length / dev->dev_attrib.block_size;
> +
> +			rc = sbc_dif_verify_read(cmd, cmd->t_task_lba, sectors,
> +						 0, fd_prot.prot_sg, 0);
> +			if (rc) {
> +				kfree(fd_prot.prot_sg);
> +				vfree(fd_prot.prot_buf);
> +				return rc;
> +			}
> +			kfree(fd_prot.prot_sg);
> +			vfree(fd_prot.prot_buf);
> +		}
>   	} else {
> +		memset(&fd_prot, 0, sizeof(struct fd_prot));
> +
> +		if (cmd->prot_type) {
> +			u32 sectors = cmd->data_length / dev->dev_attrib.block_size;
> +
> +			ret = fd_do_prot_rw(cmd, &fd_prot, false);
> +			if (ret < 0)
> +				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +
> +			rc = sbc_dif_verify_write(cmd, cmd->t_task_lba, sectors,
> +						  0, fd_prot.prot_sg, 0);
> +			if (rc) {
> +				kfree(fd_prot.prot_sg);
> +				vfree(fd_prot.prot_buf);
> +				return rc;
> +			}
> +		}
> +
>   		ret = fd_do_rw(cmd, sgl, sgl_nents, 1);
>   		/*
>   		 * Perform implicit vfs_fsync_range() for fd_do_writev() ops
> @@ -576,10 +684,19 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
>   
>   			vfs_fsync_range(fd_dev->fd_file, start, end, 1);
>   		}
> +
> +		if (ret > 0 && cmd->prot_type) {
> +			ret = fd_do_prot_rw(cmd, &fd_prot, true);
> +			if (ret < 0)
> +				return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +		}
>   	}
>   
> -	if (ret < 0)
> +	if (ret < 0) {
> +		kfree(fd_prot.prot_sg);
> +		vfree(fd_prot.prot_buf);
>   		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> +	}
>   
>   	if (ret)
>   		target_complete_cmd(cmd, SAM_STAT_GOOD);
> diff --git a/drivers/target/target_core_file.h b/drivers/target/target_core_file.h
> index 583691e..97e5e7d 100644
> --- a/drivers/target/target_core_file.h
> +++ b/drivers/target/target_core_file.h
> @@ -18,6 +18,11 @@
>   #define FDBD_HAS_BUFFERED_IO_WCE 0x04
>   #define FDBD_FORMAT_UNIT_SIZE	2048
>   
> +struct fd_prot {
> +	unsigned char	*prot_buf;
> +	struct scatterlist *prot_sg;
> +	u32 prot_sg_nents;
> +};
>   
>   struct fd_dev {
>   	struct se_device dev;

[PATCH-v2 14/17] target/rd: Refactor rd_build_device_space + rd_release_device_space

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch refactors rd_build_device_space() + rd_release_device_space()
into rd_allocate_sgl_table() + rd_release_device_space() so that they
may be used seperatly for setup + release of protection information
scatterlists.

Also add explicit memset of pages within rd_allocate_sgl_table() based
upon passed 'init_payload' value.

v2 changes:
  - Drop unused sg_table from rd_release_device_space (Wei)

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_rd.c |  113 +++++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 45 deletions(-)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 4ffe5f2..e9fa879 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -78,23 +78,14 @@ static void rd_detach_hba(struct se_hba *hba)
 	hba->hba_ptr = NULL;
 }
 
-/*	rd_release_device_space():
- *
- *
- */
-static void rd_release_device_space(struct rd_dev *rd_dev)
+static u32 rd_release_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *sg_table,
+				 u32 sg_table_count)
 {
-	u32 i, j, page_count = 0, sg_per_table;
-	struct rd_dev_sg_table *sg_table;
 	struct page *pg;
 	struct scatterlist *sg;
+	u32 i, j, page_count = 0, sg_per_table;
 
-	if (!rd_dev->sg_table_array || !rd_dev->sg_table_count)
-		return;
-
-	sg_table = rd_dev->sg_table_array;
-
-	for (i = 0; i < rd_dev->sg_table_count; i++) {
+	for (i = 0; i < sg_table_count; i++) {
 		sg = sg_table[i].sg_table;
 		sg_per_table = sg_table[i].rd_sg_count;
 
@@ -105,16 +96,28 @@ static void rd_release_device_space(struct rd_dev *rd_dev)
 				page_count++;
 			}
 		}
-
 		kfree(sg);
 	}
 
+	kfree(sg_table);
+	return page_count;
+}
+
+static void rd_release_device_space(struct rd_dev *rd_dev)
+{
+	u32 page_count;
+
+	if (!rd_dev->sg_table_array || !rd_dev->sg_table_count)
+		return;
+
+	page_count = rd_release_sgl_table(rd_dev, rd_dev->sg_table_array,
+					  rd_dev->sg_table_count);
+
 	pr_debug("CORE_RD[%u] - Released device space for Ramdisk"
 		" Device ID: %u, pages %u in %u tables total bytes %lu\n",
 		rd_dev->rd_host->rd_host_id, rd_dev->rd_dev_id, page_count,
 		rd_dev->sg_table_count, (unsigned long)page_count * PAGE_SIZE);
 
-	kfree(sg_table);
 	rd_dev->sg_table_array = NULL;
 	rd_dev->sg_table_count = 0;
 }
@@ -124,38 +127,15 @@ static void rd_release_device_space(struct rd_dev *rd_dev)
  *
  *
  */
-static int rd_build_device_space(struct rd_dev *rd_dev)
+static int rd_allocate_sgl_table(struct rd_dev *rd_dev, struct rd_dev_sg_table *sg_table,
+				 u32 total_sg_needed, unsigned char init_payload)
 {
-	u32 i = 0, j, page_offset = 0, sg_per_table, sg_tables, total_sg_needed;
+	u32 i = 0, j, page_offset = 0, sg_per_table;
 	u32 max_sg_per_table = (RD_MAX_ALLOCATION_SIZE /
 				sizeof(struct scatterlist));
-	struct rd_dev_sg_table *sg_table;
 	struct page *pg;
 	struct scatterlist *sg;
-
-	if (rd_dev->rd_page_count <= 0) {
-		pr_err("Illegal page count: %u for Ramdisk device\n",
-			rd_dev->rd_page_count);
-		return -EINVAL;
-	}
-
-	/* Don't need backing pages for NULLIO */
-	if (rd_dev->rd_flags & RDF_NULLIO)
-		return 0;
-
-	total_sg_needed = rd_dev->rd_page_count;
-
-	sg_tables = (total_sg_needed / max_sg_per_table) + 1;
-
-	sg_table = kzalloc(sg_tables * sizeof(struct rd_dev_sg_table), GFP_KERNEL);
-	if (!sg_table) {
-		pr_err("Unable to allocate memory for Ramdisk"
-			" scatterlist tables\n");
-		return -ENOMEM;
-	}
-
-	rd_dev->sg_table_array = sg_table;
-	rd_dev->sg_table_count = sg_tables;
+	unsigned char *p;
 
 	while (total_sg_needed) {
 		sg_per_table = (total_sg_needed > max_sg_per_table) ?
@@ -186,16 +166,59 @@ static int rd_build_device_space(struct rd_dev *rd_dev)
 			}
 			sg_assign_page(&sg[j], pg);
 			sg[j].length = PAGE_SIZE;
+
+			p = kmap(pg);
+			memset(p, init_payload, PAGE_SIZE);
+			kunmap(pg);
 		}
 
 		page_offset += sg_per_table;
 		total_sg_needed -= sg_per_table;
 	}
 
+	return 0;
+}
+
+static int rd_build_device_space(struct rd_dev *rd_dev)
+{
+	struct rd_dev_sg_table *sg_table;
+	u32 sg_tables, total_sg_needed;
+	u32 max_sg_per_table = (RD_MAX_ALLOCATION_SIZE /
+				sizeof(struct scatterlist));
+	int rc;
+
+	if (rd_dev->rd_page_count <= 0) {
+		pr_err("Illegal page count: %u for Ramdisk device\n",
+		       rd_dev->rd_page_count);
+		return -EINVAL;
+	}
+
+	/* Don't need backing pages for NULLIO */
+	if (rd_dev->rd_flags & RDF_NULLIO)
+		return 0;
+
+	total_sg_needed = rd_dev->rd_page_count;
+
+	sg_tables = (total_sg_needed / max_sg_per_table) + 1;
+
+	sg_table = kzalloc(sg_tables * sizeof(struct rd_dev_sg_table), GFP_KERNEL);
+	if (!sg_table) {
+		pr_err("Unable to allocate memory for Ramdisk"
+		       " scatterlist tables\n");
+		return -ENOMEM;
+	}
+
+	rd_dev->sg_table_array = sg_table;
+	rd_dev->sg_table_count = sg_tables;
+
+	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0x00);
+	if (rc)
+		return rc;
+
 	pr_debug("CORE_RD[%u] - Built Ramdisk Device ID: %u space of"
-		" %u pages in %u tables\n", rd_dev->rd_host->rd_host_id,
-		rd_dev->rd_dev_id, rd_dev->rd_page_count,
-		rd_dev->sg_table_count);
+		 " %u pages in %u tables\n", rd_dev->rd_host->rd_host_id,
+		 rd_dev->rd_dev_id, rd_dev->rd_page_count,
+		 rd_dev->sg_table_count);
 
 	return 0;
 }
-- 
1.7.10.4

[PATCH-v2 15/17] target/rd: Add support for protection SGL setup + release

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds rd_build_prot_space() + rd_release_prot_space() logic
to setup + release protection information scatterlists.

It also adds rd_init_prot() + rd_free_prot() se_subsystem_api
callbacks used by target core code for setup + release of
protection information.

v2 changes:
  - Drop unused sg_table from rd_release_prot_space (Wei)
  - Drop rd_release_prot_space call from rd_free_device

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_rd.c |   74 +++++++++++++++++++++++++++++++++++++++
 drivers/target/target_core_rd.h |    4 +++
 2 files changed, 78 insertions(+)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index e9fa879..660961d 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -223,6 +223,61 @@ static int rd_build_device_space(struct rd_dev *rd_dev)
 	return 0;
 }
 
+static void rd_release_prot_space(struct rd_dev *rd_dev)
+{
+	u32 page_count;
+
+	if (!rd_dev->sg_prot_array || !rd_dev->sg_prot_count)
+		return;
+
+	page_count = rd_release_sgl_table(rd_dev, rd_dev->sg_prot_array,
+					  rd_dev->sg_prot_count);
+
+	pr_debug("CORE_RD[%u] - Released protection space for Ramdisk"
+		 " Device ID: %u, pages %u in %u tables total bytes %lu\n",
+		 rd_dev->rd_host->rd_host_id, rd_dev->rd_dev_id, page_count,
+		 rd_dev->sg_table_count, (unsigned long)page_count * PAGE_SIZE);
+
+	rd_dev->sg_prot_array = NULL;
+	rd_dev->sg_prot_count = 0;
+}
+
+static int rd_build_prot_space(struct rd_dev *rd_dev, int prot_length)
+{
+	struct rd_dev_sg_table *sg_table;
+	u32 total_sg_needed, sg_tables;
+	u32 max_sg_per_table = (RD_MAX_ALLOCATION_SIZE /
+				sizeof(struct scatterlist));
+	int rc;
+
+	if (rd_dev->rd_flags & RDF_NULLIO)
+		return 0;
+
+	total_sg_needed = rd_dev->rd_page_count / prot_length;
+
+	sg_tables = (total_sg_needed / max_sg_per_table) + 1;
+
+	sg_table = kzalloc(sg_tables * sizeof(struct rd_dev_sg_table), GFP_KERNEL);
+	if (!sg_table) {
+		pr_err("Unable to allocate memory for Ramdisk protection"
+		       " scatterlist tables\n");
+		return -ENOMEM;
+	}
+
+	rd_dev->sg_prot_array = sg_table;
+	rd_dev->sg_prot_count = sg_tables;
+
+	rc = rd_allocate_sgl_table(rd_dev, sg_table, total_sg_needed, 0xff);
+	if (rc)
+		return rc;
+
+	pr_debug("CORE_RD[%u] - Built Ramdisk Device ID: %u prot space of"
+		 " %u pages in %u tables\n", rd_dev->rd_host->rd_host_id,
+		 rd_dev->rd_dev_id, total_sg_needed, rd_dev->sg_prot_count);
+
+	return 0;
+}
+
 static struct se_device *rd_alloc_device(struct se_hba *hba, const char *name)
 {
 	struct rd_dev *rd_dev;
@@ -479,6 +534,23 @@ static sector_t rd_get_blocks(struct se_device *dev)
 	return blocks_long;
 }
 
+static int rd_init_prot(struct se_device *dev)
+{
+	struct rd_dev *rd_dev = RD_DEV(dev);
+
+        if (!dev->dev_attrib.pi_prot_type)
+		return 0;
+
+	return rd_build_prot_space(rd_dev, dev->prot_length);
+}
+
+static void rd_free_prot(struct se_device *dev)
+{
+	struct rd_dev *rd_dev = RD_DEV(dev);
+
+	rd_release_prot_space(rd_dev);
+}
+
 static struct sbc_ops rd_sbc_ops = {
 	.execute_rw		= rd_execute_rw,
 };
@@ -504,6 +576,8 @@ static struct se_subsystem_api rd_mcp_template = {
 	.show_configfs_dev_params = rd_show_configfs_dev_params,
 	.get_device_type	= sbc_get_device_type,
 	.get_blocks		= rd_get_blocks,
+	.init_prot		= rd_init_prot,
+	.free_prot		= rd_free_prot,
 };
 
 int __init rd_module_init(void)
diff --git a/drivers/target/target_core_rd.h b/drivers/target/target_core_rd.h
index 1789d1e..cc46a6a 100644
--- a/drivers/target/target_core_rd.h
+++ b/drivers/target/target_core_rd.h
@@ -33,8 +33,12 @@ struct rd_dev {
 	u32		rd_page_count;
 	/* Number of SG tables in sg_table_array */
 	u32		sg_table_count;
+	/* Number of SG tables in sg_prot_array */
+	u32		sg_prot_count;
 	/* Array of rd_dev_sg_table_t containing scatterlists */
 	struct rd_dev_sg_table *sg_table_array;
+	/* Array of rd_dev_sg_table containing protection scatterlists */
+	struct rd_dev_sg_table *sg_prot_array;
 	/* Ramdisk HBA device is connected to */
 	struct rd_host *rd_host;
 } ____cacheline_aligned;
-- 
1.7.10.4

[PATCH-v2 16/17] target/rd: Add DIF protection into rd_execute_rw

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for DIF protection into rd_execute_rw() code
for WRITE/READ I/O using sbc_dif_verify_[write,read]() logic.

It also adds rd_get_prot_table() for locating protection SGLs
assoicated with the ramdisk backend device.

v2 changes:
  - Make rd_execute_rw() to u32 sectors count instead of sector_t
  - Drop SCF_PROT usage

Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_rd.c |   65 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/target/target_core_rd.c b/drivers/target/target_core_rd.c
index 660961d..66a5aba 100644
--- a/drivers/target/target_core_rd.c
+++ b/drivers/target/target_core_rd.c
@@ -356,6 +356,26 @@ static struct rd_dev_sg_table *rd_get_sg_table(struct rd_dev *rd_dev, u32 page)
 	return NULL;
 }
 
+static struct rd_dev_sg_table *rd_get_prot_table(struct rd_dev *rd_dev, u32 page)
+{
+	struct rd_dev_sg_table *sg_table;
+	u32 i, sg_per_table = (RD_MAX_ALLOCATION_SIZE /
+				sizeof(struct scatterlist));
+
+	i = page / sg_per_table;
+	if (i < rd_dev->sg_prot_count) {
+		sg_table = &rd_dev->sg_prot_array[i];
+		if ((sg_table->page_start_offset <= page) &&
+		     (sg_table->page_end_offset >= page))
+			return sg_table;
+	}
+
+	pr_err("Unable to locate struct prot rd_dev_sg_table for page: %u\n",
+			page);
+
+	return NULL;
+}
+
 static sense_reason_t
 rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	      enum dma_data_direction data_direction)
@@ -370,6 +390,7 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	u32 rd_page;
 	u32 src_len;
 	u64 tmp;
+	sense_reason_t rc;
 
 	if (dev->rd_flags & RDF_NULLIO) {
 		target_complete_cmd(cmd, SAM_STAT_GOOD);
@@ -392,6 +413,28 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 			data_direction == DMA_FROM_DEVICE ? "Read" : "Write",
 			cmd->t_task_lba, rd_size, rd_page, rd_offset);
 
+	if (cmd->prot_type && data_direction == DMA_TO_DEVICE) {
+		struct rd_dev_sg_table *prot_table;
+		struct scatterlist *prot_sg;
+		u32 sectors = cmd->data_length / se_dev->dev_attrib.block_size;
+		u32 prot_offset, prot_page;
+
+		tmp = cmd->t_task_lba * se_dev->prot_length;
+		prot_offset = do_div(tmp, PAGE_SIZE);
+		prot_page = tmp;
+
+		prot_table = rd_get_prot_table(dev, prot_page);
+		if (!prot_table)
+			return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+
+		prot_sg = &prot_table->sg_table[prot_page - prot_table->page_start_offset];
+
+		rc = sbc_dif_verify_write(cmd, cmd->t_task_lba, sectors, 0,
+					  prot_sg, prot_offset);
+		if (rc)
+			return rc;
+	}
+
 	src_len = PAGE_SIZE - rd_offset;
 	sg_miter_start(&m, sgl, sgl_nents,
 			data_direction == DMA_FROM_DEVICE ?
@@ -453,6 +496,28 @@ rd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	}
 	sg_miter_stop(&m);
 
+	if (cmd->prot_type && data_direction == DMA_FROM_DEVICE) {
+		struct rd_dev_sg_table *prot_table;
+		struct scatterlist *prot_sg;
+		u32 sectors = cmd->data_length / se_dev->dev_attrib.block_size;
+		u32 prot_offset, prot_page;
+
+		tmp = cmd->t_task_lba * se_dev->prot_length;
+		prot_offset = do_div(tmp, PAGE_SIZE);
+		prot_page = tmp;
+
+		prot_table = rd_get_prot_table(dev, prot_page);
+		if (!prot_table)
+			return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+
+		prot_sg = &prot_table->sg_table[prot_page - prot_table->page_start_offset];
+
+		rc = sbc_dif_verify_read(cmd, cmd->t_task_lba, sectors, 0,
+					 prot_sg, prot_offset);
+		if (rc)
+			return rc;
+	}
+
 	target_complete_cmd(cmd, SAM_STAT_GOOD);
 	return 0;
 }
-- 
1.7.10.4

[PATCH-v2 17/17] tcm_loop: Enable DIF/DIX modes in SCSI host LLD

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch updates tcm_loop_driver_probe() to set protection
information using scsi_host_set_prot() and scsi_host_set_guard(),
which currently enabled all modes of DIF/DIX protection, minus
DIX TYPE0.

Also, update tcm_loop_submission_work() to pass struct scsi_cmnd
related protection into target_submit_cmd_map_sgls() during CDB
dispatch.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Sagi Grimberg <sagig@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/loopback/tcm_loop.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 112b795..fadad7c 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -217,7 +217,8 @@ static void tcm_loop_submission_work(struct work_struct *work)
 			scsi_bufflen(sc), tcm_loop_sam_attr(sc),
 			sc->sc_data_direction, 0,
 			scsi_sglist(sc), scsi_sg_count(sc),
-			sgl_bidi, sgl_bidi_count, NULL, 0);
+			sgl_bidi, sgl_bidi_count,
+			scsi_prot_sglist(sc), scsi_prot_sg_count(sc));
 	if (rc < 0) {
 		set_host_byte(sc, DID_NO_CONNECT);
 		goto out_done;
@@ -462,7 +463,7 @@ static int tcm_loop_driver_probe(struct device *dev)
 {
 	struct tcm_loop_hba *tl_hba;
 	struct Scsi_Host *sh;
-	int error;
+	int error, host_prot;
 
 	tl_hba = to_tcm_loop_hba(dev);
 
@@ -486,6 +487,13 @@ static int tcm_loop_driver_probe(struct device *dev)
 	sh->max_channel = 0;
 	sh->max_cmd_len = TL_SCSI_MAX_CMD_LEN;
 
+	host_prot = SHOST_DIF_TYPE1_PROTECTION | SHOST_DIF_TYPE2_PROTECTION |
+		    SHOST_DIF_TYPE3_PROTECTION | SHOST_DIX_TYPE1_PROTECTION |
+		    SHOST_DIX_TYPE2_PROTECTION | SHOST_DIX_TYPE3_PROTECTION;
+
+	scsi_host_set_prot(sh, host_prot);
+	scsi_host_set_guard(sh, SHOST_DIX_GUARD_CRC);
+
 	error = scsi_add_host(sh, &tl_hba->dev);
 	if (error) {
 		pr_err("%s: scsi_add_host failed\n", __func__);
-- 
1.7.10.4