Skip to contentSkip to conversation

hptiop: HighPoint RocketRAID 3xxx controller driver

PATCH In review HighPoint Linux Team 5 participants 11 messages May 16, 2006 – Oct 15, 2007 2 messages not in the archive

Subsystems (3)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

HighPoint Linux Team

author maintainer
#2 lore raw

A wholesale replacement for the first submission.

Merge Andrew Morton's patches:
- Provide locking for global list
- Fix debug printks
- uninline function with multiple callsites
- coding style fixups
- remove unneeded casts of void*
- kfree(NULL) is legal
- Don't "succeed" if register_chrdev() failed - otherwise we'll later unregister a not-registered chrdev.
- Don't return from hptiop_do_ioctl() with the spinlock held.
- uninline __hpt_do_ioctl()

Update for Arjan van de Ven's comments:
- put all asm/ includes after the linux/ ones
- replace mdelay with msleep
- add pci posting flush
- do not set pci command reqister in map_pci_bar
- do not try merging sg elements in hptiop_buildsgl()
- remove unused outstandingcommands member from hba structure
- remove unimplemented hptiop_abort() handler
- remove typedef u32 hpt_id_t

Other updates:
- fix endianess

---

 Documentation/scsi/hptiop.txt |   92 +++
 MAINTAINERS                   |    6 
 drivers/scsi/Kconfig          |   10 
 drivers/scsi/Makefile         |    1 
 drivers/scsi/hptiop.c         | 1501 +++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/hptiop.h         |  465 +++++++++++++
 6 files changed, 2075 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/scsi/hptiop.txt
 create mode 100644 drivers/scsi/hptiop.c
 create mode 100644 drivers/scsi/hptiop.h
Documentation/scsi/hptiop.txt new file +92 -0
new file mode 100644
index 0000000..d28a312
@@ -0,0 +1,92 @@
+HIGHPOINT ROCKETRAID 3xxx RAID DRIVER (hptiop)
+
+Controller Register Map
+-------------------------
+
+The controller IOP is accessed via PCI BAR0.
+
+     BAR0 offset    Register
+            0x10    Inbound Message Register 0
+            0x14    Inbound Message Register 1
+            0x18    Outbound Message Register 0
+            0x1C    Outbound Message Register 1
+            0x20    Inbound Doorbell Register
+            0x24    Inbound Interrupt Status Register
+            0x28    Inbound Interrupt Mask Register
+            0x30    Outbound Interrupt Status Register
+            0x34    Outbound Interrupt Mask Register
+            0x40    Inbound Queue Port
+            0x44    Outbound Queue Port
+
+
+I/O Request Workflow
+----------------------
+
+All queued requests are handled via inbound/outbound queue port.
+A request packet can be allocated in either IOP or host memory.
+
+To send a request to the controller:
+
+    - Get a free request packet by reading the inbound queue port or
+      allocate a free request in host DMA coherent memory.
+
+      The value returned from the inbound queue port is an offset
+      relative to the IOP BAR0.
+
+      Requests allocated in host memory must be aligned on 32-bytes boundary.
+
+    - Fill the packet.
+
+    - Post the packet to IOP by writing it to inbound queue. For requests
+      allocated in IOP memory, write the offset to inbound queue port. For
+      requests allocated in host memory, write (0x80000000|(bus_addr>>5))
+      to the inbound queue port.
+
+    - The IOP process the request. When the request is completed, it
+      will be put into outbound queue. An outbound interrupt will be
+      generated.
+
+      For requests allocated in IOP memory, the request offset is posted to
+      outbound queue.
+
+      For requests allocated in host memory, (0x80000000|(bus_addr>>5))
+      is posted to the outbound queue. If IOP_REQUEST_FLAG_OUTPUT_CONTEXT
+      flag is set in the request, the low 32-bit context value will be
+      posted instead.
+
+    - The host read the outbound queue and complete the request.
+
+      For requests allocated in IOP memory, the host driver free the request
+      by writing it to the outbound queue.
+
+Non-queued requests (reset/flush etc) can be sent via inbound message
+register 0. An outbound message with the same value indicates the completion
+of an inbound message.
+
+
+User-level Interface
+---------------------
+
+The driver exposes following sysfs attributes:
+
+     NAME                 R/W    Description
+     driver-version        R     driver version string
+     firmware-version      R     firmware version string
+
+The driver registers char device "hptiop" to communicate with HighPoint RAID
+management software. Its ioctl routine acts as a general binary interface 
+between the IOP firmware and HighPoint RAID management software. New management
+functions can be implemented in application/firmware without modification
+in driver code.
+
+
+-----------------------------------------------------------------------------
+Copyright (C) 2006 HighPoint Technologies, Inc. All Rights Reserved.
+
+  This file is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  [email protected]
+  http://www.highpoint-tech.com
MAINTAINERS +6 -0
index 246153d..f83c892 100644
@@ -1126,6 +1126,12 @@ L:	[email protected]
 W:	http://www.nt.tuwien.ac.at/~kkudielk/Linux/
 S:	Maintained
 
+HIGHPOINT ROCKETRAID 3xxx RAID DRIVER
+P:	HighPoint Linux Team
+M:	[email protected]
+W:	http://www.highpoint-tech.com
+S:	Supported
+
 HIPPI
 P:	Jes Sorensen
 M:	[email protected]
drivers/scsi/Kconfig +10 -0
index a480a37..44728ae 100644
@@ -532,6 +532,16 @@ config SCSI_PDC_ADMA
 
 	  If unsure, say N.
 
+config SCSI_HPTIOP
+	tristate "HighPoint RocketRAID 3xxx Controller support"
+	depends on SCSI && PCI
+	help
+	  This option enables support for HighPoint RocketRAID 3xxx
+	  controllers.
+
+	  To compile this driver as a module, choose M here; the module
+	  will be called hptiop. If unsure, say N.
+
 config SCSI_SATA_QSTOR
 	tristate "Pacific Digital SATA QStor support"
 	depends on SCSI_SATA && PCI
drivers/scsi/Makefile +1 -0
index 81803a1..e7de6a0 100644
@@ -136,6 +136,7 @@ obj-$(CONFIG_SCSI_SATA_NV)	+= libata.o s
 obj-$(CONFIG_SCSI_SATA_ULI)	+= libata.o sata_uli.o
 obj-$(CONFIG_SCSI_SATA_MV)	+= libata.o sata_mv.o
 obj-$(CONFIG_SCSI_PDC_ADMA)	+= libata.o pdc_adma.o
+obj-$(CONFIG_SCSI_HPTIOP)	+= hptiop.o
 
 obj-$(CONFIG_ARM)		+= arm/
 
drivers/scsi/hptiop.c new file +1501 -0

1,501 lines not on this page · raw message

drivers/scsi/hptiop.h new file +465 -0

465 lines not on this page · raw message

#3 lore raw

On Tue, 2006-05-16 at 14:38 +0800, HighPoint Linux Team wrote:

	_req = get_req(hba);
	if (_req == NULL) {
		dprintk("hptiop_queuecmd : no free req\n");
		scp->result = DID_BUS_BUSY << 16;
		goto cmd_done;
	}

This should be doing a return SCSI_MLQUEUE_HOST_BUSY. DID_BUS_BUSY doesn't do the resource contention counting that you want (MLQUEUE_HOST_BUSY will wait until a command returns ... presumably freeing up resources before trying another).

	/*
	 * hptiop_shutdown will flash controller cache.
	 */
	if (scp->cmnd[0] == SYNCHRONIZE_CACHE)  {
		scp->result = DID_OK<<16;
		goto cmd_done;
	}

Are you really sure you want to do this? It looks like we'll be doing this in cases where shutdown won't be called (like suspend).

	host->can_queue = le32_to_cpu(iop_config.max_requests);
	host->cmd_per_lun = le32_to_cpu(iop_config.max_requests);

You might want to think about adjusting this. For the single LUN case, it's fine. For the multi-lun case it may allow commands to a single LUN to starve everything else.

However, these are minor points ... I'll put the driver in and you can fix them up later.

James

#4 lore raw
Quoted 2 lines · On Sat, 10 Jun 2006 10:36:22 -0500 James Bottomley…

I'll put the driver in and you can
fix them up later.

um, which driver? I think this email is a reply to one of the hoghpoint.com submissions? Please merge the patch I sent - it is the latest version and has fixes, I think.

I'll resend.

#5 lore raw
Quoted 3 lines · Andrew Morton

um, which driver? I think this email is a reply to one of the hoghpoint.com submissions? Please merge the patch I sent - it is the latest version and has fixes, I think.

The latest version that was sent to the SCSI list ... there shouldn't have been any updates, even in the mm tree, that didn't go over the list ...

James

author maintainer
#6 lore raw

PATCH 1/1 hptiop: adding new firmware interface and more PCI device IDs

In reply to HighPoint Linux Team

- check adapter firmware version and using appropriate interface accordingly
- add new PCI device IDs
- update driver version string

---

 hptiop.c |   57 +++++++++++++++++++++++++++++++++++++++++++--------------
 hptiop.h |    9 +++++++--
 2 files changed, 50 insertions(+), 16 deletions(-)
drivers/scsi/hptiop.c +43 -14
diff -uprN a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
@@ -1,6 +1,6 @@
 /*
  * HighPoint RR3xxx controller driver for Linux
- * Copyright (C) 2006 HighPoint Technologies, Inc. All Rights Reserved.
+ * Copyright (C) 2006-2007 HighPoint Technologies, Inc. All Rights Reserved.
  *
  * 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
@@ -42,7 +42,7 @@ MODULE_DESCRIPTION("HighPoint RocketRAID
 
 static char driver_name[] = "hptiop";
 static const char driver_name_long[] = "RocketRAID 3xxx SATA Controller driver";
-static const char driver_ver[] = "v1.0 (060426)";
+static const char driver_ver[] = "v1.2 (070810)";
 
 static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 tag);
 static void hptiop_iop_request_callback(struct hptiop_hba *hba, u32 tag);
@@ -76,7 +76,7 @@ static int iop_wait_ready(struct hpt_iop
 
 static void hptiop_request_callback(struct hptiop_hba *hba, u32 tag)
 {
-	if ((tag & IOPMU_QUEUE_MASK_HOST_BITS) == IOPMU_QUEUE_ADDR_HOST_BIT)
+	if (tag & IOPMU_QUEUE_ADDR_HOST_BIT)
 		return hptiop_host_request_callback(hba,
 				tag & ~IOPMU_QUEUE_ADDR_HOST_BIT);
 	else
@@ -323,12 +323,22 @@ static inline void free_req(struct hptio
 	hba->req_list = req;
 }
 
-static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 tag)
+static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 _tag)
 {
 	struct hpt_iop_request_scsi_command *req;
 	struct scsi_cmnd *scp;
+	u32 tag;
+
+	if (hba->firmware_version > 0x01020000 || hba->interface_version > 0x01020000) {
+		tag = _tag & ~ IOPMU_QUEUE_REQUEST_RESULT_BIT;
+		req = (struct hpt_iop_request_scsi_command *)hba->reqs[tag].req_virt;
+		if (likely(_tag & IOPMU_QUEUE_REQUEST_RESULT_BIT))
+			req->header.result = IOP_RESULT_SUCCESS;
+	} else {
+		tag = _tag;
+		req = (struct hpt_iop_request_scsi_command *)hba->reqs[tag].req_virt;
+	}
 
-	req = (struct hpt_iop_request_scsi_command *)hba->reqs[tag].req_virt;
 	dprintk("hptiop_host_request_callback: req=%p, type=%d, "
 			"result=%d, context=0x%x tag=%d\n",
 			req, req->header.type, req->header.result,
@@ -521,8 +531,20 @@ static int hptiop_queuecommand(struct sc
 
 	memcpy(req->cdb, scp->cmnd, sizeof(req->cdb));
 
-	writel(IOPMU_QUEUE_ADDR_HOST_BIT | _req->req_shifted_phy,
-			&hba->iop->inbound_queue);
+	if (hba->firmware_version > 0x01020000 ||
+				hba->interface_version > 0x01020000) {
+		u32 size_bits;
+		if (req->header.size < 256)
+			size_bits = IOPMU_QUEUE_REQUEST_SIZE_BIT;
+		else if (req->header.size < 512)
+			size_bits = IOPMU_QUEUE_ADDR_HOST_BIT;
+		else
+			size_bits = IOPMU_QUEUE_REQUEST_SIZE_BIT |
+						IOPMU_QUEUE_ADDR_HOST_BIT;
+		writel(_req->req_shifted_phy | size_bits, &hba->iop->inbound_queue);
+	} else
+		writel(_req->req_shifted_phy | IOPMU_QUEUE_ADDR_HOST_BIT,
+					&hba->iop->inbound_queue);
 
 	return 0;
 
@@ -722,6 +744,7 @@ static int __devinit hptiop_probe(struct
 	hba->max_request_size = le32_to_cpu(iop_config.request_size);
 	hba->max_sg_descriptors = le32_to_cpu(iop_config.max_sg_count);
 	hba->firmware_version = le32_to_cpu(iop_config.firmware_version);
+	hba->interface_version = le32_to_cpu(iop_config.interface_version);
 	hba->sdram_size = le32_to_cpu(iop_config.sdram_size);
 
 	host->max_sectors = le32_to_cpu(iop_config.data_transfer_length) >> 9;
@@ -731,8 +754,15 @@ static int __devinit hptiop_probe(struct
 	host->cmd_per_lun = le32_to_cpu(iop_config.max_requests);
 	host->max_cmd_len = 16;
 
-	set_config.vbus_id = cpu_to_le32(host->host_no);
+	req_size = sizeof(struct hpt_iop_request_scsi_command)
+		+ sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
+	if ((req_size & 0x1f) != 0)
+		req_size = (req_size + 0x1f) & ~0x1f;
+
+	memset(&set_config, 0, sizeof(struct hpt_iop_request_set_config));
 	set_config.iop_id = cpu_to_le32(host->host_no);
+	set_config.vbus_id = cpu_to_le16(host->host_no);
+	set_config.max_host_request_size = cpu_to_le16(req_size);
 
 	if (iop_set_config(hba, &set_config)) {
 		printk(KERN_ERR "scsi%d: set config failed\n",
@@ -750,10 +780,6 @@ static int __devinit hptiop_probe(struct
 	}
 
 	/* Allocate request mem */
-	req_size = sizeof(struct hpt_iop_request_scsi_command)
-		+ sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
-	if ((req_size& 0x1f) != 0)
-		req_size = (req_size + 0x1f) & ~0x1f;
 
 	dprintk("req_size=%d, max_requests=%d\n", req_size, hba->max_requests);
 
@@ -879,8 +905,10 @@ static void hptiop_remove(struct pci_dev
 }
 
 static struct pci_device_id hptiop_id_table[] = {
-	{ PCI_DEVICE(0x1103, 0x3220) },
-	{ PCI_DEVICE(0x1103, 0x3320) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3220) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3320) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3520) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x4320) },
 	{},
 };
 
@@ -910,3 +938,4 @@ module_init(hptiop_module_init);
 module_exit(hptiop_module_exit);
 
 MODULE_LICENSE("GPL");
+
drivers/scsi/hptiop.h +7 -2
diff -uprN a/drivers/scsi/hptiop.h b/drivers/scsi/hptiop.h
@@ -1,6 +1,6 @@
 /*
  * HighPoint RR3xxx controller driver for Linux
- * Copyright (C) 2006 HighPoint Technologies, Inc. All Rights Reserved.
+ * Copyright (C) 2006-2007 HighPoint Technologies, Inc. All Rights Reserved.
  *
  * 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
@@ -252,6 +252,8 @@ struct hpt_iopmu
 #define IOPMU_QUEUE_EMPTY            0xffffffff
 #define IOPMU_QUEUE_MASK_HOST_BITS   0xf0000000
 #define IOPMU_QUEUE_ADDR_HOST_BIT    0x80000000
+#define IOPMU_QUEUE_REQUEST_SIZE_BIT    0x40000000
+#define IOPMU_QUEUE_REQUEST_RESULT_BIT   0x40000000
 
 #define IOPMU_OUTBOUND_INT_MSG0      1
 #define IOPMU_OUTBOUND_INT_MSG1      2
@@ -336,7 +338,8 @@ struct hpt_iop_request_set_config
 {
 	struct hpt_iop_request_header header;
 	__le32 iop_id;
-	__le32 vbus_id;
+	__le16 vbus_id;
+	__le16 max_host_request_size;
 	__le32 reserve[6];
 };
 
@@ -415,6 +418,7 @@ struct hptiop_hba {
 	struct list_head link;
 
 	/* IOP config info */
+	u32     interface_version;
 	u32     firmware_version;
 	u32     sdram_size;
 	u32     max_devices;
@@ -463,3 +467,4 @@ struct hpt_ioctl_k
 #endif
 
 #endif
+
#7 lore raw

Re: [PATCH] hptiop: adding new firmware interface and more PCI device IDs

Quoted 1 line · HighPoint Linux Team
drivers/scsi/hptiop.c +1 -0
+	if (hba->firmware_version > 0x01020000 || hba->interface_version > 0x01020000) {

[...]

drivers/scsi/hptiop.c static int hptiop_queuecommand(struct sc · line 535

Quoted 2 lines
drivers/scsi/hptiop.c +2 -0
+	if (hba->firmware_version > 0x01020000 ||
+				hba->interface_version > 0x01020000) {

Rather than repeating this test, you should do it once at probe time, and set a "new interface" single-bit flag in some hptiop_hba field.

Less maintenance intensive, less prone to error, and removes expensive tests from the driver hotpath.

drivers/scsi/hptiop.c static void hptiop_remove(struct pci_dev · line 912

Quoted 8 lines

static struct pci_device_id hptiop_id_table[] = {

drivers/scsi/hptiop.c +4 -2
-	{ PCI_DEVICE(0x1103, 0x3220) },
-	{ PCI_DEVICE(0x1103, 0x3320) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3220) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3320) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x3520) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x4320) },
 	{},

Using the PCI_VDEVICE() macro can make this even shorter:

	{ PCI_VDEVICE(TTI, 0x4320) }

Regards,

Jeff

author maintainer
#8 lore raw

[PATCH/RESENT] hptiop: adding new firmware interface and more PCI device IDs

In reply to HighPoint Linux Team

updated patch based on Jeff Garzik's comments.

- check adapter firmware version and use appropriate interface accordingly
- add new PCI device IDs and use PCI_VDEVICE macro
- update driver version string
- remove unused data structures
- remove unnecessary typecasts

---
 drivers/scsi/hptiop.c |   63 +++++++++++++----
 drivers/scsi/hptiop.h |  229 ++-------------------------------------------------------------
 2 files changed, 58 insertions(+), 234 deletions(-)
drivers/scsi/hptiop.c +48 -15
diff -urpN a/drivers/scsi/hptiop.c b/drivers/scsi/hptiop.c
@@ -1,6 +1,6 @@
 /*
  * HighPoint RR3xxx controller driver for Linux
- * Copyright (C) 2006 HighPoint Technologies, Inc. All Rights Reserved.
+ * Copyright (C) 2006-2007 HighPoint Technologies, Inc. All Rights Reserved.
  *
  * 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
@@ -42,7 +42,7 @@ MODULE_DESCRIPTION("HighPoint RocketRAID
 
 static char driver_name[] = "hptiop";
 static const char driver_name_long[] = "RocketRAID 3xxx SATA Controller driver";
-static const char driver_ver[] = "v1.0 (060426)";
+static const char driver_ver[] = "v1.2 (070830)";
 
 static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 tag);
 static void hptiop_iop_request_callback(struct hptiop_hba *hba, u32 tag);
@@ -76,7 +76,7 @@ static int iop_wait_ready(struct hpt_iop
 
 static void hptiop_request_callback(struct hptiop_hba *hba, u32 tag)
 {
-	if ((tag & IOPMU_QUEUE_MASK_HOST_BITS) == IOPMU_QUEUE_ADDR_HOST_BIT)
+	if (tag & IOPMU_QUEUE_ADDR_HOST_BIT)
 		return hptiop_host_request_callback(hba,
 				tag & ~IOPMU_QUEUE_ADDR_HOST_BIT);
 	else
@@ -323,12 +323,22 @@ static inline void free_req(struct hptio
 	hba->req_list = req;
 }
 
-static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 tag)
+static void hptiop_host_request_callback(struct hptiop_hba *hba, u32 _tag)
 {
 	struct hpt_iop_request_scsi_command *req;
 	struct scsi_cmnd *scp;
+	u32 tag;
+
+	if (hba->iopintf_v2) {
+		tag = _tag & ~ IOPMU_QUEUE_REQUEST_RESULT_BIT;
+		req = hba->reqs[tag].req_virt;
+		if (likely(_tag & IOPMU_QUEUE_REQUEST_RESULT_BIT))
+			req->header.result = IOP_RESULT_SUCCESS;
+	} else {
+		tag = _tag;
+		req = hba->reqs[tag].req_virt;
+	}
 
-	req = (struct hpt_iop_request_scsi_command *)hba->reqs[tag].req_virt;
 	dprintk("hptiop_host_request_callback: req=%p, type=%d, "
 			"result=%d, context=0x%x tag=%d\n",
 			req, req->header.type, req->header.result,
@@ -497,7 +507,7 @@ static int hptiop_queuecommand(struct sc
 		goto cmd_done;
 	}
 
-	req = (struct hpt_iop_request_scsi_command *)_req->req_virt;
+	req = _req->req_virt;
 
 	/* build S/G table */
 	sg_count = hptiop_buildsgl(scp, req->sg_list);
@@ -521,8 +531,19 @@ static int hptiop_queuecommand(struct sc
 
 	memcpy(req->cdb, scp->cmnd, sizeof(req->cdb));
 
-	writel(IOPMU_QUEUE_ADDR_HOST_BIT | _req->req_shifted_phy,
-			&hba->iop->inbound_queue);
+	if (hba->iopintf_v2) {
+		u32 size_bits;
+		if (req->header.size < 256)
+			size_bits = IOPMU_QUEUE_REQUEST_SIZE_BIT;
+		else if (req->header.size < 512)
+			size_bits = IOPMU_QUEUE_ADDR_HOST_BIT;
+		else
+			size_bits = IOPMU_QUEUE_REQUEST_SIZE_BIT |
+						IOPMU_QUEUE_ADDR_HOST_BIT;
+		writel(_req->req_shifted_phy | size_bits, &hba->iop->inbound_queue);
+	} else
+		writel(_req->req_shifted_phy | IOPMU_QUEUE_ADDR_HOST_BIT,
+					&hba->iop->inbound_queue);
 
 	return 0;
 
@@ -688,6 +709,7 @@ static int __devinit hptiop_probe(struct
 	hba->pcidev = pcidev;
 	hba->host = host;
 	hba->initialized = 0;
+	hba->iopintf_v2 = 0;
 
 	atomic_set(&hba->resetting, 0);
 	atomic_set(&hba->reset_count, 0);
@@ -722,8 +744,13 @@ static int __devinit hptiop_probe(struct
 	hba->max_request_size = le32_to_cpu(iop_config.request_size);
 	hba->max_sg_descriptors = le32_to_cpu(iop_config.max_sg_count);
 	hba->firmware_version = le32_to_cpu(iop_config.firmware_version);
+	hba->interface_version = le32_to_cpu(iop_config.interface_version);
 	hba->sdram_size = le32_to_cpu(iop_config.sdram_size);
 
+	if (hba->firmware_version > 0x01020000 ||
+			hba->interface_version > 0x01020000)
+		hba->iopintf_v2 = 1;
+
 	host->max_sectors = le32_to_cpu(iop_config.data_transfer_length) >> 9;
 	host->max_id = le32_to_cpu(iop_config.max_devices);
 	host->sg_tablesize = le32_to_cpu(iop_config.max_sg_count);
@@ -731,8 +758,15 @@ static int __devinit hptiop_probe(struct
 	host->cmd_per_lun = le32_to_cpu(iop_config.max_requests);
 	host->max_cmd_len = 16;
 
-	set_config.vbus_id = cpu_to_le32(host->host_no);
+	req_size = sizeof(struct hpt_iop_request_scsi_command)
+		+ sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
+	if ((req_size & 0x1f) != 0)
+		req_size = (req_size + 0x1f) & ~0x1f;
+
+	memset(&set_config, 0, sizeof(struct hpt_iop_request_set_config));
 	set_config.iop_id = cpu_to_le32(host->host_no);
+	set_config.vbus_id = cpu_to_le16(host->host_no);
+	set_config.max_host_request_size = cpu_to_le16(req_size);
 
 	if (iop_set_config(hba, &set_config)) {
 		printk(KERN_ERR "scsi%d: set config failed\n",
@@ -750,10 +784,6 @@ static int __devinit hptiop_probe(struct
 	}
 
 	/* Allocate request mem */
-	req_size = sizeof(struct hpt_iop_request_scsi_command)
-		+ sizeof(struct hpt_iopsg) * (hba->max_sg_descriptors - 1);
-	if ((req_size& 0x1f) != 0)
-		req_size = (req_size + 0x1f) & ~0x1f;
 
 	dprintk("req_size=%d, max_requests=%d\n", req_size, hba->max_requests);
 
@@ -879,8 +909,10 @@ static void hptiop_remove(struct pci_dev
 }
 
 static struct pci_device_id hptiop_id_table[] = {
-	{ PCI_DEVICE(0x1103, 0x3220) },
-	{ PCI_DEVICE(0x1103, 0x3320) },
+	{ PCI_VDEVICE(TTI, 0x3220) },
+	{ PCI_VDEVICE(TTI, 0x3320) },
+	{ PCI_VDEVICE(TTI, 0x3520) },
+	{ PCI_VDEVICE(TTI, 0x4320) },
 	{},
 };
 
@@ -910,3 +942,4 @@ module_init(hptiop_module_init);
 module_exit(hptiop_module_exit);
 
 MODULE_LICENSE("GPL");
+
drivers/scsi/hptiop.h +10 -219

265 lines not on this page · raw message

#9 lore raw

Re: [PATCH/RESENT] hptiop: adding new firmware interface and more PCI device IDs

Quoted 9 lines · HighPoint Linux Team

updated patch based on Jeff Garzik's comments.

- check adapter firmware version and use appropriate interface accordingly
- add new PCI device IDs and use PCI_VDEVICE macro
- update driver version string
- remove unused data structures
- remove unnecessary typecasts

ACK

author maintainer
#10 lore raw

PATCH 1/1 hptiop: avoid buffer overflow when returning sense data

In reply to HighPoint Linux Team

avoid buffer overflow when returning sense data.

---

drivers/scsi/hptiop.c +3 -2
index 8b384fa..d32a4a9 100644
@@ -375,8 +375,9 @@ static void hptiop_host_request_callback
 		scp->result = SAM_STAT_CHECK_CONDITION;
 		memset(&scp->sense_buffer,
 				0, sizeof(scp->sense_buffer));
-		memcpy(&scp->sense_buffer,
-			&req->sg_list, le32_to_cpu(req->dataxfer_length));
+		memcpy(&scp->sense_buffer, &req->sg_list,
+				min(sizeof(scp->sense_buffer),
+					le32_to_cpu(req->dataxfer_length)));
 		break;
 
 	default:
#11 lore raw

Re: [PATCH] hptiop: avoid buffer overflow when returning sense data

Quoted 3 lines · On Mon, 15 Oct 2007 14:42:52 +0800 HighPoint Linux Team…

avoid buffer overflow when returning sense data.

That's really not enough information, sorry.

Quoted 5 lines

index 8b384fa..d32a4a9 100644

drivers/scsi/hptiop.c +0 -0
@@ -375,8 +375,9 @@ static void hptiop_host_request_callback
 		scp->result = SAM_STAT_CHECK_CONDITION;

drivers/scsi/hptiop.c static void hptiop_host_request_callback · line 383

Quoted 10 lines
drivers/scsi/hptiop.c +3 -2
 		memset(&scp->sense_buffer,
 				0, sizeof(scp->sense_buffer));
-		memcpy(&scp->sense_buffer,
-			&req->sg_list, le32_to_cpu(req->dataxfer_length));
+		memcpy(&scp->sense_buffer, &req->sg_list,
+				min(sizeof(scp->sense_buffer),
+					le32_to_cpu(req->dataxfer_length)));
 		break;
 
 	default:

See, we know what the fix does, but we don't know what the consequences are if the user's kernel doesn't have this fix.

So we are not able to work out whether this fix should be backported to 2.6.23.x and even to 2.6.22.x?

author maintainer
#12 lore raw

Re: [PATCH] hptiop: avoid buffer overflow when returning sense data

In reply to HighPoint Linux Team
Quoted 20 lines · Andrew Morton
Quoted 3 lines

avoid buffer overflow when returning sense data.

That's really not enough information, sorry.

Quoted 14 lines

index 8b384fa..d32a4a9 100644

drivers/scsi/hptiop.c +3 -2
@@ -375,8 +375,9 @@ static void hptiop_host_request_callback
 		scp->result = SAM_STAT_CHECK_CONDITION;
 		memset(&scp->sense_buffer,
 				0, sizeof(scp->sense_buffer));
-		memcpy(&scp->sense_buffer,
-			&req->sg_list, le32_to_cpu(req->dataxfer_length));
+		memcpy(&scp->sense_buffer, &req->sg_list,
+				min(sizeof(scp->sense_buffer),
+					le32_to_cpu(req->dataxfer_length)));
 		break;
 
Quoted 7 lines

drivers/scsi/hptiop.c static void hptiop_host_request_callback · line 383

Quoted 1 line
drivers/scsi/hptiop.c +0 -0
 	default:

See, we know what the fix does, but we don't know what the consequences are if the user's kernel doesn't have this fix.

So we are not able to work out whether this fix should be backported to 2.6.23.x and even to 2.6.22.x?

Sorry for ambiguity. With current adapter firmware the driver is working but future firmware updates may return sense data larger than 96 bytes, causing overflow on scp->sense_buffer and a kernel crash.

This fix should be backported to earlier kernels.

HighPoint Linux Team