On Thu, Aug 11, 2022 at 10:43:05PM -0500, Mike Christie wrote:
> «Внимание! Данное письмо от внешнего адресата!»
>
> 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@xxxxxxxxx>
> > Signed-off-by: Dmitry Bogdanov <d.bogdanov@xxxxxxxxx>
> > ---
> > 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.
Looks like emulate_tpws was introduced exaclty for WS+UNMAP bit case
and it can not be used in tcm_is_ws_enabled as only check. Because of
WS is actually two different commands selected by UNMAP bit it is
unable somehow to differentiate them in RSOC. So I will reformulate
the check in tcm_is_ws_enabled to be true if some of cases is
supported by the backstore device.
+ return (dev->dev_attrib.emulate_tpws && !!ops->execute_unmap) ||
+ !!ops->execute_write_same;
>
> > 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.
yep, will fix
