[PATCH v4 09/32] cxlflash: Correct naming of limbo state and waitq
Limbo is not an accurate representation of this state and is
also not consistent with the terminology that other drivers
use to represent this concept. Rename the state and and its
associated waitq to 'reset'.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h| 4 ++--
drivers/scsi/cxlflash/main.c | 26 +-
drivers/scsi/cxlflash/superpipe.c | 14 +++---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 1abe4e0..11318de 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -79,7 +79,7 @@ enum cxlflash_init_state {
enum cxlflash_state {
STATE_NORMAL, /* Normal running state, everything good */
- STATE_LIMBO,/* Limbo running state, trying to reset/recover */
+ STATE_RESET,/* Reset state, trying to reset/recover */
STATE_FAILTERM /* Failed/terminating state, error out users/threads */
};
@@ -125,7 +125,7 @@ struct cxlflash_cfg {
wait_queue_head_t tmf_waitq;
bool tmf_active;
- wait_queue_head_t limbo_waitq;
+ wait_queue_head_t reset_waitq;
enum cxlflash_state state;
};
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 6e85c77..8940336 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -382,8 +382,8 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
switch (cfg->state) {
- case STATE_LIMBO:
- dev_dbg_ratelimited(&cfg->dev->dev, "%s: device in limbo!\n",
+ case STATE_RESET:
+ dev_dbg_ratelimited(&cfg->dev->dev, "%s: device is in reset!\n",
__func__);
rc = SCSI_MLQUEUE_HOST_BUSY;
goto out;
@@ -479,8 +479,8 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
if (unlikely(rcr))
rc = FAILED;
break;
- case STATE_LIMBO:
- wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
+ case STATE_RESET:
+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
if (cfg->state == STATE_NORMAL)
break;
/* fall through */
@@ -519,7 +519,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd
*scp)
switch (cfg->state) {
case STATE_NORMAL:
- cfg->state = STATE_LIMBO;
+ cfg->state = STATE_RESET;
scsi_block_requests(cfg->host);
cxlflash_mark_contexts_error(cfg);
rcr = cxlflash_afu_reset(cfg);
@@ -528,11 +528,11 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
cfg->state = STATE_FAILTERM;
} else
cfg->state = STATE_NORMAL;
- wake_up_all(&cfg->limbo_waitq);
+ wake_up_all(&cfg->reset_waitq);
scsi_unblock_requests(cfg->host);
break;
- case STATE_LIMBO:
- wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
+ case STATE_RESET:
+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
if (cfg->state == STATE_NORMAL)
break;
/* fall through */
@@ -705,7 +705,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
cxlflash_cfg *cfg)
struct pci_dev *pdev = cfg->dev;
if (pci_channel_offline(pdev))
- wait_event_timeout(cfg->limbo_waitq,
+ wait_event_timeout(cfg->reset_waitq,
!pci_channel_offline(pdev),
CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);
}
@@ -2304,7 +2304,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->mcctx = NULL;
init_waitqueue_head(&cfg->tmf_waitq);
- init_waitqueue_head(&cfg->limbo_waitq);
+ init_waitqueue_head(&cfg->reset_waitq);
INIT_WORK(&cfg->work_q, cxlflash_worker_thread);
cfg->lr_state = LINK_RESET_INVALID;
@@ -2396,7 +2396,7 @@ static pci_ers_result_t
cxlflash_pci_error_detected(struct pci_dev *pdev,
switch (state) {
case pci_channel_io_frozen:
- cfg->state = STATE_LIMBO;
+ cfg->state = STATE_RESET;
scsi_block_requests(cfg->host);
drain_ioctls(cfg);
rc = cxlflash_mark_contexts_error(cfg);
@@ -2408,7 +2408,7 @@ static pci_ers_result_t
cxlflash_pci_error_detected(struct pci_dev *pdev,
[PATCH v4 10/32] cxlflash: Make functions static
Found during code inspection, that the following functions are not
being used outside of the file where they are defined. Make them static.
int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
int cxlflash_afu_reset(struct cxlflash_cfg *);
struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
void cxlflash_cmd_checkin(struct afu_cmd *);
void init_pcr(struct cxlflash_cfg *);
int init_global(struct cxlflash_cfg *);
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h |5 -
drivers/scsi/cxlflash/main.c | 1018
2 files changed, 509 insertions(+), 514 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 11318de..b038ac7 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -192,11 +192,6 @@ static inline u64 lun_to_lunid(u64 lun)
return swab64(lun_id);
}
-int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
-void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
-int cxlflash_afu_reset(struct cxlflash_cfg *);
-struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
-void cxlflash_cmd_checkin(struct afu_cmd *);
int cxlflash_afu_sync(struct afu *, ctx_hndl_t, res_hndl_t, u8);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 8940336..226cefe 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -36,7 +36,7 @@ MODULE_LICENSE("GPL");
/**
- * cxlflash_cmd_checkout() - checks out an AFU command
+ * cmd_checkout() - checks out an AFU command
* @afu: AFU to checkout from.
*
* Commands are checked out in a round-robin fashion. Note that since
@@ -47,7 +47,7 @@ MODULE_LICENSE("GPL");
*
* Return: The checked out command or NULL when command pool is empty.
*/
-struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
+static struct afu_cmd *cmd_checkout(struct afu *afu)
{
int k, dec = CXLFLASH_NUM_CMDS;
struct afu_cmd *cmd;
@@ -70,7 +70,7 @@ struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
}
/**
- * cxlflash_cmd_checkin() - checks in an AFU command
+ * cmd_checkin() - checks in an AFU command
* @cmd: AFU command to checkin.
*
* Safe to pass commands that have already been checked in. Several
@@ -79,7 +79,7 @@ struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
* to avoid clobbering values in the event that the command is checked
* out right away.
*/
-void cxlflash_cmd_checkin(struct afu_cmd *cmd)
+static void cmd_checkin(struct afu_cmd *cmd)
{
cmd->rcb.scp = NULL;
cmd->rcb.timeout = 0;
@@ -238,7 +238,7 @@ static void cmd_complete(struct afu_cmd *cmd)
resid = cmd->sa.resid;
cmd_is_tmf = cmd->cmd_tmf;
- cxlflash_cmd_checkin(cmd); /* Don't use cmd after here */
+ cmd_checkin(cmd); /* Don't use cmd after here */
pr_debug("%s: calling scsi_set_resid, scp=%p "
"result=%X resid=%d\n", __func__,
@@ -260,6 +260,146 @@ static void cmd_complete(struct afu_cmd *cmd)
}
/**
+ * context_reset() - timeout handler for AFU commands
+ * @cmd: AFU command that timed out.
+ *
+ * Sends a reset to the AFU.
+ */
+static void context_reset(struct afu_cmd *cmd)
+{
+ int nretry = 0;
+ u64 rrin = 0x1;
+ u64 room = 0;
+ struct afu *afu = cmd->parent;
+ ulong lock_flags;
+
+ pr_debug("%s: cmd=%p\n", __func__, cmd);
+
+ spin_lock_irqsave(&cmd->slock, lock_flags);
+
+ /* Already completed? */
+ if (cmd->sa.host_use_b[0] & B_DONE) {
+ spin_unlock_irqrestore(&cmd->slock, lock_flags);
+ return;
+ }
+
+ cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT);
+ spin_unlock_irqrestore(&cmd->slock, lock_flags);
+
+ /*
+* We really want to send this reset at all costs, so spread
+* out wait time on successive retries for available room.
+*/
+ do {
+ room = readq_be(&afu->host_map->cmd_room);
+ atomic64_set(&afu->room, room);
+ if (room)
+ goto write_rrin;
+ udelay(nretry);
+ } while (nretry++ < MC_ROOM_RETRY_CNT);
+
+ pr_err("%s: no cmd_room to send reset\n", __func__);
+ return;
+
+write_rrin:
+ nretry = 0;
+ writeq_be(rrin, &afu->host_map->ioarrin);
+ do {
+ rrin = readq_be(&afu->host_map->ioarrin);
+ if (rrin != 0x1)
+ break;
+ /* Double delay each time */
+ udelay(2 ^ nretry);
+ } while (nret
[PATCH v4 08/32] cxlflash: Fix to avoid CXL services during EEH
During an EEH freeze event, certain CXL services should not be
called until after the hardware reset has taken place. Doing so
can result in unnecessary failures and possibly cause other ill
effects by triggering hardware accesses. This translates to a
requirement to quiesce all threads that may potentially use CXL
runtime service during this window. In particular, multiple ioctls
make use of the CXL services when acting on contexts on behalf of
the user. Thus, it is essential to 'drain' running ioctls _before_
proceeding with handling the EEH freeze event.
Create the ability to drain ioctls by wrapping the ioctl handler
call in a read semaphore and then implementing a small routine that
obtains the write semaphore, effectively creating a wait point for
all currently executing ioctls.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
---
drivers/scsi/cxlflash/common.h| 2 +
drivers/scsi/cxlflash/main.c | 18 +--
drivers/scsi/cxlflash/superpipe.c | 98 ---
3 files changed, 77 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 1c56037..1abe4e0 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -16,6 +16,7 @@
#define _CXLFLASH_COMMON_H
#include
+#include
#include
#include
#include
@@ -110,6 +111,7 @@ struct cxlflash_cfg {
atomic_t recovery_threads;
struct mutex ctx_recovery_mutex;
struct mutex ctx_tbl_list_mutex;
+ struct rw_semaphore ioctl_rwsem;
struct ctx_info *ctx_tbl[MAX_CONTEXT];
struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
struct file_operations cxl_fops;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 3e3ccf1..6e85c77 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2311,6 +2311,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->lr_port = -1;
mutex_init(&cfg->ctx_tbl_list_mutex);
mutex_init(&cfg->ctx_recovery_mutex);
+ init_rwsem(&cfg->ioctl_rwsem);
INIT_LIST_HEAD(&cfg->ctx_err_recovery);
INIT_LIST_HEAD(&cfg->lluns);
@@ -2365,6 +2366,19 @@ out_remove:
}
/**
+ * drain_ioctls() - wait until all currently executing ioctls have completed
+ * @cfg: Internal structure associated with the host.
+ *
+ * Obtain write access to read/write semaphore that wraps ioctl
+ * handling to 'drain' ioctls currently executing.
+ */
+static void drain_ioctls(struct cxlflash_cfg *cfg)
+{
+ down_write(&cfg->ioctl_rwsem);
+ up_write(&cfg->ioctl_rwsem);
+}
+
+/**
* cxlflash_pci_error_detected() - called when a PCI error is detected
* @pdev: PCI device struct.
* @state: PCI channel state.
@@ -2383,16 +2397,14 @@ static pci_ers_result_t
cxlflash_pci_error_detected(struct pci_dev *pdev,
switch (state) {
case pci_channel_io_frozen:
cfg->state = STATE_LIMBO;
-
- /* Turn off legacy I/O */
scsi_block_requests(cfg->host);
+ drain_ioctls(cfg);
rc = cxlflash_mark_contexts_error(cfg);
if (unlikely(rc))
dev_err(dev, "%s: Failed to mark user contexts!(%d)\n",
__func__, rc);
term_mc(cfg, UNDO_START);
stop_afu(cfg);
-
return PCI_ERS_RESULT_NEED_RESET;
case pci_channel_io_perm_failure:
cfg->state = STATE_FAILTERM;
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 28aa9d9..655cbf1 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1214,6 +1214,46 @@ static const struct file_operations null_fops = {
};
/**
+ * check_state() - checks and responds to the current adapter state
+ * @cfg: Internal structure associated with the host.
+ *
+ * This routine can block and should only be used on process context.
+ * It assumes that the caller is an ioctl thread and holding the ioctl
+ * read semaphore. This is temporarily let up across the wait to allow
+ * for draining actively running ioctls. Also note that when waking up
+ * from waiting in reset, the state is unknown and must be checked again
+ * before proceeding.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static int check_state(struct cxlflash_cfg *cfg)
+{
+ struct device *dev = &cfg->dev->dev;
+ int rc = 0;
+
+retry:
+ switch (cfg->state) {
+ case STATE_LIMBO:
+ dev_dbg(dev, "%s: Limbo state, going to wait...\n", __func__);
+ up_read(&cfg->ioctl_rwsem);
+ rc = wait_event_interruptible(cfg->limbo_waitq,
+ cfg->state
[PATCH v4 11/32] cxlflash: Refine host/device attributes
Implement the following suggestions and add two new attributes
to allow for debugging the port LUN table.
- use scnprintf() instead of snprintf()
- use DEVICE_ATTR_RO and DEVICE_ATTR_RW
Suggested-by: Shane Seymour
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 180 +--
1 file changed, 138 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 226cefe..b44212b 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1995,33 +1995,24 @@ static int cxlflash_change_queue_depth(struct
scsi_device *sdev, int qdepth)
/**
* cxlflash_show_port_status() - queries and presents the current port status
- * @dev: Generic device associated with the host owning the port.
- * @attr: Device attribute representing the port.
+ * @port: Desired port for status reporting.
+ * @afu: AFU owning the specified port.
* @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
*
* Return: The size of the ASCII string returned in @buf.
*/
-static ssize_t cxlflash_show_port_status(struct device *dev,
-struct device_attribute *attr,
-char *buf)
+static ssize_t cxlflash_show_port_status(u32 port, struct afu *afu, char *buf)
{
- struct Scsi_Host *shost = class_to_shost(dev);
- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
- struct afu *afu = cfg->afu;
-
char *disp_status;
- int rc;
- u32 port;
u64 status;
- u64 *fc_regs;
+ __be64 __iomem *fc_regs;
- rc = kstrtouint((attr->attr.name + 4), 10, &port);
- if (rc || (port >= NUM_FC_PORTS))
+ if (port >= NUM_FC_PORTS)
return 0;
fc_regs = &afu->afu_map->global.fc_regs[port][0];
- status =
- (readq_be(&fc_regs[FC_MTIP_STATUS / 8]) & FC_MTIP_STATUS_MASK);
+ status = readq_be(&fc_regs[FC_MTIP_STATUS / 8]);
+ status &= FC_MTIP_STATUS_MASK;
if (status == FC_MTIP_STATUS_ONLINE)
disp_status = "online";
@@ -2030,31 +2021,69 @@ static ssize_t cxlflash_show_port_status(struct device
*dev,
else
disp_status = "unknown";
- return snprintf(buf, PAGE_SIZE, "%s\n", disp_status);
+ return scnprintf(buf, PAGE_SIZE, "%s\n", disp_status);
+}
+
+/**
+ * port0_show() - queries and presents the current status of port 0
+ * @dev: Generic device associated with the host owning the port.
+ * @attr: Device attribute representing the port.
+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
+ *
+ * Return: The size of the ASCII string returned in @buf.
+ */
+static ssize_t port0_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
+ struct afu *afu = cfg->afu;
+
+ return cxlflash_show_port_status(0, afu, buf);
}
/**
- * cxlflash_show_lun_mode() - presents the current LUN mode of the host
+ * port1_show() - queries and presents the current status of port 1
+ * @dev: Generic device associated with the host owning the port.
+ * @attr: Device attribute representing the port.
+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
+ *
+ * Return: The size of the ASCII string returned in @buf.
+ */
+static ssize_t port1_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
+ struct afu *afu = cfg->afu;
+
+ return cxlflash_show_port_status(1, afu, buf);
+}
+
+/**
+ * lun_mode_show() - presents the current LUN mode of the host
* @dev: Generic device associated with the host.
- * @attr: Device attribute representing the lun mode.
+ * @attr: Device attribute representing the LUN mode.
* @buf: Buffer of length PAGE_SIZE to report back the LUN mode in ASCII.
*
* Return: The size of the ASCII string returned in @buf.
*/
-static ssize_t cxlflash_show_lun_mode(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t lun_mode_show(struct device *dev,
+struct device_attribute *attr, char *buf)
{
struct Scsi_Host *shost = class_to_shost(dev);
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
struct afu *afu = cfg->afu;
- return sn
[PATCH v4 13/32] cxlflash: Fix to avoid stall while waiting on TMF
Borrowing the TMF waitq's spinlock causes a stall condition when
waiting for the TMF to complete. To remedy, introduce our own spin
lock to serialize TMF and use the appropriate wait services.
Also add a timeout while waiting for a TMF completion. When a TMF
times out, report back a failure such that a bigger hammer reset
can occur.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 55 +-
2 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index b038ac7..7a0cb5c 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -124,6 +124,7 @@ struct cxlflash_cfg {
struct list_head lluns; /* list of llun_info structs */
wait_queue_head_t tmf_waitq;
+ spinlock_t tmf_slock;
bool tmf_active;
wait_queue_head_t reset_waitq;
enum cxlflash_state state;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 527ff85..110037d 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -249,11 +249,10 @@ static void cmd_complete(struct afu_cmd *cmd)
scp->scsi_done(scp);
if (cmd_is_tmf) {
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
cfg->tmf_active = false;
wake_up_all_locked(&cfg->tmf_waitq);
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock,
- lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
}
} else
complete(&cmd->cevent);
@@ -420,6 +419,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp,
u64 tmfcmd)
struct device *dev = &cfg->dev->dev;
ulong lock_flags;
int rc = 0;
+ ulong to;
cmd = cmd_checkout(afu);
if (unlikely(!cmd)) {
@@ -428,15 +428,15 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
goto out;
}
- /* If a Task Management Function is active, do not send one more.
-*/
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ /* When Task Management Function is active do not send another */
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
if (cfg->tmf_active)
- wait_event_interruptible_locked_irq(cfg->tmf_waitq,
- !cfg->tmf_active);
+ wait_event_interruptible_lock_irq(cfg->tmf_waitq,
+ !cfg->tmf_active,
+ cfg->tmf_slock);
cfg->tmf_active = true;
cmd->cmd_tmf = true;
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
cmd->rcb.ctx_id = afu->ctx_hndl;
cmd->rcb.port_sel = port_sel;
@@ -457,15 +457,24 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
rc = send_cmd(afu, cmd);
if (unlikely(rc)) {
cmd_checkin(cmd);
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
cfg->tmf_active = false;
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
goto out;
}
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
- wait_event_interruptible_locked_irq(cfg->tmf_waitq, !cfg->tmf_active);
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
+ to = msecs_to_jiffies(5000);
+ to = wait_event_interruptible_lock_irq_timeout(cfg->tmf_waitq,
+ !cfg->tmf_active,
+ cfg->tmf_slock,
+ to);
+ if (!to) {
+ cfg->tmf_active = false;
+ dev_err(dev, "%s: TMF timed out!\n", __func__);
+ rc = -1;
+ }
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
out:
return rc;
}
@@ -512,16 +521,17 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
[PATCH v4 12/32] cxlflash: Fix to avoid spamming the kernel log
During run-time the driver can be very chatty and spam the system
kernel log. Various print statements can be limited and/or moved
to development-only mode. Additionally, numerous prints can be
converted to trace the corresponding device.
The following changes were made:
- pr_debug to pr_devel
- pr_debug to pr_debug_ratelimited
- pr_err to dev_err
- pr_debug to dev_dbg
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Conflicts:
drivers/scsi/cxlflash/main.c
---
drivers/scsi/cxlflash/main.c | 109 +++
1 file changed, 59 insertions(+), 50 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b44212b..527ff85 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -58,8 +58,8 @@ static struct afu_cmd *cmd_checkout(struct afu *afu)
cmd = &afu->cmd[k];
if (!atomic_dec_if_positive(&cmd->free)) {
- pr_debug("%s: returning found index=%d\n",
-__func__, cmd->slot);
+ pr_devel("%s: returning found index=%d cmd=%p\n",
+__func__, cmd->slot, cmd);
memset(cmd->buf, 0, CMD_BUFSIZE);
memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
return cmd;
@@ -93,7 +93,7 @@ static void cmd_checkin(struct afu_cmd *cmd)
return;
}
- pr_debug("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
+ pr_devel("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
}
/**
@@ -127,7 +127,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
}
pr_debug("%s: cmd failed afu_rc=%d scsi_rc=%d fc_rc=%d "
-"afu_extra=0x%X, scsi_entra=0x%X, fc_extra=0x%X\n",
+"afu_extra=0x%X, scsi_extra=0x%X, fc_extra=0x%X\n",
__func__, ioasa->rc.afu_rc, ioasa->rc.scsi_rc,
ioasa->rc.fc_rc, ioasa->afu_extra, ioasa->scsi_extra,
ioasa->fc_extra);
@@ -240,9 +240,9 @@ static void cmd_complete(struct afu_cmd *cmd)
cmd_is_tmf = cmd->cmd_tmf;
cmd_checkin(cmd); /* Don't use cmd after here */
- pr_debug("%s: calling scsi_set_resid, scp=%p "
-"result=%X resid=%d\n", __func__,
-scp, scp->result, resid);
+ pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "
+"ioasc=%d\n", __func__, scp, scp->result,
+cmd->sa.ioasc);
scsi_set_resid(scp, resid);
scsi_dma_unmap(scp);
@@ -417,12 +417,13 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
short lflag = 0;
struct Scsi_Host *host = scp->device->host;
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
+ struct device *dev = &cfg->dev->dev;
ulong lock_flags;
int rc = 0;
cmd = cmd_checkout(afu);
if (unlikely(!cmd)) {
- pr_err("%s: could not get a free command\n", __func__);
+ dev_err(dev, "%s: could not get a free command\n", __func__);
rc = SCSI_MLQUEUE_HOST_BUSY;
goto out;
}
@@ -493,7 +494,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
{
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
struct afu *afu = cfg->afu;
- struct pci_dev *pdev = cfg->dev;
+ struct device *dev = &cfg->dev->dev;
struct afu_cmd *cmd;
u32 port_sel = scp->device->channel + 1;
int nseg, i, ncount;
@@ -502,13 +503,14 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
short lflag = 0;
int rc = 0;
- pr_debug("%s: (scp=%p) %d/%d/%d/%llu cdb=(%08X-%08X-%08X-%08X)\n",
-__func__, scp, host->host_no, scp->device->channel,
-scp->device->id, scp->device->lun,
-get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+ dev_dbg_ratelimited(dev, "%s: (scp=%p) %d/%d/%d/%llu "
+ "cdb=(%08X-%08X-%08X-%08X)\n",
+ __func__, scp, host->host_no, scp->device->channel,
+
[PATCH v4 16/32] cxlflash: Fix async interrupt bypass logic
A bug was introduced earlier in the development cycle when cleaning
up logic statements. Instead of skipping bits that are not set, set
bits are skipped, causing async interrupts to not be handled correctly.
To fix, simply add back in the proper evaluation for an unset bit.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 98fdac1..ed9fd8c 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1315,7 +1315,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
/* check each bit that is on */
for (i = 0; reg_unmasked; i++, reg_unmasked = (reg_unmasked >> 1)) {
info = find_ainfo(1ULL << i);
- if ((reg_unmasked & 0x1) || !info)
+ if (((reg_unmasked & 0x1) == 0) || !info)
continue;
port = info->port;
--
2.1.0
--
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 v4 14/32] cxlflash: Fix location of setting resid
The resid is incorrectly set which can lead to unnecessary retry
attempts by the stack. This is due to resid _always_ being set
using a value returned from the adapter. Instead, the value
should only be interpreted and set when in an underrun scenario.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 110037d..5503a40 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -107,6 +107,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
{
struct sisl_ioarcb *ioarcb;
struct sisl_ioasa *ioasa;
+ u32 resid;
if (unlikely(!cmd))
return;
@@ -115,9 +116,10 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
ioasa = &(cmd->sa);
if (ioasa->rc.flags & SISL_RC_FLAGS_UNDERRUN) {
- pr_debug("%s: cmd underrun cmd = %p scp = %p\n",
-__func__, cmd, scp);
- scp->result = (DID_ERROR << 16);
+ resid = ioasa->resid;
+ scsi_set_resid(scp, resid);
+ pr_debug("%s: cmd underrun cmd = %p scp = %p, resid = %d\n",
+__func__, cmd, scp, resid);
}
if (ioasa->rc.flags & SISL_RC_FLAGS_OVERRUN) {
@@ -158,8 +160,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
/* If the SISL_RC_FLAGS_OVERRUN flag was set,
* then we will handle this error else where.
* If not then we must handle it here.
-* This is probably an AFU bug. We will
-* attempt a retry to see if that resolves it.
+* This is probably an AFU bug.
*/
scp->result = (DID_ERROR << 16);
}
@@ -183,7 +184,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
/* We have an AFU error */
switch (ioasa->rc.afu_rc) {
case SISL_AFU_RC_NO_CHANNELS:
- scp->result = (DID_MEDIUM_ERROR << 16);
+ scp->result = (DID_NO_CONNECT << 16);
break;
case SISL_AFU_RC_DATA_DMA_ERR:
switch (ioasa->afu_extra) {
@@ -217,7 +218,6 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
static void cmd_complete(struct afu_cmd *cmd)
{
struct scsi_cmnd *scp;
- u32 resid;
ulong lock_flags;
struct afu *afu = cmd->parent;
struct cxlflash_cfg *cfg = afu->parent;
@@ -229,14 +229,11 @@ static void cmd_complete(struct afu_cmd *cmd)
if (cmd->rcb.scp) {
scp = cmd->rcb.scp;
- if (unlikely(cmd->sa.rc.afu_rc ||
-cmd->sa.rc.scsi_rc ||
-cmd->sa.rc.fc_rc))
+ if (unlikely(cmd->sa.ioasc))
process_cmd_err(cmd, scp);
else
scp->result = (DID_OK << 16);
- resid = cmd->sa.resid;
cmd_is_tmf = cmd->cmd_tmf;
cmd_checkin(cmd); /* Don't use cmd after here */
@@ -244,7 +241,6 @@ static void cmd_complete(struct afu_cmd *cmd)
"ioasc=%d\n", __func__, scp, scp->result,
cmd->sa.ioasc);
- scsi_set_resid(scp, resid);
scsi_dma_unmap(scp);
scp->scsi_done(scp);
--
2.1.0
--
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 v4 15/32] cxlflash: Fix host link up event handling
Following a link up event, the LUNs available to the host may
have changed. Without rescanning the host, the LUN topology is
unknown to the user. In such a state, the user would be unable
to locate provisioned resources.
To remedy, the host should be rescanned after a link up event.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Conflicts:
drivers/scsi/cxlflash/common.h
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 17 +
drivers/scsi/cxlflash/main.h | 1 +
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 7a0cb5c..faf7f56 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -102,6 +102,7 @@ struct cxlflash_cfg {
enum cxlflash_init_state init_state;
enum cxlflash_lr_state lr_state;
int lr_port;
+ atomic_t scan_host_needed;
struct cxl_afu *cxl_afu;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 5503a40..98fdac1 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1119,17 +1119,17 @@ static const struct asyc_intr_info ainfo[] = {
{SISL_ASTATUS_FC0_CRC_T, "CRC threshold exceeded", 0, LINK_RESET},
{SISL_ASTATUS_FC0_LOGI_R, "login timed out, retrying", 0, 0},
{SISL_ASTATUS_FC0_LOGI_F, "login failed", 0, CLR_FC_ERROR},
- {SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, 0},
+ {SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, SCAN_HOST},
{SISL_ASTATUS_FC0_LINK_DN, "link down", 0, 0},
- {SISL_ASTATUS_FC0_LINK_UP, "link up", 0, 0},
+ {SISL_ASTATUS_FC0_LINK_UP, "link up", 0, SCAN_HOST},
{SISL_ASTATUS_FC1_OTHER, "other error", 1, CLR_FC_ERROR | LINK_RESET},
{SISL_ASTATUS_FC1_LOGO, "target initiated LOGO", 1, 0},
{SISL_ASTATUS_FC1_CRC_T, "CRC threshold exceeded", 1, LINK_RESET},
{SISL_ASTATUS_FC1_LOGI_R, "login timed out, retrying", 1, 0},
{SISL_ASTATUS_FC1_LOGI_F, "login failed", 1, CLR_FC_ERROR},
- {SISL_ASTATUS_FC1_LOGI_S, "login succeeded", 1, 0},
+ {SISL_ASTATUS_FC1_LOGI_S, "login succeeded", 1, SCAN_HOST},
{SISL_ASTATUS_FC1_LINK_DN, "link down", 1, 0},
- {SISL_ASTATUS_FC1_LINK_UP, "link up", 1, 0},
+ {SISL_ASTATUS_FC1_LINK_UP, "link up", 1, SCAN_HOST},
{0x0, "", 0, 0} /* terminator */
};
@@ -1350,6 +1350,11 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
writeq_be(reg, &global->fc_regs[port][FC_ERROR / 8]);
writeq_be(0, &global->fc_regs[port][FC_ERRCAP / 8]);
}
+
+ if (info->action & SCAN_HOST) {
+ atomic_inc(&cfg->scan_host_needed);
+ schedule_work(&cfg->work_q);
+ }
}
out:
@@ -2309,6 +2314,7 @@ MODULE_DEVICE_TABLE(pci, cxlflash_pci_table);
* - Link reset which cannot be performed on interrupt context due to
* blocking up to a few seconds
* - Read AFU command room
+ * - Rescan the host
*/
static void cxlflash_worker_thread(struct work_struct *work)
{
@@ -2351,6 +2357,9 @@ static void cxlflash_worker_thread(struct work_struct
*work)
}
spin_unlock_irqrestore(cfg->host->host_lock, lock_flags);
+
+ if (atomic_dec_if_positive(&cfg->scan_host_needed) >= 0)
+ scsi_scan_host(cfg->host);
}
/**
diff --git a/drivers/scsi/cxlflash/main.h b/drivers/scsi/cxlflash/main.h
index cf0e809..6032456 100644
--- a/drivers/scsi/cxlflash/main.h
+++ b/drivers/scsi/cxlflash/main.h
@@ -99,6 +99,7 @@ struct asyc_intr_info {
u8 action;
#define CLR_FC_ERROR 0x01
#define LINK_RESET 0x02
+#define SCAN_HOST 0x04
};
#ifndef CONFIG_CXL_EEH
--
2.1.0
--
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 v4 18/32] cxlflash: Fix AFU version access/storage and add check
The AFU version is stored as a non-terminated string of bytes within
a 64-bit little-endian register. Presently the value is read directly
(no MMIO accessor) and is stored in a buffer that is not big enough
to contain a NULL terminator. Additionally the version obtained is not
evaluated against a known value to prevent usage with unsupported AFUs.
All of these deficiencies can lead to a variety of problems.
To remedy, use the correct MMIO accessor to read the version value into
a null-terminated buffer and add a check to prevent an incompatible AFU
from being used with this driver.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h | 2 +-
drivers/scsi/cxlflash/main.c| 18 --
drivers/scsi/cxlflash/sislite.h | 2 +-
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index faf7f56..3be5754 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -179,7 +179,7 @@ struct afu {
u32 cmd_couts; /* Number of command checkouts */
u32 internal_lun; /* User-desired LUN mode for this AFU */
- char version[8];
+ char version[16];
u64 interface_version;
struct cxlflash_cfg *parent; /* Pointer back to parent cxlflash_cfg */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index d45388f..b6a2584 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1751,14 +1751,20 @@ static int init_afu(struct cxlflash_cfg *cfg)
goto err1;
}
- /* don't byte reverse on reading afu_version, else the string form */
- /* will be backwards */
- reg = afu->afu_map->global.regs.afu_version;
- memcpy(afu->version, ®, 8);
+ /* No byte reverse on reading afu_version or string will be backwards */
+ reg = readq(&afu->afu_map->global.regs.afu_version);
+ memcpy(afu->version, ®, sizeof(reg));
afu->interface_version =
readq_be(&afu->afu_map->global.regs.interface_version);
- pr_debug("%s: afu version %s, interface version 0x%llX\n",
-__func__, afu->version, afu->interface_version);
+ if ((afu->interface_version + 1) == 0) {
+ pr_err("Back level AFU, please upgrade. AFU version %s "
+ "interface version 0x%llx\n", afu->version,
+ afu->interface_version);
+ rc = -EINVAL;
+ goto err1;
+ } else
+ pr_debug("%s: afu version %s, interface version 0x%llX\n",
+__func__, afu->version, afu->interface_version);
rc = start_afu(cfg);
if (rc) {
diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
index 63bf394..8425d1a 100644
--- a/drivers/scsi/cxlflash/sislite.h
+++ b/drivers/scsi/cxlflash/sislite.h
@@ -340,7 +340,7 @@ struct sisl_global_regs {
#define SISL_AFUCONF_MBOX_CLR_READ 0x0010ULL
__be64 afu_config;
__be64 rsvd[0xf8];
- __be64 afu_version;
+ __le64 afu_version;
__be64 interface_version;
};
--
2.1.0
--
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 v4 17/32] cxlflash: Remove dual port online dependency
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index ed9fd8c..d45388f 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1030,7 +1030,7 @@ static int wait_port_offline(u64 *fc_regs, u32 delay_us,
u32 nretry)
*/
static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
{
- int ret = 0;
+ int rc = 0;
set_port_offline(fc_regs);
@@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port, u64
*fc_regs, u64 wwpn)
FC_PORT_STATUS_RETRY_CNT)) {
pr_debug("%s: wait on port %d to go offline timed out\n",
__func__, port);
- ret = -1; /* but continue on to leave the port back online */
+ rc = -1; /* but continue on to leave the port back online */
}
- if (ret == 0)
+ if (rc == 0)
writeq_be(wwpn, &fc_regs[FC_PNAME / 8]);
+ /* Always return success after programming WWPN */
+ rc = 0;
+
set_port_online(fc_regs);
if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
FC_PORT_STATUS_RETRY_CNT)) {
pr_debug("%s: wait on port %d to go online timed out\n",
__func__, port);
- ret = -1;
-
- /*
-* Override for internal lun!!!
-*/
- if (afu->internal_lun) {
- pr_debug("%s: Overriding port %d online timeout!!!\n",
-__func__, port);
- ret = 0;
- }
}
- pr_debug("%s: returning rc=%d\n", __func__, ret);
+ pr_debug("%s: returning rc=%d\n", __func__, rc);
- return ret;
+ return rc;
}
/**
--
2.1.0
--
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 v4 19/32] cxlflash: Correct usage of scsi_host_put()
Currently, scsi_host_put() is being called prematurely in the remove path and is missing entirely in an error cleanup path. The former can lead to memory being freed too early with subsequent access potentially corrupting data whilst the former would result in a memory leak. Move the usage on remove to be the last cleanup action taken and introduce a call to scsi_host_put() in the one initialization error path that does not use remove to cleanup. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index b6a2584..0bb6629 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -733,7 +733,6 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_SCSI: cxlflash_term_local_luns(cfg); scsi_remove_host(cfg->host); - scsi_host_put(cfg->host); /* Fall through */ case INIT_STATE_AFU: term_afu(cfg); @@ -743,6 +742,7 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_NONE: flush_work(&cfg->work_q); free_mem(cfg); + scsi_host_put(cfg->host); break; } @@ -2404,6 +2404,7 @@ static int cxlflash_probe(struct pci_dev *pdev, dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n", __func__); rc = -ENOMEM; + scsi_host_put(cfg->host); goto out; } -- 2.1.0 -- 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 v4 20/32] cxlflash: Fix to prevent workq from accessing freed memory
The workq can process work in parallel with a remove event, leading to a condition where the workq handler can access freed memory. To remedy, the workq should be terminated prior to freeing memory. Move the termination call earlier in remove and use cancel_work_sync() instead of flush_work() as there is not a need to process any scheduled work when shutting down. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 0bb6629..fada355 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -736,11 +736,11 @@ static void cxlflash_remove(struct pci_dev *pdev) /* Fall through */ case INIT_STATE_AFU: term_afu(cfg); + cancel_work_sync(&cfg->work_q); case INIT_STATE_PCI: pci_release_regions(cfg->dev); pci_disable_device(pdev); case INIT_STATE_NONE: - flush_work(&cfg->work_q); free_mem(cfg); scsi_host_put(cfg->host); break; -- 2.1.0 -- 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 v4 21/32] cxlflash: Correct behavior in device reset handler following EEH
When the device reset handler is entered while a reset operation
is taking place, the handler exits without actually sending a
reset (TMF) to the targeted device. This behavior is incorrect
as the device is not reset. Further complicating matters is the
fact that a success is returned even when the TMF was not sent.
To fix, the state is rechecked after coming out of the reset
state. When the state is normal, a TMF will be sent out.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index fada355..cf6a067 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1920,6 +1920,7 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+retry:
switch (cfg->state) {
case STATE_NORMAL:
rcr = send_tmf(afu, scp, TMF_LUN_RESET);
@@ -1928,9 +1929,7 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
break;
case STATE_RESET:
wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
- if (cfg->state == STATE_NORMAL)
- break;
- /* fall through */
+ goto retry;
default:
rc = FAILED;
break;
--
2.1.0
--
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 v4 22/32] cxlflash: Remove unnecessary scsi_block_requests
The host reset handler is called with I/O already blocked, thus
there is no need to explicitly block and unblock I/O in the handler.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index cf6a067..729f742 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1966,7 +1966,6 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
switch (cfg->state) {
case STATE_NORMAL:
cfg->state = STATE_RESET;
- scsi_block_requests(cfg->host);
cxlflash_mark_contexts_error(cfg);
rcr = afu_reset(cfg);
if (rcr) {
@@ -1975,7 +1974,6 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
} else
cfg->state = STATE_NORMAL;
wake_up_all(&cfg->reset_waitq);
- scsi_unblock_requests(cfg->host);
break;
case STATE_RESET:
wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
--
2.1.0
--
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 v4 27/32] cxlflash: Fix to prevent stale AFU RRQ
Following an adapter reset, the AFU RRQ that resides in host memory holds stale data. This can lead to a condition where the RRQ interrupt handler tries to process stale entries and/or endlessly loops due to an out of sync generation bit. To fix, the AFU RRQ in host memory needs to be cleared after each reset. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 24aedfb..ab11ee6 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -1598,6 +1598,9 @@ static int start_afu(struct cxlflash_cfg *cfg) init_pcr(cfg); + /* After an AFU reset, RRQ entries are stale, clear them */ + memset(&afu->rrq_entry, 0, sizeof(afu->rrq_entry)); + /* Initialize RRQ pointers */ afu->hrrq_start = &afu->rrq_entry[0]; afu->hrrq_end = &afu->rrq_entry[NUM_RRQ_ENTRY - 1]; -- 2.1.0 -- 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 v4 25/32] cxlflash: Fix to prevent EEH recovery failure
The process_sense() routine can perform a read capacity which
can take some time to complete. If an EEH occurs while waiting
on the read capacity, the EEH handler is unable to obtain the
context's mutex in order to put the context in an error state.
The EEH handler will sit and wait until the context is free,
but this wait can last longer than the EEH handler tolerates,
leading to a failed recovery.
To address this issue, make the context unavailable to new,
non-system owned threads and release the context while calling
into process_sense(). After returning from process_sense() the
context mutex is reacquired and the context is made available
again. The context can be safely moved to the error state if
needed during the unavailable window as no other threads will
hold its reference.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/superpipe.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index a6316f5..7283e83 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1787,12 +1787,21 @@ static int cxlflash_disk_verify(struct scsi_device
*sdev,
* inquiry (i.e. the Unit attention is due to the WWN changing).
*/
if (verify->hint & DK_CXLFLASH_VERIFY_HINT_SENSE) {
+ /* Can't hold mutex across process_sense/read_cap16,
+* since we could have an intervening EEH event.
+*/
+ ctxi->unavail = true;
+ mutex_unlock(&ctxi->mutex);
rc = process_sense(sdev, verify);
if (unlikely(rc)) {
dev_err(dev, "%s: Failed to validate sense data (%d)\n",
__func__, rc);
+ mutex_lock(&ctxi->mutex);
+ ctxi->unavail = false;
goto out;
}
+ mutex_lock(&ctxi->mutex);
+ ctxi->unavail = false;
}
switch (gli->mode) {
--
2.1.0
--
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 v4 26/32] cxlflash: Correct spelling, grammar, and alignment mistakes
There are several spelling and grammar mistakes throughout the
driver. Additionally there are a handful of places where there
are extra lines and unnecessary variables/statements. These are
a nuisance and pollute the driver.
Fix spelling and grammar issues. Update some comments for clarity and
consistency. Remove extra lines and a few unneeded variables/statements.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h| 2 --
drivers/scsi/cxlflash/main.c | 62 +--
drivers/scsi/cxlflash/sislite.h | 6 ++--
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/cxlflash/vlun.c | 14 -
5 files changed, 38 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index a810585..bbfe711 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -105,8 +105,6 @@ struct cxlflash_cfg {
atomic_t scan_host_needed;
struct cxl_afu *cxl_afu;
-
- struct pci_pool *cxlflash_cmd_pool;
struct pci_dev *parent_dev;
atomic_t recovery_threads;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 4893516..24aedfb 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -34,7 +34,6 @@ MODULE_AUTHOR("Manoj N. Kumar ");
MODULE_AUTHOR("Matthew R. Ochs ");
MODULE_LICENSE("GPL");
-
/**
* cmd_checkout() - checks out an AFU command
* @afu: AFU to checkout from.
@@ -730,7 +729,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
case INIT_STATE_SCSI:
cxlflash_term_local_luns(cfg);
scsi_remove_host(cfg->host);
- /* Fall through */
+ /* fall through */
case INIT_STATE_AFU:
term_afu(cfg);
cancel_work_sync(&cfg->work_q);
@@ -763,9 +762,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg)
char *buf = NULL;
struct device *dev = &cfg->dev->dev;
- /* This allocation is about 12K, i.e. only 1 64k page
-* and upto 4 4k pages
-*/
+ /* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */
cfg->afu = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
get_order(sizeof(struct afu)));
if (unlikely(!cfg->afu)) {
@@ -1295,10 +1292,10 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
goto out;
}
- /* it is OK to clear AFU status before FC_ERROR */
+ /* FYI, it is 'okay' to clear AFU status before FC_ERROR */
writeq_be(reg_unmasked, &global->regs.aintr_clear);
- /* check each bit that is on */
+ /* Check each bit that is on */
for (i = 0; reg_unmasked; i++, reg_unmasked = (reg_unmasked >> 1)) {
info = find_ainfo(1ULL << i);
if (((reg_unmasked & 0x1) == 0) || !info)
@@ -1311,7 +1308,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
readq_be(&global->fc_regs[port][FC_STATUS / 8]));
/*
-* do link reset first, some OTHER errors will set FC_ERROR
+* Do link reset first, some OTHER errors will set FC_ERROR
* again if cleared before or w/o a reset
*/
if (info->action & LINK_RESET) {
@@ -1326,7 +1323,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
reg = readq_be(&global->fc_regs[port][FC_ERROR / 8]);
/*
-* since all errors are unmasked, FC_ERROR and FC_ERRCAP
+* Since all errors are unmasked, FC_ERROR and FC_ERRCAP
* should be the same and tracing one is sufficient.
*/
@@ -1472,23 +1469,22 @@ static void init_pcr(struct cxlflash_cfg *cfg)
for (i = 0; i < MAX_CONTEXT; i++) {
ctrl_map = &afu->afu_map->ctrls[i].ctrl;
- /* disrupt any clients that could be running */
- /* e. g. clients that survived a master restart */
+ /* Disrupt any clients that could be running */
+ /* e.g. clients that survived a master restart */
writeq_be(0, &ctrl_map->rht_start);
writeq_be(0, &ctrl_map->rht_cnt_id);
writeq_be(0, &ctrl_map->ctx_cap);
}
- /* copy frequently used fields into afu */
+ /* Copy frequently used fields into afu */
afu->ctx_hndl = (u16) cxl_process_element(cfg->mcctx);
- /* ctx_hndl is 16 bits in CAIA */
afu->host_map = &afu->afu_map->hosts[afu->ctx_hndl].host;
afu->
[PATCH v4 24/32] cxlflash: Fix MMIO and endianness errors
Sparse uncovered several errors with MMIO operations (accessing
directly) and handling endianness. These can cause issues when
running in different environments.
Introduce __iomem and proper endianness tags/swaps where
appropriate to make driver sparse clean.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h| 10 +-
drivers/scsi/cxlflash/main.c | 25 +
drivers/scsi/cxlflash/superpipe.c | 6 +++---
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
5 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 3be5754..a810585 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -164,9 +164,9 @@ struct afu {
/* AFU HW */
struct cxl_ioctl_start_work work;
- struct cxlflash_afu_map *afu_map; /* entire MMIO map */
- struct sisl_host_map *host_map; /* MC host map */
- struct sisl_ctrl_map *ctrl_map; /* MC control map */
+ struct cxlflash_afu_map __iomem *afu_map; /* entire MMIO map */
+ struct sisl_host_map __iomem *host_map; /* MC host map */
+ struct sisl_ctrl_map __iomem *ctrl_map; /* MC control map */
ctx_hndl_t ctx_hndl;/* master's context handle */
u64 *hrrq_start;
@@ -188,10 +188,10 @@ struct afu {
static inline u64 lun_to_lunid(u64 lun)
{
- u64 lun_id;
+ __be64 lun_id;
int_to_scsilun(lun, (struct scsi_lun *)&lun_id);
- return swab64(lun_id);
+ return be64_to_cpu(lun_id);
}
int cxlflash_afu_sync(struct afu *, ctx_hndl_t, res_hndl_t, u8);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 7c76227..4893516 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -644,7 +644,7 @@ static void stop_afu(struct cxlflash_cfg *cfg)
complete(&afu->cmd[i].cevent);
if (likely(afu->afu_map)) {
- cxl_psa_unmap((void *)afu->afu_map);
+ cxl_psa_unmap((void __iomem *)afu->afu_map);
afu->afu_map = NULL;
}
}
@@ -914,7 +914,7 @@ out:
* that the FC link layer has synced, completed the handshaking process, and
* is ready for login to start.
*/
-static void set_port_online(u64 *fc_regs)
+static void set_port_online(__be64 __iomem *fc_regs)
{
u64 cmdcfg;
@@ -930,7 +930,7 @@ static void set_port_online(u64 *fc_regs)
*
* The provided MMIO region must be mapped prior to call.
*/
-static void set_port_offline(u64 *fc_regs)
+static void set_port_offline(__be64 __iomem *fc_regs)
{
u64 cmdcfg;
@@ -954,7 +954,7 @@ static void set_port_offline(u64 *fc_regs)
* FALSE (0) when the specified port fails to come online after timeout
* -EINVAL when @delay_us is less than 1000
*/
-static int wait_port_online(u64 *fc_regs, u32 delay_us, u32 nretry)
+static int wait_port_online(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
{
u64 status;
@@ -985,7 +985,7 @@ static int wait_port_online(u64 *fc_regs, u32 delay_us, u32
nretry)
* FALSE (0) when the specified port fails to go offline after timeout
* -EINVAL when @delay_us is less than 1000
*/
-static int wait_port_offline(u64 *fc_regs, u32 delay_us, u32 nretry)
+static int wait_port_offline(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
{
u64 status;
@@ -1020,7 +1020,8 @@ static int wait_port_offline(u64 *fc_regs, u32 delay_us,
u32 nretry)
* 0 when the WWPN is successfully written and the port comes back online
* -1 when the port fails to go offline or come back up online
*/
-static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
+static int afu_set_wwpn(struct afu *afu, int port, __be64 __iomem *fc_regs,
+ u64 wwpn)
{
int rc = 0;
@@ -1065,7 +1066,7 @@ static int afu_set_wwpn(struct afu *afu, int port, u64
*fc_regs, u64 wwpn)
* the alternate port exclusively while the reset takes place.
* failure to come online is overridden.
*/
-static void afu_link_reset(struct afu *afu, int port, u64 *fc_regs)
+static void afu_link_reset(struct afu *afu, int port, __be64 __iomem *fc_regs)
{
u64 port_sel;
@@ -1280,7 +1281,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
struct device *dev = &cfg->dev->dev;
u64 reg_unmasked;
const struct asyc_intr_info *info;
- struct sisl_global_map *global = &afu->afu_map->global;
+ struct sisl_global_map __iomem *global = &afu->afu_map->global;
u64 reg;
u8 port;
int i;
@@ -1466,7 +1467,7 @@ out:
static void init_pcr(struct cxlflash_cfg *cfg)
{
struct a
[PATCH v4 23/32] cxlflash: Fix function prolog parameters and return codes
Several function prologs have incorrect parameter names and return
code descriptions. This can lead to confusion when reviewing the
source and creates inaccurate documentation.
To remedy, update the function prologs to properly reflect parameter
names and return codes.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 68
1 file changed, 25 insertions(+), 43 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 729f742..7c76227 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -401,8 +401,7 @@ static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
* @tmfcmd:TMF command to send.
*
* Return:
- * 0 on success
- * SCSI_MLQUEUE_HOST_BUSY when host is busy
+ * 0 on success or SCSI_MLQUEUE_HOST_BUSY
*/
static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
{
@@ -491,9 +490,7 @@ static const char *cxlflash_driver_info(struct Scsi_Host
*host)
* @host: SCSI host associated with device.
* @scp: SCSI command to send.
*
- * Return:
- * 0 on success
- * SCSI_MLQUEUE_HOST_BUSY when host is busy
+ * Return: 0 on success or SCSI_MLQUEUE_HOST_BUSY
*/
static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)
{
@@ -597,7 +594,7 @@ out:
/**
* cxlflash_wait_for_pci_err_recovery() - wait for error recovery during probe
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*/
static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg)
{
@@ -611,7 +608,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
cxlflash_cfg *cfg)
/**
* free_mem() - free memory associated with the AFU
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*/
static void free_mem(struct cxlflash_cfg *cfg)
{
@@ -633,7 +630,7 @@ static void free_mem(struct cxlflash_cfg *cfg)
/**
* stop_afu() - stops the AFU command timers and unmaps the MMIO space
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Safe to call with AFU in a partially allocated/initialized state.
*/
@@ -655,7 +652,7 @@ static void stop_afu(struct cxlflash_cfg *cfg)
/**
* term_mc() - terminates the master context
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
* @level: Depth of allocation, where to begin waterfall tear down.
*
* Safe to call with AFU/MC in partially allocated/initialized state.
@@ -691,7 +688,7 @@ static void term_mc(struct cxlflash_cfg *cfg, enum
undo_level level)
/**
* term_afu() - terminates the AFU
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Safe to call with AFU/MC in partially allocated/initialized state.
*/
@@ -751,7 +748,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
/**
* alloc_mem() - allocates the AFU and its command pool
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* A partially allocated state remains on failure.
*
@@ -804,12 +801,9 @@ out:
/**
* init_pci() - initializes the host as a PCI device
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
- * Return:
- * 0 on success
- * -EIO on unable to communicate with device
- * A return code from the PCI sub-routines
+ * Return: 0 on success, -errno on failure
*/
static int init_pci(struct cxlflash_cfg *cfg)
{
@@ -889,11 +883,9 @@ out_release_regions:
/**
* init_scsi() - adds the host to the SCSI stack and kicks off host scan
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
- * Return:
- * 0 on success
- * A return code from adding the host
+ * Return: 0 on success, -errno on failure
*/
static int init_scsi(struct cxlflash_cfg *cfg)
{
@@ -1357,7 +1349,7 @@ out:
/**
* start_context() - starts the master context
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Return: A success or failure value from CXL services.
*/
@@ -1375,12 +1367,10 @@ static int start_context(struct cxlflash_cfg *cfg)
/**
* read_vpd() - obtains the WWPNs from VPD
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
* @wwpn: Array of size NUM_FC_PORTS to pass back WWPNs
*
- * Return:
- * 0 on success
- * -ENODEV when VPD or WWPN keywords not found
+ * Return: 0 on success
[PATCH v4 29/32] cxlflash: Fix to double the delay each time
From: Manoj Kumar The operator used to double the delay is incorrect and does not result in delay doubling. To fix, use a left shift instead of the XOR operator. Reported-by: Tomas Henzl Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index ab11ee6..be78906 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -303,7 +303,7 @@ write_rrin: if (rrin != 0x1) break; /* Double delay each time */ - udelay(2 ^ nretry); + udelay(2 << nretry); } while (nretry++ < MC_ROOM_RETRY_CNT); } -- 2.1.0 -- 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 v4 30/32] cxlflash: Fix to avoid corrupting adapter fops
The fops owned by the adapter can be corrupted in certain scenarios,
opening a window where certain fops are temporarily NULLed before being
reset to their proper value. This can potentially lead software to make
incorrect decisions, leaving the user with the inability to function as
intended.
An example of this behavior can be observed when there are a number of
users with a high rate of turn around (attach to LUN, perform an I/O,
detach from LUN, repeat). Every so often a user is given a valid
context and adapter file descriptor, but the file associated with the
descriptor lacks the correct read permission bit (FMODE_CAN_READ) and
thus the read system call bails before calling the valid read fop.
Background:
The fops is stored in the adapter structure to provide the ability to
lookup the adapter structure from within the fop handler. CXL services
use the file's private_data and at present, the CXL context does not
have a private section. In an effort to limit areas of the cxlflash
driver with code specific the superpipe function, a design choice was
made to keep the details of the fops situated away from the legacy
portions of the driver. This drove the behavior that the adapter fops
is set at the beginning of the disk attach ioctl handler when there
are no users present.
The corruption that this fix remedies is due to the fact that the fops
is initially defaulted to values found within a static structure. When
the fops is handed down to the CXL services later in the attach path,
certain services are patched. The fops structure remains correct until
the user count drops to 0 and the fops is reset, triggering the process
to repeat again. The user counts are tightly coupled with the creation
and deletion of the user context. If multiple users perform a disk
attach at the same time, when the user count is currently 0, some users
can be in the middle of obtaining a file descriptor and have not yet
reached the context creation code that [in addition to creating the
context] increments the user count. Subsequent users coming in to
perform the attach see that the user count is still 0, and reinitialize
the fops, temporarily removing the patched fops. The users that are in
the middle obtaining their file descriptor may then receive an invalid
descriptor.
The fix simply removes the user count altogether and moves the fops
initialization to probe time such that it is only performed one time
for the life of the adapter. In the future, if the CXL services adopt
a private member for their context, that could be used to store the
adapter structure reference and cxlflash could revert to a model that
does not require an embedded fops.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
---
drivers/scsi/cxlflash/common.h| 3 +--
drivers/scsi/cxlflash/main.c | 1 +
drivers/scsi/cxlflash/superpipe.c | 11 +--
3 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index bbfe711..c11cd19 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -21,6 +21,7 @@
#include
#include
+extern const struct file_operations cxlflash_cxl_fops;
#define MAX_CONTEXT CXLFLASH_MAX_CONTEXT /* num contexts per afu */
@@ -115,8 +116,6 @@ struct cxlflash_cfg {
struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
struct file_operations cxl_fops;
- atomic_t num_user_contexts;
-
/* Parameters that are LUN table related */
int last_lun_index[CXLFLASH_NUM_FC_PORTS];
int promote_lun_index;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index be78906..38e7edc 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2386,6 +2386,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->init_state = INIT_STATE_NONE;
cfg->dev = pdev;
+ cfg->cxl_fops = cxlflash_cxl_fops;
/*
* The promoted LUNs move to the top of the LUN table. The rest stay
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 3cc8609..f625e07 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -712,7 +712,6 @@ static void destroy_context(struct cxlflash_cfg *cfg,
kfree(ctxi->rht_needs_ws);
kfree(ctxi->rht_lun);
kfree(ctxi);
- atomic_dec_if_positive(&cfg->num_user_contexts);
}
/**
@@ -769,7 +768,6 @@ static struct ctx_info *create_context(struct cxlflash_cfg
*cfg,
INIT_LIST_HEAD(&ctxi->luns);
INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
- atomic_inc(&cfg->num_user_contexts);
mutex_lock(&ctxi->mutex);
out:
return ctxi;
@@ -1164,10 +1162,7 @@ out:
return rc;
}
-/*
- * Local fops for adapter file descriptor
- */
-static const struct file_operati
[PATCH v4 28/32] MAINTAINERS: Add cxlflash driver
Add stanza for cxlflash SCSI driver. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King Reviewed-by: Andrew Donnellan --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 274f854..f2f3046 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3153,6 +3153,15 @@ F: Documentation/powerpc/cxl.txt F: Documentation/powerpc/cxl.txt F: Documentation/ABI/testing/sysfs-class-cxl +CXLFLASH (IBM Coherent Accelerator Processor Interface CAPI Flash) SCSI DRIVER +M: Manoj N. Kumar +M: Matthew R. Ochs +L: linux-scsi@vger.kernel.org +S: Supported +F: drivers/scsi/cxlflash/ +F: include/uapi/scsi/cxlflash_ioctls.h +F: Documentation/powerpc/cxlflash.txt + STMMAC ETHERNET DRIVER M: Giuseppe Cavallaro L: net...@vger.kernel.org -- 2.1.0 -- 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 v4 31/32] cxlflash: Correct trace string
The trace following the failure of alloc_mem() incorrectly identifies
which function failed. This can lead to misdiagnosing a failure.
Fix the string to correctly indicate that alloc_mem() failed.
Reported-by: Brian King
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 38e7edc..e8f8c7e 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2377,7 +2377,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->host = host;
rc = alloc_mem(cfg);
if (rc) {
- dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n",
+ dev_err(&pdev->dev, "%s: call to alloc_mem failed!\n",
__func__);
rc = -ENOMEM;
scsi_host_put(cfg->host);
--
2.1.0
--
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 v4 32/32] cxlflash: Fix to avoid potential deadlock on EEH
Ioctl threads that use scsi_execute() can run for an excessive amount
of time due to the fact that they have lengthy timeouts and retry logic
built in. Under normal operation this is not an issue. However, once EEH
enters the picture, a long execution time coupled with the possibility
that a timeout can trigger entry to the driver via registered reset
callbacks becomes a liability.
In particular, a deadlock can occur when an EEH event is encountered
while in running in scsi_execute(). As part of the recovery, the EEH
handler drains all currently running ioctls, waiting until they have
completed before proceeding with a reset. As the scsi_execute()'s are
situated on the ioctl path, the EEH handler will wait until they (and
the remainder of the ioctl handler they're associated with) have
completed. Normally this would not be much of an issue aside from the
longer recovery period. Unfortunately, the scsi_execute() triggers a
reset when it times out. The reset handler will see that the device is
already being reset and wait until that reset completed. This creates
a condition where the EEH handler becomes stuck, infinitely waiting for
the ioctl thread to complete.
To avoid this behavior, temporarily unmark the scsi_execute() threads
as an ioctl thread by releasing the ioctl read semaphore. This allows
the EEH handler to proceed with a recovery while the thread is still
running. Once the scsi_execute() returns, the ioctl read semaphore is
reacquired and the adapter state is rechecked in case it changed while
inside of scsi_execute(). The state check will wait if the adapter is
still being recovered or returns a failure if the recovery failed. In
the event that the adapter reset failed, the failure is simply returned
as the ioctl would be unable to continue.
Reported-by: Brian King
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
---
drivers/scsi/cxlflash/superpipe.c | 30 +-
drivers/scsi/cxlflash/superpipe.h | 2 ++
drivers/scsi/cxlflash/vlun.c | 29 +
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index f625e07..8af7cdc 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -283,6 +283,24 @@ out:
* @sdev: SCSI device associated with LUN.
* @lli: LUN destined for capacity request.
*
+ * The READ_CAP16 can take quite a while to complete. Should an EEH occur while
+ * in scsi_execute(), the EEH handler will attempt to recover. As part of the
+ * recovery, the handler drains all currently running ioctls, waiting until
they
+ * have completed before proceeding with a reset. As this routine is used on
the
+ * ioctl path, this can create a condition where the EEH handler becomes stuck,
+ * infinitely waiting for this ioctl thread. To avoid this behavior,
temporarily
+ * unmark this thread as an ioctl thread by releasing the ioctl read semaphore.
+ * This will allow the EEH handler to proceed with a recovery while this thread
+ * is still running. Once the scsi_execute() returns, reacquire the ioctl read
+ * semaphore and check the adapter state in case it changed while inside of
+ * scsi_execute(). The state check will wait if the adapter is still being
+ * recovered or return a failure if the recovery failed. In the event that the
+ * adapter reset failed, simply return the failure as the ioctl would be unable
+ * to continue.
+ *
+ * Note that the above puts a requirement on this routine to only be called on
+ * an ioctl thread.
+ *
* Return: 0 on success, -errno on failure
*/
static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
@@ -314,8 +332,18 @@ retry:
dev_dbg(dev, "%s: %ssending cmd(0x%x)\n", __func__,
retry_cnt ? "re" : "", scsi_cmd[0]);
+ /* Drop the ioctl read semahpore across lengthy call */
+ up_read(&cfg->ioctl_rwsem);
result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
+ down_read(&cfg->ioctl_rwsem);
+ rc = check_state(cfg);
+ if (rc) {
+ dev_err(dev, "%s: Failed state! result=0x08%X\n",
+ __func__, result);
+ rc = -ENODEV;
+ goto out;
+ }
if (driver_byte(result) == DRIVER_SENSE) {
result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -1221,7 +1249,7 @@ static const struct file_operations null_fops = {
*
* Return: 0 on success, -errno on failure
*/
-static int check_state(struct cxlflash_cfg *cfg)
+int check_state(struct cxlflash_cfg *cfg)
{
struct device *dev = &cfg->dev->dev;
int rc = 0;
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers/scsi/cxlflash/superpipe.h
index 7df88ee..06a805a 10
Re: [PATCH v4 08/32] cxlflash: Fix to avoid CXL services during EEH
> On Sep 28, 2015, at 6:05 PM, Daniel Axtens wrote:
>
> You have two versions of check_state() below, which is a bit
> confusing. It looks like you've moved the function and also added the
> up/down of the read semaphore. I assume that's all that changed?
Correct.
It was originally moved to meet a dependency due to it being defined statically.
>
>>
>> /**
>> + * check_state() - checks and responds to the current adapter state
>> + * @cfg:Internal structure associated with the host.
>> + *
>> + * This routine can block and should only be used on process context.
>> + * It assumes that the caller is an ioctl thread and holding the ioctl
>> + * read semaphore. This is temporarily let up across the wait to allow
>> + * for draining actively running ioctls. Also note that when waking up
>> + * from waiting in reset, the state is unknown and must be checked again
>> + * before proceeding.
>> + *
>> + * Return: 0 on success, -errno on failure
>> + */
>> +static int check_state(struct cxlflash_cfg *cfg)
>> +{
>> +struct device *dev = &cfg->dev->dev;
>> +int rc = 0;
>> +
>> +retry:
>> +switch (cfg->state) {
>> +case STATE_LIMBO:
>> +dev_dbg(dev, "%s: Limbo state, going to wait...\n", __func__);
>> +up_read(&cfg->ioctl_rwsem);
>> +rc = wait_event_interruptible(cfg->limbo_waitq,
>> + cfg->state != STATE_LIMBO);
>> +down_read(&cfg->ioctl_rwsem);
>> +if (unlikely(rc))
>> +break;
>> +goto retry;
>> +case STATE_FAILTERM:
>> +dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
>> +rc = -ENODEV;
>> +break;
>> +default:
>> +break;
>> +}
>> +
>> +return rc;
>> +}
>> +
>> +/**
>> * cxlflash_disk_attach() - attach a LUN to a context
>> * @sdev:SCSI device associated with LUN.
>> * @attach: Attach ioctl data structure.
>> @@ -1523,41 +1563,6 @@ err1:
>> }
>>
>> /**
>> - * check_state() - checks and responds to the current adapter state
>> - * @cfg:Internal structure associated with the host.
>> - *
>> - * This routine can block and should only be used on process context.
>> - * Note that when waking up from waiting in limbo, the state is unknown
>> - * and must be checked again before proceeding.
>> - *
>> - * Return: 0 on success, -errno on failure
>> - */
>> -static int check_state(struct cxlflash_cfg *cfg)
>> -{
>> -struct device *dev = &cfg->dev->dev;
>> -int rc = 0;
>> -
>> -retry:
>> -switch (cfg->state) {
>> -case STATE_LIMBO:
>> -dev_dbg(dev, "%s: Limbo, going to wait...\n", __func__);
>> -rc = wait_event_interruptible(cfg->limbo_waitq,
>> - cfg->state != STATE_LIMBO);
>> -if (unlikely(rc))
>> -break;
>> -goto retry;
>> -case STATE_FAILTERM:
>> -dev_dbg(dev, "%s: Failed/Terminating!\n", __func__);
>> -rc = -ENODEV;
>> -break;
>> -default:
>> -break;
>> -}
>> -
>> -return rc;
>> -}
>> -
>> -/**
>> * cxlflash_afu_recover() - initiates AFU recovery
>> * @sdev:SCSI device associated with LUN.
>> * @recover: Recover ioctl data structure.
>> @@ -1646,9 +1651,14 @@ retry_recover:
>> /* Test if in error state */
>> reg = readq_be(&afu->ctrl_map->mbox_r);
>> if (reg == -1) {
>> -dev_dbg(dev, "%s: MMIO read fail! Wait for recovery...\n",
>> -__func__);
>> -mutex_unlock(&ctxi->mutex);
>> +dev_dbg(dev, "%s: MMIO fail, wait for recovery.\n", __func__);
>> +
>> +/*
>> + * Before checking the state, put back the context obtained with
>> + * get_context() as it is no longer needed and sleep for a short
>> + * period of time (see prolog notes).
>> + */
>> +put_context(ctxi);
>
> Is this needed for the drain to work? It looks like it fixes a
> refcounting bug in the function, but I'm not sure I understand how it
> interacts with the rest of this patch.
This was simply some "while I'm here" refactoring as the commit originally
included a change here. The main point of this change was to replace the
mutex_unlock() with put_context(), which is a wrapper around the unlocking
of the context's mutex.
>
> Anyway, the patch overall looks good to me, and makes your driver
> interact with CXL's EEH support in the way I intended when I wrote it.
Thanks for reviewing.
--
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
Re: [PATCH v4 17/32] cxlflash: Remove dual port online dependency
> On Sep 28, 2015, at 6:37 PM, Daniel Axtens wrote:
>
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA512
>
> Hi,
>
>> static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
>> {
>> -int ret = 0;
>> +int rc = 0;
> I realise it's nice to have things consistent, but making this change
> now makes the rest of the patch quite difficult to follow.
Next time I will try to separate out a change like this.
>
>>
>> set_port_offline(fc_regs);
>>
>> @@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port,
>> u64 *fc_regs, u64 wwpn)
>> FC_PORT_STATUS_RETRY_CNT)) {
>> pr_debug("%s: wait on port %d to go offline timed out\n",
>> __func__, port);
>> -ret = -1; /* but continue on to leave the port back online */
>> +rc = -1; /* but continue on to leave the port back online */
>> }
>>
>> -if (ret == 0)
>> +if (rc == 0)
>> writeq_be(wwpn, &fc_regs[FC_PNAME / 8]);
>>
>> +/* Always return success after programming WWPN */
>> +rc = 0;
>> +
>> set_port_online(fc_regs);
>>
>> if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
>>FC_PORT_STATUS_RETRY_CNT)) {
>> pr_debug("%s: wait on port %d to go online timed out\n",
>> __func__, port);
>> -ret = -1;
>> -
>> -/*
>> - * Override for internal lun!!!
>> - */
>> -if (afu->internal_lun) {
>> -pr_debug("%s: Overriding port %d online timeout!!!\n",
>> - __func__, port);
>> -ret = 0;
>> -}
>> }
>>
>> -pr_debug("%s: returning rc=%d\n", __func__, ret);
>> +pr_debug("%s: returning rc=%d\n", __func__, rc);
> I'm not sure I fully understand the flow of this function, but it looks
> like you set rc=0 regardless of how things actually go: is this ever
> going to print a return value other than zero?
Correct, this function behaves more like a void for the time being. The
overall goal of this is to allow a card to configure even when the link is
down. At some later point when the link is transitioned to 'up', a link state
change interrupt will trigger the port configuration. I left this with a return
code for right now in case we need to alter the behavior again (based
upon testing) and actually return a value other than 0.
--
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
Re: [PATCH v4 32/32] cxlflash: Fix to avoid potential deadlock on EEH
> On Sep 28, 2015, at 6:41 PM, Brian King wrote:
> On 09/25/2015 06:19 PM, Matthew R. Ochs wrote:
>> static int write_same16(struct scsi_device *sdev,
>> @@ -433,9 +451,20 @@ static int write_same16(struct scsi_device *sdev,
>> put_unaligned_be32(ws_limit < left ? ws_limit : left,
>> &scsi_cmd[10]);
>>
>> +/* Drop the ioctl read semahpore across lengthy call */
>> +up_read(&cfg->ioctl_rwsem);
>> result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
>>CMD_BUFSIZE, sense_buf, to, CMD_RETRIES,
>>0, NULL);
>> +down_read(&cfg->ioctl_rwsem);
>> +rc = check_state(cfg);
>> +if (rc) {
>> +dev_err(dev, "%s: Failed state! result=0x08%X\n",
>> +__func__, result);
>> +rc = -ENODEV;
>
> Since check_state only returns 0 or -ENODEV, this is a bit redundant, but not
> worth redoing the
> patch in my mind.
Agreed. This occurred to me the other day after submitting this patch when I was
reviewing the state locking code. Will look at revising in a future patch.
Thanks again for reviewing.
--
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
Re: [PATCH v4 25/32] cxlflash: Fix to prevent EEH recovery failure
> On Sep 28, 2015, at 8:25 PM, Daniel Axtens wrote: > > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > "Matthew R. Ochs" writes: > > >> The process_sense() routine can perform a read capacity which >> can take some time to complete. If an EEH occurs while waiting >> on the read capacity, the EEH handler is unable to obtain the >> context's mutex in order to put the context in an error state. >> The EEH handler will sit and wait until the context is free, >> but this wait can last longer than the EEH handler tolerates, >> leading to a failed recovery. > > I'm not quite clear on what you mean by the EEH handler timing > out. AFAIK there's nothing in eehd and the EEH core that times out if a > driver doesn't respond - indeed, it's pretty easy to hang eehd with a > misbehaving driver. > > Are you referring to your own internal timeouts? > cxlflash_wait_for_pci_err_recovery and anything else that uses > CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT? Reading through this again I can see how this is misleading. This is actually similar and related to the deadlock scenario described in "Fix to avoid potential deadlock on EEH". Without this fix, you'd end up in a similar situation but deadlocked on the context mutex instead of the ioctl semaphore. -- 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
Re: [PATCH v4 27/32] cxlflash: Fix to prevent stale AFU RRQ
> On Sep 28, 2015, at 8:36 PM, Daniel Axtens wrote: > > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > "Matthew R. Ochs" writes: > >> Following an adapter reset, the AFU RRQ that resides in host memory >> holds stale data. This can lead to a condition where the RRQ interrupt >> handler tries to process stale entries and/or endlessly loops due to an >> out of sync generation bit. >> >> To fix, the AFU RRQ in host memory needs to be cleared after each reset. > > This looks good. Do you need anything to bail out of cxlflash_rrq_irq if > the data goes stale or to all Fs while that function is running? We're not performing an MMIO here, so I'm not sure how the all Fs check would apply. We're also protected fairly well by the generation bit. I suppose we could look at adding some type of 'max iterations' count to protect against a runaway handler but that would be in a future patch. -- 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
Re: [PATCH v4 23/32] cxlflash: Fix function prolog parameters and return codes
> On Sep 28, 2015, at 11:36 PM, Andrew Donnellan
> wrote:
> On 26/09/15 09:18, Matthew R. Ochs wrote:
>>
>> */
>> static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
>> {
>> @@ -491,9 +490,7 @@ static const char *cxlflash_driver_info(struct Scsi_Host
>> *host)
>> * @host: SCSI host associated with device.
>> * @scp:SCSI command to send.
>> *
>> - * Return:
>> - * 0 on success
>> - * SCSI_MLQUEUE_HOST_BUSY when host is busy
>> + * Return: 0 on success or SCSI_MLQUEUE_HOST_BUSY
>> */
>
> I'd prefer it to say "SCSI_MLQUEUE_HOST_BUSY on failure". (Aesthetically I
> prefer having it on a separate line, but that's just personal preference.)
>
> As an aside, while checking the correctness of this, I found that the comment
> for cxlflash_send_cmd() states that it returns -1 on failure, when the only
> error value it actually returns is SCSI_MLQUEUE_HOST_BUSY. If you send a v5
> you might want to fix this.
I'll make a note of this.
--
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
Re: [PATCH v4 29/32] cxlflash: Fix to double the delay each time
> On Sep 28, 2015, at 8:40 PM, Daniel Axtens wrote: > > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > "Matthew R. Ochs" writes: > >> From: Manoj Kumar >> >> The operator used to double the delay is incorrect and >> does not result in delay doubling. >> >> To fix, use a left shift instead of the XOR operator. >> > I can see that the patch is correct, but this commit message is a bit > confusing. What delay? In what circumstances are you doubling it? Why? This is the response delay while resetting the master context. The reset is performed by writing a bit and then waiting for it to clear. While waiting for it to clear, the code relaxes the delta between MMIO reads. -- 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
Re: [PATCH v4 12/32] cxlflash: Fix to avoid spamming the kernel log
> On Sep 29, 2015, at 12:05 AM, Andrew Donnellan
> wrote:
> On 26/09/15 09:15, Matthew R. Ochs wrote:
>> During run-time the driver can be very chatty and spam the system
>> kernel log. Various print statements can be limited and/or moved
>> to development-only mode. Additionally, numerous prints can be
>> converted to trace the corresponding device.
>>
>> The following changes were made:
>> - pr_debug to pr_devel
>> - pr_debug to pr_debug_ratelimited
>> - pr_err to dev_err
>> - pr_debug to dev_dbg
>>
>> Signed-off-by: Matthew R. Ochs
>> Signed-off-by: Manoj N. Kumar
>> Reviewed-by: Brian King
>
> Reviewed-by: Andrew Donnellan
>
> Changes mostly look fine, further comments below.
>
>> --- a/drivers/scsi/cxlflash/main.c
>> +++ b/drivers/scsi/cxlflash/main.c
>> @@ -58,8 +58,8 @@ static struct afu_cmd *cmd_checkout(struct afu *afu)
>> cmd = &afu->cmd[k];
>>
>> if (!atomic_dec_if_positive(&cmd->free)) {
>> -pr_debug("%s: returning found index=%d\n",
>> - __func__, cmd->slot);
>> +pr_devel("%s: returning found index=%d cmd=%p\n",
>> + __func__, cmd->slot, cmd);
>
>> pr_debug("%s: cmd failed afu_rc=%d scsi_rc=%d fc_rc=%d "
>> - "afu_extra=0x%X, scsi_entra=0x%X, fc_extra=0x%X\n",
>> + "afu_extra=0x%X, scsi_extra=0x%X, fc_extra=0x%X\n",
>> __func__, ioasa->rc.afu_rc, ioasa->rc.scsi_rc,
>> ioasa->rc.fc_rc, ioasa->afu_extra, ioasa->scsi_extra,
>> ioasa->fc_extra);
>
> Minor nitpicking: mention that you fix these in the commit message.
Noted for the future.
>> @@ -240,9 +240,9 @@ static void cmd_complete(struct afu_cmd *cmd)
>> cmd_is_tmf = cmd->cmd_tmf;
>> cmd_checkin(cmd); /* Don't use cmd after here */
>>
>> -pr_debug("%s: calling scsi_set_resid, scp=%p "
>> - "result=%X resid=%d\n", __func__,
>> - scp, scp->result, resid);
>> +pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "
>> + "ioasc=%d\n", __func__, scp, scp->result,
>> + cmd->sa.ioasc);
>>
>> scsi_set_resid(scp, resid);
>> scsi_dma_unmap(scp);
>
> Why has the message changed from scsi_set_resid to scsi_done, and should the
> message be moved to immediately before the scsi_done call?
In a later patch in the series the scsi_set_resid() is actually moved.
--
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
Re: [PATCH 3/3] cxlflash: drop unlikely before IS_ERR_OR_NULL
Acked-by: Matthew R. Ochs -- 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
Re: [PATCH v4 17/32] cxlflash: Remove dual port online dependency
> On Sep 30, 2015, at 6:50 PM, Daniel Axtens wrote:
> (resending to the list this time, apologies!)
>
>>> I'm not sure I fully understand the flow of this function, but it looks
>>> like you set rc=0 regardless of how things actually go: is this ever
>>> going to print a return value other than zero?
>>
>> Correct, this function behaves more like a void for the time being. The
>> overall goal of this is to allow a card to configure even when the link is
>> down. At some later point when the link is transitioned to 'up', a link state
>> change interrupt will trigger the port configuration. I left this with a
>> return
>> code for right now in case we need to alter the behavior again (based
>> upon testing) and actually return a value other than 0.
>
> OK. That makes more sense - it wasn't clear to me how it could be
> correct to proceed if the links were down but now I understand how that
> works. I think that explanation should go in the commit message.
>
if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
FC_PORT_STATUS_RETRY_CNT)) {
pr_debug("%s: wait on port %d to go online timed out\n",
__func__, port);
>
> As an aside, should this be a bit noisier? It seems like something
> a user would probably want to know - especially in the case where
> something has actually gone wrong so there's no link state change
> interrupt forthcoming regardless of how long you wait.
You bring up a good point. There is another place where we are noisier
with respect to the link being down, so we'll do the same here. I'll include
this in v5 along with an updated commit message as requested.
--
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 v5 00/34] cxlflash: Miscellaneous bug fixes and corrections
This patch set contains various fixes and corrections for issues that
were found during test and code review. The series is based upon the
code upstreamed in 4.3 and is intended for the rc phase. The entire
set is bisectable. Please reference the changelog below for details
on what has been altered from previous versions of this patch set.
v5 Changes:
- Incorporate comments from Daniel Axtens
- Incorporate comments from Andrew Donnellan
- Added additional clarifications to several commit messages
- Specified some return codes as failures in "Fix function prolog..."
- Made port online failure noisier in "Remove dual port online..."
- Added patch to properly cleanup when encountering an unsupported AFU
- Added patch to escalate a link reset on login timeout
v4 Changes:
- Incorporate comments from Brian King
- Removed unnecessary check_state() parameter from "Fix to avoid CXL..."
- Added patch to fix potential deadlock on EEH
- Removed patch to avoid state change collision
- Changed fops initialization location in "Fix to avoid corrupting..."
v3 Changes:
- Rebased the series on top of patch by Dan Carpenter ("a couple off...")
- Incorporate comments from David Laight
- Incorporate comments from Tomas Henzl
- Incorporate comments from Brian King
- Removed patch to stop interrupt processing on remove
- Removed double scsi_device_put() from "Fix potential oops"
- Fixed usage of scnprintf() in "Refine host/device attributes"
- Removed unnecessary parenthesis from "Fix read capacity timeout"
- Added patch to use correct operator for doubling delay
- Changed location of cancel_work_sync() in "Fix to prevent workq..."
- Removed local mutex from cxlflash_afu_sync() in "Fix to avoid state..."
- Added patch to correctly identify a failed function in a trace
- Added patch to fix a fops corruption bug
v2 Changes:
- Incorporate comments from Ian Munsie
- Rework commit messages to be more descriptive
- Add state change serialization patch
Manoj Kumar (5):
cxlflash: Fix to avoid invalid port_sel value
cxlflash: Replace magic numbers with literals
cxlflash: Fix read capacity timeout
cxlflash: Fix to double the delay each time
cxlflash: Fix to escalate to LINK_RESET on login timeout
Matthew R. Ochs (29):
cxlflash: Fix potential oops following LUN removal
cxlflash: Fix data corruption when vLUN used over multiple cards
cxlflash: Fix to avoid sizeof(bool)
cxlflash: Fix context encode mask width
cxlflash: Fix to avoid CXL services during EEH
cxlflash: Correct naming of limbo state and waitq
cxlflash: Make functions static
cxlflash: Refine host/device attributes
cxlflash: Fix to avoid spamming the kernel log
cxlflash: Fix to avoid stall while waiting on TMF
cxlflash: Fix location of setting resid
cxlflash: Fix host link up event handling
cxlflash: Fix async interrupt bypass logic
cxlflash: Remove dual port online dependency
cxlflash: Fix AFU version access/storage and add check
cxlflash: Correct usage of scsi_host_put()
cxlflash: Fix to prevent workq from accessing freed memory
cxlflash: Correct behavior in device reset handler following EEH
cxlflash: Remove unnecessary scsi_block_requests
cxlflash: Fix function prolog parameters and return codes
cxlflash: Fix MMIO and endianness errors
cxlflash: Fix to prevent EEH recovery failure
cxlflash: Correct spelling, grammar, and alignment mistakes
cxlflash: Fix to prevent stale AFU RRQ
MAINTAINERS: Add cxlflash driver
cxlflash: Fix to avoid corrupting adapter fops
cxlflash: Correct trace string
cxlflash: Fix to avoid potential deadlock on EEH
cxlflash: Fix to avoid leaving dangling interrupt resources
MAINTAINERS |9 +
drivers/scsi/cxlflash/common.h| 30 +-
drivers/scsi/cxlflash/lunmgt.c|9 +-
drivers/scsi/cxlflash/main.c | 1549 -
drivers/scsi/cxlflash/main.h |1 +
drivers/scsi/cxlflash/sislite.h |8 +-
drivers/scsi/cxlflash/superpipe.c | 204 +++--
drivers/scsi/cxlflash/superpipe.h | 13 +-
drivers/scsi/cxlflash/vlun.c | 68 +-
9 files changed, 1055 insertions(+), 836 deletions(-)
--
2.1.0
--
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 v5 01/34] cxlflash: Fix to avoid invalid port_sel value
From: Manoj Kumar
If two concurrent MANAGE_LUN ioctls are issued with the same
WWID parameter, it would result in an incorrect value of port_sel.
This is because port_sel is modified without any locks being
held. If the first caller stalls after the return from
find_and_create_lun(), the value of port_sel will be set
incorrectly to indicate a single port, though in this case
it should have been set to both ports.
To fix, use the global mutex to serialize the lookup of the
WWID and the subsequent modification of port_sel.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/lunmgt.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxlflash/lunmgt.c b/drivers/scsi/cxlflash/lunmgt.c
index d98ad0f..8c372fc 100644
--- a/drivers/scsi/cxlflash/lunmgt.c
+++ b/drivers/scsi/cxlflash/lunmgt.c
@@ -120,7 +120,8 @@ static struct glun_info *lookup_global(u8 *wwid)
*
* The LUN is kept both in a local list (per adapter) and in a global list
* (across all adapters). Certain attributes of the LUN are local to the
- * adapter (such as index, port selection mask etc.).
+ * adapter (such as index, port selection mask, etc.).
+ *
* The block allocation map is shared across all adapters (i.e. associated
* wih the global list). Since different attributes are associated with
* the per adapter and global entries, allocate two separate structures for
each
@@ -128,6 +129,8 @@ static struct glun_info *lookup_global(u8 *wwid)
*
* Keep a pointer back from the local to the global entry.
*
+ * This routine assumes the caller holds the global mutex.
+ *
* Return: Found/Allocated local lun_info structure on success, NULL on failure
*/
static struct llun_info *find_and_create_lun(struct scsi_device *sdev, u8
*wwid)
@@ -137,7 +140,6 @@ static struct llun_info *find_and_create_lun(struct
scsi_device *sdev, u8 *wwid)
struct Scsi_Host *shost = sdev->host;
struct cxlflash_cfg *cfg = shost_priv(shost);
- mutex_lock(&global.mutex);
if (unlikely(!wwid))
goto out;
@@ -169,7 +171,6 @@ static struct llun_info *find_and_create_lun(struct
scsi_device *sdev, u8 *wwid)
list_add(&gli->list, &global.gluns);
out:
- mutex_unlock(&global.mutex);
pr_debug("%s: returning %p\n", __func__, lli);
return lli;
}
@@ -235,6 +236,7 @@ int cxlflash_manage_lun(struct scsi_device *sdev,
u64 flags = manage->hdr.flags;
u32 chan = sdev->channel;
+ mutex_lock(&global.mutex);
lli = find_and_create_lun(sdev, manage->wwid);
pr_debug("%s: ENTER: WWID = %016llX%016llX, flags = %016llX li = %p\n",
__func__, get_unaligned_le64(&manage->wwid[0]),
@@ -261,6 +263,7 @@ int cxlflash_manage_lun(struct scsi_device *sdev,
}
out:
+ mutex_unlock(&global.mutex);
pr_debug("%s: returning rc=%d\n", __func__, rc);
return rc;
}
--
2.1.0
--
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 v5 10/34] cxlflash: Make functions static
Found during code inspection, that the following functions are not
being used outside of the file where they are defined. Make them static.
int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
int cxlflash_afu_reset(struct cxlflash_cfg *);
struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
void cxlflash_cmd_checkin(struct afu_cmd *);
void init_pcr(struct cxlflash_cfg *);
int init_global(struct cxlflash_cfg *);
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h |5 -
drivers/scsi/cxlflash/main.c | 1018
2 files changed, 509 insertions(+), 514 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 11318de..b038ac7 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -192,11 +192,6 @@ static inline u64 lun_to_lunid(u64 lun)
return swab64(lun_id);
}
-int cxlflash_send_cmd(struct afu *, struct afu_cmd *);
-void cxlflash_wait_resp(struct afu *, struct afu_cmd *);
-int cxlflash_afu_reset(struct cxlflash_cfg *);
-struct afu_cmd *cxlflash_cmd_checkout(struct afu *);
-void cxlflash_cmd_checkin(struct afu_cmd *);
int cxlflash_afu_sync(struct afu *, ctx_hndl_t, res_hndl_t, u8);
void cxlflash_list_init(void);
void cxlflash_term_global_luns(void);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 8940336..226cefe 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -36,7 +36,7 @@ MODULE_LICENSE("GPL");
/**
- * cxlflash_cmd_checkout() - checks out an AFU command
+ * cmd_checkout() - checks out an AFU command
* @afu: AFU to checkout from.
*
* Commands are checked out in a round-robin fashion. Note that since
@@ -47,7 +47,7 @@ MODULE_LICENSE("GPL");
*
* Return: The checked out command or NULL when command pool is empty.
*/
-struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
+static struct afu_cmd *cmd_checkout(struct afu *afu)
{
int k, dec = CXLFLASH_NUM_CMDS;
struct afu_cmd *cmd;
@@ -70,7 +70,7 @@ struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
}
/**
- * cxlflash_cmd_checkin() - checks in an AFU command
+ * cmd_checkin() - checks in an AFU command
* @cmd: AFU command to checkin.
*
* Safe to pass commands that have already been checked in. Several
@@ -79,7 +79,7 @@ struct afu_cmd *cxlflash_cmd_checkout(struct afu *afu)
* to avoid clobbering values in the event that the command is checked
* out right away.
*/
-void cxlflash_cmd_checkin(struct afu_cmd *cmd)
+static void cmd_checkin(struct afu_cmd *cmd)
{
cmd->rcb.scp = NULL;
cmd->rcb.timeout = 0;
@@ -238,7 +238,7 @@ static void cmd_complete(struct afu_cmd *cmd)
resid = cmd->sa.resid;
cmd_is_tmf = cmd->cmd_tmf;
- cxlflash_cmd_checkin(cmd); /* Don't use cmd after here */
+ cmd_checkin(cmd); /* Don't use cmd after here */
pr_debug("%s: calling scsi_set_resid, scp=%p "
"result=%X resid=%d\n", __func__,
@@ -260,6 +260,146 @@ static void cmd_complete(struct afu_cmd *cmd)
}
/**
+ * context_reset() - timeout handler for AFU commands
+ * @cmd: AFU command that timed out.
+ *
+ * Sends a reset to the AFU.
+ */
+static void context_reset(struct afu_cmd *cmd)
+{
+ int nretry = 0;
+ u64 rrin = 0x1;
+ u64 room = 0;
+ struct afu *afu = cmd->parent;
+ ulong lock_flags;
+
+ pr_debug("%s: cmd=%p\n", __func__, cmd);
+
+ spin_lock_irqsave(&cmd->slock, lock_flags);
+
+ /* Already completed? */
+ if (cmd->sa.host_use_b[0] & B_DONE) {
+ spin_unlock_irqrestore(&cmd->slock, lock_flags);
+ return;
+ }
+
+ cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT);
+ spin_unlock_irqrestore(&cmd->slock, lock_flags);
+
+ /*
+* We really want to send this reset at all costs, so spread
+* out wait time on successive retries for available room.
+*/
+ do {
+ room = readq_be(&afu->host_map->cmd_room);
+ atomic64_set(&afu->room, room);
+ if (room)
+ goto write_rrin;
+ udelay(nretry);
+ } while (nretry++ < MC_ROOM_RETRY_CNT);
+
+ pr_err("%s: no cmd_room to send reset\n", __func__);
+ return;
+
+write_rrin:
+ nretry = 0;
+ writeq_be(rrin, &afu->host_map->ioarrin);
+ do {
+ rrin = readq_be(&afu->host_map->ioarrin);
+ if (rrin != 0x1)
+ break;
+ /* Double delay each time */
+ udelay(2 ^ nretry);
+ } while (nret
[PATCH v5 11/34] cxlflash: Refine host/device attributes
Implement the following suggestions and add two new attributes
to allow for debugging the port LUN table.
- use scnprintf() instead of snprintf()
- use DEVICE_ATTR_RO and DEVICE_ATTR_RW
Suggested-by: Shane Seymour
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 180 +--
1 file changed, 138 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 226cefe..b44212b 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1995,33 +1995,24 @@ static int cxlflash_change_queue_depth(struct
scsi_device *sdev, int qdepth)
/**
* cxlflash_show_port_status() - queries and presents the current port status
- * @dev: Generic device associated with the host owning the port.
- * @attr: Device attribute representing the port.
+ * @port: Desired port for status reporting.
+ * @afu: AFU owning the specified port.
* @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
*
* Return: The size of the ASCII string returned in @buf.
*/
-static ssize_t cxlflash_show_port_status(struct device *dev,
-struct device_attribute *attr,
-char *buf)
+static ssize_t cxlflash_show_port_status(u32 port, struct afu *afu, char *buf)
{
- struct Scsi_Host *shost = class_to_shost(dev);
- struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
- struct afu *afu = cfg->afu;
-
char *disp_status;
- int rc;
- u32 port;
u64 status;
- u64 *fc_regs;
+ __be64 __iomem *fc_regs;
- rc = kstrtouint((attr->attr.name + 4), 10, &port);
- if (rc || (port >= NUM_FC_PORTS))
+ if (port >= NUM_FC_PORTS)
return 0;
fc_regs = &afu->afu_map->global.fc_regs[port][0];
- status =
- (readq_be(&fc_regs[FC_MTIP_STATUS / 8]) & FC_MTIP_STATUS_MASK);
+ status = readq_be(&fc_regs[FC_MTIP_STATUS / 8]);
+ status &= FC_MTIP_STATUS_MASK;
if (status == FC_MTIP_STATUS_ONLINE)
disp_status = "online";
@@ -2030,31 +2021,69 @@ static ssize_t cxlflash_show_port_status(struct device
*dev,
else
disp_status = "unknown";
- return snprintf(buf, PAGE_SIZE, "%s\n", disp_status);
+ return scnprintf(buf, PAGE_SIZE, "%s\n", disp_status);
+}
+
+/**
+ * port0_show() - queries and presents the current status of port 0
+ * @dev: Generic device associated with the host owning the port.
+ * @attr: Device attribute representing the port.
+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
+ *
+ * Return: The size of the ASCII string returned in @buf.
+ */
+static ssize_t port0_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
+ struct afu *afu = cfg->afu;
+
+ return cxlflash_show_port_status(0, afu, buf);
}
/**
- * cxlflash_show_lun_mode() - presents the current LUN mode of the host
+ * port1_show() - queries and presents the current status of port 1
+ * @dev: Generic device associated with the host owning the port.
+ * @attr: Device attribute representing the port.
+ * @buf: Buffer of length PAGE_SIZE to report back port status in ASCII.
+ *
+ * Return: The size of the ASCII string returned in @buf.
+ */
+static ssize_t port1_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct Scsi_Host *shost = class_to_shost(dev);
+ struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
+ struct afu *afu = cfg->afu;
+
+ return cxlflash_show_port_status(1, afu, buf);
+}
+
+/**
+ * lun_mode_show() - presents the current LUN mode of the host
* @dev: Generic device associated with the host.
- * @attr: Device attribute representing the lun mode.
+ * @attr: Device attribute representing the LUN mode.
* @buf: Buffer of length PAGE_SIZE to report back the LUN mode in ASCII.
*
* Return: The size of the ASCII string returned in @buf.
*/
-static ssize_t cxlflash_show_lun_mode(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t lun_mode_show(struct device *dev,
+struct device_attribute *attr, char *buf)
{
struct Scsi_Host *shost = class_to_shost(dev);
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)shost->hostdata;
struct afu *afu = cfg->afu;
- return sn
[PATCH v5 07/34] cxlflash: Fix context encode mask width
The context encode mask covers more than 32-bits, making it
a long integer. This should be noted by appending the ULL
width suffix to the mask.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/superpipe.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers/scsi/cxlflash/superpipe.h
index 72d53cf..7947091 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -87,7 +87,7 @@ enum ctx_ctrl {
CTX_CTRL_FILE = (1 << 5)
};
-#define ENCODE_CTXID(_ctx, _id)(u64)_ctx) & 0x0) << 28) |
_id)
+#define ENCODE_CTXID(_ctx, _id)(u64)_ctx) & 0x0ULL) << 28)
| _id)
#define DECODE_CTXID(_val) (_val & 0x)
struct ctx_info {
--
2.1.0
--
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 v5 06/34] cxlflash: Fix to avoid sizeof(bool)
Using sizeof(bool) is considered poor form for various reasons and
sparse warns us of that. Correct by changing type from bool to u8.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/cxlflash/superpipe.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index ffa68cc..28aa9d9 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -737,7 +737,7 @@ static struct ctx_info *create_context(struct cxlflash_cfg
*cfg,
struct afu *afu = cfg->afu;
struct ctx_info *ctxi = NULL;
struct llun_info **lli = NULL;
- bool *ws = NULL;
+ u8 *ws = NULL;
struct sisl_rht_entry *rhte;
ctxi = kzalloc(sizeof(*ctxi), GFP_KERNEL);
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers/scsi/cxlflash/superpipe.h
index fffb179..72d53cf 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -97,7 +97,7 @@ struct ctx_info {
u32 rht_out;/* Number of checked out RHT entries */
u32 rht_perms; /* User-defined permissions for RHT entries */
struct llun_info **rht_lun; /* Mapping of RHT entries to LUNs */
- bool *rht_needs_ws; /* User-desired write-same function per RHTE */
+ u8 *rht_needs_ws; /* User-desired write-same function per RHTE */
struct cxl_ioctl_start_work work;
u64 ctxid;
--
2.1.0
--
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 v5 05/34] cxlflash: Fix data corruption when vLUN used over multiple cards
If the same virtual LUN is accessed over multiple cards, only accesses
made over the first card will be valid. Accesses made over the second
card will go to the wrong LUN causing data corruption.
This is because the global LUN's mode word was being used to determine
whether the LUN table for that card needs to be programmed. The mode
word would be setup by the first card, causing the LUN table for the
second card to not be programmed.
By unconditionally initializing the LUN table (not depending on the
mode word), the problem is avoided.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/vlun.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/cxlflash/vlun.c b/drivers/scsi/cxlflash/vlun.c
index 68994c4..96b074f 100644
--- a/drivers/scsi/cxlflash/vlun.c
+++ b/drivers/scsi/cxlflash/vlun.c
@@ -915,16 +915,9 @@ int cxlflash_disk_virtual_open(struct scsi_device *sdev,
void *arg)
pr_debug("%s: ctxid=%llu ls=0x%llx\n", __func__, ctxid, lun_size);
+ /* Setup the LUNs block allocator on first call */
mutex_lock(&gli->mutex);
if (gli->mode == MODE_NONE) {
- /* Setup the LUN table and block allocator on first call */
- rc = init_luntable(cfg, lli);
- if (rc) {
- dev_err(dev, "%s: call to init_luntable failed "
- "rc=%d!\n", __func__, rc);
- goto err0;
- }
-
rc = init_vlun(lli);
if (rc) {
dev_err(dev, "%s: call to init_vlun failed rc=%d!\n",
@@ -942,6 +935,13 @@ int cxlflash_disk_virtual_open(struct scsi_device *sdev,
void *arg)
}
mutex_unlock(&gli->mutex);
+ rc = init_luntable(cfg, lli);
+ if (rc) {
+ dev_err(dev, "%s: call to init_luntable failed rc=%d!\n",
+ __func__, rc);
+ goto err1;
+ }
+
ctxi = get_context(cfg, rctxid, lli, 0);
if (unlikely(!ctxi)) {
dev_err(dev, "%s: Bad context! (%llu)\n", __func__, ctxid);
--
2.1.0
--
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 v5 03/34] cxlflash: Fix read capacity timeout
From: Manoj Kumar
The timeout value for read capacity is too small. Certain devices
may take longer to respond and thus the command may prematurely
timeout. Additionally the literal used for the timeout is stale.
Update the timeout to 30 seconds (matches the value used in sd.c)
and rework the timeout literal to a more appropriate description.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/superpipe.c | 9 -
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 4db15a4..4e44a48 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -296,7 +296,7 @@ static int read_cap16(struct scsi_device *sdev, struct
llun_info *lli)
int rc = 0;
int result = 0;
int retry_cnt = 0;
- u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
+ u32 to = CMD_TIMEOUT * HZ;
retry:
cmd_buf = kzalloc(CMD_BUFSIZE, GFP_KERNEL);
@@ -315,8 +315,7 @@ retry:
retry_cnt ? "re" : "", scsi_cmd[0]);
result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
- CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
- 0, NULL);
+ CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
if (driver_byte(result) == DRIVER_SENSE) {
result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -1376,8 +1375,8 @@ out_attach:
attach->block_size = gli->blk_len;
attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
attach->last_lba = gli->max_lba;
- attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
- gli->blk_len;
+ attach->max_xfer = sdev->host->max_sectors * MAX_SECTOR_UNIT;
+ attach->max_xfer /= gli->blk_len;
out:
attach->adap_fd = fd;
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers/scsi/cxlflash/superpipe.h
index 3f7856b..fffb179 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -28,7 +28,7 @@ extern struct cxlflash_global global;
*/
#define MC_CHUNK_SIZE (1 << MC_RHT_NMASK) /* in LBAs */
-#define MC_DISCOVERY_TIMEOUT 5 /* 5 secs */
+#define CMD_TIMEOUT 30 /* 30 secs */
#define CMD_RETRIES 5 /* 5 retries for scsi_execute */
#define MAX_SECTOR_UNIT 512 /* max_sector is in 512 byte multiples */
diff --git a/drivers/scsi/cxlflash/vlun.c b/drivers/scsi/cxlflash/vlun.c
index 6d6608b..68994c4 100644
--- a/drivers/scsi/cxlflash/vlun.c
+++ b/drivers/scsi/cxlflash/vlun.c
@@ -414,7 +414,7 @@ static int write_same16(struct scsi_device *sdev,
int ws_limit = SISLITE_MAX_WS_BLOCKS;
u64 offset = lba;
int left = nblks;
- u32 tout = sdev->request_queue->rq_timeout;
+ u32 to = sdev->request_queue->rq_timeout;
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)sdev->host->hostdata;
struct device *dev = &cfg->dev->dev;
@@ -434,7 +434,7 @@ static int write_same16(struct scsi_device *sdev,
&scsi_cmd[10]);
result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
- CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
+ CMD_BUFSIZE, sense_buf, to, CMD_RETRIES,
0, NULL);
if (result) {
dev_err_ratelimited(dev, "%s: command failed for "
--
2.1.0
--
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 v5 04/34] cxlflash: Fix potential oops following LUN removal
When a LUN is removed, the sdev that is associated with the LUN
remains intact until its reference count drops to 0. In order
to prevent an sdev from being removed while a context is still
associated with it, obtain an additional reference per-context
for each LUN attached to the context.
This resolves a potential Oops in the release handler when a
dealing with a LUN that has already been removed.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/superpipe.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 4e44a48..ffa68cc 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -880,6 +880,9 @@ static int _cxlflash_disk_detach(struct scsi_device *sdev,
sys_close(lfd);
}
+ /* Release the sdev reference that bound this LUN to the context */
+ scsi_device_put(sdev);
+
out:
if (put_ctx)
put_context(ctxi);
@@ -1287,11 +1290,17 @@ static int cxlflash_disk_attach(struct scsi_device
*sdev,
}
}
+ rc = scsi_device_get(sdev);
+ if (unlikely(rc)) {
+ dev_err(dev, "%s: Unable to get sdev reference!\n", __func__);
+ goto out;
+ }
+
lun_access = kzalloc(sizeof(*lun_access), GFP_KERNEL);
if (unlikely(!lun_access)) {
dev_err(dev, "%s: Unable to allocate lun_access!\n", __func__);
rc = -ENOMEM;
- goto out;
+ goto err0;
}
lun_access->lli = lli;
@@ -1311,21 +1320,21 @@ static int cxlflash_disk_attach(struct scsi_device
*sdev,
dev_err(dev, "%s: Could not initialize context %p\n",
__func__, ctx);
rc = -ENODEV;
- goto err0;
+ goto err1;
}
ctxid = cxl_process_element(ctx);
if (unlikely((ctxid >= MAX_CONTEXT) || (ctxid < 0))) {
dev_err(dev, "%s: ctxid (%d) invalid!\n", __func__, ctxid);
rc = -EPERM;
- goto err1;
+ goto err2;
}
file = cxl_get_fd(ctx, &cfg->cxl_fops, &fd);
if (unlikely(fd < 0)) {
rc = -ENODEV;
dev_err(dev, "%s: Could not get file descriptor\n", __func__);
- goto err1;
+ goto err2;
}
/* Translate read/write O_* flags from fcntl.h to AFU permission bits */
@@ -1335,7 +1344,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
if (unlikely(!ctxi)) {
dev_err(dev, "%s: Failed to create context! (%d)\n",
__func__, ctxid);
- goto err2;
+ goto err3;
}
work = &ctxi->work;
@@ -1346,13 +1355,13 @@ static int cxlflash_disk_attach(struct scsi_device
*sdev,
if (unlikely(rc)) {
dev_dbg(dev, "%s: Could not start context rc=%d\n",
__func__, rc);
- goto err3;
+ goto err4;
}
rc = afu_attach(cfg, ctxi);
if (unlikely(rc)) {
dev_err(dev, "%s: Could not attach AFU rc %d\n", __func__, rc);
- goto err4;
+ goto err5;
}
/*
@@ -1388,13 +1397,13 @@ out:
__func__, ctxid, fd, attach->block_size, rc, attach->last_lba);
return rc;
-err4:
+err5:
cxl_stop_context(ctx);
-err3:
+err4:
put_context(ctxi);
destroy_context(cfg, ctxi);
ctxi = NULL;
-err2:
+err3:
/*
* Here, we're overriding the fops with a dummy all-NULL fops because
* fput() calls the release fop, which will cause us to mistakenly
@@ -1406,10 +1415,12 @@ err2:
fput(file);
put_unused_fd(fd);
fd = -1;
-err1:
+err2:
cxl_release_context(ctx);
-err0:
+err1:
kfree(lun_access);
+err0:
+ scsi_device_put(sdev);
goto out;
}
--
2.1.0
--
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 v5 02/34] cxlflash: Replace magic numbers with literals
From: Manoj Kumar
Magic numbers are not meaningful and can create confusion. As a
remedy, replace them with descriptive literals.
Replace 512 with literal MAX_SECTOR_UNIT.
Replace 5 with literal CMD_RETRIES.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
---
drivers/scsi/cxlflash/superpipe.c | 6 --
drivers/scsi/cxlflash/superpipe.h | 3 +++
drivers/scsi/cxlflash/vlun.c | 3 ++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 05e0ecf..4db15a4 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -315,7 +315,8 @@ retry:
retry_cnt ? "re" : "", scsi_cmd[0]);
result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
- CMD_BUFSIZE, sense_buf, tout, 5, 0, NULL);
+ CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
+ 0, NULL);
if (driver_byte(result) == DRIVER_SENSE) {
result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -1375,7 +1376,8 @@ out_attach:
attach->block_size = gli->blk_len;
attach->mmio_size = sizeof(afu->afu_map->hosts[0].harea);
attach->last_lba = gli->max_lba;
- attach->max_xfer = (sdev->host->max_sectors * 512) / gli->blk_len;
+ attach->max_xfer = (sdev->host->max_sectors * MAX_SECTOR_UNIT) /
+ gli->blk_len;
out:
attach->adap_fd = fd;
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers/scsi/cxlflash/superpipe.h
index d7dc88b..3f7856b 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -29,6 +29,9 @@ extern struct cxlflash_global global;
#define MC_CHUNK_SIZE (1 << MC_RHT_NMASK) /* in LBAs */
#define MC_DISCOVERY_TIMEOUT 5 /* 5 secs */
+#define CMD_RETRIES 5 /* 5 retries for scsi_execute */
+
+#define MAX_SECTOR_UNIT 512 /* max_sector is in 512 byte multiples */
#define CHAN2PORT(_x) ((_x) + 1)
#define PORT2CHAN(_x) ((_x) - 1)
diff --git a/drivers/scsi/cxlflash/vlun.c b/drivers/scsi/cxlflash/vlun.c
index 6155cb1..6d6608b 100644
--- a/drivers/scsi/cxlflash/vlun.c
+++ b/drivers/scsi/cxlflash/vlun.c
@@ -434,7 +434,8 @@ static int write_same16(struct scsi_device *sdev,
&scsi_cmd[10]);
result = scsi_execute(sdev, scsi_cmd, DMA_TO_DEVICE, cmd_buf,
- CMD_BUFSIZE, sense_buf, tout, 5, 0, NULL);
+ CMD_BUFSIZE, sense_buf, tout, CMD_RETRIES,
+ 0, NULL);
if (result) {
dev_err_ratelimited(dev, "%s: command failed for "
"offset %lld result=0x%x\n",
--
2.1.0
--
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 v5 17/34] cxlflash: Remove dual port online dependency
At present, both ports must be online for the device to
configure properly. Remove this dependency and the unnecessary
internal LUN override logic as well. Additionally, as a refactoring
measure, change the return code variable name to match that used
throughout the driver.
With this change, the card will be able to configure even when the
link is down. At some later point when the link is transitioned to
'up', a link state change interrupt will trigger the port configuration.
Note that despite its void-like behavior, the function was left with a
return code for right now in case its behavior needs to be altered again
in the near future based on testing.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index ed9fd8c..c25efc3 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1030,7 +1030,7 @@ static int wait_port_offline(u64 *fc_regs, u32 delay_us,
u32 nretry)
*/
static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
{
- int ret = 0;
+ int rc = 0;
set_port_offline(fc_regs);
@@ -1038,33 +1038,26 @@ static int afu_set_wwpn(struct afu *afu, int port, u64
*fc_regs, u64 wwpn)
FC_PORT_STATUS_RETRY_CNT)) {
pr_debug("%s: wait on port %d to go offline timed out\n",
__func__, port);
- ret = -1; /* but continue on to leave the port back online */
+ rc = -1; /* but continue on to leave the port back online */
}
- if (ret == 0)
+ if (rc == 0)
writeq_be(wwpn, &fc_regs[FC_PNAME / 8]);
+ /* Always return success after programming WWPN */
+ rc = 0;
+
set_port_online(fc_regs);
if (!wait_port_online(fc_regs, FC_PORT_STATUS_RETRY_INTERVAL_US,
FC_PORT_STATUS_RETRY_CNT)) {
- pr_debug("%s: wait on port %d to go online timed out\n",
-__func__, port);
- ret = -1;
-
- /*
-* Override for internal lun!!!
-*/
- if (afu->internal_lun) {
- pr_debug("%s: Overriding port %d online timeout!!!\n",
-__func__, port);
- ret = 0;
- }
+ pr_err("%s: wait on port %d to go online timed out\n",
+ __func__, port);
}
- pr_debug("%s: returning rc=%d\n", __func__, ret);
+ pr_debug("%s: returning rc=%d\n", __func__, rc);
- return ret;
+ return rc;
}
/**
--
2.1.0
--
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 v5 18/34] cxlflash: Fix AFU version access/storage and add check
The AFU version is stored as a non-terminated string of bytes within
a 64-bit little-endian register. Presently the value is read directly
(no MMIO accessor) and is stored in a buffer that is not big enough
to contain a NULL terminator. Additionally the version obtained is not
evaluated against a known value to prevent usage with unsupported AFUs.
All of these deficiencies can lead to a variety of problems.
To remedy, use the correct MMIO accessor to read the version value into
a null-terminated buffer and add a check to prevent an incompatible AFU
from being used with this driver.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h | 2 +-
drivers/scsi/cxlflash/main.c| 18 --
drivers/scsi/cxlflash/sislite.h | 2 +-
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index faf7f56..3be5754 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -179,7 +179,7 @@ struct afu {
u32 cmd_couts; /* Number of command checkouts */
u32 internal_lun; /* User-desired LUN mode for this AFU */
- char version[8];
+ char version[16];
u64 interface_version;
struct cxlflash_cfg *parent; /* Pointer back to parent cxlflash_cfg */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index c25efc3..c1d5c88 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1751,14 +1751,20 @@ static int init_afu(struct cxlflash_cfg *cfg)
goto err1;
}
- /* don't byte reverse on reading afu_version, else the string form */
- /* will be backwards */
- reg = afu->afu_map->global.regs.afu_version;
- memcpy(afu->version, ®, 8);
+ /* No byte reverse on reading afu_version or string will be backwards */
+ reg = readq(&afu->afu_map->global.regs.afu_version);
+ memcpy(afu->version, ®, sizeof(reg));
afu->interface_version =
readq_be(&afu->afu_map->global.regs.interface_version);
- pr_debug("%s: afu version %s, interface version 0x%llX\n",
-__func__, afu->version, afu->interface_version);
+ if ((afu->interface_version + 1) == 0) {
+ pr_err("Back level AFU, please upgrade. AFU version %s "
+ "interface version 0x%llx\n", afu->version,
+ afu->interface_version);
+ rc = -EINVAL;
+ goto err1;
+ } else
+ pr_debug("%s: afu version %s, interface version 0x%llX\n",
+__func__, afu->version, afu->interface_version);
rc = start_afu(cfg);
if (rc) {
diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
index 63bf394..8425d1a 100644
--- a/drivers/scsi/cxlflash/sislite.h
+++ b/drivers/scsi/cxlflash/sislite.h
@@ -340,7 +340,7 @@ struct sisl_global_regs {
#define SISL_AFUCONF_MBOX_CLR_READ 0x0010ULL
__be64 afu_config;
__be64 rsvd[0xf8];
- __be64 afu_version;
+ __le64 afu_version;
__be64 interface_version;
};
--
2.1.0
--
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 v5 12/34] cxlflash: Fix to avoid spamming the kernel log
During run-time the driver can be very chatty and spam the system
kernel log. Various print statements can be limited and/or moved
to development-only mode. Additionally, numerous prints can be
converted to trace the corresponding device. Lastly, one spelling
correction was made: 'entra' to 'extra'.
The following changes were made:
- pr_debug to pr_devel
- pr_debug to pr_debug_ratelimited
- pr_err to dev_err
- pr_debug to dev_dbg
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
Conflicts:
drivers/scsi/cxlflash/main.c
---
drivers/scsi/cxlflash/main.c | 109 +++
1 file changed, 59 insertions(+), 50 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b44212b..527ff85 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -58,8 +58,8 @@ static struct afu_cmd *cmd_checkout(struct afu *afu)
cmd = &afu->cmd[k];
if (!atomic_dec_if_positive(&cmd->free)) {
- pr_debug("%s: returning found index=%d\n",
-__func__, cmd->slot);
+ pr_devel("%s: returning found index=%d cmd=%p\n",
+__func__, cmd->slot, cmd);
memset(cmd->buf, 0, CMD_BUFSIZE);
memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb));
return cmd;
@@ -93,7 +93,7 @@ static void cmd_checkin(struct afu_cmd *cmd)
return;
}
- pr_debug("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
+ pr_devel("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot);
}
/**
@@ -127,7 +127,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
}
pr_debug("%s: cmd failed afu_rc=%d scsi_rc=%d fc_rc=%d "
-"afu_extra=0x%X, scsi_entra=0x%X, fc_extra=0x%X\n",
+"afu_extra=0x%X, scsi_extra=0x%X, fc_extra=0x%X\n",
__func__, ioasa->rc.afu_rc, ioasa->rc.scsi_rc,
ioasa->rc.fc_rc, ioasa->afu_extra, ioasa->scsi_extra,
ioasa->fc_extra);
@@ -240,9 +240,9 @@ static void cmd_complete(struct afu_cmd *cmd)
cmd_is_tmf = cmd->cmd_tmf;
cmd_checkin(cmd); /* Don't use cmd after here */
- pr_debug("%s: calling scsi_set_resid, scp=%p "
-"result=%X resid=%d\n", __func__,
-scp, scp->result, resid);
+ pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X "
+"ioasc=%d\n", __func__, scp, scp->result,
+cmd->sa.ioasc);
scsi_set_resid(scp, resid);
scsi_dma_unmap(scp);
@@ -417,12 +417,13 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
short lflag = 0;
struct Scsi_Host *host = scp->device->host;
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
+ struct device *dev = &cfg->dev->dev;
ulong lock_flags;
int rc = 0;
cmd = cmd_checkout(afu);
if (unlikely(!cmd)) {
- pr_err("%s: could not get a free command\n", __func__);
+ dev_err(dev, "%s: could not get a free command\n", __func__);
rc = SCSI_MLQUEUE_HOST_BUSY;
goto out;
}
@@ -493,7 +494,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
{
struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata;
struct afu *afu = cfg->afu;
- struct pci_dev *pdev = cfg->dev;
+ struct device *dev = &cfg->dev->dev;
struct afu_cmd *cmd;
u32 port_sel = scp->device->channel + 1;
int nseg, i, ncount;
@@ -502,13 +503,14 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
short lflag = 0;
int rc = 0;
- pr_debug("%s: (scp=%p) %d/%d/%d/%llu cdb=(%08X-%08X-%08X-%08X)\n",
-__func__, scp, host->host_no, scp->device->channel,
-scp->device->id, scp->device->lun,
-get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
-get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+ dev_dbg_ratelimited(dev, "%s: (scp=%p) %d/%d/%d/%llu "
+ "cdb=(%08X-%08X-%08X-%08X)\n",
+
[PATCH v5 14/34] cxlflash: Fix location of setting resid
The resid is incorrectly set which can lead to unnecessary retry
attempts by the stack. This is due to resid _always_ being set
using a value returned from the adapter. Instead, the value
should only be interpreted and set when in an underrun scenario.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 110037d..5503a40 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -107,6 +107,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
{
struct sisl_ioarcb *ioarcb;
struct sisl_ioasa *ioasa;
+ u32 resid;
if (unlikely(!cmd))
return;
@@ -115,9 +116,10 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
ioasa = &(cmd->sa);
if (ioasa->rc.flags & SISL_RC_FLAGS_UNDERRUN) {
- pr_debug("%s: cmd underrun cmd = %p scp = %p\n",
-__func__, cmd, scp);
- scp->result = (DID_ERROR << 16);
+ resid = ioasa->resid;
+ scsi_set_resid(scp, resid);
+ pr_debug("%s: cmd underrun cmd = %p scp = %p, resid = %d\n",
+__func__, cmd, scp, resid);
}
if (ioasa->rc.flags & SISL_RC_FLAGS_OVERRUN) {
@@ -158,8 +160,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
/* If the SISL_RC_FLAGS_OVERRUN flag was set,
* then we will handle this error else where.
* If not then we must handle it here.
-* This is probably an AFU bug. We will
-* attempt a retry to see if that resolves it.
+* This is probably an AFU bug.
*/
scp->result = (DID_ERROR << 16);
}
@@ -183,7 +184,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
/* We have an AFU error */
switch (ioasa->rc.afu_rc) {
case SISL_AFU_RC_NO_CHANNELS:
- scp->result = (DID_MEDIUM_ERROR << 16);
+ scp->result = (DID_NO_CONNECT << 16);
break;
case SISL_AFU_RC_DATA_DMA_ERR:
switch (ioasa->afu_extra) {
@@ -217,7 +218,6 @@ static void process_cmd_err(struct afu_cmd *cmd, struct
scsi_cmnd *scp)
static void cmd_complete(struct afu_cmd *cmd)
{
struct scsi_cmnd *scp;
- u32 resid;
ulong lock_flags;
struct afu *afu = cmd->parent;
struct cxlflash_cfg *cfg = afu->parent;
@@ -229,14 +229,11 @@ static void cmd_complete(struct afu_cmd *cmd)
if (cmd->rcb.scp) {
scp = cmd->rcb.scp;
- if (unlikely(cmd->sa.rc.afu_rc ||
-cmd->sa.rc.scsi_rc ||
-cmd->sa.rc.fc_rc))
+ if (unlikely(cmd->sa.ioasc))
process_cmd_err(cmd, scp);
else
scp->result = (DID_OK << 16);
- resid = cmd->sa.resid;
cmd_is_tmf = cmd->cmd_tmf;
cmd_checkin(cmd); /* Don't use cmd after here */
@@ -244,7 +241,6 @@ static void cmd_complete(struct afu_cmd *cmd)
"ioasc=%d\n", __func__, scp, scp->result,
cmd->sa.ioasc);
- scsi_set_resid(scp, resid);
scsi_dma_unmap(scp);
scp->scsi_done(scp);
--
2.1.0
--
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 v5 13/34] cxlflash: Fix to avoid stall while waiting on TMF
Borrowing the TMF waitq's spinlock causes a stall condition when
waiting for the TMF to complete. To remedy, introduce our own spin
lock to serialize TMF and use the appropriate wait services.
Also add a timeout while waiting for a TMF completion. When a TMF
times out, report back a failure such that a bigger hammer reset
can occur.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 55 +-
2 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index b038ac7..7a0cb5c 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -124,6 +124,7 @@ struct cxlflash_cfg {
struct list_head lluns; /* list of llun_info structs */
wait_queue_head_t tmf_waitq;
+ spinlock_t tmf_slock;
bool tmf_active;
wait_queue_head_t reset_waitq;
enum cxlflash_state state;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 527ff85..110037d 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -249,11 +249,10 @@ static void cmd_complete(struct afu_cmd *cmd)
scp->scsi_done(scp);
if (cmd_is_tmf) {
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
cfg->tmf_active = false;
wake_up_all_locked(&cfg->tmf_waitq);
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock,
- lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
}
} else
complete(&cmd->cevent);
@@ -420,6 +419,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp,
u64 tmfcmd)
struct device *dev = &cfg->dev->dev;
ulong lock_flags;
int rc = 0;
+ ulong to;
cmd = cmd_checkout(afu);
if (unlikely(!cmd)) {
@@ -428,15 +428,15 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
goto out;
}
- /* If a Task Management Function is active, do not send one more.
-*/
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ /* When Task Management Function is active do not send another */
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
if (cfg->tmf_active)
- wait_event_interruptible_locked_irq(cfg->tmf_waitq,
- !cfg->tmf_active);
+ wait_event_interruptible_lock_irq(cfg->tmf_waitq,
+ !cfg->tmf_active,
+ cfg->tmf_slock);
cfg->tmf_active = true;
cmd->cmd_tmf = true;
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
cmd->rcb.ctx_id = afu->ctx_hndl;
cmd->rcb.port_sel = port_sel;
@@ -457,15 +457,24 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd
*scp, u64 tmfcmd)
rc = send_cmd(afu, cmd);
if (unlikely(rc)) {
cmd_checkin(cmd);
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
cfg->tmf_active = false;
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
goto out;
}
- spin_lock_irqsave(&cfg->tmf_waitq.lock, lock_flags);
- wait_event_interruptible_locked_irq(cfg->tmf_waitq, !cfg->tmf_active);
- spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
+ spin_lock_irqsave(&cfg->tmf_slock, lock_flags);
+ to = msecs_to_jiffies(5000);
+ to = wait_event_interruptible_lock_irq_timeout(cfg->tmf_waitq,
+ !cfg->tmf_active,
+ cfg->tmf_slock,
+ to);
+ if (!to) {
+ cfg->tmf_active = false;
+ dev_err(dev, "%s: TMF timed out!\n", __func__);
+ rc = -1;
+ }
+ spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
out:
return rc;
}
@@ -512,16 +521,17 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
[PATCH v5 16/34] cxlflash: Fix async interrupt bypass logic
A bug was introduced earlier in the development cycle when cleaning
up logic statements. Instead of skipping bits that are not set, set
bits are skipped, causing async interrupts to not be handled correctly.
To fix, simply add back in the proper evaluation for an unset bit.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 98fdac1..ed9fd8c 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1315,7 +1315,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
/* check each bit that is on */
for (i = 0; reg_unmasked; i++, reg_unmasked = (reg_unmasked >> 1)) {
info = find_ainfo(1ULL << i);
- if ((reg_unmasked & 0x1) || !info)
+ if (((reg_unmasked & 0x1) == 0) || !info)
continue;
port = info->port;
--
2.1.0
--
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 v5 15/34] cxlflash: Fix host link up event handling
Following a link up event, the LUNs available to the host may
have changed. Without rescanning the host, the LUN topology is
unknown to the user. In such a state, the user would be unable
to locate provisioned resources.
To remedy, the host should be rescanned after a link up event.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Conflicts:
drivers/scsi/cxlflash/common.h
---
drivers/scsi/cxlflash/common.h | 1 +
drivers/scsi/cxlflash/main.c | 17 +
drivers/scsi/cxlflash/main.h | 1 +
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 7a0cb5c..faf7f56 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -102,6 +102,7 @@ struct cxlflash_cfg {
enum cxlflash_init_state init_state;
enum cxlflash_lr_state lr_state;
int lr_port;
+ atomic_t scan_host_needed;
struct cxl_afu *cxl_afu;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 5503a40..98fdac1 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1119,17 +1119,17 @@ static const struct asyc_intr_info ainfo[] = {
{SISL_ASTATUS_FC0_CRC_T, "CRC threshold exceeded", 0, LINK_RESET},
{SISL_ASTATUS_FC0_LOGI_R, "login timed out, retrying", 0, 0},
{SISL_ASTATUS_FC0_LOGI_F, "login failed", 0, CLR_FC_ERROR},
- {SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, 0},
+ {SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, SCAN_HOST},
{SISL_ASTATUS_FC0_LINK_DN, "link down", 0, 0},
- {SISL_ASTATUS_FC0_LINK_UP, "link up", 0, 0},
+ {SISL_ASTATUS_FC0_LINK_UP, "link up", 0, SCAN_HOST},
{SISL_ASTATUS_FC1_OTHER, "other error", 1, CLR_FC_ERROR | LINK_RESET},
{SISL_ASTATUS_FC1_LOGO, "target initiated LOGO", 1, 0},
{SISL_ASTATUS_FC1_CRC_T, "CRC threshold exceeded", 1, LINK_RESET},
{SISL_ASTATUS_FC1_LOGI_R, "login timed out, retrying", 1, 0},
{SISL_ASTATUS_FC1_LOGI_F, "login failed", 1, CLR_FC_ERROR},
- {SISL_ASTATUS_FC1_LOGI_S, "login succeeded", 1, 0},
+ {SISL_ASTATUS_FC1_LOGI_S, "login succeeded", 1, SCAN_HOST},
{SISL_ASTATUS_FC1_LINK_DN, "link down", 1, 0},
- {SISL_ASTATUS_FC1_LINK_UP, "link up", 1, 0},
+ {SISL_ASTATUS_FC1_LINK_UP, "link up", 1, SCAN_HOST},
{0x0, "", 0, 0} /* terminator */
};
@@ -1350,6 +1350,11 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
writeq_be(reg, &global->fc_regs[port][FC_ERROR / 8]);
writeq_be(0, &global->fc_regs[port][FC_ERRCAP / 8]);
}
+
+ if (info->action & SCAN_HOST) {
+ atomic_inc(&cfg->scan_host_needed);
+ schedule_work(&cfg->work_q);
+ }
}
out:
@@ -2309,6 +2314,7 @@ MODULE_DEVICE_TABLE(pci, cxlflash_pci_table);
* - Link reset which cannot be performed on interrupt context due to
* blocking up to a few seconds
* - Read AFU command room
+ * - Rescan the host
*/
static void cxlflash_worker_thread(struct work_struct *work)
{
@@ -2351,6 +2357,9 @@ static void cxlflash_worker_thread(struct work_struct
*work)
}
spin_unlock_irqrestore(cfg->host->host_lock, lock_flags);
+
+ if (atomic_dec_if_positive(&cfg->scan_host_needed) >= 0)
+ scsi_scan_host(cfg->host);
}
/**
diff --git a/drivers/scsi/cxlflash/main.h b/drivers/scsi/cxlflash/main.h
index cf0e809..6032456 100644
--- a/drivers/scsi/cxlflash/main.h
+++ b/drivers/scsi/cxlflash/main.h
@@ -99,6 +99,7 @@ struct asyc_intr_info {
u8 action;
#define CLR_FC_ERROR 0x01
#define LINK_RESET 0x02
+#define SCAN_HOST 0x04
};
#ifndef CONFIG_CXL_EEH
--
2.1.0
--
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 v5 23/34] cxlflash: Fix function prolog parameters and return codes
Several function prologs have incorrect parameter names and return
code descriptions. This can lead to confusion when reviewing the
source and creates inaccurate documentation.
To remedy, update the function prologs to properly reflect parameter
names and return codes.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
---
drivers/scsi/cxlflash/main.c | 70
1 file changed, 26 insertions(+), 44 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 441e897..f37e968 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -314,7 +314,7 @@ write_rrin:
* @cmd: AFU command to send.
*
* Return:
- * 0 on success or SCSI_MLQUEUE_HOST_BUSY
+ * 0 on success, SCSI_MLQUEUE_HOST_BUSY on failure
*/
static int send_cmd(struct afu *afu, struct afu_cmd *cmd)
{
@@ -401,8 +401,7 @@ static void wait_resp(struct afu *afu, struct afu_cmd *cmd)
* @tmfcmd:TMF command to send.
*
* Return:
- * 0 on success
- * SCSI_MLQUEUE_HOST_BUSY when host is busy
+ * 0 on success, SCSI_MLQUEUE_HOST_BUSY on failure
*/
static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
{
@@ -491,9 +490,7 @@ static const char *cxlflash_driver_info(struct Scsi_Host
*host)
* @host: SCSI host associated with device.
* @scp: SCSI command to send.
*
- * Return:
- * 0 on success
- * SCSI_MLQUEUE_HOST_BUSY when host is busy
+ * Return: 0 on success, SCSI_MLQUEUE_HOST_BUSY on failure
*/
static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp)
{
@@ -597,7 +594,7 @@ out:
/**
* cxlflash_wait_for_pci_err_recovery() - wait for error recovery during probe
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*/
static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg)
{
@@ -611,7 +608,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
cxlflash_cfg *cfg)
/**
* free_mem() - free memory associated with the AFU
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*/
static void free_mem(struct cxlflash_cfg *cfg)
{
@@ -633,7 +630,7 @@ static void free_mem(struct cxlflash_cfg *cfg)
/**
* stop_afu() - stops the AFU command timers and unmaps the MMIO space
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Safe to call with AFU in a partially allocated/initialized state.
*/
@@ -655,7 +652,7 @@ static void stop_afu(struct cxlflash_cfg *cfg)
/**
* term_mc() - terminates the master context
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
* @level: Depth of allocation, where to begin waterfall tear down.
*
* Safe to call with AFU/MC in partially allocated/initialized state.
@@ -691,7 +688,7 @@ static void term_mc(struct cxlflash_cfg *cfg, enum
undo_level level)
/**
* term_afu() - terminates the AFU
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Safe to call with AFU/MC in partially allocated/initialized state.
*/
@@ -751,7 +748,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
/**
* alloc_mem() - allocates the AFU and its command pool
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* A partially allocated state remains on failure.
*
@@ -804,12 +801,9 @@ out:
/**
* init_pci() - initializes the host as a PCI device
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
- * Return:
- * 0 on success
- * -EIO on unable to communicate with device
- * A return code from the PCI sub-routines
+ * Return: 0 on success, -errno on failure
*/
static int init_pci(struct cxlflash_cfg *cfg)
{
@@ -889,11 +883,9 @@ out_release_regions:
/**
* init_scsi() - adds the host to the SCSI stack and kicks off host scan
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
- * Return:
- * 0 on success
- * A return code from adding the host
+ * Return: 0 on success, -errno on failure
*/
static int init_scsi(struct cxlflash_cfg *cfg)
{
@@ -1357,7 +1349,7 @@ out:
/**
* start_context() - starts the master context
- * @cxlflash: Internal structure associated with the host.
+ * @cfg: Internal structure associated with the host.
*
* Return: A success or failure value from CXL services.
*/
@@ -1375,12 +1367,10 @@ static int start_context(struct cxlflash_cfg *cfg)
/**
* read_vpd() - obtains the WWPNs
[PATCH v5 21/34] cxlflash: Correct behavior in device reset handler following EEH
When the device reset handler is entered while a reset operation
is taking place, the handler exits without actually sending a
reset (TMF) to the targeted device. This behavior is incorrect
as the device is not reset. Further complicating matters is the
fact that a success is returned even when the TMF was not sent.
To fix, the state is rechecked after coming out of the reset
state. When the state is normal, a TMF will be sent out.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index d0b9972..89bd4c3 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1920,6 +1920,7 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+retry:
switch (cfg->state) {
case STATE_NORMAL:
rcr = send_tmf(afu, scp, TMF_LUN_RESET);
@@ -1928,9 +1929,7 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
break;
case STATE_RESET:
wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
- if (cfg->state == STATE_NORMAL)
- break;
- /* fall through */
+ goto retry;
default:
rc = FAILED;
break;
--
2.1.0
--
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 v5 19/34] cxlflash: Correct usage of scsi_host_put()
Currently, scsi_host_put() is being called prematurely in the remove path and is missing entirely in an error cleanup path. The former can lead to memory being freed too early with subsequent access potentially corrupting data whilst the former would result in a memory leak. Move the usage on remove to be the last cleanup action taken and introduce a call to scsi_host_put() in the one initialization error path that does not use remove to cleanup. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index c1d5c88..6b8b159 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -733,7 +733,6 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_SCSI: cxlflash_term_local_luns(cfg); scsi_remove_host(cfg->host); - scsi_host_put(cfg->host); /* Fall through */ case INIT_STATE_AFU: term_afu(cfg); @@ -743,6 +742,7 @@ static void cxlflash_remove(struct pci_dev *pdev) case INIT_STATE_NONE: flush_work(&cfg->work_q); free_mem(cfg); + scsi_host_put(cfg->host); break; } @@ -2404,6 +2404,7 @@ static int cxlflash_probe(struct pci_dev *pdev, dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n", __func__); rc = -ENOMEM; + scsi_host_put(cfg->host); goto out; } -- 2.1.0 -- 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 v5 20/34] cxlflash: Fix to prevent workq from accessing freed memory
The workq can process work in parallel with a remove event, leading to a condition where the workq handler can access freed memory. To remedy, the workq should be terminated prior to freeing memory. Move the termination call earlier in remove and use cancel_work_sync() instead of flush_work() as there is not a need to process any scheduled work when shutting down. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King --- drivers/scsi/cxlflash/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 6b8b159..d0b9972 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -736,11 +736,11 @@ static void cxlflash_remove(struct pci_dev *pdev) /* Fall through */ case INIT_STATE_AFU: term_afu(cfg); + cancel_work_sync(&cfg->work_q); case INIT_STATE_PCI: pci_release_regions(cfg->dev); pci_disable_device(pdev); case INIT_STATE_NONE: - flush_work(&cfg->work_q); free_mem(cfg); scsi_host_put(cfg->host); break; -- 2.1.0 -- 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 v5 22/34] cxlflash: Remove unnecessary scsi_block_requests
The host reset handler is called with I/O already blocked, thus
there is no need to explicitly block and unblock I/O in the handler.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
---
drivers/scsi/cxlflash/main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 89bd4c3..441e897 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1966,7 +1966,6 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
switch (cfg->state) {
case STATE_NORMAL:
cfg->state = STATE_RESET;
- scsi_block_requests(cfg->host);
cxlflash_mark_contexts_error(cfg);
rcr = afu_reset(cfg);
if (rcr) {
@@ -1975,7 +1974,6 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
} else
cfg->state = STATE_NORMAL;
wake_up_all(&cfg->reset_waitq);
- scsi_unblock_requests(cfg->host);
break;
case STATE_RESET:
wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
--
2.1.0
--
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 v5 28/34] MAINTAINERS: Add cxlflash driver
Add stanza for cxlflash SCSI driver. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King Reviewed-by: Andrew Donnellan --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 274f854..f2f3046 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3153,6 +3153,15 @@ F: Documentation/powerpc/cxl.txt F: Documentation/powerpc/cxl.txt F: Documentation/ABI/testing/sysfs-class-cxl +CXLFLASH (IBM Coherent Accelerator Processor Interface CAPI Flash) SCSI DRIVER +M: Manoj N. Kumar +M: Matthew R. Ochs +L: linux-scsi@vger.kernel.org +S: Supported +F: drivers/scsi/cxlflash/ +F: include/uapi/scsi/cxlflash_ioctls.h +F: Documentation/powerpc/cxlflash.txt + STMMAC ETHERNET DRIVER M: Giuseppe Cavallaro L: net...@vger.kernel.org -- 2.1.0 -- 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 v5 25/34] cxlflash: Fix to prevent EEH recovery failure
The process_sense() routine can perform a read capacity which
can take some time to complete. If an EEH occurs while waiting
on the read capacity, the EEH handler will wait to obtain the
context's mutex in order to put the context in an error state.
The EEH handler will sit and wait until the context is free,
but this wait can potentially last forever (deadlock) if the
scsi_execute() that performs the read capacity experiences a
timeout and calls into the reset callback. When that occurs,
the reset callback sees that the device is already being reset
and waits for the reset to complete. This leaves two threads
waiting on the other.
To address this issue, make the context unavailable to new,
non-system owned threads and release the context while calling
into process_sense(). After returning from process_sense() the
context mutex is reacquired and the context is made available
again. The context can be safely moved to the error state if
needed during the unavailable window as no other threads will
hold its reference.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/superpipe.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index a6316f5..7283e83 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1787,12 +1787,21 @@ static int cxlflash_disk_verify(struct scsi_device
*sdev,
* inquiry (i.e. the Unit attention is due to the WWN changing).
*/
if (verify->hint & DK_CXLFLASH_VERIFY_HINT_SENSE) {
+ /* Can't hold mutex across process_sense/read_cap16,
+* since we could have an intervening EEH event.
+*/
+ ctxi->unavail = true;
+ mutex_unlock(&ctxi->mutex);
rc = process_sense(sdev, verify);
if (unlikely(rc)) {
dev_err(dev, "%s: Failed to validate sense data (%d)\n",
__func__, rc);
+ mutex_lock(&ctxi->mutex);
+ ctxi->unavail = false;
goto out;
}
+ mutex_lock(&ctxi->mutex);
+ ctxi->unavail = false;
}
switch (gli->mode) {
--
2.1.0
--
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 v5 34/34] cxlflash: Fix to escalate to LINK_RESET on login timeout
From: Manoj Kumar
A 'login timed out' asynchronous error interrupt is generated if no
response is seen to a FLOGI within 2 seconds. If the time out error
is not escalated to a LINK_RESET the port will not be available for
use. This fix provides the required escalation.
Signed-off-by: Manoj N. Kumar
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index c152703..afaf533 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1100,7 +1100,7 @@ static const struct asyc_intr_info ainfo[] = {
{SISL_ASTATUS_FC0_OTHER, "other error", 0, CLR_FC_ERROR | LINK_RESET},
{SISL_ASTATUS_FC0_LOGO, "target initiated LOGO", 0, 0},
{SISL_ASTATUS_FC0_CRC_T, "CRC threshold exceeded", 0, LINK_RESET},
- {SISL_ASTATUS_FC0_LOGI_R, "login timed out, retrying", 0, 0},
+ {SISL_ASTATUS_FC0_LOGI_R, "login timed out, retrying", 0, LINK_RESET},
{SISL_ASTATUS_FC0_LOGI_F, "login failed", 0, CLR_FC_ERROR},
{SISL_ASTATUS_FC0_LOGI_S, "login succeeded", 0, SCAN_HOST},
{SISL_ASTATUS_FC0_LINK_DN, "link down", 0, 0},
--
2.1.0
--
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 v5 32/34] cxlflash: Fix to avoid potential deadlock on EEH
Ioctl threads that use scsi_execute() can run for an excessive amount
of time due to the fact that they have lengthy timeouts and retry logic
built in. Under normal operation this is not an issue. However, once EEH
enters the picture, a long execution time coupled with the possibility
that a timeout can trigger entry to the driver via registered reset
callbacks becomes a liability.
In particular, a deadlock can occur when an EEH event is encountered
while in running in scsi_execute(). As part of the recovery, the EEH
handler drains all currently running ioctls, waiting until they have
completed before proceeding with a reset. As the scsi_execute()'s are
situated on the ioctl path, the EEH handler will wait until they (and
the remainder of the ioctl handler they're associated with) have
completed. Normally this would not be much of an issue aside from the
longer recovery period. Unfortunately, the scsi_execute() triggers a
reset when it times out. The reset handler will see that the device is
already being reset and wait until that reset completed. This creates
a condition where the EEH handler becomes stuck, infinitely waiting for
the ioctl thread to complete.
To avoid this behavior, temporarily unmark the scsi_execute() threads
as an ioctl thread by releasing the ioctl read semaphore. This allows
the EEH handler to proceed with a recovery while the thread is still
running. Once the scsi_execute() returns, the ioctl read semaphore is
reacquired and the adapter state is rechecked in case it changed while
inside of scsi_execute(). The state check will wait if the adapter is
still being recovered or returns a failure if the recovery failed. In
the event that the adapter reset failed, the failure is simply returned
as the ioctl would be unable to continue.
Reported-by: Brian King
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/superpipe.c | 30 +-
drivers/scsi/cxlflash/superpipe.h | 2 ++
drivers/scsi/cxlflash/vlun.c | 29 +
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index f625e07..8af7cdc 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -283,6 +283,24 @@ out:
* @sdev: SCSI device associated with LUN.
* @lli: LUN destined for capacity request.
*
+ * The READ_CAP16 can take quite a while to complete. Should an EEH occur while
+ * in scsi_execute(), the EEH handler will attempt to recover. As part of the
+ * recovery, the handler drains all currently running ioctls, waiting until
they
+ * have completed before proceeding with a reset. As this routine is used on
the
+ * ioctl path, this can create a condition where the EEH handler becomes stuck,
+ * infinitely waiting for this ioctl thread. To avoid this behavior,
temporarily
+ * unmark this thread as an ioctl thread by releasing the ioctl read semaphore.
+ * This will allow the EEH handler to proceed with a recovery while this thread
+ * is still running. Once the scsi_execute() returns, reacquire the ioctl read
+ * semaphore and check the adapter state in case it changed while inside of
+ * scsi_execute(). The state check will wait if the adapter is still being
+ * recovered or return a failure if the recovery failed. In the event that the
+ * adapter reset failed, simply return the failure as the ioctl would be unable
+ * to continue.
+ *
+ * Note that the above puts a requirement on this routine to only be called on
+ * an ioctl thread.
+ *
* Return: 0 on success, -errno on failure
*/
static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
@@ -314,8 +332,18 @@ retry:
dev_dbg(dev, "%s: %ssending cmd(0x%x)\n", __func__,
retry_cnt ? "re" : "", scsi_cmd[0]);
+ /* Drop the ioctl read semahpore across lengthy call */
+ up_read(&cfg->ioctl_rwsem);
result = scsi_execute(sdev, scsi_cmd, DMA_FROM_DEVICE, cmd_buf,
CMD_BUFSIZE, sense_buf, to, CMD_RETRIES, 0, NULL);
+ down_read(&cfg->ioctl_rwsem);
+ rc = check_state(cfg);
+ if (rc) {
+ dev_err(dev, "%s: Failed state! result=0x08%X\n",
+ __func__, result);
+ rc = -ENODEV;
+ goto out;
+ }
if (driver_byte(result) == DRIVER_SENSE) {
result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
@@ -1221,7 +1249,7 @@ static const struct file_operations null_fops = {
*
* Return: 0 on success, -errno on failure
*/
-static int check_state(struct cxlflash_cfg *cfg)
+int check_state(struct cxlflash_cfg *cfg)
{
struct device *dev = &cfg->dev->dev;
int rc = 0;
diff --git a/drivers/scsi/cxlflash/superpipe.h
b/drivers
[PATCH v5 26/34] cxlflash: Correct spelling, grammar, and alignment mistakes
There are several spelling and grammar mistakes throughout the
driver. Additionally there are a handful of places where there
are extra lines and unnecessary variables/statements. These are
a nuisance and pollute the driver.
Fix spelling and grammar issues. Update some comments for clarity and
consistency. Remove extra lines and a few unneeded variables/statements.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
---
drivers/scsi/cxlflash/common.h| 2 --
drivers/scsi/cxlflash/main.c | 62 +--
drivers/scsi/cxlflash/sislite.h | 6 ++--
drivers/scsi/cxlflash/superpipe.c | 2 +-
drivers/scsi/cxlflash/vlun.c | 14 -
5 files changed, 38 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index a810585..bbfe711 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -105,8 +105,6 @@ struct cxlflash_cfg {
atomic_t scan_host_needed;
struct cxl_afu *cxl_afu;
-
- struct pci_pool *cxlflash_cmd_pool;
struct pci_dev *parent_dev;
atomic_t recovery_threads;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 14fb9b4..eeb1c47 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -34,7 +34,6 @@ MODULE_AUTHOR("Manoj N. Kumar ");
MODULE_AUTHOR("Matthew R. Ochs ");
MODULE_LICENSE("GPL");
-
/**
* cmd_checkout() - checks out an AFU command
* @afu: AFU to checkout from.
@@ -730,7 +729,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
case INIT_STATE_SCSI:
cxlflash_term_local_luns(cfg);
scsi_remove_host(cfg->host);
- /* Fall through */
+ /* fall through */
case INIT_STATE_AFU:
term_afu(cfg);
cancel_work_sync(&cfg->work_q);
@@ -763,9 +762,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg)
char *buf = NULL;
struct device *dev = &cfg->dev->dev;
- /* This allocation is about 12K, i.e. only 1 64k page
-* and upto 4 4k pages
-*/
+ /* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */
cfg->afu = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
get_order(sizeof(struct afu)));
if (unlikely(!cfg->afu)) {
@@ -1295,10 +1292,10 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
goto out;
}
- /* it is OK to clear AFU status before FC_ERROR */
+ /* FYI, it is 'okay' to clear AFU status before FC_ERROR */
writeq_be(reg_unmasked, &global->regs.aintr_clear);
- /* check each bit that is on */
+ /* Check each bit that is on */
for (i = 0; reg_unmasked; i++, reg_unmasked = (reg_unmasked >> 1)) {
info = find_ainfo(1ULL << i);
if (((reg_unmasked & 0x1) == 0) || !info)
@@ -1311,7 +1308,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
readq_be(&global->fc_regs[port][FC_STATUS / 8]));
/*
-* do link reset first, some OTHER errors will set FC_ERROR
+* Do link reset first, some OTHER errors will set FC_ERROR
* again if cleared before or w/o a reset
*/
if (info->action & LINK_RESET) {
@@ -1326,7 +1323,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
reg = readq_be(&global->fc_regs[port][FC_ERROR / 8]);
/*
-* since all errors are unmasked, FC_ERROR and FC_ERRCAP
+* Since all errors are unmasked, FC_ERROR and FC_ERRCAP
* should be the same and tracing one is sufficient.
*/
@@ -1472,23 +1469,22 @@ static void init_pcr(struct cxlflash_cfg *cfg)
for (i = 0; i < MAX_CONTEXT; i++) {
ctrl_map = &afu->afu_map->ctrls[i].ctrl;
- /* disrupt any clients that could be running */
- /* e. g. clients that survived a master restart */
+ /* Disrupt any clients that could be running */
+ /* e.g. clients that survived a master restart */
writeq_be(0, &ctrl_map->rht_start);
writeq_be(0, &ctrl_map->rht_cnt_id);
writeq_be(0, &ctrl_map->ctx_cap);
}
- /* copy frequently used fields into afu */
+ /* Copy frequently used fields into afu */
afu->ctx_hndl = (u16) cxl_process_element(cfg->mcctx);
- /* ctx_hndl is 16 bits in CAIA */
afu->host_map = &afu->afu_map->hosts[afu->
[PATCH v5 27/34] cxlflash: Fix to prevent stale AFU RRQ
Following an adapter reset, the AFU RRQ that resides in host memory holds stale data. This can lead to a condition where the RRQ interrupt handler tries to process stale entries and/or endlessly loops due to an out of sync generation bit. To fix, the AFU RRQ in host memory needs to be cleared after each reset. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King Reviewed-by: Daniel Axtens --- drivers/scsi/cxlflash/main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index eeb1c47..c77cb92 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -1598,6 +1598,9 @@ static int start_afu(struct cxlflash_cfg *cfg) init_pcr(cfg); + /* After an AFU reset, RRQ entries are stale, clear them */ + memset(&afu->rrq_entry, 0, sizeof(afu->rrq_entry)); + /* Initialize RRQ pointers */ afu->hrrq_start = &afu->rrq_entry[0]; afu->hrrq_end = &afu->rrq_entry[NUM_RRQ_ENTRY - 1]; -- 2.1.0 -- 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 v5 31/34] cxlflash: Correct trace string
The trace following the failure of alloc_mem() incorrectly identifies
which function failed. This can lead to misdiagnosing a failure.
Fix the string to correctly indicate that alloc_mem() failed.
Reported-by: Brian King
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
---
drivers/scsi/cxlflash/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 3f43879..998373e 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2377,7 +2377,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->host = host;
rc = alloc_mem(cfg);
if (rc) {
- dev_err(&pdev->dev, "%s: call to scsi_host_alloc failed!\n",
+ dev_err(&pdev->dev, "%s: call to alloc_mem failed!\n",
__func__);
rc = -ENOMEM;
scsi_host_put(cfg->host);
--
2.1.0
--
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 v5 30/34] cxlflash: Fix to avoid corrupting adapter fops
The fops owned by the adapter can be corrupted in certain scenarios,
opening a window where certain fops are temporarily NULLed before being
reset to their proper value. This can potentially lead software to make
incorrect decisions, leaving the user with the inability to function as
intended.
An example of this behavior can be observed when there are a number of
users with a high rate of turn around (attach to LUN, perform an I/O,
detach from LUN, repeat). Every so often a user is given a valid
context and adapter file descriptor, but the file associated with the
descriptor lacks the correct read permission bit (FMODE_CAN_READ) and
thus the read system call bails before calling the valid read fop.
Background:
The fops is stored in the adapter structure to provide the ability to
lookup the adapter structure from within the fop handler. CXL services
use the file's private_data and at present, the CXL context does not
have a private section. In an effort to limit areas of the cxlflash
driver with code specific the superpipe function, a design choice was
made to keep the details of the fops situated away from the legacy
portions of the driver. This drove the behavior that the adapter fops
is set at the beginning of the disk attach ioctl handler when there
are no users present.
The corruption that this fix remedies is due to the fact that the fops
is initially defaulted to values found within a static structure. When
the fops is handed down to the CXL services later in the attach path,
certain services are patched. The fops structure remains correct until
the user count drops to 0 and the fops is reset, triggering the process
to repeat again. The user counts are tightly coupled with the creation
and deletion of the user context. If multiple users perform a disk
attach at the same time, when the user count is currently 0, some users
can be in the middle of obtaining a file descriptor and have not yet
reached the context creation code that [in addition to creating the
context] increments the user count. Subsequent users coming in to
perform the attach see that the user count is still 0, and reinitialize
the fops, temporarily removing the patched fops. The users that are in
the middle obtaining their file descriptor may then receive an invalid
descriptor.
The fix simply removes the user count altogether and moves the fops
initialization to probe time such that it is only performed one time
for the life of the adapter. In the future, if the CXL services adopt
a private member for their context, that could be used to store the
adapter structure reference and cxlflash could revert to a model that
does not require an embedded fops.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/common.h| 3 +--
drivers/scsi/cxlflash/main.c | 1 +
drivers/scsi/cxlflash/superpipe.c | 11 +--
3 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index bbfe711..c11cd19 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -21,6 +21,7 @@
#include
#include
+extern const struct file_operations cxlflash_cxl_fops;
#define MAX_CONTEXT CXLFLASH_MAX_CONTEXT /* num contexts per afu */
@@ -115,8 +116,6 @@ struct cxlflash_cfg {
struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
struct file_operations cxl_fops;
- atomic_t num_user_contexts;
-
/* Parameters that are LUN table related */
int last_lun_index[CXLFLASH_NUM_FC_PORTS];
int promote_lun_index;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 51883be..3f43879 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2386,6 +2386,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->init_state = INIT_STATE_NONE;
cfg->dev = pdev;
+ cfg->cxl_fops = cxlflash_cxl_fops;
/*
* The promoted LUNs move to the top of the LUN table. The rest stay
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 3cc8609..f625e07 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -712,7 +712,6 @@ static void destroy_context(struct cxlflash_cfg *cfg,
kfree(ctxi->rht_needs_ws);
kfree(ctxi->rht_lun);
kfree(ctxi);
- atomic_dec_if_positive(&cfg->num_user_contexts);
}
/**
@@ -769,7 +768,6 @@ static struct ctx_info *create_context(struct cxlflash_cfg
*cfg,
INIT_LIST_HEAD(&ctxi->luns);
INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
- atomic_inc(&cfg->num_user_contexts);
mutex_lock(&ctxi->mutex);
out:
return ctxi;
@@ -1164,10 +1162,7 @@ out:
return rc;
}
-/*
- * Lo
[PATCH v5 24/34] cxlflash: Fix MMIO and endianness errors
Sparse uncovered several errors with MMIO operations (accessing
directly) and handling endianness. These can cause issues when
running in different environments.
Introduce __iomem and proper endianness tags/swaps where
appropriate to make driver sparse clean.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Andrew Donnellan
---
drivers/scsi/cxlflash/common.h| 10 +-
drivers/scsi/cxlflash/main.c | 25 +
drivers/scsi/cxlflash/superpipe.c | 6 +++---
drivers/scsi/cxlflash/superpipe.h | 2 +-
drivers/scsi/cxlflash/vlun.c | 4 ++--
5 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 3be5754..a810585 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -164,9 +164,9 @@ struct afu {
/* AFU HW */
struct cxl_ioctl_start_work work;
- struct cxlflash_afu_map *afu_map; /* entire MMIO map */
- struct sisl_host_map *host_map; /* MC host map */
- struct sisl_ctrl_map *ctrl_map; /* MC control map */
+ struct cxlflash_afu_map __iomem *afu_map; /* entire MMIO map */
+ struct sisl_host_map __iomem *host_map; /* MC host map */
+ struct sisl_ctrl_map __iomem *ctrl_map; /* MC control map */
ctx_hndl_t ctx_hndl;/* master's context handle */
u64 *hrrq_start;
@@ -188,10 +188,10 @@ struct afu {
static inline u64 lun_to_lunid(u64 lun)
{
- u64 lun_id;
+ __be64 lun_id;
int_to_scsilun(lun, (struct scsi_lun *)&lun_id);
- return swab64(lun_id);
+ return be64_to_cpu(lun_id);
}
int cxlflash_afu_sync(struct afu *, ctx_hndl_t, res_hndl_t, u8);
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index f37e968..14fb9b4 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -644,7 +644,7 @@ static void stop_afu(struct cxlflash_cfg *cfg)
complete(&afu->cmd[i].cevent);
if (likely(afu->afu_map)) {
- cxl_psa_unmap((void *)afu->afu_map);
+ cxl_psa_unmap((void __iomem *)afu->afu_map);
afu->afu_map = NULL;
}
}
@@ -914,7 +914,7 @@ out:
* that the FC link layer has synced, completed the handshaking process, and
* is ready for login to start.
*/
-static void set_port_online(u64 *fc_regs)
+static void set_port_online(__be64 __iomem *fc_regs)
{
u64 cmdcfg;
@@ -930,7 +930,7 @@ static void set_port_online(u64 *fc_regs)
*
* The provided MMIO region must be mapped prior to call.
*/
-static void set_port_offline(u64 *fc_regs)
+static void set_port_offline(__be64 __iomem *fc_regs)
{
u64 cmdcfg;
@@ -954,7 +954,7 @@ static void set_port_offline(u64 *fc_regs)
* FALSE (0) when the specified port fails to come online after timeout
* -EINVAL when @delay_us is less than 1000
*/
-static int wait_port_online(u64 *fc_regs, u32 delay_us, u32 nretry)
+static int wait_port_online(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
{
u64 status;
@@ -985,7 +985,7 @@ static int wait_port_online(u64 *fc_regs, u32 delay_us, u32
nretry)
* FALSE (0) when the specified port fails to go offline after timeout
* -EINVAL when @delay_us is less than 1000
*/
-static int wait_port_offline(u64 *fc_regs, u32 delay_us, u32 nretry)
+static int wait_port_offline(__be64 __iomem *fc_regs, u32 delay_us, u32 nretry)
{
u64 status;
@@ -1020,7 +1020,8 @@ static int wait_port_offline(u64 *fc_regs, u32 delay_us,
u32 nretry)
* 0 when the WWPN is successfully written and the port comes back online
* -1 when the port fails to go offline or come back up online
*/
-static int afu_set_wwpn(struct afu *afu, int port, u64 *fc_regs, u64 wwpn)
+static int afu_set_wwpn(struct afu *afu, int port, __be64 __iomem *fc_regs,
+ u64 wwpn)
{
int rc = 0;
@@ -1065,7 +1066,7 @@ static int afu_set_wwpn(struct afu *afu, int port, u64
*fc_regs, u64 wwpn)
* the alternate port exclusively while the reset takes place.
* failure to come online is overridden.
*/
-static void afu_link_reset(struct afu *afu, int port, u64 *fc_regs)
+static void afu_link_reset(struct afu *afu, int port, __be64 __iomem *fc_regs)
{
u64 port_sel;
@@ -1280,7 +1281,7 @@ static irqreturn_t cxlflash_async_err_irq(int irq, void
*data)
struct device *dev = &cfg->dev->dev;
u64 reg_unmasked;
const struct asyc_intr_info *info;
- struct sisl_global_map *global = &afu->afu_map->global;
+ struct sisl_global_map __iomem *global = &afu->afu_map->global;
u64 reg;
u8 port;
int i;
@@ -1466,7 +1467,7 @@ out:
static void init_pcr(struct cxlflash_cfg *
[PATCH v5 08/34] cxlflash: Fix to avoid CXL services during EEH
During an EEH freeze event, certain CXL services should not be
called until after the hardware reset has taken place. Doing so
can result in unnecessary failures and possibly cause other ill
effects by triggering hardware accesses. This translates to a
requirement to quiesce all threads that may potentially use CXL
runtime service during this window. In particular, multiple ioctls
make use of the CXL services when acting on contexts on behalf of
the user. Thus, it is essential to 'drain' running ioctls _before_
proceeding with handling the EEH freeze event.
Create the ability to drain ioctls by wrapping the ioctl handler
call in a read semaphore and then implementing a small routine that
obtains the write semaphore, effectively creating a wait point for
all currently executing ioctls.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/common.h| 2 +
drivers/scsi/cxlflash/main.c | 18 +--
drivers/scsi/cxlflash/superpipe.c | 98 ---
3 files changed, 77 insertions(+), 41 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 1c56037..1abe4e0 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -16,6 +16,7 @@
#define _CXLFLASH_COMMON_H
#include
+#include
#include
#include
#include
@@ -110,6 +111,7 @@ struct cxlflash_cfg {
atomic_t recovery_threads;
struct mutex ctx_recovery_mutex;
struct mutex ctx_tbl_list_mutex;
+ struct rw_semaphore ioctl_rwsem;
struct ctx_info *ctx_tbl[MAX_CONTEXT];
struct list_head ctx_err_recovery; /* contexts w/ recovery pending */
struct file_operations cxl_fops;
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 3e3ccf1..6e85c77 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2311,6 +2311,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->lr_port = -1;
mutex_init(&cfg->ctx_tbl_list_mutex);
mutex_init(&cfg->ctx_recovery_mutex);
+ init_rwsem(&cfg->ioctl_rwsem);
INIT_LIST_HEAD(&cfg->ctx_err_recovery);
INIT_LIST_HEAD(&cfg->lluns);
@@ -2365,6 +2366,19 @@ out_remove:
}
/**
+ * drain_ioctls() - wait until all currently executing ioctls have completed
+ * @cfg: Internal structure associated with the host.
+ *
+ * Obtain write access to read/write semaphore that wraps ioctl
+ * handling to 'drain' ioctls currently executing.
+ */
+static void drain_ioctls(struct cxlflash_cfg *cfg)
+{
+ down_write(&cfg->ioctl_rwsem);
+ up_write(&cfg->ioctl_rwsem);
+}
+
+/**
* cxlflash_pci_error_detected() - called when a PCI error is detected
* @pdev: PCI device struct.
* @state: PCI channel state.
@@ -2383,16 +2397,14 @@ static pci_ers_result_t
cxlflash_pci_error_detected(struct pci_dev *pdev,
switch (state) {
case pci_channel_io_frozen:
cfg->state = STATE_LIMBO;
-
- /* Turn off legacy I/O */
scsi_block_requests(cfg->host);
+ drain_ioctls(cfg);
rc = cxlflash_mark_contexts_error(cfg);
if (unlikely(rc))
dev_err(dev, "%s: Failed to mark user contexts!(%d)\n",
__func__, rc);
term_mc(cfg, UNDO_START);
stop_afu(cfg);
-
return PCI_ERS_RESULT_NEED_RESET;
case pci_channel_io_perm_failure:
cfg->state = STATE_FAILTERM;
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
index 28aa9d9..655cbf1 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1214,6 +1214,46 @@ static const struct file_operations null_fops = {
};
/**
+ * check_state() - checks and responds to the current adapter state
+ * @cfg: Internal structure associated with the host.
+ *
+ * This routine can block and should only be used on process context.
+ * It assumes that the caller is an ioctl thread and holding the ioctl
+ * read semaphore. This is temporarily let up across the wait to allow
+ * for draining actively running ioctls. Also note that when waking up
+ * from waiting in reset, the state is unknown and must be checked again
+ * before proceeding.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+static int check_state(struct cxlflash_cfg *cfg)
+{
+ struct device *dev = &cfg->dev->dev;
+ int rc = 0;
+
+retry:
+ switch (cfg->state) {
+ case STATE_LIMBO:
+ dev_dbg(dev, "%s: Limbo state, going to wait...\n", __func__);
+ up_read(&cfg->ioctl_rwsem);
+ rc = wait_event_interruptible(cfg->limbo_waitq,
+
[PATCH v5 09/34] cxlflash: Correct naming of limbo state and waitq
Limbo is not an accurate representation of this state and is
also not consistent with the terminology that other drivers
use to represent this concept. Rename the state and and its
associated waitq to 'reset'.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
Reviewed-by: Brian King
Reviewed-by: Daniel Axtens
---
drivers/scsi/cxlflash/common.h| 4 ++--
drivers/scsi/cxlflash/main.c | 26 +-
drivers/scsi/cxlflash/superpipe.c | 14 +++---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 1abe4e0..11318de 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -79,7 +79,7 @@ enum cxlflash_init_state {
enum cxlflash_state {
STATE_NORMAL, /* Normal running state, everything good */
- STATE_LIMBO,/* Limbo running state, trying to reset/recover */
+ STATE_RESET,/* Reset state, trying to reset/recover */
STATE_FAILTERM /* Failed/terminating state, error out users/threads */
};
@@ -125,7 +125,7 @@ struct cxlflash_cfg {
wait_queue_head_t tmf_waitq;
bool tmf_active;
- wait_queue_head_t limbo_waitq;
+ wait_queue_head_t reset_waitq;
enum cxlflash_state state;
};
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 6e85c77..8940336 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -382,8 +382,8 @@ static int cxlflash_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scp)
spin_unlock_irqrestore(&cfg->tmf_waitq.lock, lock_flags);
switch (cfg->state) {
- case STATE_LIMBO:
- dev_dbg_ratelimited(&cfg->dev->dev, "%s: device in limbo!\n",
+ case STATE_RESET:
+ dev_dbg_ratelimited(&cfg->dev->dev, "%s: device is in reset!\n",
__func__);
rc = SCSI_MLQUEUE_HOST_BUSY;
goto out;
@@ -479,8 +479,8 @@ static int cxlflash_eh_device_reset_handler(struct
scsi_cmnd *scp)
if (unlikely(rcr))
rc = FAILED;
break;
- case STATE_LIMBO:
- wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
+ case STATE_RESET:
+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
if (cfg->state == STATE_NORMAL)
break;
/* fall through */
@@ -519,7 +519,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd
*scp)
switch (cfg->state) {
case STATE_NORMAL:
- cfg->state = STATE_LIMBO;
+ cfg->state = STATE_RESET;
scsi_block_requests(cfg->host);
cxlflash_mark_contexts_error(cfg);
rcr = cxlflash_afu_reset(cfg);
@@ -528,11 +528,11 @@ static int cxlflash_eh_host_reset_handler(struct
scsi_cmnd *scp)
cfg->state = STATE_FAILTERM;
} else
cfg->state = STATE_NORMAL;
- wake_up_all(&cfg->limbo_waitq);
+ wake_up_all(&cfg->reset_waitq);
scsi_unblock_requests(cfg->host);
break;
- case STATE_LIMBO:
- wait_event(cfg->limbo_waitq, cfg->state != STATE_LIMBO);
+ case STATE_RESET:
+ wait_event(cfg->reset_waitq, cfg->state != STATE_RESET);
if (cfg->state == STATE_NORMAL)
break;
/* fall through */
@@ -705,7 +705,7 @@ static void cxlflash_wait_for_pci_err_recovery(struct
cxlflash_cfg *cfg)
struct pci_dev *pdev = cfg->dev;
if (pci_channel_offline(pdev))
- wait_event_timeout(cfg->limbo_waitq,
+ wait_event_timeout(cfg->reset_waitq,
!pci_channel_offline(pdev),
CXLFLASH_PCI_ERROR_RECOVERY_TIMEOUT);
}
@@ -2304,7 +2304,7 @@ static int cxlflash_probe(struct pci_dev *pdev,
cfg->mcctx = NULL;
init_waitqueue_head(&cfg->tmf_waitq);
- init_waitqueue_head(&cfg->limbo_waitq);
+ init_waitqueue_head(&cfg->reset_waitq);
INIT_WORK(&cfg->work_q, cxlflash_worker_thread);
cfg->lr_state = LINK_RESET_INVALID;
@@ -2396,7 +2396,7 @@ static pci_ers_result_t
cxlflash_pci_error_detected(struct pci_dev *pdev,
switch (state) {
case pci_channel_io_frozen:
- cfg->state = STATE_LIMBO;
+ cfg->state = STATE_RESET;
scsi_block_requests(cfg->host);
drain_ioctls(cfg);
rc = cxlflash_mark_contexts_error(cfg);
@@ -2408,7 +2408,7 @@ static pci_ers_result_t
cxlflash
[PATCH v5 29/34] cxlflash: Fix to double the delay each time
From: Manoj Kumar The operator used to double the master context response delay is incorrect and does not result in delay doubling. To fix, use a left shift instead of the XOR operator. Reported-by: Tomas Henzl Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar Reviewed-by: Brian King Reviewed-by: Andrew Donnellan --- drivers/scsi/cxlflash/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index c77cb92..51883be 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -303,7 +303,7 @@ write_rrin: if (rrin != 0x1) break; /* Double delay each time */ - udelay(2 ^ nretry); + udelay(2 << nretry); } while (nretry++ < MC_ROOM_RETRY_CNT); } -- 2.1.0 -- 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 v5 33/34] cxlflash: Fix to avoid leaving dangling interrupt resources
When running with an unsupported AFU, the cxlflash driver fails
the probe. When the driver is removed, the following Oops is
encountered on a show_interrupts() thread:
Call Trace:
[c01fba5a7a10] [0003] 0x3 (unreliable)
[c01fba5a7a60] [c053dcf4] vsnprintf+0x204/0x4c0
[c01fba5a7ae0] [c030045c] seq_vprintf+0x5c/0xd0
[c01fba5a7b20] [c030051c] seq_printf+0x4c/0x60
[c01fba5a7b50] [c013e140] show_interrupts+0x370/0x4f0
[c01fba5a7c10] [c02ff898] seq_read+0xe8/0x530
[c01fba5a7ca0] [c035d5c0] proc_reg_read+0xb0/0x110
[c01fba5a7cf0] [c02ca74c] __vfs_read+0x6c/0x180
[c01fba5a7d90] [c02cb464] vfs_read+0xa4/0x1c0
[c01fba5a7de0] [c02cc51c] SyS_read+0x6c/0x110
[c01fba5a7e30] [c0009204] system_call+0x38/0xb4
The Oops is due to not cleaning up correctly on the unsupported
AFU error path, leaving various allocated and registered resources.
In this case, interrupts are in a semi-allocated/registered state,
which the show_interrupts() thread attempts to use.
To fix, the cleanup logic in init_afu() is consolidated to error
gates at the bottom of the function and the appropriate goto is
added to each error path. As a mini side fix while refactoring
in this routine, the else statement following the AFU version
evaluation is eliminated as it is not needed.
Signed-off-by: Matthew R. Ochs
---
drivers/scsi/cxlflash/main.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 998373e..c152703 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1721,15 +1721,14 @@ static int init_afu(struct cxlflash_cfg *cfg)
if (rc) {
dev_err(dev, "%s: call to init_mc failed, rc=%d!\n",
__func__, rc);
- goto err1;
+ goto out;
}
/* Map the entire MMIO space of the AFU */
afu->afu_map = cxl_psa_map(cfg->mcctx);
if (!afu->afu_map) {
- rc = -ENOMEM;
- term_mc(cfg, UNDO_START);
dev_err(dev, "%s: call to cxl_psa_map failed!\n", __func__);
+ rc = -ENOMEM;
goto err1;
}
@@ -1743,19 +1742,17 @@ static int init_afu(struct cxlflash_cfg *cfg)
"interface version 0x%llx\n", afu->version,
afu->interface_version);
rc = -EINVAL;
- goto err1;
- } else
- pr_debug("%s: afu version %s, interface version 0x%llX\n",
-__func__, afu->version, afu->interface_version);
+ goto err2;
+ }
+
+ pr_debug("%s: afu version %s, interface version 0x%llX\n", __func__,
+afu->version, afu->interface_version);
rc = start_afu(cfg);
if (rc) {
dev_err(dev, "%s: call to start_afu failed, rc=%d!\n",
__func__, rc);
- term_mc(cfg, UNDO_START);
- cxl_psa_unmap((void __iomem *)afu->afu_map);
- afu->afu_map = NULL;
- goto err1;
+ goto err2;
}
afu_err_intr_init(cfg->afu);
@@ -1763,9 +1760,16 @@ static int init_afu(struct cxlflash_cfg *cfg)
/* Restore the LUN mappings */
cxlflash_restore_luntable(cfg);
-err1:
+out:
pr_debug("%s: returning rc=%d\n", __func__, rc);
return rc;
+
+err2:
+ cxl_psa_unmap((void __iomem *)afu->afu_map);
+ afu->afu_map = NULL;
+err1:
+ term_mc(cfg, UNDO_START);
+ goto out;
}
/**
--
2.1.0
--
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
Re: [PATCH v5 34/34] cxlflash: Fix to escalate to LINK_RESET on login timeout
Acked-by: Matthew R. Ochs -- 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
Re: [PATCH v5 18/34] cxlflash: Fix AFU version access/storage and add check
> On Oct 1, 2015, at 7:56 PM, Daniel Axtens wrote:
>
> "Matthew R. Ochs" writes:
>
>> The AFU version is stored as a non-terminated string of bytes within
>> a 64-bit little-endian register. Presently the value is read directly
>> (no MMIO accessor) and is stored in a buffer that is not big enough
>> to contain a NULL terminator. Additionally the version obtained is not
>> evaluated against a known value to prevent usage with unsupported AFUs.
>> All of these deficiencies can lead to a variety of problems.
>>
>> +if ((afu->interface_version + 1) == 0) {
>> +pr_err("Back level AFU, please upgrade. AFU version %s "
>> + "interface version 0x%llx\n", afu->version,
>> + afu->interface_version);
>> +rc = -EINVAL;
>> +goto err1;
>
> I'm confused by this if statement. If afu->interface_version + 1 == 0,
> and interface_version is a 64bit unsigned int, that would mean that
> afu->interface_version was 0x .
>
> Are you trying to check against all Fs? Is that value significant in the
> hardware?
Correct, downlevel (unsupported) AFUs don't implement the interface_version
register and thus will return -1 at that offset.
--
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
Re: [PATCH v5 00/34] cxlflash: Miscellaneous bug fixes and corrections
Hi James,
This series has been fairly well vetted. With cxlflash being a new driver the
majority
of these patches fix critical bugs. Is there anything else you're looking for
in order to
get this set pulled into 4.3-rc?
-matt
> On Oct 1, 2015, at 10:52 AM, Matthew R. Ochs
> wrote:
>
> This patch set contains various fixes and corrections for issues that
> were found during test and code review. The series is based upon the
> code upstreamed in 4.3 and is intended for the rc phase. The entire
> set is bisectable. Please reference the changelog below for details
> on what has been altered from previous versions of this patch set.
>
> v5 Changes:
> - Incorporate comments from Daniel Axtens
> - Incorporate comments from Andrew Donnellan
> - Added additional clarifications to several commit messages
> - Specified some return codes as failures in "Fix function prolog..."
> - Made port online failure noisier in "Remove dual port online..."
> - Added patch to properly cleanup when encountering an unsupported AFU
> - Added patch to escalate a link reset on login timeout
>
> v4 Changes:
> - Incorporate comments from Brian King
> - Removed unnecessary check_state() parameter from "Fix to avoid CXL..."
> - Added patch to fix potential deadlock on EEH
> - Removed patch to avoid state change collision
> - Changed fops initialization location in "Fix to avoid corrupting..."
>
> v3 Changes:
> - Rebased the series on top of patch by Dan Carpenter ("a couple off...")
> - Incorporate comments from David Laight
> - Incorporate comments from Tomas Henzl
> - Incorporate comments from Brian King
> - Removed patch to stop interrupt processing on remove
> - Removed double scsi_device_put() from "Fix potential oops"
> - Fixed usage of scnprintf() in "Refine host/device attributes"
> - Removed unnecessary parenthesis from "Fix read capacity timeout"
> - Added patch to use correct operator for doubling delay
> - Changed location of cancel_work_sync() in "Fix to prevent workq..."
> - Removed local mutex from cxlflash_afu_sync() in "Fix to avoid state..."
> - Added patch to correctly identify a failed function in a trace
> - Added patch to fix a fops corruption bug
>
> v2 Changes:
> - Incorporate comments from Ian Munsie
> - Rework commit messages to be more descriptive
> - Add state change serialization patch
>
> Manoj Kumar (5):
> cxlflash: Fix to avoid invalid port_sel value
> cxlflash: Replace magic numbers with literals
> cxlflash: Fix read capacity timeout
> cxlflash: Fix to double the delay each time
> cxlflash: Fix to escalate to LINK_RESET on login timeout
>
> Matthew R. Ochs (29):
> cxlflash: Fix potential oops following LUN removal
> cxlflash: Fix data corruption when vLUN used over multiple cards
> cxlflash: Fix to avoid sizeof(bool)
> cxlflash: Fix context encode mask width
> cxlflash: Fix to avoid CXL services during EEH
> cxlflash: Correct naming of limbo state and waitq
> cxlflash: Make functions static
> cxlflash: Refine host/device attributes
> cxlflash: Fix to avoid spamming the kernel log
> cxlflash: Fix to avoid stall while waiting on TMF
> cxlflash: Fix location of setting resid
> cxlflash: Fix host link up event handling
> cxlflash: Fix async interrupt bypass logic
> cxlflash: Remove dual port online dependency
> cxlflash: Fix AFU version access/storage and add check
> cxlflash: Correct usage of scsi_host_put()
> cxlflash: Fix to prevent workq from accessing freed memory
> cxlflash: Correct behavior in device reset handler following EEH
> cxlflash: Remove unnecessary scsi_block_requests
> cxlflash: Fix function prolog parameters and return codes
> cxlflash: Fix MMIO and endianness errors
> cxlflash: Fix to prevent EEH recovery failure
> cxlflash: Correct spelling, grammar, and alignment mistakes
> cxlflash: Fix to prevent stale AFU RRQ
> MAINTAINERS: Add cxlflash driver
> cxlflash: Fix to avoid corrupting adapter fops
> cxlflash: Correct trace string
> cxlflash: Fix to avoid potential deadlock on EEH
> cxlflash: Fix to avoid leaving dangling interrupt resources
>
> MAINTAINERS |9 +
> drivers/scsi/cxlflash/common.h| 30 +-
> drivers/scsi/cxlflash/lunmgt.c|9 +-
> drivers/scsi/cxlflash/main.c | 1549 -
> drivers/scsi/cxlflash/main.h |1 +
> drivers/scsi/cxlflash/sislite.h |8 +-
> drivers/scsi/cxlflash/superpipe.c | 204 +++--
> drivers/scsi/cxlflash/superpipe.h | 13 +-
> drivers/scsi/cxlflash/vlun.c | 68 +-
> 9 files changed, 1055 insertions(+), 836 deletions(-)
>
> --
> 2.1.0
Re: [PATCH v2] ipr: fix out-of-bounds null overwrite
On Jan 6, 2016, at 7:53 AM, Insu Yun wrote: > > Return value of snprintf is not bound by size value, 2nd argument. > (https://www.kernel.org/doc/htmldocs/kernel-api/API-snprintf.html). > Return value is number of printed chars, can be larger than 2nd argument. > Therefore, it can write null byte out of bounds ofbuffer. > Since snprintf puts null, it does not need to put additional null byte. > > Signed-off-by: Insu Yun > --- > drivers/scsi/ipr.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c > index 536cd5a..caf63f9 100644 > --- a/drivers/scsi/ipr.c > +++ b/drivers/scsi/ipr.c > @@ -4003,13 +4003,12 @@ static ssize_t ipr_store_update_fw(struct device *dev, > struct ipr_sglist *sglist; > char fname[100]; > char *src; > - int len, result, dnld_size; > + int result, dnld_size; > > if (!capable(CAP_SYS_ADMIN)) > return -EACCES; > > - len = snprintf(fname, 99, "%s", buf); > - fname[len-1] = '\0'; > + snprintf(fname, 99, "%s", buf); Changing the 99 to sizeof(fname) as part of this change would also make for cleaner code in my opinion. Will leave it up to you if you'd like to do a v3. Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH] scsi: export function scsi_scan.c:sanitize_inquiry_string
> On Jan 20, 2016, at 1:41 PM, Don Brace wrote:
>
> The hpsa driver uses this function to cleanup inquiry
> data. Our new pqi driver will also use this
> function. This function was copied into both drivers.
>
> This patch exports sanitize_inquiry_string so the hpsa
> and the pqi drivers can use this function directly.
>
> Hannes recommended the change in review:
> https://www.marc.info/?l=linux-scsi&m=144619249003064&w=2
> that using the existing function in scsi_scan would be
> preferrable.
I also made this suggestion when reviewing the same patch:
https://www.marc.info/?l=linux-scsi&m=144613827316617&w=2
Reviewed-by: Matthew R. Ochs
>
> Suggested-by: Hannes Reinecke
> Reviewed-by: Kevin Barnett
> Reviewed-by: Justin Lindley
> Reviewed-by: Scott Teel
> Reviewed-by: Hannes Reinecke
> Signed-off-by: Don Brace
> ---
> drivers/scsi/scsi_scan.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..1f02e84 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -518,7 +518,8 @@ void scsi_target_reap(struct scsi_target *starget)
> }
>
> /**
> - * sanitize_inquiry_string - remove non-graphical chars from an INQUIRY
> result string
> + * scsi_sanitize_inquiry_string - remove non-graphical chars from an
> + *INQUIRY result string
> * @s: INQUIRY result string to sanitize
> * @len: length of the string
> *
> @@ -531,7 +532,7 @@ void scsi_target_reap(struct scsi_target *starget)
> *string terminator, so all the following characters are set to
> *spaces.
> **/
> -static void sanitize_inquiry_string(unsigned char *s, int len)
> +void scsi_sanitize_inquiry_string(unsigned char *s, int len)
> {
> int terminated = 0;
>
> @@ -542,6 +543,7 @@ static void sanitize_inquiry_string(unsigned char *s, int
> len)
> *s = ' ';
> }
> }
> +EXPORT_SYMBOL(scsi_sanitize_inquiry_string);
>
> /**
> * scsi_probe_lun - probe a single LUN using a SCSI INQUIRY
> @@ -627,9 +629,9 @@ static int scsi_probe_lun(struct scsi_device *sdev,
> unsigned char *inq_result,
> }
>
> if (result == 0) {
> - sanitize_inquiry_string(&inq_result[8], 8);
> - sanitize_inquiry_string(&inq_result[16], 16);
> - sanitize_inquiry_string(&inq_result[32], 4);
> + scsi_sanitize_inquiry_string(&inq_result[8], 8);
> + scsi_sanitize_inquiry_string(&inq_result[16], 16);
> + scsi_sanitize_inquiry_string(&inq_result[32], 4);
>
> response_len = inq_result[4] + 5;
> if (response_len > 255)
>
> --
> 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
>
--
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
Re: [PATCH 1/7] hpsa: do not get enclosure info for external devices
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 2/7] hpsa: add SMR drive support
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 5/7] hpsa: check for a null phys_disk pointer in ioaccel2 path
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 4/7] hpsa: correct abort tmf for hba devices
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 3/7] hpsa: correct lun data caching bitmap definition
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 6/7] hpsa: remove function definition for sanitize_inquiry_string
Thanks again for implementing this feedback! Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH 7/7] hpsa: update copyright information
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH] hpsa: update MAINTAINERS with new e-mail
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH] imm: check parport_claim
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH v2] osd: remove deadcode
Reviewed-by: Matthew R. Ochs -- 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
Re: [PATCH v6 19/20] cxlflash: Use new cxl_pci_read_adapter_vpd() API
This patch should have also been sent to the SCSI list (included now).
Changes look fine.
Acked-by: Matthew R. Ochs
> On Mar 4, 2016, at 5:26 AM, Frederic Barrat
> wrote:
>
> To read the adapter VPD, drivers can't rely on pci config APIs, as it
> wouldn't work on powerVM. cxl introduced a new kernel API especially
> for this, so start using it.
>
> Co-authored-by: Christophe Lombard
> Signed-off-by: Frederic Barrat
> Signed-off-by: Christophe Lombard
> ---
> drivers/scsi/cxlflash/common.h | 1 -
> drivers/scsi/cxlflash/main.c | 18 ++
> 2 files changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
> index 5ada926..580f370 100644
> --- a/drivers/scsi/cxlflash/common.h
> +++ b/drivers/scsi/cxlflash/common.h
> @@ -106,7 +106,6 @@ struct cxlflash_cfg {
> atomic_t scan_host_needed;
>
> struct cxl_afu *cxl_afu;
> - struct pci_dev *parent_dev;
>
> atomic_t recovery_threads;
> struct mutex ctx_recovery_mutex;
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index f6d90ce..e04aae7 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -1407,7 +1407,7 @@ static int start_context(struct cxlflash_cfg *cfg)
> */
> static int read_vpd(struct cxlflash_cfg *cfg, u64 wwpn[])
> {
> - struct pci_dev *dev = cfg->parent_dev;
> + struct pci_dev *dev = cfg->dev;
> int rc = 0;
> int ro_start, ro_size, i, j, k;
> ssize_t vpd_size;
> @@ -1416,7 +1416,7 @@ static int read_vpd(struct cxlflash_cfg *cfg, u64
> wwpn[])
> char *wwpn_vpd_tags[NUM_FC_PORTS] = { "V5", "V6" };
>
> /* Get the VPD data from the device */
> - vpd_size = pci_read_vpd(dev, 0, sizeof(vpd_data), vpd_data);
> + vpd_size = cxl_read_adapter_vpd(dev, vpd_data, sizeof(vpd_data));
> if (unlikely(vpd_size <= 0)) {
> dev_err(&dev->dev, "%s: Unable to read VPD (size = %ld)\n",
> __func__, vpd_size);
> @@ -2392,7 +2392,6 @@ static int cxlflash_probe(struct pci_dev *pdev,
> {
> struct Scsi_Host *host;
> struct cxlflash_cfg *cfg = NULL;
> - struct device *phys_dev;
> struct dev_dependent_vals *ddv;
> int rc = 0;
>
> @@ -2458,19 +2457,6 @@ static int cxlflash_probe(struct pci_dev *pdev,
>
> pci_set_drvdata(pdev, cfg);
>
> - /*
> - * Use the special service provided to look up the physical
> - * PCI device, since we are called on the probe of the virtual
> - * PCI host bus (vphb)
> - */
> - phys_dev = cxl_get_phys_dev(pdev);
> - if (!dev_is_pci(phys_dev)) {
> - dev_err(&pdev->dev, "%s: not a pci dev\n", __func__);
> - rc = -ENODEV;
> - goto out_remove;
> - }
> - cfg->parent_dev = to_pci_dev(phys_dev);
> -
> cfg->cxl_afu = cxl_pci_to_afu(pdev);
>
> rc = init_pci(cfg);
> --
> 1.9.1
>
> ___
> Linuxppc-dev mailing list
> linuxppc-...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
--
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
Re: [PATCH 1/7] cxlflash: Simplify PCI registration
> On Mar 4, 2016, at 3:55 PM, Uma Krishnan wrote: > > From: "Manoj N. Kumar" > > The calls to pci_request_regions(), pci_resource_start(), > pci_set_dma_mask(), pci_set_master() and pci_save_state() are all > unnecessary for the IBM CXL flash adapter since data buffers > are not required to be mapped to the device's memory. > > The use of services such as pci_set_dma_mask() are problematic on > hypervisor managed systems as the IBM CXL flash adapter is operating > under a virtual PCI Host Bridge (virtual PHB) which does not support > these services. > > cxlflash 0001:00:00.0: init_pci: Failed to set PCI DMA mask rc=-5 > > The resolution is to simplify init_pci(), to a point where it does the > bare minimum (pci_enable_device). Similarly, remove the call the > pci_release_regions() from cxlflash_remove(). > > Signed-off-by: Manoj N. Kumar Acked-by: Matthew R. Ochs -- 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
Re: [PATCH 2/7] cxlflash: Unmap problem state area before detaching master context
> On Mar 4, 2016, at 3:55 PM, Uma Krishnan wrote: > > When operating in the PowerVM environment, the cxlflash module can > receive an error from the hypervisor indicating that there are > existing mappings in the page table for the process MMIO space. > > This issue exists because term_afu() currently invokes term_mc() > before stop_afu(), allowing for the master context to be detached > first and the problem state area to be unmapped second. > > To resolve this issue, stop_afu() should be called before term_mc(). > > Signed-off-by: Uma Krishnan Acked-by: Matthew R. Ochs -- 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
Re: [PATCH 5/7] cxlflash: Reorder user context initialization
> On Mar 4, 2016, at 3:55 PM, Uma Krishnan wrote: > > In order to support cxlflash in the PowerVM environment, underlying > hypervisor APIs have imposed a kernel API ordering change. > > For the superpipe access to LUN, user applications need a context. > The cxlflash module creates this context by making a sequence of > cxl calls. In the current code, a context is initialized via > cxl_dev_context_init() followed by cxl_process_element(), a function > that obtains the process element id. Finally, cxl_start_work() > is called to attach the process element. > > In the PowerVM environment, a process element id cannot be obtained > from the hypervisor until the process element is attached. The > cxlflash module is unable to create contexts without a valid > process element id. > > To fix this problem, cxl_start_work() is called before obtaining > the process element id. > > Signed-off-by: Uma Krishnan Acked-by: Matthew R. Ochs -- 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
Re: [PATCH 6/7] cxlflash: Fix to avoid unnecessary scan with internal LUNs
> On Mar 4, 2016, at 3:55 PM, Uma Krishnan wrote: > > From: "Manoj N. Kumar" > > When switching to the internal LUN defined on the > IBM CXL flash adapter, there is an unnecessary > scan occurring on the second port. This scan leads > to the following extra lines in the log: > > Dec 17 10:09:00 tul83p1 kernel: [ 3708.561134] cxlflash 0008:00:00.0: > cxlflash_queuecommand: (scp=c000fc1f0f00) 11/1/0/0 > cdb=(A000--1000-) > Dec 17 10:09:00 tul83p1 kernel: [ 3708.561147] process_cmd_err: cmd failed > afu_rc=32 scsi_rc=0 fc_rc=0 afu_extra=0xE, scsi_extra=0x0, fc_extra=0x0 > > By definition, both of the internal LUNs are on the first port/channel. > > When the lun_mode is switched to internal LUN the > same value for host->max_channel is retained. This > causes an unnecessary scan over the second port/channel. > > This fix alters the host->max_channel to 0 (1 port), if internal > LUNs are configured and switches it back to 1 (2 ports) while > going back to external LUNs. > > Signed-off-by: Manoj N. Kumar Acked-by: Matthew R. Ochs -- 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
Re: [PATCH 7/7] cxlflash: Increase cmd_per_lun for better throughput
> On Mar 4, 2016, at 3:55 PM, Uma Krishnan wrote: > > From: "Manoj N. Kumar" > > With the current value of cmd_per_lun at 16, the throughput > over a single adapter is limited to around 150kIOPS. > > Increase the value of cmd_per_lun to 256 to improve > throughput. With this change a single adapter is able to > attain close to the maximum throughput (380kIOPS). > Also change the number of RRQ entries that can be queued. > > Signed-off-by: Manoj N. Kumar Acked-by: Matthew R. Ochs -- 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
Re: [PATCH 1/2] cxlflash: Fix regression issue with re-ordering patch
> On Mar 23, 2016, at 2:50 PM, Uma Krishnan wrote:
>
> From: "Manoj N. Kumar"
>
> While running 'sg_reset -H' back to back the following exception was seen:
>
> [ 735.115695] Faulting instruction address: 0xd98c0864
> cpu 0x0: Vector: 300 (Data Access) at [c00afa80]
>pc: d98c0864: cxlflash_async_err_irq+0x84/0x5c0 [cxlflash]
>lr: c013aed0: handle_irq_event_percpu+0xa0/0x310
>sp: c00afd00
> msr: 90009033
> dar: 201
> dsisr: 4000
> current = 0xc1510880
> paca= 0xcfb8 softe: 0irq_happened: 0x01
>pid = 0, comm = swapper/0
>
> Linux version 4.5.0-491-26f710d+
>
> enter ? for help
> [c00afe10] c013aed0 handle_irq_event_percpu+0xa0/0x310
> [c00afed0] c013b1a8 handle_irq_event+0x68/0xc0
> [c00aff00] c01404ec handle_fasteoi_irq+0xec/0x2a0
> [c00aff30] c013a084 generic_handle_irq+0x54/0x80
> [c00aff60] c0011130 __do_irq+0x80/0x1d0
> [c00aff90] c0024d40 call_do_irq+0x14/0x24
> [c1573a20] c0011318 do_IRQ+0x98/0x140
> [c1573a70] c0002594 hardware_interrupt_common+0x114/0x180
>
> This exception is being hit because the async_err interrupt path performs
> an MMIO to read the interrupt status register. The MMIO region in this
> case is not available.
>
> Commit 6ded8b3cbd9a ("cxlflash: Unmap problem state area before detaching
> master context") re-ordered the sequence in which term_mc() and stop_afu()
> are called. This introduces a window for interrupts to come in with the
> problem space area unmapped, that did not exist previously.
>
> The fix is to separate the disabling of all AFU interrupts to a distinct
> function, term_intr() so that it is the first thing that is done in the
> tear down process.
>
> To keep the initialization process symmetric, separate the AFU interrupt
> setup also to a distinct function: init_intr().
>
> Fixes: 6ded8b3cbd9a ("cxlflash: Unmap problem state area before detaching
> master context")
> Signed-off-by: Manoj N. Kumar
Looks good, thanks for the quick turnaround!
Acked-by: Matthew R. Ochs
--
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
Re: [PATCH 2/2] cxlflash: Move to exponential back-off when cmd_room is not available
> On Mar 23, 2016, at 2:50 PM, Uma Krishnan wrote: > > From: "Manoj N. Kumar" > > While profiling the cxlflash_queuecommand() path under a heavy load it > was found that number of retries to find cmd_room was fairly high. > > There are two problems with the current back-off: > a) It starts with a udelay of 0 > b) It backs-off linearly > > Tried several approaches (a higher multiple 10*n, 100*n, as well as n^2, > 2^n) and found that the exponential back-off(2^n) approach had the least > overall cost. Cost as being defined as overall time spent waiting. > > The fix is to change the linear back-off to an exponential back-off. > This solution also takes care of the problem with the initial > delay (starts with 1 usec). > > Signed-off-by: Manoj N. Kumar Acked-by: Matthew R. Ochs -- 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 RFC 0/2] IBM CXL Flash Adapter
This patch set adds support for the IBM CXL flash (cxlflash) adapter. The device is presented to the system as a SCSI adapter and may be accessed and used via the normal filesystem paths. Also included is support for a special mode that allows direct access from userspace. The IBM Power processor architecture provides support for CAPI (Coherent Accelerator Power Interface), which is available to certain PCIe slots on Power 8 systems. CAPI can be thought of as a special tunneling protocol through PCIe that allow PCIe adapters to look like special purpose co-processors which can read or write an application's memory and generate page faults. As a result, the host interface to an adapter running in CAPI mode does not require data buffers to be mapped to the device's memory (IOMMU bypass) nor does it require memory to be pinned. Application specific accelerators are constructed by burning logic to either an FPGA or ASIC that accelerates a certain function in hardware. This logic is referred to as a an Accelerator Function Unit or AFU. AFUs and their associated software are designed to leverage the benefits that CAPI provides to reduce the burden on CPUs and achieve higher performance. Examples of AFUs include compression, encryption, sorting, etc. The cxlflash adapter contains an AFU that enhances the performance of accessing an external flash storage device by allowing userspace applications to establish a 'superpipe' through which they may directly access the storage, bypassing the traditional storage stack and reducing pathlength per-I/O by more than an order of magnitude. The AFU also supports a translation function that allows users to segment a physical device into 'n' virtual devices [by programmatic means] and refer to these segments as if they were a true physical device. This function enables a more efficient use of a physical device and provides for a secure multi-tenancy environment. At a high-level, the cxlflash adapter looks and behaves very much like a SCSI HBA. Like other SCSI adapters it understands SCSI CDBs and LUN discovery. It also provides health monitoring, error recovery, and link event reporting. It is for these reasons that we have decided to situate support for the cxlflash adapter as a low level SCSI device driver. At a lower level, the cxlflash adapter requires some additional items not found in a traditional SCSI HBA driver. These include the following: - A programmatic API (implemented as ioctls) that user applications interact with when they desire to take advantage of the superpipe access from userspace. These ioctls allow the user to gain access to the CAPI resources (ie: interrupts, MMIO space, etc.) that are required to use the superpipe. Additionally, they allow applications to use the AFUs virtual partitioning function. Note that while the ioctls are new, under the covers they make use of existing functionality found in the cxl driver (drivers/misc/cxl). - A block allocation table (implemented as a bitmap) per physical device attached to the cxlflash adapter that is operating in the virtual partitioned mode. This table manages the segmentation of the physical device and is used to derive the entries found in the LUN mapping table. - A LUN mapping table that is shared with the AFU and used by the AFU to associate the resource handles referring to a specific virtual device with blocks on the physical device. - The ability to send a limited set of SCSI commands directly to the adapter to determine capacity and identification data as well as wipe blocks that are no longer in use when a virtual device is released. This set of commands includes READ_CAPACITY, INQUIRY, and WRITE_SAME. Accompanying this adapter driver but not included here is a userspace library (known as the block library) that will hide the interaction between userspace and the cxlflash driver. Most (if not all) users will chose to use this library when developing superpipe-aware applications. The block library can be found on Github: https://github.com/mikehollinger/ibmcapikv More technical details can be found within Documentation/powerpc/cxlflash.txt The following patches are bisectable and are dependent on support that has not yet been merged in with the mainline. This code may be found on Github: https://github.com/mikey/linux/tree/cxl/kernelapi The forked (from cxl-kernelapi) cxlflash source may also be found on Github: https://github.com/mikehollinger/linux/tree/cflash/dev Patch 1 contains support for only filesystem access. Patch 2 adds support for userspace applications that wish to communicate using the superpipe in lieu of filesystem access. Any thoughts on our approach for supporing this adapter/function? Matthew R. Ochs (2): cxlflash: Base support for IBM CXL Flash Adapter cxlflash: Support for superpipe I/O API Documentation/powerpc/cxlflash.txt | 275 + drivers/scsi/Kconfig |
[PATCH RFC 2/2] cxlflash: Support for superpipe I/O API
Allow userspace applications to obtain CXL resources and exploit the
superpipe functionality of the IBM CXL Flash adapter.
Signed-off-by: Matthew R. Ochs
Signed-off-by: Manoj N. Kumar
---
drivers/scsi/cxlflash/Makefile |2 +-
drivers/scsi/cxlflash/main.c |1 +
drivers/scsi/cxlflash/main.h |1 +
drivers/scsi/cxlflash/superpipe.c | 1673
drivers/scsi/cxlflash/superpipe.h | 67 ++
include/uapi/scsi/Kbuild |1 +
include/uapi/scsi/cxlflash_ioctl.h | 133 +++
7 files changed, 1877 insertions(+), 1 deletion(-)
create mode 100644 drivers/scsi/cxlflash/superpipe.c
create mode 100644 drivers/scsi/cxlflash/superpipe.h
create mode 100644 include/uapi/scsi/cxlflash_ioctl.h
diff --git a/drivers/scsi/cxlflash/Makefile b/drivers/scsi/cxlflash/Makefile
index 90e9382..5ddcae4 100644
--- a/drivers/scsi/cxlflash/Makefile
+++ b/drivers/scsi/cxlflash/Makefile
@@ -1,4 +1,4 @@
obj-$(CONFIG_CXLFLASH) += cxlflash.o
-cxlflash-y += main.o
+cxlflash-y += main.o superpipe.o
ccflags-y += -DCONFIG_PRINTK
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 2533ecb..4612ccd 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -601,6 +601,7 @@ static struct scsi_host_template driver_template = {
.module = THIS_MODULE,
.name = CXLFLASH_ADAPTER_NAME,
.info = cxlflash_driver_info,
+ .ioctl = cxlflash_ioctl,
.proc_name = CXLFLASH_NAME,
.queuecommand = cxlflash_queuecommand,
.eh_device_reset_handler = cxlflash_eh_device_reset_handler,
diff --git a/drivers/scsi/cxlflash/main.h b/drivers/scsi/cxlflash/main.h
index b79d4f3..67332f1 100644
--- a/drivers/scsi/cxlflash/main.h
+++ b/drivers/scsi/cxlflash/main.h
@@ -120,5 +120,6 @@ static inline u64 lun_to_lunid(u64 lun)
/*
* Externs and Prototypes
*/
+int cxlflash_ioctl(struct scsi_device *, int, void __user *);
#endif /* _CXLFLASH_MAIN_H */
diff --git a/drivers/scsi/cxlflash/superpipe.c
b/drivers/scsi/cxlflash/superpipe.c
new file mode 100644
index 000..9cc7666
--- /dev/null
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -0,0 +1,1673 @@
+/*
+ * CXL Flash Device Driver
+ *
+ * Written by: Manoj N. Kumar , IBM Corporation
+ * Matthew R. Ochs , IBM Corporation
+ *
+ * Copyright (C) 2015 IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+#include "sislite.h"
+#include "common.h"
+#include "superpipe.h"
+
+static void marshall_virt_to_resize(struct dk_cxlflash_uvirtual *virt,
+ struct dk_cxlflash_resize *resize)
+{
+ resize->hdr = virt->hdr;
+ resize->context_id = virt->context_id;
+ resize->rsrc_handle = virt->rsrc_handle;
+ resize->req_size = virt->lun_size;
+ resize->last_lba = virt->last_lba;
+}
+
+static void marshall_rele_to_resize(struct dk_cxlflash_release *release,
+ struct dk_cxlflash_resize *resize)
+{
+ resize->hdr = release->hdr;
+ resize->context_id = release->context_id;
+ resize->rsrc_handle = release->rsrc_handle;
+}
+
+static void marshall_det_to_rele(struct dk_cxlflash_detach *detach,
+struct dk_cxlflash_release *release)
+{
+ release->hdr = detach->hdr;
+ release->context_id = detach->context_id;
+}
+
+static void marshall_clone_to_rele(struct dk_cxlflash_clone *clone,
+ struct dk_cxlflash_release *release)
+{
+ release->hdr = clone->hdr;
+ release->context_id = clone->context_id_dst;
+}
+
+static int ba_init(struct ba_lun *ba_lun)
+{
+ struct ba_lun_info *lun_info = NULL;
+ int lun_size_au = 0, i = 0;
+ int last_word_underflow = 0;
+ u64 *lam;
+
+ cxlflash_info("Initializing LUN: lun_id = %llX, "
+ "ba_lun->lsize = %lX, ba_lun->au_size = %lX",
+ ba_lun->lun_id, ba_lun->lsize, ba_lun->au_size);
+
+ /* Calculate bit map size */
+ lun_size_au = ba_lun->lsize / ba_lun->au_size;
+ if (lun_size_au == 0) {
+ cxlflash_err("Requested LUN size of 0!");
+ return -EINVAL;
+ }
+
+ /* Allocate lun_fino */
+ lun_info = kzalloc(sizeof(struct ba_lun_info), GFP_KERNEL);
+ if (unlikely(!lun_info)) {
+ cxlflash_err("Failed to allocate lun_info for lun_id %llX",
+ba_lun->lun_id);
+ return -ENOMEM;
+ }
+
+ lun_info->total_aus =
[PATCH RFC 1/2] cxlflash: Base support for IBM CXL Flash Adapter
SCSI device driver to support filesystem access on the IBM CXL Flash adapter. Signed-off-by: Matthew R. Ochs Signed-off-by: Manoj N. Kumar --- Documentation/powerpc/cxlflash.txt | 275 + drivers/scsi/Kconfig |1 + drivers/scsi/Makefile |1 + drivers/scsi/cxlflash/Kconfig | 11 + drivers/scsi/cxlflash/Makefile |4 + drivers/scsi/cxlflash/common.h | 287 + drivers/scsi/cxlflash/main.c | 2081 drivers/scsi/cxlflash/main.h | 124 +++ drivers/scsi/cxlflash/sislite.h| 407 +++ 9 files changed, 3191 insertions(+) create mode 100644 Documentation/powerpc/cxlflash.txt create mode 100644 drivers/scsi/cxlflash/Kconfig create mode 100644 drivers/scsi/cxlflash/Makefile create mode 100644 drivers/scsi/cxlflash/common.h create mode 100644 drivers/scsi/cxlflash/main.c create mode 100644 drivers/scsi/cxlflash/main.h create mode 100755 drivers/scsi/cxlflash/sislite.h diff --git a/Documentation/powerpc/cxlflash.txt b/Documentation/powerpc/cxlflash.txt new file mode 100644 index 000..c31d13e --- /dev/null +++ b/Documentation/powerpc/cxlflash.txt @@ -0,0 +1,275 @@ +Introduction + + +The IBM Power architecture provides support for CAPI (Coherent +Accelerator Power Interface), which is available to certain PCIe +slots on Power 8 systems. CAPI can be thought of as a special +tunneling protocol through PCIe that allow PCIe adapters to look +like special purpose co-processors which can read or write an +application's memory and generate page faults. As a result, the +host interface to an adapter running in CAPI mode do not require +the data buffers to be mapped to the device's memory (IOMMU bypass) +nor does it require memory to be pinned. + +On Linux, Coherent Accelerator (CXL) kernel services present CAPI +devices as a PCI device by implementing a virtual PCI host bridge. +This abstraction simplifies the infrastructure and programming model, +allowing for drivers to look similar to other native PCI device drivers. + +CXL provides a mechanism by which user space applications can +directly talk to a device (network or storage) bypassing the +typical kernel/device driver stack. The CXL Flash Adapter Driver +enables a user space application direct access to Flash storage. + +The CXL Flash Adapter Driver is a kernel module that sits in +the SCSI stack as a low level device driver (below the SCSI disk +and protocol drivers) for the IBM CXL Flash Adapter. This driver +is responsible for the initialization of the adapter, setting up +the special path for user space access, and performing error +recovery. It communicates directly the Flash Accelerator Functional +Unit (AFU) as described in Documentation/powerpc/cxl.txt. + +The cxlflash driver supports two, mutually exclusive, modes of +operation at the device (LUN) level: + +- Any flash device (LUN) can be configured to be accessed as a + regular disk device (ie: /dev/sdc). This is the default mode. + +- Any flash device (LUN) can be configured to be accessed from + user space with a special block library. This mode further + specifies the means of accessing the device and provides for + either raw access to the entire LUN (referred to as direct or + physical LUN access) or access to a kernel/AFU-mediated partition + of the LUN (referred to as virtual LUN access). The segmentation + of a disk device into virtual LUNs is assisted by special + translation services provided by the Flash AFU. + +Overview + + +The Coherent Accelerator Interface Architecture (CAIA) introduces +a concept of a master context. A master typically has special +privileges granted to it by the kernel or hypervisor allowing +it to perform AFU wide management and control. The master may or +may not be involved directly in each user I/O, but at the minimum +is involved in the initial setup before the user application is +allowed to send requests directly to the AFU. + +The CXL Flash Adapter Driver establishes a master context with +the AFU. It uses memory mapped I/O (MMIO) for this control and +setup. The Adapter Problem Space Memory Map looks like this: + + +---+ + |512 * 64 KB User MMIO | + |(per context) | + | User Accessible | + +---+ + |512 * 128 B per context| + |Provisioning and Control | + | Trusted Process accessible | + +---+ + | 64
Re: [PATCH 1/1] cxlflash: Base support for IBM CXL Flash Adapter
Mikey,
Thanks for reviewing this patch. Responses are inline below.
-matt
On May 20, 2015, at 12:51 AM, Michael Neuling wrote:
> Does this driver work when compiled big endian?
We haven't tested on big endian but agree that the code should support both
little
and big endian.
>>
>> +#define CXLFLASH_MAX_CMDS 16
>> +#define CXLFLASH_MAX_CMDS_PER_LUN CXLFLASH_MAX_CMDS
>> +
>> +#define NOT_POW2(_x) ((_x) & ((_x) & ((_x) - 1)))
>
> include/linux/log2 has is_power_of_2()
>
This is used for compile-time enforcement. The items in the log2 header are
runtime
functions. Will add a comment for clarity.
>> +#if NOT_POW2(CXLFLASH_NUM_CMDS)
>> +#error "CXLFLASH_NUM_CMDS is not a power of 2!"
>> +#endif
>> +
>> +#define CMD_BUFSIZE PAGE_SIZE_4K
>
> Does this mean we can't compile with 64K pages? How is this related to PAGES?
>
No, 64K pages are fully supported. We are simply borrowing the 4K page #define
as
the size of the AFU's command buffer is 4096 bytes. Will add a comment for
clarity.
>> +#define CONFN(_s) "%s: "_s"\n"
>> +#define cxlflash_err(_s, ...) pr_err(CONFN(_s), __func__,
>> ##__VA_ARGS__)
>> +#define cxlflash_warn(_s, ...) pr_warn(CONFN(_s), __func__,
>> ##__VA_ARGS__)
>> +#define cxlflash_info(_s, ...) pr_info(CONFN(_s), __func__,
>> ##__VA_ARGS__)
>> +#define cxlflash_dbg(_s, ...) pr_debug(CONFN(_s), __func__,
>> ##__VA_ARGS__)
>
> Please don't redefine these. Just makes it less readable for others.
>
Okay.
>> +#define cxlflash_dev_dbg(_d, _s, ...) \
>> +dev_dbg(_d, CONFN(_s), __func__, ##__VA_ARGS__)
>> +
>
> Same here...
Okay.
>>
>> +u32 padding;
>> +struct cxl_context *ctx;
>> +struct list_head luns; /* LUNs attached to this context */
>> +};
>
> Might be nice to pack these to ensure there is no holes. I'm not sure that's
> the case currently.
We'll look into this.
>
>> +
>> +struct cxlflash {
>
> I really don't like this name. What does this struct actually represent? Is
> it just some table, info, queue? Calling it just "cxlflash" doesn't give the
> reader any idea what it is. Plus it makes it really hard to search for since
> "cxlflash" is being used a lot in this driver for other stuff and you often do
> struct cxlflash *cxlflash;
>
> If you used something like
> struct cxlflash_desc {};
> struct cxlflash_desc *desc;
> I could search for desc and find all the uses.
>
Sounds reasonable. This is our per-device handle. How about the name
struct cxlflash_cfg and we locally call the variable 'cfg'?
>>
>> +
>> +struct pci_pool *cxlflash_cmd_pool;
>> +struct pci_dev *parent_dev;
>> +
>> +int num_user_contexts;
>
> You init this to 0 but never use it.
>
>> +struct ctx_info *ctx_info[MAX_CONTEXT];
>> +struct file_operations cxl_fops;
>
> This is unused in this patch.
>
These are used in other areas of the driver not included in this patch. Will
look
at not including them in the next patch with updates based on feedback.
>> +char *buf; /* per command buffer */
>> +struct afu *back;
>
> Can we call this parent, rather than back?
>
Sure.
>> +int slot;
>> +atomic_t free;
>
> Looks like you're just doing ref counting with this so it should probably be a
> struct kref.
>
We'll leave this as an atomic for now to support our round-robin allocation.
>> +u8 special:1;
>> +u8 internal:1;
>> +u8 sync:1;
>> +
>> +} __aligned(cache_line_size());
>
> Why is this cacheline aigned?
The IOASA and IOARCB sit at the top of this structure. We keep this aligned on
a cacheline so that these two sit in the same cacheline for better performance.
>> +/* Housekeeping data */
>> +struct mutex afu_mutex; /* for anything that needs serialization
>
> This is never used
>
>> + e. g. to access afu */
>> +struct mutex err_mutex; /* for signalling error thread */
>
> This is never used
>
Will remove these with other unused fields.
>> +
>> +struct cxlflash *back; /* Pointer back to parent cxlflash */
>
> Can we just call this parent?
Sure.
>
>> +
>> +} __aligned(PAGE_SIZE_4K);
>
> Why 4K aligned? What has this got to do with page size?
We have removed this.
>> +while (dec--) {
>> +k = (afu->cmd_couts++ & (CXLFLASH_NUM_CMDS - 1));
>> +
>> +cmd = &afu->cmd[k];
>> +
>> +if (!atomic_dec_if_positive(&cmd->free)) {
>
> I guess this is ok with only 32 commands but it's a linear search. I would be
> nice to use a standard allocator here instead. I think you could use ida for
> this and remove the need for cmd->free. Probably not a bit issue though.
We're going to continue to use this simple round-robin search. The command pool
is larger than the hardware queue depth and therefore we should rarely hit a
case
where we're looping more than a time or two here to find a free command.
>> +
>> +spin_lock_irqsave(&cmd->slock, lock_flags);
>> +
[PATCH v2] IBM CXL Flash Adapter
v2 Changes: - Update to candidate upstream CXL APIs - Add several clarification comments - Renamed some structures for clarity - Removed the 'cxlflash' prefix from internal functions - Address endianness concerns/issues - Remove unused code - Revert to using pr_*/dev_* print services - Remove unneeded locking and barrier instructions This patch adds support for the IBM CXL flash (cxlflash) adapter. The device is presented to the system as a SCSI adapter and may be accessed and used via the normal filesystem paths. Note that the patch is dependent on support that has not yet been merged in with the mainline (submission expected shortly). In the interim, this code may be found on Github: https://github.com/mikey/linux/tree/cxl/kernelapi Matthew R. Ochs (1): cxlflash: Base support for IBM CXL Flash Adapter drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/cxlflash/Kconfig | 11 + drivers/scsi/cxlflash/Makefile |2 + drivers/scsi/cxlflash/common.h | 180 drivers/scsi/cxlflash/main.c| 2242 +++ drivers/scsi/cxlflash/main.h| 111 ++ drivers/scsi/cxlflash/sislite.h | 465 8 files changed, 3013 insertions(+) create mode 100644 drivers/scsi/cxlflash/Kconfig create mode 100644 drivers/scsi/cxlflash/Makefile create mode 100644 drivers/scsi/cxlflash/common.h create mode 100644 drivers/scsi/cxlflash/main.c create mode 100644 drivers/scsi/cxlflash/main.h create mode 100755 drivers/scsi/cxlflash/sislite.h -- 2.1.0 -- 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
