[PATCH 0/3] cxlflash: Shutdown notification and reset patch

[PATCH 0/3] cxlflash: Shutdown notification and reset patch

[Uma Krishnan]

This patch set contains support to notify CXL Flash devices of an
impending shutdown and a fix to drain operations prior to a reset.

This series is intended for 4.8 and is bisectable.

Manoj N. Kumar (1):
  cxlflash: Fix to drain operations from previous reset

Uma Krishnan (2):
  cxlflash: Add device dependent flags
  cxlflash: Shutdown notify support for CXL Flash cards

 drivers/scsi/cxlflash/main.c    | 98 ++++++++++++++++++++++++++++++++++-------
 drivers/scsi/cxlflash/main.h    |  2 +
 drivers/scsi/cxlflash/sislite.h |  6 +++
 3 files changed, 91 insertions(+), 15 deletions(-)

-- 
2.1.0

[PATCH 1/3] cxlflash: Fix to drain operations from previous reset

[Uma Krishnan]

From: "Manoj N. Kumar" <manoj@linux.vnet.ibm.com>

While running 'sg_reset -H' in a loop with a user-space application active,
hit the following exception:

cpu 0x2: Vector: 300 (Data Access)
    pc: : afu_attach+0x50/0x240 [cxlflash]
    lr: : cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
    pid   = 20365, comm = run_block_fvt

Linux version 4.5.0-491-26f710d+

cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
cxlflash_ioctl+0x5a8/0x6f0 [cxlflash]
scsi_ioctl+0x3b0/0x4c0
sd_ioctl+0x110/0x190
blkdev_ioctl+0x28c/0xc20
block_ioctl+0xa4/0xd0
do_vfs_ioctl+0xd8/0x8c0
SyS_ioctl+0xd4/0xf0
system_call+0x38/0xb4

The problem here is that the problem space area is unmapped while the
application issues the DK_CXLFLASH_RECOVER_AFU ioctl.

This is the order I observe:

proc1				proc2
1) sg_reset
				2) ioctl(DK_CXLFLASH_RECOVER_AFU)
3) sg_reset again
   causing a PSA unmap
				4) continues RECOVER_AFU processing

The resolution to this problem is to have the reset handler drain all
outstanding user space initiated ioctls before proceeding.  It is safe
to drain after the state has been changed to STATE_RESET. Also since
drain_ioctls() was static, it had to be moved up a bit to be before
cxlflash_eh_host_reset_handler().

Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 8fb9643..f1f977f 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1916,6 +1916,19 @@ static int afu_reset(struct cxlflash_cfg *cfg)
 }
 
 /**
+ * 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_eh_device_reset_handler() - reset a single LUN
  * @scp:	SCSI command to send.
  *
@@ -1986,6 +1999,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)
 	switch (cfg->state) {
 	case STATE_NORMAL:
 		cfg->state = STATE_RESET;
+		drain_ioctls(cfg);
 		cxlflash_mark_contexts_error(cfg);
 		rcr = afu_reset(cfg);
 		if (rcr) {
@@ -2504,19 +2518,6 @@ 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.
-- 
2.1.0

Re: [PATCH 1/3] cxlflash: Fix to drain operations from previous reset

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> wrote:
> 
> From: "Manoj N. Kumar" <manoj@linux.vnet.ibm.com>
> 
> While running 'sg_reset -H' in a loop with a user-space application active,
> hit the following exception:
> 
> cpu 0x2: Vector: 300 (Data Access)
>    pc: : afu_attach+0x50/0x240 [cxlflash]
>    lr: : cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
>    pid   = 20365, comm = run_block_fvt
> 
> Linux version 4.5.0-491-26f710d+
> 
> cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
> cxlflash_ioctl+0x5a8/0x6f0 [cxlflash]
> scsi_ioctl+0x3b0/0x4c0
> sd_ioctl+0x110/0x190
> blkdev_ioctl+0x28c/0xc20
> block_ioctl+0xa4/0xd0
> do_vfs_ioctl+0xd8/0x8c0
> SyS_ioctl+0xd4/0xf0
> system_call+0x38/0xb4
> 
> The problem here is that the problem space area is unmapped while the
> application issues the DK_CXLFLASH_RECOVER_AFU ioctl.
> 
> This is the order I observe:
> 
> proc1				proc2
> 1) sg_reset
> 				2) ioctl(DK_CXLFLASH_RECOVER_AFU)
> 3) sg_reset again
>   causing a PSA unmap
> 				4) continues RECOVER_AFU processing
> 
> The resolution to this problem is to have the reset handler drain all
> outstanding user space initiated ioctls before proceeding.  It is safe
> to drain after the state has been changed to STATE_RESET. Also since
> drain_ioctls() was static, it had to be moved up a bit to be before
> cxlflash_eh_host_reset_handler().
> 
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH 1/3] cxlflash: Fix to drain operations from previous reset

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> =
wrote:
>=20
> From: "Manoj N. Kumar" <manoj@linux.vnet.ibm.com>
>=20
> While running 'sg_reset -H' in a loop with a user-space application =
active,
> hit the following exception:
>=20
> cpu 0x2: Vector: 300 (Data Access)
>    pc: : afu_attach+0x50/0x240 [cxlflash]
>    lr: : cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
>    pid   =3D 20365, comm =3D run_block_fvt
>=20
> Linux version 4.5.0-491-26f710d+
>=20
> cxlflash_afu_recover+0x3dc/0x7d0 [cxlflash]
> cxlflash_ioctl+0x5a8/0x6f0 [cxlflash]
> scsi_ioctl+0x3b0/0x4c0
> sd_ioctl+0x110/0x190
> blkdev_ioctl+0x28c/0xc20
> block_ioctl+0xa4/0xd0
> do_vfs_ioctl+0xd8/0x8c0
> SyS_ioctl+0xd4/0xf0
> system_call+0x38/0xb4
>=20
> The problem here is that the problem space area is unmapped while the
> application issues the DK_CXLFLASH_RECOVER_AFU ioctl.
>=20
> This is the order I observe:
>=20
> proc1				proc2
> 1) sg_reset
> 				2) ioctl(DK_CXLFLASH_RECOVER_AFU)
> 3) sg_reset again
>   causing a PSA unmap
> 				4) continues RECOVER_AFU processing
>=20
> The resolution to this problem is to have the reset handler drain all
> outstanding user space initiated ioctls before proceeding.  It is safe
> to drain after the state has been changed to STATE_RESET. Also since
> drain_ioctls() was static, it had to be moved up a bit to be before
> cxlflash_eh_host_reset_handler().
>=20
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

[PATCH 2/3] cxlflash: Add device dependent flags

[Uma Krishnan]

Device dependent flags are needed to support functions that are
specific to a particular device.

One such case is - some CXL Flash cards need to be notified of
device shutdown. For other CXL devices, this feature does not prove
to be useful yet. Such distinct features need to be identified in
the driver to bypass or invoke specific functionality.

In this patch, a member 'flags' has been added to device dependent
values. These flags will be used and expanded in the future to
support various device specific functions.

Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 6 ++++--
 drivers/scsi/cxlflash/main.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index f1f977f..e027fa0 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2333,8 +2333,10 @@ static struct scsi_host_template driver_template = {
 /*
  * Device dependent values
  */
-static struct dev_dependent_vals dev_corsa_vals = { CXLFLASH_MAX_SECTORS };
-static struct dev_dependent_vals dev_flash_gt_vals = { CXLFLASH_MAX_SECTORS };
+static struct dev_dependent_vals dev_corsa_vals = { CXLFLASH_MAX_SECTORS,
+					0ULL };
+static struct dev_dependent_vals dev_flash_gt_vals = { CXLFLASH_MAX_SECTORS,
+					0ULL };
 
 /*
  * PCI device binding table
diff --git a/drivers/scsi/cxlflash/main.h b/drivers/scsi/cxlflash/main.h
index eb9d8f7..029f517 100644
--- a/drivers/scsi/cxlflash/main.h
+++ b/drivers/scsi/cxlflash/main.h
@@ -88,6 +88,7 @@ enum undo_level {
 
 struct dev_dependent_vals {
 	u64 max_sectors;
+	u64 flags;
 };
 
 struct asyc_intr_info {
-- 
2.1.0

Re: [PATCH 2/3] cxlflash: Add device dependent flags

[Manoj Kumar]

On 6/15/2016 6:49 PM, Uma Krishnan wrote:
> Device dependent flags are needed to support functions that are
> specific to a particular device.
>


Acked-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>

Re: [PATCH 2/3] cxlflash: Add device dependent flags

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> wrote:
> 
> Device dependent flags are needed to support functions that are
> specific to a particular device.
> 
> One such case is - some CXL Flash cards need to be notified of
> device shutdown. For other CXL devices, this feature does not prove
> to be useful yet. Such distinct features need to be identified in
> the driver to bypass or invoke specific functionality.
> 
> In this patch, a member 'flags' has been added to device dependent
> values. These flags will be used and expanded in the future to
> support various device specific functions.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH 2/3] cxlflash: Add device dependent flags

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> =
wrote:
>=20
> Device dependent flags are needed to support functions that are
> specific to a particular device.
>=20
> One such case is - some CXL Flash cards need to be notified of
> device shutdown. For other CXL devices, this feature does not prove
> to be useful yet. Such distinct features need to be identified in
> the driver to bypass or invoke specific functionality.
>=20
> In this patch, a member 'flags' has been added to device dependent
> values. These flags will be used and expanded in the future to
> support various device specific functions.
>=20
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

[PATCH 3/3] cxlflash: Shutdown notify support for CXL Flash cards

[Uma Krishnan]

Some CXL Flash cards need notification of device shutdown
in order to flush pending I/Os.

A PCI notification hook for shutdown has been added where
the driver notifies the card and returns. When the device
is removed in the PCI remove path, notification code will
wait for shutdown processing to complete.

Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c    | 67 ++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/cxlflash/main.h    |  1 +
 drivers/scsi/cxlflash/sislite.h |  6 ++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index e027fa0..860008d 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -765,6 +765,67 @@ static void term_afu(struct cxlflash_cfg *cfg)
 }
 
 /**
+ * notify_shutdown() - notifies device of pending shutdown
+ * @cfg:	Internal structure associated with the host.
+ * @wait:	Whether to wait for shutdown processing to complete.
+ *
+ * This function will notify the AFU that the adapter is being shutdown
+ * and will wait for shutdown processing to complete if wait is true.
+ * This notification should flush pending I/Os to the device and halt
+ * further I/Os until the next AFU reset is issued and device restarted.
+ */
+static void notify_shutdown(struct cxlflash_cfg *cfg, bool wait)
+{
+	struct afu *afu = cfg->afu;
+	struct device *dev = &cfg->dev->dev;
+	struct sisl_global_map __iomem *global = &afu->afu_map->global;
+	struct dev_dependent_vals *ddv;
+	u64 reg, status;
+	int i, retry_cnt = 0;
+
+	ddv = (struct dev_dependent_vals *)cfg->dev_id->driver_data;
+	if (!(ddv->flags & CXLFLASH_NOTIFY_SHUTDOWN))
+		return;
+
+	/* Notify AFU */
+	for (i = 0; i < NUM_FC_PORTS; i++) {
+		reg = readq_be(&global->fc_regs[i][FC_CONFIG2 / 8]);
+		reg |= SISL_FC_SHUTDOWN_NORMAL;
+		writeq_be(reg, &global->fc_regs[i][FC_CONFIG2 / 8]);
+	}
+
+	if (!wait)
+		return;
+
+	/* Wait up to 1.5 seconds for shutdown processing to complete */
+	for (i = 0; i < NUM_FC_PORTS; i++) {
+		retry_cnt = 0;
+		while (true) {
+			status = readq_be(&global->fc_regs[i][FC_STATUS / 8]);
+			if (status & SISL_STATUS_SHUTDOWN_COMPLETE)
+				break;
+			if (++retry_cnt >= MC_RETRY_CNT) {
+				dev_dbg(dev, "%s: port %d shutdown processing "
+					"not yet completed\n", __func__, i);
+				break;
+			}
+			msleep(100 * retry_cnt);
+		}
+	}
+}
+
+/**
+ * cxlflash_shutdown() - shutdown handler
+ * @pdev:	PCI device associated with the host.
+ */
+static void cxlflash_shutdown(struct pci_dev *pdev)
+{
+	struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
+
+	notify_shutdown(cfg, false);
+}
+
+/**
  * cxlflash_remove() - PCI entry point to tear down host
  * @pdev:	PCI device associated with the host.
  *
@@ -785,6 +846,9 @@ static void cxlflash_remove(struct pci_dev *pdev)
 						  cfg->tmf_slock);
 	spin_unlock_irqrestore(&cfg->tmf_slock, lock_flags);
 
+	/* Notify AFU and wait for shutdown processing to complete */
+	notify_shutdown(cfg, true);
+
 	cfg->state = STATE_FAILTERM;
 	cxlflash_stop_term_user_contexts(cfg);
 
@@ -2336,7 +2400,7 @@ static struct scsi_host_template driver_template = {
 static struct dev_dependent_vals dev_corsa_vals = { CXLFLASH_MAX_SECTORS,
 					0ULL };
 static struct dev_dependent_vals dev_flash_gt_vals = { CXLFLASH_MAX_SECTORS,
-					0ULL };
+					CXLFLASH_NOTIFY_SHUTDOWN };
 
 /*
  * PCI device binding table
@@ -2613,6 +2677,7 @@ static struct pci_driver cxlflash_driver = {
 	.id_table = cxlflash_pci_table,
 	.probe = cxlflash_probe,
 	.remove = cxlflash_remove,
+	.shutdown = cxlflash_shutdown,
 	.err_handler = &cxlflash_err_handler,
 };
 
diff --git a/drivers/scsi/cxlflash/main.h b/drivers/scsi/cxlflash/main.h
index 029f517..f54bbd5 100644
--- a/drivers/scsi/cxlflash/main.h
+++ b/drivers/scsi/cxlflash/main.h
@@ -89,6 +89,7 @@ enum undo_level {
 struct dev_dependent_vals {
 	u64 max_sectors;
 	u64 flags;
+#define CXLFLASH_NOTIFY_SHUTDOWN   0x0000000000000001ULL
 };
 
 struct asyc_intr_info {
diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h
index 0b3366f..347fc16 100644
--- a/drivers/scsi/cxlflash/sislite.h
+++ b/drivers/scsi/cxlflash/sislite.h
@@ -311,6 +311,12 @@ struct sisl_global_regs {
 #define SISL_FC_INTERNAL_MASK	~(SISL_FC_INTERNAL_UNMASK)
 #define SISL_FC_INTERNAL_SHIFT	32
 
+#define SISL_FC_SHUTDOWN_NORMAL		0x0000000000000010ULL
+#define SISL_FC_SHUTDOWN_ABRUPT		0x0000000000000020ULL
+
+#define SISL_STATUS_SHUTDOWN_ACTIVE	0x0000000000000010ULL
+#define SISL_STATUS_SHUTDOWN_COMPLETE	0x0000000000000020ULL
+
 #define SISL_ASTATUS_UNMASK	0xFFFFULL		/* 1 means unmasked */
 #define SISL_ASTATUS_MASK	~(SISL_ASTATUS_UNMASK)	/* 1 means masked */
 
-- 
2.1.0

Re: [PATCH 3/3] cxlflash: Shutdown notify support for CXL Flash cards

[Manoj Kumar]

On 6/15/2016 6:49 PM, Uma Krishnan wrote:
> Some CXL Flash cards need notification of device shutdown
> in order to flush pending I/Os.
>
> A PCI notification hook for shutdown has been added where
> the driver notifies the card and returns. When the device
> is removed in the PCI remove path, notification code will
> wait for shutdown processing to complete.
>
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Acked-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>

Re: [PATCH 3/3] cxlflash: Shutdown notify support for CXL Flash cards

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> wrote:
> 
> Some CXL Flash cards need notification of device shutdown
> in order to flush pending I/Os.
> 
> A PCI notification hook for shutdown has been added where
> the driver notifies the card and returns. When the device
> is removed in the PCI remove path, notification code will
> wait for shutdown processing to complete.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH 3/3] cxlflash: Shutdown notify support for CXL Flash cards

[Matthew R. Ochs]

> On Jun 15, 2016, at 6:49 PM, Uma Krishnan <ukrishn@linux.vnet.ibm.com> =
wrote:
>=20
> Some CXL Flash cards need notification of device shutdown
> in order to flush pending I/Os.
>=20
> A PCI notification hook for shutdown has been added where
> the driver notifies the card and returns. When the device
> is removed in the PCI remove path, notification code will
> wait for shutdown processing to complete.
>=20
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>

Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>

Re: [PATCH 0/3] cxlflash: Shutdown notification and reset patch

[Martin K. Petersen]

>>>>> "Uma" == Uma Krishnan <ukrishn@linux.vnet.ibm.com> writes:

Uma> This patch set contains support to notify CXL Flash devices of an
Uma> impending shutdown and a fix to drain operations prior to a reset.

Uma> This series is intended for 4.8 and is bisectable.

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 0/3] cxlflash: Shutdown notification and reset patch

[Martin K. Petersen]

>>>>> "Uma" == Uma Krishnan <ukrishn@linux.vnet.ibm.com> writes:

Uma> This patch set contains support to notify CXL Flash devices of an
Uma> impending shutdown and a fix to drain operations prior to a reset.

Uma> This series is intended for 4.8 and is bisectable.

Applied to 4.8/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering