[PATCH v1 3/3] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire
In various areas of the code, it is assumed that
se_cmd-data_length describes pure data. In case
that protection information exists over the wire
(protect bits is are on) the target core decrease
the protection length from the data length (instead
of each transport peeking in the cdb).
Modify loopback device to include protection information
in the transferred data length (like other scsi transports).
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
drivers/target/loopback/tcm_loop.c | 15 ---
drivers/target/target_core_sbc.c | 15 +--
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
index c886ad1..1f4c015 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -179,7 +179,7 @@ static void tcm_loop_submission_work(struct work_struct
*work)
struct tcm_loop_hba *tl_hba;
struct tcm_loop_tpg *tl_tpg;
struct scatterlist *sgl_bidi = NULL;
- u32 sgl_bidi_count = 0;
+ u32 sgl_bidi_count = 0, transfer_length;
int rc;
tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
@@ -213,12 +213,21 @@ static void tcm_loop_submission_work(struct work_struct
*work)
}
- if (!scsi_prot_sg_count(sc) scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
+ transfer_length = scsi_transfer_length(sc);
+ if (!scsi_prot_sg_count(sc)
+ scsi_get_prot_op(sc) != SCSI_PROT_NORMAL) {
se_cmd-prot_pto = true;
+ /*
+* loopback transport doesn't support
+* WRITE_GENERATE, READ_STRIP protection
+* information operations, go ahead unprotected.
+*/
+ transfer_length = scsi_bufflen(sc);
+ }
rc = target_submit_cmd_map_sgls(se_cmd, tl_nexus-se_sess, sc-cmnd,
tl_cmd-tl_sense_buf[0], tl_cmd-sc-device-lun,
- scsi_bufflen(sc), tcm_loop_sam_attr(sc),
+ transfer_length, tcm_loop_sam_attr(sc),
sc-sc_data_direction, 0,
scsi_sglist(sc), scsi_sg_count(sc),
sgl_bidi, sgl_bidi_count,
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index e022959..06f8ecd 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -665,8 +665,19 @@ sbc_check_prot(struct se_device *dev, struct se_cmd *cmd,
unsigned char *cdb,
cmd-prot_type = dev-dev_attrib.pi_prot_type;
cmd-prot_length = dev-prot_length * sectors;
- pr_debug(%s: prot_type=%d, prot_length=%d prot_op=%d prot_checks=%d\n,
-__func__, cmd-prot_type, cmd-prot_length,
+
+ /**
+* In case protection information exists over the wire
+* we modify command data length to describe pure data.
+* The actual transfer length is data length + protection
+* length
+**/
+ if (protect)
+ cmd-data_length -= cmd-prot_length;
+
+ pr_debug(%s: prot_type=%d, data_length=%d, prot_length=%d
+prot_op=%d prot_checks=%d\n,
+__func__, cmd-prot_type, cmd-data_length, cmd-prot_length,
cmd-prot_op, cmd-prot_checks);
return true;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 0/3] Include protection information in iscsi header
At the SCSI transport level, there is no distinction between user data and protection information. Thus, iscsi header field expected data transfer length should include protection information. Patch #1 introduces scsi helpers scsi_transfer_length (compute wire transfer byte count) and scsi_prot_len (compute protection information byte count). Patch #2 modifies iscsi initiator to set correct wire transfer length in iscsi header data_length field (and modifies iser accordingly). Patch #3 modifies target core to expect protection information included in the wire transfer length (and modifies loopback transport to do so). I have 2 patches that convert lpfc/qla2xxx drivers to use scsi helpers but these are completely untested at the moment. Once we get this set to land upstream, I can queue them up as RFCs. Changes from v0: - Introduce scsi helpers to compute correct transfer length in the presence of protection information (instead of having each transport doing the same computation). - Modify iscsi to set correct transfer length using scsi helpers - Modify loopback transport to set correct transfer length using scsi helpers Sagi Grimberg (3): scsi_cmnd: Introduce scsi_transfer_length helper libiscsi, iser: Adjust data_length to include protection information TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire drivers/infiniband/ulp/iser/iser_initiator.c | 34 ++ drivers/scsi/libiscsi.c | 18 ++-- drivers/target/loopback/tcm_loop.c | 15 -- drivers/target/target_core_sbc.c | 15 - include/scsi/scsi_cmnd.h | 39 ++ 5 files changed, 83 insertions(+), 38 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 1/3] scsi_cmnd: Introduce scsi_transfer_length helper
In case protection information exists on the wire
scsi transports should include it in the transfer
byte count (even if protection information does not
exist in the host memory space). This helper will
compute the total transfer length from the scsi
command data length and protection attributes.
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
include/scsi/scsi_cmnd.h | 39 +++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index dd7c998..84d9593 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -7,6 +7,7 @@
#include linux/types.h
#include linux/timer.h
#include linux/scatterlist.h
+#include scsi/scsi_device.h
struct Scsi_Host;
struct scsi_device;
@@ -306,4 +307,42 @@ static inline void set_driver_byte(struct scsi_cmnd *cmd,
char status)
cmd-result = (cmd-result 0x00ff) | (status 24);
}
+static inline unsigned scsi_prot_length(unsigned data_length,
+ unsigned sector_size)
+{
+ switch (sector_size) {
+ case 512:
+ return (data_length 9) * 8;
+ case 1024:
+ return (data_length 10) * 8;
+ case 2048:
+ return (data_length 11) * 8;
+ case 4096:
+ return (data_length 12) * 8;
+ default:
+ return (data_length ilog2(sector_size)) * 8;
+ }
+}
+
+static inline unsigned scsi_transfer_length(struct scsi_cmnd *cmd)
+{
+ unsigned data_length;
+
+ if (cmd-sc_data_direction == DMA_FROM_DEVICE) {
+ data_length = scsi_in(cmd)-length;
+ if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL ||
+ scsi_get_prot_op(cmd) == SCSI_PROT_READ_INSERT)
+ return data_length;
+ } else {
+ data_length = scsi_out(cmd)-length;
+ if (scsi_get_prot_op(cmd) == SCSI_PROT_NORMAL ||
+ scsi_get_prot_op(cmd) == SCSI_PROT_WRITE_STRIP)
+ return data_length;
+ }
+
+ /* Protection information exists on the wire */
+ return data_length + scsi_prot_length(data_length,
+ cmd-device-sector_size);
+}
+
#endif /* _SCSI_SCSI_CMND_H */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1 2/3] libiscsi, iser: Adjust data_length to include protection information
In case protection information exists over the wire
iscsi header data_length is required to include it.
Use protection information aware scsi helpers to set
the correct transfer length.
In order to avoid breakage, remove iser transfer length
checks for each task as they are not always true and
somewhat redundant anyway.
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
drivers/infiniband/ulp/iser/iser_initiator.c | 34 +++--
drivers/scsi/libiscsi.c | 18 +++---
2 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index 2e2d903..e3b0af5 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -41,11 +41,11 @@
#include iscsi_iser.h
/* Register user buffer memory and initialize passive rdma
- * dto descriptor. Total data size is stored in
- * iser_task-data[ISER_DIR_IN].data_len
+ * dto descriptor. Data size is stored in
+ * task-data[ISER_DIR_IN].data_len, Protection size
+ * os stored in task-prot[ISER_DIR_IN].data_len
*/
-static int iser_prepare_read_cmd(struct iscsi_task *task,
-unsigned int edtl)
+static int iser_prepare_read_cmd(struct iscsi_task *task)
{
struct iscsi_iser_task *iser_task = task-dd_data;
@@ -73,14 +73,6 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
return err;
}
- if (edtl iser_task-data[ISER_DIR_IN].data_len) {
- iser_err(Total data length: %ld, less than EDTL:
-%d, in READ cmd BHS itt: %d, conn: 0x%p\n,
-iser_task-data[ISER_DIR_IN].data_len, edtl,
-task-itt, iser_task-ib_conn);
- return -EINVAL;
- }
-
err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
if (err) {
iser_err(Failed to set up Data-IN RDMA\n);
@@ -100,8 +92,9 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
}
/* Register user buffer memory and initialize passive rdma
- * dto descriptor. Total data size is stored in
- * task-data[ISER_DIR_OUT].data_len
+ * dto descriptor. Data size is stored in
+ * task-data[ISER_DIR_OUT].data_len, Protection size
+ * is stored at task-prot[ISER_DIR_OUT].data_len
*/
static int
iser_prepare_write_cmd(struct iscsi_task *task,
@@ -135,14 +128,6 @@ iser_prepare_write_cmd(struct iscsi_task *task,
return err;
}
- if (edtl iser_task-data[ISER_DIR_OUT].data_len) {
- iser_err(Total data length: %ld, less than EDTL: %d,
-in WRITE cmd BHS itt: %d, conn: 0x%p\n,
-iser_task-data[ISER_DIR_OUT].data_len,
-edtl, task-itt, task-conn);
- return -EINVAL;
- }
-
err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
if (err != 0) {
iser_err(Failed to register write cmd RDMA mem\n);
@@ -417,11 +402,12 @@ int iser_send_command(struct iscsi_conn *conn,
if (scsi_prot_sg_count(sc)) {
prot_buf-buf = scsi_prot_sglist(sc);
prot_buf-size = scsi_prot_sg_count(sc);
- prot_buf-data_len = sc-prot_sdb-length;
+ prot_buf-data_len = scsi_prot_length(data_buf-data_len,
+ sc-device-sector_size);
}
if (hdr-flags ISCSI_FLAG_CMD_READ) {
- err = iser_prepare_read_cmd(task, edtl);
+ err = iser_prepare_read_cmd(task);
if (err)
goto send_command_error;
}
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 26dc005..3f46234 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -338,7 +338,7 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
struct iscsi_session *session = conn-session;
struct scsi_cmnd *sc = task-sc;
struct iscsi_scsi_req *hdr;
- unsigned hdrlength, cmd_len;
+ unsigned hdrlength, cmd_len, transfer_length;
itt_t itt;
int rc;
@@ -391,11 +391,11 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task
*task)
if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
task-protected = true;
+ transfer_length = scsi_transfer_length(sc);
+ hdr-data_length = cpu_to_be32(transfer_length);
if (sc-sc_data_direction == DMA_TO_DEVICE) {
- unsigned out_len = scsi_out(sc)-length;
struct iscsi_r2t_info *r2t = task-unsol_r2t;
- hdr-data_length = cpu_to_be32(out_len);
hdr-flags |= ISCSI_FLAG_CMD_WRITE;
/*
* Write counters:
@@ -414,18 +414,19 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task
*task)
memset(r2t, 0,
Re: [PATCH-v2 0/6] vhost/scsi: Add T10 PI SGL passthrough support
On Thu, May 22, 2014 at 02:26:16AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi MST, MKP, Paolo Co, Here is the v2 patch series for adding T1O protection information (PI) SGL passthrough support between virtio-scsi LLD + vhost-scsi fabric endpoints. Following MST's recommendation, it includes the changes for using bytes intead of number of iovecs in virtio_scsi_cmd_req_pi along with the associated changes to virtio-scsi + vhost/scsi code. For vhost/scsi, this includes walking the leading iovec's length(s) to determine where protection payload ends, and real data payload starts. For virtio-scsi LLD code, this includes locating struct blk_integrity for using blk_rq_sectors + -tuple_size to calculate the total bytes for outgoing cmd_pi-pi_bytes[out,in] values. The full list of changes from last series include: - Use pi_bytesout + pi_bytesin instead of niovs in virtio-scsi PI header (mst + paolo) - Add prot_pto=1 in tcm_vhost_submission_work() when no PI buffer exists (nab) - Get rid of req + cdb pointer casts in vhost_scsi_handle_vq (mst) - Ensure that virtio_scsi_cmd_req_pi processing happens regardless of data_num in vhost_scsi_handle_vq (nab) - Pass TARGET_PROT_ALL into transport_init_session_tags() (nab) - Convert vhost_scsi_handle_vq to use memcpy_fromiovecend() (mst) - Convert vhost_scsi_handle_vq to use pi_bytesout + pi_bytesin (nab) - Convert virtio_scsi_init_hdr_pi() to use pi_bytesout + pi_bytesin (mst + paolo + nab) - Use blk_integrity-tuple_size to calculate pi bytes (nab) Please review for v3.16-rc1 code. Thanks! --nab OK, finally went over this, looks good to me: Acked-by: Michael S. Tsirkin m...@redhat.com However, we really should stop making more changes before fixing ANY_LAYOUT in this driver. virtio 1.0 should be out soon and that makes ANY_LAYOUT a required feature. Nicholas Bellinger (6): virtio-scsi.h: Add virtio_scsi_cmd_req_pi + VIRTIO_SCSI_F_T10_PI bits vhost/scsi: Move sanity check into vhost_scsi_map_iov_to_sgl vhost/scsi: Add preallocation of protection SGLs vhost/scsi: Add T10 PI IOV - SGL memory mapping logic vhost/scsi: Enable T10 PI IOV - SGL memory mapping virtio-scsi: Enable DIF/DIX modes in SCSI host LLD drivers/scsi/virtio_scsi.c | 86 +--- drivers/vhost/scsi.c| 305 +-- include/linux/virtio_scsi.h | 15 ++- 3 files changed, 292 insertions(+), 114 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND] [SCSI] bfa: Fix undefined bit shift on big-endian architectures with 32-bit DMA address
bfa_swap_words() shifts its argument (assumed to be 64-bit) by 32 bits
each way. In two places the argument type is dma_addr_t, which may be
32-bit, in which case the effect of the bit shift is undefined:
drivers/scsi/bfa/bfa_fcpim.c: In function 'bfa_ioim_send_ioreq':
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: left shift count = width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: right shift count = width of
type [enabled by default]
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: left shift count = width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: right shift count = width of
type [enabled by default]
Avoid this by adding casts to u64 in bfa_swap_words().
Compile-tested only.
Signed-off-by: Ben Hutchings b...@decadent.org.uk
Cc: sta...@vger.kernel.org
Fixes: f16a17507b09 ('[SCSI] bfa: remove all OS wrappers')
---
drivers/scsi/bfa/bfa_ioc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 2e28392..a38aafa0 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -72,7 +72,7 @@ struct bfa_sge_s {
} while (0)
#define bfa_swap_words(_x) ( \
- ((_x) 32) | ((_x) 32))
+ ((u64)(_x) 32) | ((u64)(_x) 32))
#ifdef __BIG_ENDIAN
#define bfa_sge_to_be(_x)
--
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.
signature.asc
Description: This is a digitally signed message part
[PATCH RESEND] [SCSI] aic94xx: Remove broken fallback for missing 'Ctrl-A' user settings
asd_process_ctrl_a_user() attempts to find user settings in flash, and
if they are missing it prepares a substitute structure containing
default values for PHY settings. But having done so, it will still
try to read user settings - from some random address in flash, as the
local variable 'offs' has not been initialised.
Since asd_common_setup() already sets default PHY settings, there
seems to be no need to repeat them here, and we can just return 0.
Compile-tested only.
Signed-off-by: Ben Hutchings b...@decadent.org.uk
---
drivers/scsi/aic94xx/aic94xx_sds.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..f5d51d2 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -981,29 +981,15 @@ static int asd_process_ctrla_phy_settings(struct
asd_ha_struct *asd_ha,
static int asd_process_ctrl_a_user(struct asd_ha_struct *asd_ha,
struct asd_flash_dir *flash_dir)
{
- int err, i;
+ int err;
u32 offs, size;
struct asd_ll_el *el;
struct asd_ctrla_phy_settings *ps;
- struct asd_ctrla_phy_settings dflt_ps;
err = asd_find_flash_de(flash_dir, FLASH_DE_CTRL_A_USER, offs, size);
if (err) {
ASD_DPRINTK(couldn't find CTRL-A user settings section\n);
- ASD_DPRINTK(Creating default CTRL-A user settings section\n);
-
- dflt_ps.id0 = 'h';
- dflt_ps.num_phys = 8;
- for (i =0; i ASD_MAX_PHYS; i++) {
- memcpy(dflt_ps.phy_ent[i].sas_addr,
- asd_ha-hw_prof.sas_addr, SAS_ADDR_SIZE);
- dflt_ps.phy_ent[i].sas_link_rates = 0x98;
- dflt_ps.phy_ent[i].flags = 0x0;
- dflt_ps.phy_ent[i].sata_link_rates = 0x0;
- }
-
- size = sizeof(struct asd_ctrla_phy_settings);
- ps = dflt_ps;
+ return 0;
}
if (size == 0)
--
Ben Hutchings
Never attribute to conspiracy what can adequately be explained by stupidity.
signature.asc
Description: This is a digitally signed message part
[PATCH RESEND] [SCSI] bfa: Fix undefined bit shift on big-endian architectures with 32-bit DMA address
bfa_swap_words() shifts its argument (assumed to be 64-bit) by 32 bits
each way. In two places the argument type is dma_addr_t, which may be
32-bit, in which case the effect of the bit shift is undefined:
drivers/scsi/bfa/bfa_fcpim.c: In function 'bfa_ioim_send_ioreq':
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: left shift count = width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2497:4: warning: right shift count = width of
type [enabled by default]
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: left shift count = width of type
[enabled by default]
addr = bfa_sgaddr_le(sg_dma_address(sg));
^
drivers/scsi/bfa/bfa_fcpim.c:2509:4: warning: right shift count = width of
type [enabled by default]
Avoid this by adding casts to u64 in bfa_swap_words().
Compile-tested only.
Signed-off-by: Ben Hutchings b...@decadent.org.uk
Cc: sta...@vger.kernel.org
Fixes: f16a17507b09 ('[SCSI] bfa: remove all OS wrappers')
---
Resending yet again, this time with current maintainer addresses.
Ben.
drivers/scsi/bfa/bfa_ioc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bfa/bfa_ioc.h b/drivers/scsi/bfa/bfa_ioc.h
index 2e28392..a38aafa0 100644
--- a/drivers/scsi/bfa/bfa_ioc.h
+++ b/drivers/scsi/bfa/bfa_ioc.h
@@ -72,7 +72,7 @@ struct bfa_sge_s {
} while (0)
#define bfa_swap_words(_x) ( \
- ((_x) 32) | ((_x) 32))
+ ((u64)(_x) 32) | ((u64)(_x) 32))
#ifdef __BIG_ENDIAN
#define bfa_sge_to_be(_x)
--
Ben Hutchings
One of the nice things about standards is that there are so many of them.
signature.asc
Description: This is a digitally signed message part
