2011-05-03 20:03:37

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 00/16] cciss: May 3, 2011 updates

The following series mostly contains fixes to improve kdump behavior
and esp. to make older controllers which cannot be hard reset work by
doing a soft reset instead. There are a few patches factoring out
various functionality into individual functions make way for the soft
reset functionality. There is also a bugfix which prevents PCI write
combining from potentially causing commands to get lost.

---

Stephen M. Cameron (16):
cciss: add readl after writel in interrupt mask setting code
cciss: do a better job of detecting controller reset failure
cciss: factor out command pool allocation functions
cciss: factor out scatterlist allocation functions
cciss: factor out irq request code
cciss: fix reply pool and block fetch table memory leaks
cciss: get rid of message related magic numbers
cciss: increase time to wait for board reset to start
cciss: clarify messages around reset behavior
cciss: increase timeouts for post-reset no-ops
cciss: use new doorbell-bit-5 reset method
cciss: do soft reset if hard reset is broken
cciss: remove superfluous sleeps around reset code
cciss: do not attempt PCI power management reset method if we know it won't work.
cciss: do not use bit 2 doorbell reset
cciss: add cciss_tape_cmds module paramter


Documentation/blockdev/cciss.txt | 15 +
drivers/block/cciss.c | 571 ++++++++++++++++++++++++++++++--------
drivers/block/cciss.h | 11 +
drivers/block/cciss_cmd.h | 11 +
drivers/block/cciss_scsi.c | 41 ++-
drivers/block/cciss_scsi.h | 4
6 files changed, 515 insertions(+), 138 deletions(-)

--
-- steve


2011-05-03 19:52:57

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 01/16] cciss: add readl after writel in interrupt mask setting code

From: Stephen M. Cameron <[email protected]>

This is to ensure the board interrupts are really off when
these functions return.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.h | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index 554bbd9..9b49439 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -239,11 +239,13 @@ static void SA5_intr_mask(ctlr_info_t *h, unsigned long val)
{ /* Turn interrupts on */
h->interrupts_enabled = 1;
writel(0, h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
} else /* Turn them off */
{
h->interrupts_enabled = 0;
writel( SA5_INTR_OFF,
h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
}
}
/*
@@ -257,11 +259,13 @@ static void SA5B_intr_mask(ctlr_info_t *h, unsigned long val)
{ /* Turn interrupts on */
h->interrupts_enabled = 1;
writel(0, h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
} else /* Turn them off */
{
h->interrupts_enabled = 0;
writel( SA5B_INTR_OFF,
h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
}
}

@@ -271,10 +275,12 @@ static void SA5_performant_intr_mask(ctlr_info_t *h, unsigned long val)
if (val) { /* turn on interrupts */
h->interrupts_enabled = 1;
writel(0, h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
} else {
h->interrupts_enabled = 0;
writel(SA5_PERF_INTR_OFF,
h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
+ (void) readl(h->vaddr + SA5_REPLY_INTR_MASK_OFFSET);
}
}

2011-05-03 19:53:04

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 02/16] cciss: do a better job of detecting controller reset failure

From: Stephen M. Cameron <[email protected]>

Detect failure of controller reset by noticing if the 32 bytes of
"driver version" we store on the hardware in the config table
fail to get zeroed out. Previously we noticed if the controller
did not transition to "simple mode", but this did not detect reset
failure if the controller was already in simple mode prior to
the reset attempt (e.g. due to module parameter hpsa_simple_mode=1).

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 82 ++++++++++++++++++++++++++++++++++++++++-----
drivers/block/cciss_cmd.h | 1 +
2 files changed, 73 insertions(+), 10 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 9bf1398..ed6aa83 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -194,6 +194,8 @@ static int __devinit cciss_find_cfg_addrs(struct pci_dev *pdev,
static int __devinit cciss_pci_find_memory_BAR(struct pci_dev *pdev,
unsigned long *memory_bar);
static inline u32 cciss_tag_discard_error_bits(ctlr_info_t *h, u32 tag);
+static __devinit int write_driver_ver_to_cfgtable(
+ CfgTable_struct __iomem *cfgtable);

/* performant mode helper functions */
static void calc_bucket_map(int *bucket, int num_buckets, int nsgs,
@@ -4078,6 +4080,9 @@ static int __devinit cciss_find_cfgtables(ctlr_info_t *h)
cfg_base_addr_index) + cfg_offset, sizeof(h->cfgtable));
if (!h->cfgtable)
return -ENOMEM;
+ rc = write_driver_ver_to_cfgtable(h->cfgtable);
+ if (rc)
+ return rc;
/* Find performant mode table. */
trans_offset = readl(&h->cfgtable->TransMethodOffset);
h->transtable = remap_pci_mem(pci_resource_start(h->pdev,
@@ -4428,6 +4433,60 @@ static int cciss_controller_hard_reset(struct pci_dev *pdev,
return 0;
}

+static __devinit void init_driver_version(char *driver_version, int len)
+{
+ memset(driver_version, 0, len);
+ strncpy(driver_version, "cciss " DRIVER_NAME, len - 1);
+}
+
+static __devinit int write_driver_ver_to_cfgtable(
+ CfgTable_struct __iomem *cfgtable)
+{
+ char *driver_version;
+ int i, size = sizeof(cfgtable->driver_version);
+
+ driver_version = kmalloc(size, GFP_KERNEL);
+ if (!driver_version)
+ return -ENOMEM;
+
+ init_driver_version(driver_version, size);
+ for (i = 0; i < size; i++)
+ writeb(driver_version[i], &cfgtable->driver_version[i]);
+ kfree(driver_version);
+ return 0;
+}
+
+static __devinit void read_driver_ver_from_cfgtable(
+ CfgTable_struct __iomem *cfgtable, unsigned char *driver_ver)
+{
+ int i;
+
+ for (i = 0; i < sizeof(cfgtable->driver_version); i++)
+ driver_ver[i] = readb(&cfgtable->driver_version[i]);
+}
+
+static __devinit int controller_reset_failed(
+ CfgTable_struct __iomem *cfgtable)
+{
+
+ char *driver_ver, *old_driver_ver;
+ int rc, size = sizeof(cfgtable->driver_version);
+
+ old_driver_ver = kmalloc(2 * size, GFP_KERNEL);
+ if (!old_driver_ver)
+ return -ENOMEM;
+ driver_ver = old_driver_ver + size;
+
+ /* After a reset, the 32 bytes of "driver version" in the cfgtable
+ * should have been changed, otherwise we know the reset failed.
+ */
+ init_driver_version(old_driver_ver, size);
+ read_driver_ver_from_cfgtable(cfgtable, driver_ver);
+ rc = !memcmp(driver_ver, old_driver_ver, size);
+ kfree(old_driver_ver);
+ return rc;
+}
+
/* This does a hard reset of the controller using PCI power management
* states or using the doorbell register. */
static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
@@ -4437,7 +4496,7 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
u64 cfg_base_addr_index;
void __iomem *vaddr;
unsigned long paddr;
- u32 misc_fw_support, active_transport;
+ u32 misc_fw_support;
int rc;
CfgTable_struct __iomem *cfgtable;
bool use_doorbell;
@@ -4497,6 +4556,9 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
rc = -ENOMEM;
goto unmap_vaddr;
}
+ rc = write_driver_ver_to_cfgtable(cfgtable);
+ if (rc)
+ goto unmap_vaddr;

/* If reset via doorbell register is supported, use that. */
misc_fw_support = readl(&cfgtable->misc_fw_support);
@@ -4535,21 +4597,21 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
"failed waiting for board to become ready\n");
goto unmap_cfgtable;
}
- dev_info(&pdev->dev, "board ready.\n");

- /* Controller should be in simple mode at this point. If it's not,
- * It means we're on one of those controllers which doesn't support
- * the doorbell reset method and on which the PCI power management reset
- * method doesn't work (P800, for example.)
- * In those cases, don't try to proceed, as it generally doesn't work.
- */
- active_transport = readl(&cfgtable->TransportActive);
- if (active_transport & PERFORMANT_MODE) {
+ rc = controller_reset_failed(vaddr);
+ if (rc < 0)
+ goto unmap_cfgtable;
+ if (rc) {
dev_warn(&pdev->dev, "Unable to successfully reset controller,"
" Ignoring controller.\n");
rc = -ENODEV;
+ goto unmap_cfgtable;
+ } else {
+ dev_info(&pdev->dev, "board ready.\n");
}

+ dev_info(&pdev->dev, "board ready.\n");
+
unmap_cfgtable:
iounmap(cfgtable);

diff --git a/drivers/block/cciss_cmd.h b/drivers/block/cciss_cmd.h
index cd441be..a2e68d2 100644
--- a/drivers/block/cciss_cmd.h
+++ b/drivers/block/cciss_cmd.h
@@ -235,6 +235,7 @@ typedef struct _CfgTable_struct {
u8 reserved[0x78 - 0x58];
u32 misc_fw_support; /* offset 0x78 */
#define MISC_FW_DOORBELL_RESET (0x02)
+ u8 driver_version[32];
} CfgTable_struct;

struct TransTable_struct {

2011-05-03 19:53:08

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 03/16] cciss: factor out command pool allocation functions

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 67 ++++++++++++++++++++++++++-----------------------
1 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index ed6aa83..152cb40 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4653,6 +4653,39 @@ static __devinit int cciss_init_reset_devices(struct pci_dev *pdev)
return 0;
}

+static __devinit int cciss_allocate_cmd_pool(ctlr_info_t *h)
+{
+ h->cmd_pool_bits = kmalloc(
+ DIV_ROUND_UP(h->nr_cmds, BITS_PER_LONG) *
+ sizeof(unsigned long), GFP_KERNEL);
+ h->cmd_pool = pci_alloc_consistent(h->pdev,
+ h->nr_cmds * sizeof(CommandList_struct),
+ &(h->cmd_pool_dhandle));
+ h->errinfo_pool = pci_alloc_consistent(h->pdev,
+ h->nr_cmds * sizeof(ErrorInfo_struct),
+ &(h->errinfo_pool_dhandle));
+ if ((h->cmd_pool_bits == NULL)
+ || (h->cmd_pool == NULL)
+ || (h->errinfo_pool == NULL)) {
+ dev_err(&h->pdev->dev, "out of memory");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static void cciss_free_cmd_pool(ctlr_info_t *h)
+{
+ kfree(h->cmd_pool_bits);
+ if (h->cmd_pool)
+ pci_free_consistent(h->pdev,
+ h->nr_cmds * sizeof(CommandList_struct),
+ h->cmd_pool, h->cmd_pool_dhandle);
+ if (h->errinfo_pool)
+ pci_free_consistent(h->pdev,
+ h->nr_cmds * sizeof(ErrorInfo_struct),
+ h->errinfo_pool, h->errinfo_pool_dhandle);
+}
+
/*
* This is it. Find all the controllers and register them. I really hate
* stealing all these major device numbers.
@@ -4745,23 +4778,8 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
h->devname, pdev->device, pci_name(pdev),
h->intr[PERF_MODE_INT], dac ? "" : " not");

- h->cmd_pool_bits =
- kmalloc(DIV_ROUND_UP(h->nr_cmds, BITS_PER_LONG)
- * sizeof(unsigned long), GFP_KERNEL);
- h->cmd_pool = (CommandList_struct *)
- pci_alloc_consistent(h->pdev,
- h->nr_cmds * sizeof(CommandList_struct),
- &(h->cmd_pool_dhandle));
- h->errinfo_pool = (ErrorInfo_struct *)
- pci_alloc_consistent(h->pdev,
- h->nr_cmds * sizeof(ErrorInfo_struct),
- &(h->errinfo_pool_dhandle));
- if ((h->cmd_pool_bits == NULL)
- || (h->cmd_pool == NULL)
- || (h->errinfo_pool == NULL)) {
- dev_err(&h->pdev->dev, "out of memory");
+ if (cciss_allocate_cmd_pool(h))
goto clean4;
- }

/* Need space for temp scatter list */
h->scatter_list = kmalloc(h->max_commands *
@@ -4837,21 +4855,12 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
return 1;

clean4:
- kfree(h->cmd_pool_bits);
+ cciss_free_cmd_pool(h);
/* Free up sg elements */
for (k-- ; k >= 0; k--)
kfree(h->scatter_list[k]);
kfree(h->scatter_list);
cciss_free_sg_chain_blocks(h->cmd_sg_list, h->nr_cmds);
- if (h->cmd_pool)
- pci_free_consistent(h->pdev,
- h->nr_cmds * sizeof(CommandList_struct),
- h->cmd_pool, h->cmd_pool_dhandle);
- if (h->errinfo_pool)
- pci_free_consistent(h->pdev,
- h->nr_cmds * sizeof(ErrorInfo_struct),
- h->errinfo_pool,
- h->errinfo_pool_dhandle);
free_irq(h->intr[PERF_MODE_INT], h);
clean2:
unregister_blkdev(h->major, h->devname);
@@ -4949,11 +4958,7 @@ static void __devexit cciss_remove_one(struct pci_dev *pdev)
iounmap(h->cfgtable);
iounmap(h->vaddr);

- pci_free_consistent(h->pdev, h->nr_cmds * sizeof(CommandList_struct),
- h->cmd_pool, h->cmd_pool_dhandle);
- pci_free_consistent(h->pdev, h->nr_cmds * sizeof(ErrorInfo_struct),
- h->errinfo_pool, h->errinfo_pool_dhandle);
- kfree(h->cmd_pool_bits);
+ cciss_free_cmd_pool(h);
/* Free up sg elements */
for (j = 0; j < h->nr_cmds; j++)
kfree(h->scatter_list[j]);

2011-05-03 19:53:15

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 04/16] cciss: factor out scatterlist allocation functions

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 55 +++++++++++++++++++++++++++++++------------------
1 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 152cb40..55ca1f4 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4673,6 +4673,39 @@ static __devinit int cciss_allocate_cmd_pool(ctlr_info_t *h)
return 0;
}

+static __devinit int cciss_allocate_scatterlists(ctlr_info_t *h)
+{
+ int i;
+
+ /* zero it, so that on free we need not know how many were alloc'ed */
+ h->scatter_list = kzalloc(h->max_commands *
+ sizeof(struct scatterlist *), GFP_KERNEL);
+ if (!h->scatter_list)
+ return -ENOMEM;
+
+ for (i = 0; i < h->nr_cmds; i++) {
+ h->scatter_list[i] = kmalloc(sizeof(struct scatterlist) *
+ h->maxsgentries, GFP_KERNEL);
+ if (h->scatter_list[i] == NULL) {
+ dev_err(&h->pdev->dev, "could not allocate "
+ "s/g lists\n");
+ return -ENOMEM;
+ }
+ }
+ return 0;
+}
+
+static void cciss_free_scatterlists(ctlr_info_t *h)
+{
+ int i;
+
+ if (h->scatter_list) {
+ for (i = 0; i < h->nr_cmds; i++)
+ kfree(h->scatter_list[i]);
+ kfree(h->scatter_list);
+ }
+}
+
static void cciss_free_cmd_pool(ctlr_info_t *h)
{
kfree(h->cmd_pool_bits);
@@ -4696,7 +4729,6 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
{
int i;
int j = 0;
- int k = 0;
int rc;
int dac, return_code;
InquiryData_struct *inq_buff;
@@ -4781,23 +4813,9 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
if (cciss_allocate_cmd_pool(h))
goto clean4;

- /* Need space for temp scatter list */
- h->scatter_list = kmalloc(h->max_commands *
- sizeof(struct scatterlist *),
- GFP_KERNEL);
- if (!h->scatter_list)
+ if (cciss_allocate_scatterlists(h))
goto clean4;

- for (k = 0; k < h->nr_cmds; k++) {
- h->scatter_list[k] = kmalloc(sizeof(struct scatterlist) *
- h->maxsgentries,
- GFP_KERNEL);
- if (h->scatter_list[k] == NULL) {
- dev_err(&h->pdev->dev,
- "could not allocate s/g lists\n");
- goto clean4;
- }
- }
h->cmd_sg_list = cciss_allocate_sg_chain_blocks(h,
h->chainsize, h->nr_cmds);
if (!h->cmd_sg_list && h->chainsize > 0)
@@ -4856,10 +4874,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,

clean4:
cciss_free_cmd_pool(h);
- /* Free up sg elements */
- for (k-- ; k >= 0; k--)
- kfree(h->scatter_list[k]);
- kfree(h->scatter_list);
+ cciss_free_scatterlists(h);
cciss_free_sg_chain_blocks(h->cmd_sg_list, h->nr_cmds);
free_irq(h->intr[PERF_MODE_INT], h);
clean2:

2011-05-03 19:53:18

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 05/16] cciss: factor out irq request code

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 41 +++++++++++++++++++++++++----------------
1 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 55ca1f4..63fe05a 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4719,6 +4719,28 @@ static void cciss_free_cmd_pool(ctlr_info_t *h)
h->errinfo_pool, h->errinfo_pool_dhandle);
}

+static int cciss_request_irq(ctlr_info_t *h,
+ irqreturn_t (*msixhandler)(int, void *),
+ irqreturn_t (*intxhandler)(int, void *))
+{
+ if (h->msix_vector || h->msi_vector) {
+ if (!request_irq(h->intr[PERF_MODE_INT], msixhandler,
+ IRQF_DISABLED, h->devname, h))
+ return 0;
+ dev_err(&h->pdev->dev, "Unable to get msi irq %d"
+ " for %s\n", h->intr[PERF_MODE_INT],
+ h->devname);
+ return -1;
+ }
+
+ if (!request_irq(h->intr[PERF_MODE_INT], intxhandler,
+ IRQF_DISABLED, h->devname, h))
+ return 0;
+ dev_err(&h->pdev->dev, "Unable to get irq %d for %s\n",
+ h->intr[PERF_MODE_INT], h->devname);
+ return -1;
+}
+
/*
* This is it. Find all the controllers and register them. I really hate
* stealing all these major device numbers.
@@ -4789,22 +4811,9 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,

/* make sure the board interrupts are off */
h->access.set_intr_mask(h, CCISS_INTR_OFF);
- if (h->msi_vector || h->msix_vector) {
- if (request_irq(h->intr[PERF_MODE_INT],
- do_cciss_msix_intr,
- IRQF_DISABLED, h->devname, h)) {
- dev_err(&h->pdev->dev, "Unable to get irq %d for %s\n",
- h->intr[PERF_MODE_INT], h->devname);
- goto clean2;
- }
- } else {
- if (request_irq(h->intr[PERF_MODE_INT], do_cciss_intx,
- IRQF_DISABLED, h->devname, h)) {
- dev_err(&h->pdev->dev, "Unable to get irq %d for %s\n",
- h->intr[PERF_MODE_INT], h->devname);
- goto clean2;
- }
- }
+ rc = cciss_request_irq(h, do_cciss_msix_intr, do_cciss_intx);
+ if (rc)
+ goto clean2;

dev_info(&h->pdev->dev, "%s: <0x%x> at PCI %s IRQ %d%s using DAC\n",
h->devname, pdev->device, pci_name(pdev),

2011-05-03 19:53:23

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 06/16] cciss: fix reply pool and block fetch table memory leaks

From: Stephen M. Cameron <[email protected]>


---
drivers/block/cciss.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 63fe05a..d604489 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4988,6 +4988,10 @@ static void __devexit cciss_remove_one(struct pci_dev *pdev)
kfree(h->scatter_list[j]);
kfree(h->scatter_list);
cciss_free_sg_chain_blocks(h->cmd_sg_list, h->nr_cmds);
+ kfree(h->blockFetchTable);
+ if (h->reply_pool)
+ pci_free_consistent(h->pdev, h->max_commands * sizeof(__u64),
+ h->reply_pool, h->reply_pool_dhandle);
/*
* Deliberately omit pci_disable_device(): it does something nasty to
* Smart Array controllers that pci_enable_device does not undo

2011-05-03 19:53:30

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 07/16] cciss: get rid of message related magic numbers

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 8 ++++----
drivers/block/cciss_cmd.h | 8 ++++++++
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index d604489..1a4dff0 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -2569,7 +2569,7 @@ static int fill_cmd(ctlr_info_t *h, CommandList_struct *c, __u8 cmd, void *buff,
}
} else if (cmd_type == TYPE_MSG) {
switch (cmd) {
- case 0: /* ABORT message */
+ case CCISS_ABORT_MSG:
c->Request.CDBLen = 12;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_WRITE;
@@ -2579,16 +2579,16 @@ static int fill_cmd(ctlr_info_t *h, CommandList_struct *c, __u8 cmd, void *buff,
/* buff contains the tag of the command to abort */
memcpy(&c->Request.CDB[4], buff, 8);
break;
- case 1: /* RESET message */
+ case CCISS_RESET_MSG:
c->Request.CDBLen = 16;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_NONE;
c->Request.Timeout = 0;
memset(&c->Request.CDB[0], 0, sizeof(c->Request.CDB));
c->Request.CDB[0] = cmd; /* reset */
- c->Request.CDB[1] = 0x03; /* reset a target */
+ c->Request.CDB[1] = CCISS_RESET_TYPE_TARGET;
break;
- case 3: /* No-Op message */
+ case CCISS_NOOP_MSG:
c->Request.CDBLen = 1;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_WRITE;
diff --git a/drivers/block/cciss_cmd.h b/drivers/block/cciss_cmd.h
index a2e68d2..3b20c31 100644
--- a/drivers/block/cciss_cmd.h
+++ b/drivers/block/cciss_cmd.h
@@ -142,6 +142,14 @@ typedef struct _ReadCapdata_struct_16
#define BMIC_CACHE_FLUSH 0xc2
#define CCISS_CACHE_FLUSH 0x01 /* C2 was already being used by CCISS */

+#define CCISS_ABORT_MSG 0x00
+#define CCISS_RESET_MSG 0x01
+#define CCISS_RESET_TYPE_CONTROLLER 0x00
+#define CCISS_RESET_TYPE_BUS 0x01
+#define CCISS_RESET_TYPE_TARGET 0x03
+#define CCISS_RESET_TYPE_LUN 0x04
+#define CCISS_NOOP_MSG 0x03
+
/* Command List Structure */
#define CTLR_LUNID "\0\0\0\0\0\0\0\0"

2011-05-03 19:53:35

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 08/16] cciss: increase time to wait for board reset to start

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index 9b49439..21ec628 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -200,7 +200,7 @@ struct ctlr_info
* the above.
*/
#define CCISS_BOARD_READY_WAIT_SECS (120)
-#define CCISS_BOARD_NOT_READY_WAIT_SECS (10)
+#define CCISS_BOARD_NOT_READY_WAIT_SECS (100)
#define CCISS_BOARD_READY_POLL_INTERVAL_MSECS (100)
#define CCISS_BOARD_READY_ITERATIONS \
((CCISS_BOARD_READY_WAIT_SECS * 1000) / \

2011-05-03 20:01:17

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 09/16] cciss: clarify messages around reset behavior

From: Stephen M. Cameron <[email protected]>

When waiting for the board to become "not ready"
don't print a message saying "waiting for board to
become ready" (possibly followed by a message saying
"failed waiting for board to become not ready". Instead,
it should be "waiting for board to reset" and "failed
waiting for board to reset."

Signed-off-by: Stephen M. Cameron <[email protected]>
"
---
drivers/block/cciss.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 1a4dff0..3b55723 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4586,11 +4586,11 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
msleep(CCISS_POST_RESET_PAUSE_MSECS);

/* Wait for board to become not ready, then ready. */
- dev_info(&pdev->dev, "Waiting for board to become ready.\n");
+ dev_info(&pdev->dev, "Waiting for board to reset.\n");
rc = cciss_wait_for_board_state(pdev, vaddr, BOARD_NOT_READY);
if (rc) /* Don't bail, might be E500, etc. which can't be reset */
dev_warn(&pdev->dev,
- "failed waiting for board to become not ready\n");
+ "failed waiting for board to reset\n");
rc = cciss_wait_for_board_state(pdev, vaddr, BOARD_READY);
if (rc) {
dev_warn(&pdev->dev,
@@ -4641,6 +4641,7 @@ static __devinit int cciss_init_reset_devices(struct pci_dev *pdev)
return -ENODEV;

/* Now try to get the controller to respond to a no-op */
+ dev_warn(&pdev->dev, "Waiting for controller to respond to no-op\n");
for (i = 0; i < CCISS_POST_RESET_NOOP_RETRIES; i++) {
if (cciss_noop(pdev) == 0)
break;

2011-05-03 20:03:36

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 10/16] cciss: increase timeouts for post-reset no-ops

From: Stephen M. Cameron <[email protected]>

Just to reduce the messages about timeouts that appear.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 2 +-
drivers/block/cciss.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 3b55723..b9f8658 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4353,7 +4353,7 @@ static __devinit int cciss_message(struct pci_dev *pdev, unsigned char opcode, u
tag = readl(vaddr + SA5_REPLY_PORT_OFFSET);
if ((tag & ~3) == paddr32)
break;
- schedule_timeout_uninterruptible(HZ);
+ msleep(CCISS_POST_RESET_NOOP_TIMEOUT_MSECS);
}

iounmap(vaddr);
diff --git a/drivers/block/cciss.h b/drivers/block/cciss.h
index 21ec628..16b4d58 100644
--- a/drivers/block/cciss.h
+++ b/drivers/block/cciss.h
@@ -209,8 +209,9 @@ struct ctlr_info
((CCISS_BOARD_NOT_READY_WAIT_SECS * 1000) / \
CCISS_BOARD_READY_POLL_INTERVAL_MSECS)
#define CCISS_POST_RESET_PAUSE_MSECS (3000)
-#define CCISS_POST_RESET_NOOP_INTERVAL_MSECS (1000)
+#define CCISS_POST_RESET_NOOP_INTERVAL_MSECS (4000)
#define CCISS_POST_RESET_NOOP_RETRIES (12)
+#define CCISS_POST_RESET_NOOP_TIMEOUT_MSECS (10000)

/*
Send the command to the hardware

2011-05-03 20:01:13

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 11/16] cciss: use new doorbell-bit-5 reset method

From: Stephen M. Cameron <[email protected]>

The bit-2-doorbell reset method seemed to cause (survivable) NMIs
on some systems and (unsurvivable) IOCK NMIs on some G7 servers.
Firmware guys implemented a new doorbell method to alleviate these
problems triggered by bit 5 of the doorbell register. We want to
use it if it's available.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 25 ++++++++++++++-----------
drivers/block/cciss_cmd.h | 2 ++
2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index b9f8658..df58c59 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4384,7 +4384,7 @@ static __devinit int cciss_message(struct pci_dev *pdev, unsigned char opcode, u
#define cciss_noop(p) cciss_message(p, 3, 0)

static int cciss_controller_hard_reset(struct pci_dev *pdev,
- void * __iomem vaddr, bool use_doorbell)
+ void * __iomem vaddr, u32 use_doorbell)
{
u16 pmcsr;
int pos;
@@ -4395,7 +4395,7 @@ static int cciss_controller_hard_reset(struct pci_dev *pdev,
* other way using the doorbell register.
*/
dev_info(&pdev->dev, "using doorbell to reset controller\n");
- writel(DOORBELL_CTLR_RESET, vaddr + SA5_DOORBELL);
+ writel(use_doorbell, vaddr + SA5_DOORBELL);
msleep(1000);
} else { /* Try to do it the PCI power state way */

@@ -4499,7 +4499,7 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
u32 misc_fw_support;
int rc;
CfgTable_struct __iomem *cfgtable;
- bool use_doorbell;
+ u32 use_doorbell;
u32 board_id;
u16 command_register;

@@ -4560,15 +4560,18 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
if (rc)
goto unmap_vaddr;

- /* If reset via doorbell register is supported, use that. */
- misc_fw_support = readl(&cfgtable->misc_fw_support);
- use_doorbell = misc_fw_support & MISC_FW_DOORBELL_RESET;
-
- /* The doorbell reset seems to cause lockups on some Smart
- * Arrays (e.g. P410, P410i, maybe others). Until this is
- * fixed or at least isolated, avoid the doorbell reset.
+ /* If reset via doorbell register is supported, use that.
+ * There are two such methods. Favor the newest method.
*/
- use_doorbell = 0;
+ misc_fw_support = readl(&cfgtable->misc_fw_support);
+ use_doorbell = misc_fw_support & MISC_FW_DOORBELL_RESET2;
+ if (use_doorbell) {
+ use_doorbell = DOORBELL_CTLR_RESET2;
+ } else {
+ use_doorbell = misc_fw_support & MISC_FW_DOORBELL_RESET;
+ if (use_doorbell)
+ use_doorbell = DOORBELL_CTLR_RESET;
+ }

rc = cciss_controller_hard_reset(pdev, vaddr, use_doorbell);
if (rc)
diff --git a/drivers/block/cciss_cmd.h b/drivers/block/cciss_cmd.h
index 3b20c31..d9be6b4 100644
--- a/drivers/block/cciss_cmd.h
+++ b/drivers/block/cciss_cmd.h
@@ -53,6 +53,7 @@
#define CFGTBL_ChangeReq 0x00000001l
#define CFGTBL_AccCmds 0x00000001l
#define DOORBELL_CTLR_RESET 0x00000004l
+#define DOORBELL_CTLR_RESET2 0x00000020l

#define CFGTBL_Trans_Simple 0x00000002l
#define CFGTBL_Trans_Performant 0x00000004l
@@ -243,6 +244,7 @@ typedef struct _CfgTable_struct {
u8 reserved[0x78 - 0x58];
u32 misc_fw_support; /* offset 0x78 */
#define MISC_FW_DOORBELL_RESET (0x02)
+#define MISC_FW_DOORBELL_RESET2 (0x10)
u8 driver_version[32];
} CfgTable_struct;

2011-05-03 20:02:54

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 12/16] cciss: do soft reset if hard reset is broken

From: Stephen M. Cameron <[email protected]>

on driver load, if reset_devices is set, and the hard reset
attempts fail, try to bring up the controller to the point that
a command can be sent, and send it a soft reset command, then
after the reset undo whatever driver initialization was done to get
it to the point to take a command, and re-do it after the reset.

This is to get kdump to work on all the "non-resettable" controllers
(except 64xx controllers which can't be reset due to the potentially
shared cache module.)

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 221 insertions(+), 15 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index df58c59..23b0ba4 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -2477,6 +2477,31 @@ static int deregister_disk(ctlr_info_t *h, int drv_index,
return 0;
}

+static int __devinit cciss_send_reset(ctlr_info_t *h, unsigned char *scsi3addr,
+ u8 reset_type)
+{
+ CommandList_struct *c;
+ int return_status;
+
+ c = cmd_alloc(h);
+ if (!c)
+ return -ENOMEM;
+ return_status = fill_cmd(h, c, CCISS_RESET_MSG, NULL, 0, 0,
+ CTLR_LUNID, TYPE_MSG);
+ c->Request.CDB[1] = reset_type; /* fill_cmd defaults to target reset */
+ if (return_status != IO_OK) {
+ cmd_special_free(h, c);
+ return return_status;
+ }
+ c->waiting = NULL;
+ enqueue_cmd_and_start_io(h, c);
+ /* Don't wait for completion, the reset won't complete. Don't free
+ * the command either. This is the last command we will send before
+ * re-initializing everything, so it doesn't matter and won't leak.
+ */
+ return 0;
+}
+
static int fill_cmd(ctlr_info_t *h, CommandList_struct *c, __u8 cmd, void *buff,
size_t size, __u8 page_code, unsigned char *scsi3addr,
int cmd_type)
@@ -3463,6 +3488,63 @@ static inline u32 process_nonindexed_cmd(ctlr_info_t *h, u32 raw_tag)
return next_command(h);
}

+/* Some controllers, like p400, will give us one interrupt
+ * after a soft reset, even if we turned interrupts off.
+ * Only need to check for this in the cciss_xxx_discard_completions
+ * functions.
+ */
+static int ignore_bogus_interrupt(ctlr_info_t *h)
+{
+ if (likely(!reset_devices))
+ return 0;
+
+ if (likely(h->interrupts_enabled))
+ return 0;
+
+ dev_info(&h->pdev->dev, "Received interrupt while interrupts disabled "
+ "(known firmware bug.) Ignoring.\n");
+
+ return 1;
+}
+
+static irqreturn_t cciss_intx_discard_completions(int irq, void *dev_id)
+{
+ ctlr_info_t *h = dev_id;
+ unsigned long flags;
+ u32 raw_tag;
+
+ if (ignore_bogus_interrupt(h))
+ return IRQ_NONE;
+
+ if (interrupt_not_for_us(h))
+ return IRQ_NONE;
+ spin_lock_irqsave(&h->lock, flags);
+ while (interrupt_pending(h)) {
+ raw_tag = get_next_completion(h);
+ while (raw_tag != FIFO_EMPTY)
+ raw_tag = next_command(h);
+ }
+ spin_unlock_irqrestore(&h->lock, flags);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t cciss_msix_discard_completions(int irq, void *dev_id)
+{
+ ctlr_info_t *h = dev_id;
+ unsigned long flags;
+ u32 raw_tag;
+
+ if (ignore_bogus_interrupt(h))
+ return IRQ_NONE;
+
+ spin_lock_irqsave(&h->lock, flags);
+ raw_tag = get_next_completion(h);
+ while (raw_tag != FIFO_EMPTY)
+ raw_tag = next_command(h);
+ spin_unlock_irqrestore(&h->lock, flags);
+ return IRQ_HANDLED;
+}
+
static irqreturn_t do_cciss_intx(int irq, void *dev_id)
{
ctlr_info_t *h = dev_id;
@@ -4380,7 +4462,6 @@ static __devinit int cciss_message(struct pci_dev *pdev, unsigned char opcode, u
return 0;
}

-#define cciss_soft_reset_controller(p) cciss_message(p, 1, 0)
#define cciss_noop(p) cciss_message(p, 3, 0)

static int cciss_controller_hard_reset(struct pci_dev *pdev,
@@ -4591,13 +4672,17 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
/* Wait for board to become not ready, then ready. */
dev_info(&pdev->dev, "Waiting for board to reset.\n");
rc = cciss_wait_for_board_state(pdev, vaddr, BOARD_NOT_READY);
- if (rc) /* Don't bail, might be E500, etc. which can't be reset */
- dev_warn(&pdev->dev,
- "failed waiting for board to reset\n");
+ if (rc) {
+ dev_warn(&pdev->dev, "Failed waiting for board to hard reset."
+ " Will try soft reset.\n");
+ rc = -ENOTSUPP; /* Not expected, but try soft reset later */
+ goto unmap_cfgtable;
+ }
rc = cciss_wait_for_board_state(pdev, vaddr, BOARD_READY);
if (rc) {
dev_warn(&pdev->dev,
- "failed waiting for board to become ready\n");
+ "failed waiting for board to become ready "
+ "after hard reset\n");
goto unmap_cfgtable;
}

@@ -4605,16 +4690,13 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
if (rc < 0)
goto unmap_cfgtable;
if (rc) {
- dev_warn(&pdev->dev, "Unable to successfully reset controller,"
- " Ignoring controller.\n");
- rc = -ENODEV;
- goto unmap_cfgtable;
+ dev_warn(&pdev->dev, "Unable to successfully hard reset "
+ "controller. Will try soft reset.\n");
+ rc = -ENOTSUPP; /* Not expected, but try soft reset later */
} else {
- dev_info(&pdev->dev, "board ready.\n");
+ dev_info(&pdev->dev, "Board ready after hard reset.\n");
}

- dev_info(&pdev->dev, "board ready.\n");
-
unmap_cfgtable:
iounmap(cfgtable);

@@ -4639,7 +4721,7 @@ static __devinit int cciss_init_reset_devices(struct pci_dev *pdev)
* due to concerns about shared bbwc between 6402/6404 pair.
*/
if (rc == -ENOTSUPP)
- return 0; /* just try to do the kdump anyhow. */
+ return rc; /* just try to do the kdump anyhow. */
if (rc)
return -ENODEV;

@@ -4745,6 +4827,60 @@ static int cciss_request_irq(ctlr_info_t *h,
return -1;
}

+static int __devinit cciss_kdump_soft_reset(ctlr_info_t *h)
+{
+ if (cciss_send_reset(h, CTLR_LUNID, CCISS_RESET_TYPE_CONTROLLER)) {
+ dev_warn(&h->pdev->dev, "Resetting array controller failed.\n");
+ return -EIO;
+ }
+
+ dev_info(&h->pdev->dev, "Waiting for board to soft reset.\n");
+ if (cciss_wait_for_board_state(h->pdev, h->vaddr, BOARD_NOT_READY)) {
+ dev_warn(&h->pdev->dev, "Soft reset had no effect.\n");
+ return -1;
+ }
+
+ dev_info(&h->pdev->dev, "Board reset, awaiting READY status.\n");
+ if (cciss_wait_for_board_state(h->pdev, h->vaddr, BOARD_READY)) {
+ dev_warn(&h->pdev->dev, "Board failed to become ready "
+ "after soft reset.\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static void cciss_undo_allocations_after_kdump_soft_reset(ctlr_info_t *h)
+{
+ int ctlr = h->ctlr;
+
+ free_irq(h->intr[PERF_MODE_INT], h);
+#ifdef CONFIG_PCI_MSI
+ if (h->msix_vector)
+ pci_disable_msix(h->pdev);
+ else if (h->msi_vector)
+ pci_disable_msi(h->pdev);
+#endif /* CONFIG_PCI_MSI */
+ cciss_free_sg_chain_blocks(h->cmd_sg_list, h->nr_cmds);
+ cciss_free_scatterlists(h);
+ cciss_free_cmd_pool(h);
+ kfree(h->blockFetchTable);
+ if (h->reply_pool)
+ pci_free_consistent(h->pdev, h->max_commands * sizeof(__u64),
+ h->reply_pool, h->reply_pool_dhandle);
+ if (h->transtable)
+ iounmap(h->transtable);
+ if (h->cfgtable)
+ iounmap(h->cfgtable);
+ if (h->vaddr)
+ iounmap(h->vaddr);
+ unregister_blkdev(h->major, h->devname);
+ cciss_destroy_hba_sysfs_entry(h);
+ pci_release_regions(h->pdev);
+ kfree(h);
+ hba[ctlr] = NULL;
+}
+
/*
* This is it. Find all the controllers and register them. I really hate
* stealing all these major device numbers.
@@ -4756,13 +4892,27 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
int i;
int j = 0;
int rc;
+ int try_soft_reset = 0;
int dac, return_code;
InquiryData_struct *inq_buff;
ctlr_info_t *h;
+ unsigned long flags;

rc = cciss_init_reset_devices(pdev);
- if (rc)
- return rc;
+ if (rc) {
+ if (rc != -ENOTSUPP)
+ return rc;
+ /* If the reset fails in a particular way (it has no way to do
+ * a proper hard reset, so returns -ENOTSUPP) we can try to do
+ * a soft reset once we get the controller configured up to the
+ * point that it can accept a command.
+ */
+ try_soft_reset = 1;
+ rc = 0;
+ }
+
+reinit_after_soft_reset:
+
i = alloc_cciss_hba(pdev);
if (i < 0)
return -1;
@@ -4852,6 +5002,62 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
h->gendisk[j] = NULL;
}

+ /* At this point, the controller is ready to take commands.
+ * Now, if reset_devices and the hard reset didn't work, try
+ * the soft reset and see if that works.
+ */
+ if (try_soft_reset) {
+
+ /* This is kind of gross. We may or may not get a completion
+ * from the soft reset command, and if we do, then the value
+ * from the fifo may or may not be valid. So, we wait 10 secs
+ * after the reset throwing away any completions we get during
+ * that time. Unregister the interrupt handler and register
+ * fake ones to scoop up any residual completions.
+ */
+ spin_lock_irqsave(&h->lock, flags);
+ h->access.set_intr_mask(h, CCISS_INTR_OFF);
+ spin_unlock_irqrestore(&h->lock, flags);
+ free_irq(h->intr[PERF_MODE_INT], h);
+ rc = cciss_request_irq(h, cciss_msix_discard_completions,
+ cciss_intx_discard_completions);
+ if (rc) {
+ dev_warn(&h->pdev->dev, "Failed to request_irq after "
+ "soft reset.\n");
+ goto clean4;
+ }
+
+ rc = cciss_kdump_soft_reset(h);
+ if (rc) {
+ dev_warn(&h->pdev->dev, "Soft reset failed.\n");
+ goto clean4;
+ }
+
+ dev_info(&h->pdev->dev, "Board READY.\n");
+ dev_info(&h->pdev->dev,
+ "Waiting for stale completions to drain.\n");
+ h->access.set_intr_mask(h, CCISS_INTR_ON);
+ msleep(10000);
+ h->access.set_intr_mask(h, CCISS_INTR_OFF);
+
+ rc = controller_reset_failed(h->cfgtable);
+ if (rc)
+ dev_info(&h->pdev->dev,
+ "Soft reset appears to have failed.\n");
+
+ /* since the controller's reset, we have to go back and re-init
+ * everything. Easiest to just forget what we've done and do it
+ * all over again.
+ */
+ cciss_undo_allocations_after_kdump_soft_reset(h);
+ try_soft_reset = 0;
+ if (rc)
+ /* don't go to clean4, we already unallocated */
+ return -ENODEV;
+
+ goto reinit_after_soft_reset;
+ }
+
cciss_scsi_setup(h);

/* Turn the interrupts on so we can service requests */

2011-05-03 19:54:00

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 13/16] cciss: remove superfluous sleeps around reset code

From: Stephen M. Cameron <[email protected]>

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 23b0ba4..7990b98 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4477,7 +4477,6 @@ static int cciss_controller_hard_reset(struct pci_dev *pdev,
*/
dev_info(&pdev->dev, "using doorbell to reset controller\n");
writel(use_doorbell, vaddr + SA5_DOORBELL);
- msleep(1000);
} else { /* Try to do it the PCI power state way */

/* Quoting from the Open CISS Specification: "The Power
@@ -4508,8 +4507,6 @@ static int cciss_controller_hard_reset(struct pci_dev *pdev,
pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
pmcsr |= PCI_D0;
pci_write_config_word(pdev, pos + PCI_PM_CTRL, pmcsr);
-
- msleep(500);
}
return 0;
}

2011-05-03 20:02:52

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 14/16] cciss: do not attempt PCI power management reset method if we know it won't work.

From: Stephen M. Cameron <[email protected]>

Just go straight to the soft-reset method instead.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 36 +++++++++++++++++++++++++++++++-----
1 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 7990b98..865484e 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -558,7 +558,7 @@ static void __devinit cciss_procinit(ctlr_info_t *h)
#define to_hba(n) container_of(n, struct ctlr_info, dev)
#define to_drv(n) container_of(n, drive_info_struct, dev)

-/* List of controllers which cannot be reset on kexec with reset_devices */
+/* List of controllers which cannot be hard reset on kexec with reset_devices */
static u32 unresettable_controller[] = {
0x324a103C, /* Smart Array P712m */
0x324b103C, /* SmartArray P711m */
@@ -576,23 +576,45 @@ static u32 unresettable_controller[] = {
0x409D0E11, /* Smart Array 6400 EM */
};

-static int ctlr_is_resettable(struct ctlr_info *h)
+/* List of controllers which cannot even be soft reset */
+static u32 soft_unresettable_controller[] = {
+ 0x409C0E11, /* Smart Array 6400 */
+ 0x409D0E11, /* Smart Array 6400 EM */
+};
+
+static int ctlr_is_hard_resettable(u32 board_id)
{
int i;

for (i = 0; i < ARRAY_SIZE(unresettable_controller); i++)
- if (unresettable_controller[i] == h->board_id)
+ if (unresettable_controller[i] == board_id)
return 0;
return 1;
}

+static int ctlr_is_soft_resettable(u32 board_id)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(soft_unresettable_controller); i++)
+ if (soft_unresettable_controller[i] == board_id)
+ return 0;
+ return 1;
+}
+
+static int ctlr_is_resettable(u32 board_id)
+{
+ return ctlr_is_hard_resettable(board_id) ||
+ ctlr_is_soft_resettable(board_id);
+}
+
static ssize_t host_show_resettable(struct device *dev,
struct device_attribute *attr,
char *buf)
{
struct ctlr_info *h = to_hba(dev);

- return snprintf(buf, 20, "%d\n", ctlr_is_resettable(h));
+ return snprintf(buf, 20, "%d\n", ctlr_is_resettable(h->board_id));
}
static DEVICE_ATTR(resettable, S_IRUGO, host_show_resettable, NULL);

@@ -4601,12 +4623,16 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
* likely not be happy. Just forbid resetting this conjoined mess.
*/
cciss_lookup_board_id(pdev, &board_id);
- if (board_id == 0x409C0E11 || board_id == 0x409D0E11) {
+ if (!ctlr_is_resettable(board_id)) {
dev_warn(&pdev->dev, "Cannot reset Smart Array 640x "
"due to shared cache module.");
return -ENODEV;
}

+ /* if controller is soft- but not hard resettable... */
+ if (!ctlr_is_hard_resettable(board_id))
+ return -ENOTSUPP; /* try soft reset later. */
+
/* Save the PCI command register */
pci_read_config_word(pdev, 4, &command_register);
/* Turn the board off. This is so that later pci_restore_state()

2011-05-03 19:54:12

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 15/16] cciss: do not use bit 2 doorbell reset

From: Stephen M. Cameron <[email protected]>

It causes NMIs which are undesirable at best, unsurvivable at worst.
Prefer the soft reset instead.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
drivers/block/cciss.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 865484e..9528e94 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -4673,8 +4673,14 @@ static __devinit int cciss_kdump_hard_reset_controller(struct pci_dev *pdev)
use_doorbell = DOORBELL_CTLR_RESET2;
} else {
use_doorbell = misc_fw_support & MISC_FW_DOORBELL_RESET;
- if (use_doorbell)
- use_doorbell = DOORBELL_CTLR_RESET;
+ if (use_doorbell) {
+ dev_warn(&pdev->dev, "Controller claims that "
+ "'Bit 2 doorbell reset' is "
+ "supported, but not 'bit 5 doorbell reset'. "
+ "Firmware update is recommended.\n");
+ rc = -ENOTSUPP; /* use the soft reset */
+ goto unmap_cfgtable;
+ }
}

rc = cciss_controller_hard_reset(pdev, vaddr, use_doorbell);

2011-05-03 20:01:14

by Stephen M. Cameron

[permalink] [raw]
Subject: [PATCH 16/16] cciss: add cciss_tape_cmds module paramter

From: Stephen M. Cameron <[email protected]>

This is to allow number of commands reserved for use by SCSI tape drives
and medium changers to be adjusted at driver load time via the kernel
parameter cciss_tape_cmds, with a default value of 6, and a range
of 2 - 16 inclusive. Previously, the driver limited the number of
commands which could be queued to the SCSI half of the the driver
to only 2. This is to fix the problem that if you had more than
two tape drives, you couldn't, for example, erase or rewind them all
at the same time.

Signed-off-by: Stephen M. Cameron <[email protected]>
---
Documentation/blockdev/cciss.txt | 15 ++++++++++++++
drivers/block/cciss.c | 11 +++++++++-
drivers/block/cciss_scsi.c | 41 +++++++++++++++++++++-----------------
drivers/block/cciss_scsi.h | 4 ----
4 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/Documentation/blockdev/cciss.txt b/Documentation/blockdev/cciss.txt
index 89698e8..c00c6a5 100644
--- a/Documentation/blockdev/cciss.txt
+++ b/Documentation/blockdev/cciss.txt
@@ -169,3 +169,18 @@ is issued which positions the tape to a known position. Typically you
must rewind the tape (by issuing "mt -f /dev/st0 rewind" for example)
before i/o can proceed again to a tape drive which was reset.

+There is a cciss_tape_cmds module parameter which can be used to make cciss
+allocate more commands for use by tape drives. Ordinarily only a few commands
+(6) are allocated for tape drives because tape drives are slow and
+infrequently used and the primary purpose of Smart Array controllers is to
+act as a RAID controller for disk drives, so the vast majority of commands
+are allocated for disk devices. However, if you have more than a few tape
+drives attached to a smart array, the default number of commands may not be
+enought (for example, if you have 8 tape drives, you could only rewind 6
+at one time with the default number of commands.) The cciss_tape_cmds module
+parameter allows more commands (up to 16 more) to be allocated for use by
+tape drives. For example:
+
+ insmod cciss.ko cciss_tape_cmds=16
+
+Or, as a kernel boot parameter passed in via grub: cciss.cciss_tape_cmds=8
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 9528e94..abe90c9 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -64,6 +64,10 @@ MODULE_DESCRIPTION("Driver for HP Smart Array Controllers");
MODULE_SUPPORTED_DEVICE("HP Smart Array Controllers");
MODULE_VERSION("3.6.26");
MODULE_LICENSE("GPL");
+static int cciss_tape_cmds = 6;
+module_param(cciss_tape_cmds, int, 0644);
+MODULE_PARM_DESC(cciss_tape_cmds,
+ "number of commands to allocate for tape devices (default: 6)");

static DEFINE_MUTEX(cciss_mutex);
static struct proc_dir_entry *proc_cciss;
@@ -4221,7 +4225,7 @@ static void __devinit cciss_get_max_perf_mode_cmds(struct ctlr_info *h)
static void __devinit cciss_find_board_params(ctlr_info_t *h)
{
cciss_get_max_perf_mode_cmds(h);
- h->nr_cmds = h->max_commands - 4; /* Allow room for some ioctls */
+ h->nr_cmds = h->max_commands - 4 - cciss_tape_cmds;
h->maxsgentries = readl(&(h->cfgtable->MaxSGElements));
/*
* Limit in-command s/g elements to 32 save dma'able memory.
@@ -4959,6 +4963,11 @@ reinit_after_soft_reset:
sprintf(h->devname, "cciss%d", i);
h->ctlr = i;

+ if (cciss_tape_cmds < 2)
+ cciss_tape_cmds = 2;
+ if (cciss_tape_cmds > 16)
+ cciss_tape_cmds = 16;
+
init_completion(&h->scan_wait);

if (cciss_create_hba_sysfs_entry(h))
diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index df79380..6961002 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -84,7 +84,6 @@ static struct scsi_host_template cciss_driver_template = {
.proc_name = "cciss",
.proc_info = cciss_scsi_proc_info,
.queuecommand = cciss_scsi_queue_command,
- .can_queue = SCSI_CCISS_CAN_QUEUE,
.this_id = 7,
.cmd_per_lun = 1,
.use_clustering = DISABLE_CLUSTERING,
@@ -108,16 +107,13 @@ struct cciss_scsi_cmd_stack_elem_t {

#pragma pack()

-#define CMD_STACK_SIZE (SCSI_CCISS_CAN_QUEUE * \
- CCISS_MAX_SCSI_DEVS_PER_HBA + 2)
- // plus two for init time usage
-
#pragma pack(1)
struct cciss_scsi_cmd_stack_t {
struct cciss_scsi_cmd_stack_elem_t *pool;
- struct cciss_scsi_cmd_stack_elem_t *elem[CMD_STACK_SIZE];
+ struct cciss_scsi_cmd_stack_elem_t **elem;
dma_addr_t cmd_pool_handle;
int top;
+ int nelems;
};
#pragma pack()

@@ -191,7 +187,7 @@ scsi_cmd_free(ctlr_info_t *h, CommandList_struct *c)
sa = h->scsi_ctlr;
stk = &sa->cmd_stack;
stk->top++;
- if (stk->top >= CMD_STACK_SIZE) {
+ if (stk->top >= stk->nelems) {
dev_err(&h->pdev->dev,
"scsi_cmd_free called too many times.\n");
BUG();
@@ -206,13 +202,14 @@ scsi_cmd_stack_setup(ctlr_info_t *h, struct cciss_scsi_adapter_data_t *sa)
struct cciss_scsi_cmd_stack_t *stk;
size_t size;

+ stk = &sa->cmd_stack;
+ stk->nelems = cciss_tape_cmds + 2;
sa->cmd_sg_list = cciss_allocate_sg_chain_blocks(h,
- h->chainsize, CMD_STACK_SIZE);
+ h->chainsize, stk->nelems);
if (!sa->cmd_sg_list && h->chainsize > 0)
return -ENOMEM;

- stk = &sa->cmd_stack;
- size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * CMD_STACK_SIZE;
+ size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * stk->nelems;

/* Check alignment, see cciss_cmd.h near CommandList_struct def. */
BUILD_BUG_ON((sizeof(*stk->pool) % COMMANDLIST_ALIGNMENT) != 0);
@@ -221,18 +218,23 @@ scsi_cmd_stack_setup(ctlr_info_t *h, struct cciss_scsi_adapter_data_t *sa)
pci_alloc_consistent(h->pdev, size, &stk->cmd_pool_handle);

if (stk->pool == NULL) {
- cciss_free_sg_chain_blocks(sa->cmd_sg_list, CMD_STACK_SIZE);
+ cciss_free_sg_chain_blocks(sa->cmd_sg_list, stk->nelems);
sa->cmd_sg_list = NULL;
return -ENOMEM;
}
-
- for (i=0; i<CMD_STACK_SIZE; i++) {
+ stk->elem = kmalloc(sizeof(stk->elem[0]) * stk->nelems, GFP_KERNEL);
+ if (!stk->elem) {
+ pci_free_consistent(h->pdev, size, stk->pool,
+ stk->cmd_pool_handle);
+ return -1;
+ }
+ for (i = 0; i < stk->nelems; i++) {
stk->elem[i] = &stk->pool[i];
stk->elem[i]->busaddr = (__u32) (stk->cmd_pool_handle +
(sizeof(struct cciss_scsi_cmd_stack_elem_t) * i));
stk->elem[i]->cmdindex = i;
}
- stk->top = CMD_STACK_SIZE-1;
+ stk->top = stk->nelems-1;
return 0;
}

@@ -245,16 +247,18 @@ scsi_cmd_stack_free(ctlr_info_t *h)

sa = h->scsi_ctlr;
stk = &sa->cmd_stack;
- if (stk->top != CMD_STACK_SIZE-1) {
+ if (stk->top != stk->nelems-1) {
dev_warn(&h->pdev->dev,
"bug: %d scsi commands are still outstanding.\n",
- CMD_STACK_SIZE - stk->top);
+ stk->nelems - stk->top);
}
- size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * CMD_STACK_SIZE;
+ size = sizeof(struct cciss_scsi_cmd_stack_elem_t) * stk->nelems;

pci_free_consistent(h->pdev, size, stk->pool, stk->cmd_pool_handle);
stk->pool = NULL;
- cciss_free_sg_chain_blocks(sa->cmd_sg_list, CMD_STACK_SIZE);
+ cciss_free_sg_chain_blocks(sa->cmd_sg_list, stk->nelems);
+ kfree(stk->elem);
+ stk->elem = NULL;
}

#if 0
@@ -859,6 +863,7 @@ cciss_scsi_detect(ctlr_info_t *h)
sh->io_port = 0; // good enough? FIXME,
sh->n_io_port = 0; // I don't think we use these two...
sh->this_id = SELF_SCSI_ID;
+ sh->can_queue = cciss_tape_cmds;
sh->sg_tablesize = h->maxsgentries;
sh->max_cmd_len = MAX_COMMAND_SIZE;

diff --git a/drivers/block/cciss_scsi.h b/drivers/block/cciss_scsi.h
index 6d5822f..e71d986 100644
--- a/drivers/block/cciss_scsi.h
+++ b/drivers/block/cciss_scsi.h
@@ -36,13 +36,9 @@
addressible natively, and may in fact turn
out to be not scsi at all. */

-#define SCSI_CCISS_CAN_QUEUE 2

/*

-Note, cmd_per_lun could give us some trouble, so I'm setting it very low.
-Likewise, SCSI_CCISS_CAN_QUEUE is set very conservatively.
-
If the upper scsi layer tries to track how many commands we have
outstanding, it will be operating under the misapprehension that it is
the only one sending us requests. We also have the block interface,

2011-05-06 14:29:15

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 00/16] cciss: May 3, 2011 updates

On 2011-05-03 13:52, Stephen M. Cameron wrote:
> The following series mostly contains fixes to improve kdump behavior
> and esp. to make older controllers which cannot be hard reset work by
> doing a soft reset instead. There are a few patches factoring out
> various functionality into individual functions make way for the soft
> reset functionality. There is also a bugfix which prevents PCI write
> combining from potentially causing commands to get lost.

Applied to for-2.6.40/drivers


--
Jens Axboe