Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Thu, 2015-07-23 at 15:27 -0700, Spencer Baugh wrote: > From: Brian Bunker > > AIX servers using VIOS servers that virtualize FC cards will have a > problem booting without support for START_STOP_UNIT. > > v2: Cite sb3r36 exactly, clean up if conditions > > Signed-off-by: Brian Bunker > Signed-off-by: Spencer Baugh > --- > drivers/target/target_core_sbc.c | 36 > 1 file changed, 36 insertions(+) > Applied to target-pending/for-next with extra sb3 -> sbc3 typo fixup. Thanks Brian + Spencer! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Thu, 2015-07-23 at 15:27 -0700, Spencer Baugh wrote: From: Brian Bunker br...@purestorage.com AIX servers using VIOS servers that virtualize FC cards will have a problem booting without support for START_STOP_UNIT. v2: Cite sb3r36 exactly, clean up if conditions Signed-off-by: Brian Bunker br...@purestorage.com Signed-off-by: Spencer Baugh sba...@catern.com --- drivers/target/target_core_sbc.c | 36 1 file changed, 36 insertions(+) Applied to target-pending/for-next with extra sb3 - sbc3 typo fixup. Thanks Brian + Spencer! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Fri, Jul 24, 2015 at 03:06:12AM +, Elliott, Robert (Server Storage) wrote: > Note that the officially published versions of the ISO and ANSI > standards don't carry that revision number r36; they just have > the standard name and year. SBC-3 revision 36 became > "ANSI INCITS 514-2014 Information technology - SCSI Block > Commands - 3 (SBC-3)". > > T10 isn't really obligated to keep making particular working > drafts available, although the ones that have been assigned > version descriptors (in SPC-n) are more likely to stick > around. For SBC-3, only revisions 35 and 36 earned those. Unfortunately the final T10 standards are only available for a high monetary cost, so they aren't useful for Free Software development. We work arounds this by referencing specific drafts that are available. If T10 would regress even more by not providing them we'd have to find other work arounds. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Fri, Jul 24, 2015 at 03:06:12AM +, Elliott, Robert (Server Storage) wrote: Note that the officially published versions of the ISO and ANSI standards don't carry that revision number r36; they just have the standard name and year. SBC-3 revision 36 became ANSI INCITS 514-2014 Information technology - SCSI Block Commands - 3 (SBC-3). T10 isn't really obligated to keep making particular working drafts available, although the ones that have been assigned version descriptors (in SPC-n) are more likely to stick around. For SBC-3, only revisions 35 and 36 earned those. Unfortunately the final T10 standards are only available for a high monetary cost, so they aren't useful for Free Software development. We work arounds this by referencing specific drafts that are available. If T10 would regress even more by not providing them we'd have to find other work arounds. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Spencer Baugh
> Sent: Thursday, July 23, 2015 5:28 PM
> To: Christoph Hellwig ; Spencer Baugh
>
...
> Subject: Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
>
> From: Brian Bunker
>
> AIX servers using VIOS servers that virtualize FC cards will have a
> problem booting without support for START_STOP_UNIT.
>
> v2: Cite sb3r36 exactly, clean up if conditions
>
> Signed-off-by: Brian Bunker
> Signed-off-by: Spencer Baugh
...
> v2: Cite sb3r36 exactly, clean up if conditions
>
...
> diff --git a/drivers/target/target_core_sbc.c
> b/drivers/target/target_core_sbc.c
> index e318ddb..85c3c0a 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -154,6 +154,38 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd)
> return 0;
> }
>
> +static sense_reason_t
> +sbc_emulate_startstop(struct se_cmd *cmd)
> +{
> + unsigned char *cdb = cmd->t_task_cdb;
> +
> + /*
> + * See sb3r36 section 5.25
Global typo in this patch - sb3 should be sbc3.
Editorial comment:
Note that the officially published versions of the ISO and ANSI
standards don't carry that revision number r36; they just have
the standard name and year. SBC-3 revision 36 became
"ANSI INCITS 514-2014 Information technology - SCSI Block
Commands - 3 (SBC-3)".
T10 isn't really obligated to keep making particular working
drafts available, although the ones that have been assigned
version descriptors (in SPC-n) are more likely to stick
around. For SBC-3, only revisions 35 and 36 earned those.
Section numbers are quite volatile, too, so you might be
better off including the section name. It's starting to
become standardese, but this kind of wording might be better:
"See the SBC-3 START STOP UNIT command description
(e.g., sbc3r36 5.25)"
> + /* From SBC-3:
> + * Immediate bit should be set since there is nothing to complete
> + * POWER CONDITION MODIFIER 0h
> + */
> + if (!(cdb[1] & 1) || (cdb[2] | cdb[3]))
> + return TCM_INVALID_CDB_FIELD;
Technical comment:
The application client is not obligated to set the IMMED bit here.
In fact, that's an unusual choice. Using the IMMED bit means the
application client must handle the initial status for the
command, then poll for the functional results with REQUEST SENSE
and TEST UNIT READY commands, which is much more complicated.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
From: Brian Bunker
AIX servers using VIOS servers that virtualize FC cards will have a
problem booting without support for START_STOP_UNIT.
v2: Cite sb3r36 exactly, clean up if conditions
Signed-off-by: Brian Bunker
Signed-off-by: Spencer Baugh
---
drivers/target/target_core_sbc.c | 36
1 file changed, 36 insertions(+)
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index e318ddb..85c3c0a 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -154,6 +154,38 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd)
return 0;
}
+static sense_reason_t
+sbc_emulate_startstop(struct se_cmd *cmd)
+{
+ unsigned char *cdb = cmd->t_task_cdb;
+
+ /*
+* See sb3r36 section 5.25
+* Immediate bit should be set since there is nothing to complete
+* POWER CONDITION MODIFIER 0h
+*/
+ if (!(cdb[1] & 1) || cdb[2] || cdb[3])
+ return TCM_INVALID_CDB_FIELD;
+
+ /*
+* See sb3r36 section 5.25
+* POWER CONDITION 0h START_VALID - process START and LOEJ
+*/
+ if (cdb[4] >> 4 & 0xf)
+ return TCM_INVALID_CDB_FIELD;
+
+ /*
+* See sb3r36 section 5.25
+* LOEJ 0h - nothing to load or unload
+* START 1h - we are ready
+*/
+ if (!(cdb[4] & 1) || (cdb[4] & 2) || (cdb[4] & 4))
+ return TCM_INVALID_CDB_FIELD;
+
+ target_complete_cmd(cmd, SAM_STAT_GOOD);
+ return 0;
+}
+
sector_t sbc_get_write_same_sectors(struct se_cmd *cmd)
{
u32 num_blocks;
@@ -1069,6 +1101,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
size = 0;
cmd->execute_cmd = sbc_emulate_noop;
break;
+ case START_STOP:
+ size = 0;
+ cmd->execute_cmd = sbc_emulate_startstop;
+ break;
default:
ret = spc_parse_cdb(cmd, );
if (ret)
--
2.5.0.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Tue, Jul 21, 2015 at 03:07:53PM -0700, Spencer Baugh wrote:
> +static sense_reason_t
> +sbc_emulate_startstop(struct se_cmd *cmd)
> +{
> + unsigned char *cdb = cmd->t_task_cdb;
> +
> + /* From SBC-3:
> + * Immediate bit should be set since there is nothing to complete
> + * POWER CONDITION MODIFIER 0h
> + */
Mot of the target code mentions the exact document and section, e.g.
drivers/target/target_core_alua.c: * See sbc3r35 section 5.23
drivers/target/target_core_sbc.c:* Set Thin Provisioning Enable bit
following sbc3r22 in section
drivers/target/target_core_sbc.c:* From sbc3r22.pdf section 5.48
XDWRITEREAD (10) command
Also if you fix thing up anyway the Linux style is to keep the opening
"/*" on a separate line.
> + if (!(cdb[1] & 1) || (cdb[2] | cdb[3]))
> + return TCM_INVALID_CDB_FIELD;
The mix of || and | here is odd, why not just:
if (!(cdb[1] & 1) || cdb[2] || cdb[3])
> + if (!(cdb[4] & 1) || ((cdb[4] & 2) | (cdb[4] & 4)))
Same here.
But except for this nitpicking the patch looks good:
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
From: Brian Bunker br...@purestorage.com
AIX servers using VIOS servers that virtualize FC cards will have a
problem booting without support for START_STOP_UNIT.
v2: Cite sb3r36 exactly, clean up if conditions
Signed-off-by: Brian Bunker br...@purestorage.com
Signed-off-by: Spencer Baugh sba...@catern.com
---
drivers/target/target_core_sbc.c | 36
1 file changed, 36 insertions(+)
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index e318ddb..85c3c0a 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -154,6 +154,38 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd)
return 0;
}
+static sense_reason_t
+sbc_emulate_startstop(struct se_cmd *cmd)
+{
+ unsigned char *cdb = cmd-t_task_cdb;
+
+ /*
+* See sb3r36 section 5.25
+* Immediate bit should be set since there is nothing to complete
+* POWER CONDITION MODIFIER 0h
+*/
+ if (!(cdb[1] 1) || cdb[2] || cdb[3])
+ return TCM_INVALID_CDB_FIELD;
+
+ /*
+* See sb3r36 section 5.25
+* POWER CONDITION 0h START_VALID - process START and LOEJ
+*/
+ if (cdb[4] 4 0xf)
+ return TCM_INVALID_CDB_FIELD;
+
+ /*
+* See sb3r36 section 5.25
+* LOEJ 0h - nothing to load or unload
+* START 1h - we are ready
+*/
+ if (!(cdb[4] 1) || (cdb[4] 2) || (cdb[4] 4))
+ return TCM_INVALID_CDB_FIELD;
+
+ target_complete_cmd(cmd, SAM_STAT_GOOD);
+ return 0;
+}
+
sector_t sbc_get_write_same_sectors(struct se_cmd *cmd)
{
u32 num_blocks;
@@ -1069,6 +1101,10 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
size = 0;
cmd-execute_cmd = sbc_emulate_noop;
break;
+ case START_STOP:
+ size = 0;
+ cmd-execute_cmd = sbc_emulate_startstop;
+ break;
default:
ret = spc_parse_cdb(cmd, size);
if (ret)
--
2.5.0.rc3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
-Original Message-
From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
ow...@vger.kernel.org] On Behalf Of Spencer Baugh
Sent: Thursday, July 23, 2015 5:28 PM
To: Christoph Hellwig h...@infradead.org; Spencer Baugh
sba...@catern.com
...
Subject: Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
From: Brian Bunker br...@purestorage.com
AIX servers using VIOS servers that virtualize FC cards will have a
problem booting without support for START_STOP_UNIT.
v2: Cite sb3r36 exactly, clean up if conditions
Signed-off-by: Brian Bunker br...@purestorage.com
Signed-off-by: Spencer Baugh sba...@catern.com
...
v2: Cite sb3r36 exactly, clean up if conditions
...
diff --git a/drivers/target/target_core_sbc.c
b/drivers/target/target_core_sbc.c
index e318ddb..85c3c0a 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -154,6 +154,38 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd)
return 0;
}
+static sense_reason_t
+sbc_emulate_startstop(struct se_cmd *cmd)
+{
+ unsigned char *cdb = cmd-t_task_cdb;
+
+ /*
+ * See sb3r36 section 5.25
Global typo in this patch - sb3 should be sbc3.
Editorial comment:
Note that the officially published versions of the ISO and ANSI
standards don't carry that revision number r36; they just have
the standard name and year. SBC-3 revision 36 became
ANSI INCITS 514-2014 Information technology - SCSI Block
Commands - 3 (SBC-3).
T10 isn't really obligated to keep making particular working
drafts available, although the ones that have been assigned
version descriptors (in SPC-n) are more likely to stick
around. For SBC-3, only revisions 35 and 36 earned those.
Section numbers are quite volatile, too, so you might be
better off including the section name. It's starting to
become standardese, but this kind of wording might be better:
See the SBC-3 START STOP UNIT command description
(e.g., sbc3r36 5.25)
+ /* From SBC-3:
+ * Immediate bit should be set since there is nothing to complete
+ * POWER CONDITION MODIFIER 0h
+ */
+ if (!(cdb[1] 1) || (cdb[2] | cdb[3]))
+ return TCM_INVALID_CDB_FIELD;
Technical comment:
The application client is not obligated to set the IMMED bit here.
In fact, that's an unusual choice. Using the IMMED bit means the
application client must handle the initial status for the
command, then poll for the functional results with REQUEST SENSE
and TEST UNIT READY commands, which is much more complicated.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] target: add support for START_STOP_UNIT SCSI opcode
On Tue, Jul 21, 2015 at 03:07:53PM -0700, Spencer Baugh wrote:
+static sense_reason_t
+sbc_emulate_startstop(struct se_cmd *cmd)
+{
+ unsigned char *cdb = cmd-t_task_cdb;
+
+ /* From SBC-3:
+ * Immediate bit should be set since there is nothing to complete
+ * POWER CONDITION MODIFIER 0h
+ */
Mot of the target code mentions the exact document and section, e.g.
drivers/target/target_core_alua.c: * See sbc3r35 section 5.23
drivers/target/target_core_sbc.c:* Set Thin Provisioning Enable bit
following sbc3r22 in section
drivers/target/target_core_sbc.c:* From sbc3r22.pdf section 5.48
XDWRITEREAD (10) command
Also if you fix thing up anyway the Linux style is to keep the opening
/* on a separate line.
+ if (!(cdb[1] 1) || (cdb[2] | cdb[3]))
+ return TCM_INVALID_CDB_FIELD;
The mix of || and | here is odd, why not just:
if (!(cdb[1] 1) || cdb[2] || cdb[3])
+ if (!(cdb[4] 1) || ((cdb[4] 2) | (cdb[4] 4)))
Same here.
But except for this nitpicking the patch looks good:
Reviewed-by: Christoph Hellwig h...@lst.de
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
