target: fix DPO and FUA bit checks

Drivers may override the WCE flag, in which case the DPOFUA flag in
MODE SENSE might differ from the check used to reject invalid FUA
bits in sbc_check_dpofua.  Also now that we reject invalid FUA
bits early there is no need to duplicate the same buggy check
down in the fileio code.

As the DPOFUA flag controls th support for FUA bits on read and
write commands as well as DPO key off all the checks off a single
helper, and deprecate the emulate_dpo and emulate_fua_read attributs.

This fixes various failures in the libiscsi testsuite.

Personally I'd prefer to also remove the emulate_fua_write attribute
as there is no good reason to disable it, but I'll leave that for
a separate discussion.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index ce5f768181ff6593a7afac365214c77b0f0aceab..9ee194d1cd4be682e2bd5829ad66b304cd03719e 100644 (file)
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -760,16 +760,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_model_alias);
 
 int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
 {
 
 int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
 {

-       if (flag != 0 && flag != 1) {
-               pr_err("Illegal value %d\n", flag);
-               return -EINVAL;
-       }
-
-       if (flag) {
-               pr_err("dpo_emulated not supported\n");
-               return -EINVAL;
-       }
-
+       printk_once(KERN_WARNING
+               "ignoring deprecated emulate_dpo attribute\n");

        return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_dpo);
        return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_dpo);


@@ -799,16 +791,8 @@ EXPORT_SYMBOL(se_dev_set_emulate_fua_write);
 
 int se_dev_set_emulate_fua_read(struct se_device *dev, int flag)
 {
 
 int se_dev_set_emulate_fua_read(struct se_device *dev, int flag)
 {

-       if (flag != 0 && flag != 1) {
-               pr_err("Illegal value %d\n", flag);
-               return -EINVAL;
-       }
-
-       if (flag) {
-               pr_err("ua read emulated not supported\n");
-               return -EINVAL;
-       }
-
+       printk_once(KERN_WARNING
+               "ignoring deprecated emulate_fua_read attribute\n");

        return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_fua_read);
        return 0;
 }
 EXPORT_SYMBOL(se_dev_set_emulate_fua_read);


@@ -1513,9 +1497,9 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 
        dev->dev_attrib.da_dev = dev;
        dev->dev_attrib.emulate_model_alias = DA_EMULATE_MODEL_ALIAS;
 
        dev->dev_attrib.da_dev = dev;
        dev->dev_attrib.emulate_model_alias = DA_EMULATE_MODEL_ALIAS;

-       dev->dev_attrib.emulate_dpo = DA_EMULATE_DPO;
-       dev->dev_attrib.emulate_fua_write = DA_EMULATE_FUA_WRITE;
-       dev->dev_attrib.emulate_fua_read = DA_EMULATE_FUA_READ;
+       dev->dev_attrib.emulate_dpo = 1;
+       dev->dev_attrib.emulate_fua_write = 1;
+       dev->dev_attrib.emulate_fua_read = 1;

        dev->dev_attrib.emulate_write_cache = DA_EMULATE_WRITE_CACHE;
        dev->dev_attrib.emulate_ua_intlck_ctrl = DA_EMULATE_UA_INTLLCK_CTRL;
        dev->dev_attrib.emulate_tas = DA_EMULATE_TAS;
        dev->dev_attrib.emulate_write_cache = DA_EMULATE_WRITE_CACHE;
        dev->dev_attrib.emulate_ua_intlck_ctrl = DA_EMULATE_UA_INTLLCK_CTRL;
        dev->dev_attrib.emulate_tas = DA_EMULATE_TAS;

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index fe6c19c1e001caa3e0ef5e4e474ce69e0ce28315..e865885352da01550edd6b86ad268cfedba7e797 100644 (file)
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -610,9 +610,7 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
                 * for SCSI WRITEs with Forced Unit Access (FUA) set.
                 * Allow this to happen independent of WCE=0 setting.
                 */
                 * for SCSI WRITEs with Forced Unit Access (FUA) set.
                 * Allow this to happen independent of WCE=0 setting.
                 */

-               if (ret > 0 &&
-                   dev->dev_attrib.emulate_fua_write > 0 &&
-                   (cmd->se_cmd_flags & SCF_FUA)) {
+               if (ret > 0 && (cmd->se_cmd_flags & SCF_FUA)) {

                        loff_t start = cmd->t_task_lba *
                                dev->dev_attrib.block_size;
                        loff_t end;
                        loff_t start = cmd->t_task_lba *
                                dev->dev_attrib.block_size;
                        loff_t end;

diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 68bd7f5d9f73cf6feacd2dfefb951db99dd21c4f..75338c348be4c9b1161c16345eba0ad0af9c8164 100644 (file)
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -81,6 +81,8 @@ int   transport_clear_lun_ref(struct se_lun *);
 void   transport_send_task_abort(struct se_cmd *);
 sense_reason_t target_cmd_size_check(struct se_cmd *cmd, unsigned int size);
 void   target_qf_do_work(struct work_struct *work);
 void   transport_send_task_abort(struct se_cmd *);
 sense_reason_t target_cmd_size_check(struct se_cmd *cmd, unsigned int size);
 void   target_qf_do_work(struct work_struct *work);

+bool   target_check_wce(struct se_device *dev);
+bool   target_check_fua(struct se_device *dev);

 
 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);
 
 /* target_core_stat.c */
 void   target_stat_setup_dev_default_groups(struct se_device *);

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 3df2cd53847831f09476aae1d614035c599ceb42..d441975604db6e386a25d7fa45a1f1c7b883e14f 100644 (file)
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -738,14 +738,15 @@ static int
 sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
 {
        if (cdb[1] & 0x10) {
 sbc_check_dpofua(struct se_device *dev, struct se_cmd *cmd, unsigned char *cdb)
 {
        if (cdb[1] & 0x10) {

-               if (!dev->dev_attrib.emulate_dpo) {
+               /* see explanation in spc_emulate_modesense */
+               if (!target_check_fua(dev)) {

                        pr_err("Got CDB: 0x%02x with DPO bit set, but device"
                               " does not advertise support for DPO\n", cdb[0]);
                        return -EINVAL;
                }
        }
        if (cdb[1] & 0x8) {
                        pr_err("Got CDB: 0x%02x with DPO bit set, but device"
                               " does not advertise support for DPO\n", cdb[0]);
                        return -EINVAL;
                }
        }
        if (cdb[1] & 0x8) {

-               if (!dev->dev_attrib.emulate_fua_write || !se_dev_check_wce(dev)) {
+               if (!target_check_fua(dev)) {

                        pr_err("Got CDB: 0x%02x with FUA bit set, but device"
                               " does not advertise support for FUA write\n",
                               cdb[0]);
                        pr_err("Got CDB: 0x%02x with FUA bit set, but device"
                               " does not advertise support for FUA write\n",
                               cdb[0]);

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 7912aa1243857a8968dcff5fa0f643b8e249dd6e..988c158cf65d149e787de81caf02ff829a8c3167 100644 (file)
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -481,7 +481,7 @@ spc_emulate_evpd_86(struct se_cmd *cmd, unsigned char *buf)
        buf[5] = 0x07;
 
        /* If WriteCache emulation is enabled, set V_SUP */
        buf[5] = 0x07;
 
        /* If WriteCache emulation is enabled, set V_SUP */

-       if (se_dev_check_wce(dev))
+       if (target_check_wce(dev))

                buf[6] = 0x01;
        /* If an LBA map is present set R_SUP */
        spin_lock(&cmd->se_dev->t10_alua.lba_map_lock);
                buf[6] = 0x01;
        /* If an LBA map is present set R_SUP */
        spin_lock(&cmd->se_dev->t10_alua.lba_map_lock);


@@ -888,7 +888,7 @@ static int spc_modesense_caching(struct se_cmd *cmd, u8 pc, u8 *p)
        if (pc == 1)
                goto out;
 
        if (pc == 1)
                goto out;
 

-       if (se_dev_check_wce(dev))
+       if (target_check_wce(dev))

                p[2] = 0x04; /* Write Cache Enable */
        p[12] = 0x20; /* Disabled Read Ahead */
 
                p[2] = 0x04; /* Write Cache Enable */
        p[12] = 0x20; /* Disabled Read Ahead */
 


@@ -1000,8 +1000,12 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
             (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY)))
                spc_modesense_write_protect(&buf[length], type);
 
             (cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_READ_ONLY)))
                spc_modesense_write_protect(&buf[length], type);
 

-       if ((se_dev_check_wce(dev)) &&
-           (dev->dev_attrib.emulate_fua_write > 0))
+       /*
+        * SBC only allows us to enable FUA and DPO together.  Fortunately
+        * DPO is explicitly specified as a hint, so a noop is a perfectly
+        * valid implementation.
+        */
+       if (target_check_fua(dev))

                spc_modesense_dpofua(&buf[length], type);
 
        ++length;
                spc_modesense_dpofua(&buf[length], type);
 
        ++length;

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 105f4fd0fad4f8675af14a874005ec1393edde65..231812d613576568c6ad8f05501dff53d517a250 100644 (file)
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -3075,3 +3075,22 @@ int transport_generic_handle_tmr(
        return 0;
 }
 EXPORT_SYMBOL(transport_generic_handle_tmr);
        return 0;
 }
 EXPORT_SYMBOL(transport_generic_handle_tmr);

+
+bool
+target_check_wce(struct se_device *dev)
+{
+       bool wce = false;
+
+       if (dev->transport->get_write_cache)
+               wce = dev->transport->get_write_cache(dev);
+       else if (dev->dev_attrib.emulate_write_cache > 0)
+               wce = true;
+
+       return wce;
+}
+
+bool
+target_check_fua(struct se_device *dev)
+{
+       return target_check_wce(dev) && dev->dev_attrib.emulate_fua_write > 0;
+}

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 480e9f82dfea861a70fa5e1df45076770c719f2e..7f4c7de3a4ce7a2506c1db5117f71fea72f5f530 100644 (file)
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -79,12 +79,6 @@
 #define DA_MAX_WRITE_SAME_LEN                  0
 /* Use a model alias based on the configfs backend device name */
 #define DA_EMULATE_MODEL_ALIAS                 0
 #define DA_MAX_WRITE_SAME_LEN                  0
 /* Use a model alias based on the configfs backend device name */
 #define DA_EMULATE_MODEL_ALIAS                 0

-/* Emulation for Direct Page Out */
-#define DA_EMULATE_DPO                         0
-/* Emulation for Forced Unit Access WRITEs */
-#define DA_EMULATE_FUA_WRITE                   1
-/* Emulation for Forced Unit Access READs */
-#define DA_EMULATE_FUA_READ                    0

 /* Emulation for WriteCache and SYNCHRONIZE_CACHE */
 #define DA_EMULATE_WRITE_CACHE                 0
 /* Emulation for UNIT ATTENTION Interlock Control */
 /* Emulation for WriteCache and SYNCHRONIZE_CACHE */
 #define DA_EMULATE_WRITE_CACHE                 0
 /* Emulation for UNIT ATTENTION Interlock Control */