Re: [PATCH 3/6] scsi: target: core: dynamic opcode support in RSOC

[Mike Christie <michael.christie@oracle.com>]

On 7/18/22 7:01 AM, Dmitry Bogdanov wrote:
> Report supported opcodes depending on a dynamic device configuration
> 
> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
> ---
>  drivers/target/target_core_spc.c  | 118 ++++++++++++++++++++++++++++--
>  include/target/target_core_base.h |   1 +
>  2 files changed, 114 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
> index 506e28b14e5a..cf516136b933 100644
> --- a/drivers/target/target_core_spc.c
> +++ b/drivers/target/target_core_spc.c
> @@ -1424,6 +1424,13 @@ static struct target_opcode_descriptor tcm_opcode_xdwriteread32 = {
>  		       0xff, 0xff, 0xff, 0xff},
>  };
>  
> +static bool tcm_is_ws_enabled(struct se_cmd *cmd)
> +{
> +	struct se_device *dev = cmd->se_dev;
> +
> +	return dev->dev_attrib.emulate_tpws;
> +}
> +
>  static struct target_opcode_descriptor tcm_opcode_write_same32 = {
>  	.support = SCSI_SUPPORT_FULL,
>  	.serv_action_valid = 1,
> @@ -1438,8 +1445,16 @@ static struct target_opcode_descriptor tcm_opcode_write_same32 = {
>  		       0x00, 0x00, 0x00, 0x00,
>  		       0x00, 0x00, 0x00, 0x00,
>  		       0xff, 0xff, 0xff, 0xff},
> +	.enabled = tcm_is_ws_enabled,
>  };

I'm not sure what's incorrect. I think your patch is correct but the write
same code is wrong.

If emulate_tpws is 0, we will still execute the command. We actually only fail
with TCM_UNSUPPORTED_SCSI_OPCODE if it's a WRITE_SAME with the UNMAP bit = 1
and emulate_tpws=0.

If it's just a normal WRITE_SAME we maybe go by if by max_write_same_len is
greater than zero? Maybe that was a mistake and sbc_setup_write_same needs
a emulate_tpws check.



>  static struct target_opcode_descriptor tcm_opcode_sync_cache = {
> @@ -1502,6 +1533,14 @@ static struct target_opcode_descriptor tcm_opcode_sync_cache16 = {
>  		       0xff, 0xff, SCSI_GROUP_NUMBER_MASK, SCSI_CONTROL_MASK},
>  };
>  
> +static bool tcm_is_unmap_enabled(struct se_cmd *cmd)
> +{
> +	struct sbc_ops *ops = cmd->protocol_data;
> +	struct se_device *dev = cmd->se_dev;
> +
> +	return ops->execute_unmap  && dev->dev_attrib.emulate_tpu;
> +}

Just a trivial nit. You had an extra space there.