From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: linux1394-devel@lists.sourceforge.net
Cc: Ben Collins <bcollins@ubuntu.com>, linux-kernel@vger.kernel.org
Subject: [PATCH 2.6.18-rc1-mm2 5/6] ieee1394: sbp2: more checks of status block
Date: Sun, 23 Jul 2006 22:59:47 +0200 (CEST) [thread overview]
Message-ID: <tkrat.1e07144d0042077e@s5r6.in-berlin.de> (raw)
In-Reply-To: <tkrat.25e69df87688def6@s5r6.in-berlin.de>
- Add checks for the (very unlikely) cases that the target writes too
little or too much status data or writes unsolicited status.
- Indicate that these and similar conditions are unlikely().
- Check the 'resp' and 'sbp_status' fields for possible failure status.
- Slightly optimize access macros for the status block bitfields.
- Unify a few related log messages.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
drivers/ieee1394/sbp2.c | 70 +++++++++++++++++-----------------------
drivers/ieee1394/sbp2.h | 14 ++++----
2 files changed, 38 insertions(+), 46 deletions(-)
TODO:
Check if status' 'src'==1, then withhold the respective ORB from reuse
until status for any subsequent ORB was received.
Index: linux/drivers/ieee1394/sbp2.h
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.h 2006-07-23 10:11:34.000000000 +0200
+++ linux/drivers/ieee1394/sbp2.h 2006-07-23 10:21:31.000000000 +0200
@@ -180,12 +180,14 @@ struct sbp2_unrestricted_page_table {
#define SBP2_SCSI_STATUS_SELECTION_TIMEOUT 0xff
-#define STATUS_GET_ORB_OFFSET_HI(value) (value & 0xffff)
-#define STATUS_GET_SBP_STATUS(value) ((value >> 16) & 0xff)
-#define STATUS_GET_LENGTH(value) ((value >> 24) & 0x7)
-#define STATUS_GET_DEAD_BIT(value) ((value >> 27) & 0x1)
-#define STATUS_GET_RESP(value) ((value >> 28) & 0x3)
-#define STATUS_GET_SRC(value) ((value >> 30) & 0x3)
+#define STATUS_GET_SRC(value) (((value) >> 30) & 0x3)
+#define STATUS_GET_LEN(value) (((value) >> 24) & 0x7)
+#define STATUS_GET_ORB_OFFSET_HI(value) ((value) & 0x0000ffff)
+#define STATUS_TEST_D(value) ((value) & 0x08000000)
+/* test 'resp' | 'sbp2_status' */
+#define STATUS_TEST_RS(value) ((value) & 0x30ff0000)
+/* test 'resp' | 'dead' | 'sbp2_status' */
+#define STATUS_TEST_RDS(value) ((value) & 0x38ff0000)
struct sbp2_status_block {
u32 ORB_offset_hi_misc;
Index: linux/drivers/ieee1394/sbp2.c
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.c 2006-07-23 10:21:07.000000000 +0200
+++ linux/drivers/ieee1394/sbp2.c 2006-07-23 10:21:31.000000000 +0200
@@ -1201,11 +1201,8 @@ static int sbp2_query_logins(struct scsi
return -EIO;
}
- if (STATUS_GET_RESP(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_DEAD_BIT(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_SBP_STATUS(scsi_id->status_block.ORB_offset_hi_misc)) {
-
- SBP2_INFO("Error querying logins to SBP-2 device - timed out");
+ if (STATUS_TEST_RDS(scsi_id->status_block.ORB_offset_hi_misc)) {
+ SBP2_INFO("Error querying logins to SBP-2 device - failed");
return -EIO;
}
@@ -1298,18 +1295,12 @@ static int sbp2_login_device(struct scsi
* Sanity. Make sure status returned matches login orb.
*/
if (scsi_id->status_block.ORB_offset_lo != scsi_id->login_orb_dma) {
- SBP2_ERR("Error logging into SBP-2 device - login timed-out");
+ SBP2_ERR("Error logging into SBP-2 device - timed out");
return -EIO;
}
- /*
- * Check status
- */
- if (STATUS_GET_RESP(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_DEAD_BIT(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_SBP_STATUS(scsi_id->status_block.ORB_offset_hi_misc)) {
-
- SBP2_ERR("Error logging into SBP-2 device - login failed");
+ if (STATUS_TEST_RDS(scsi_id->status_block.ORB_offset_hi_misc)) {
+ SBP2_ERR("Error logging into SBP-2 device - failed");
return -EIO;
}
@@ -1333,9 +1324,7 @@ static int sbp2_login_device(struct scsi
scsi_id->sbp2_command_block_agent_addr &= 0x0000ffffffffffffULL;
SBP2_INFO("Logged into SBP-2 device");
-
return 0;
-
}
/*
@@ -1466,25 +1455,17 @@ static int sbp2_reconnect_device(struct
* Sanity. Make sure status returned matches reconnect orb.
*/
if (scsi_id->status_block.ORB_offset_lo != scsi_id->reconnect_orb_dma) {
- SBP2_ERR("Error reconnecting to SBP-2 device - reconnect timed-out");
+ SBP2_ERR("Error reconnecting to SBP-2 device - timed out");
return -EIO;
}
- /*
- * Check status
- */
- if (STATUS_GET_RESP(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_DEAD_BIT(scsi_id->status_block.ORB_offset_hi_misc) ||
- STATUS_GET_SBP_STATUS(scsi_id->status_block.ORB_offset_hi_misc)) {
-
- SBP2_ERR("Error reconnecting to SBP-2 device - reconnect failed");
+ if (STATUS_TEST_RDS(scsi_id->status_block.ORB_offset_hi_misc)) {
+ SBP2_ERR("Error reconnecting to SBP-2 device - failed");
return -EIO;
}
HPSB_DEBUG("Reconnected to SBP-2 device");
-
return 0;
-
}
/*
@@ -2115,18 +2096,19 @@ static int sbp2_handle_status_write(stru
sbp2util_packet_dump(data, length, "sbp2 status write by device", (u32)addr);
- if (!host) {
+ if (unlikely(length < 8 || length > sizeof(struct sbp2_status_block))) {
+ SBP2_ERR("Wrong size of status block");
+ return RCODE_ADDRESS_ERROR;
+ }
+ if (unlikely(!host)) {
SBP2_ERR("host is NULL - this is bad!");
return RCODE_ADDRESS_ERROR;
}
-
hi = hpsb_get_hostinfo(&sbp2_highlevel, host);
-
- if (!hi) {
+ if (unlikely(!hi)) {
SBP2_ERR("host info is NULL - this is bad!");
return RCODE_ADDRESS_ERROR;
}
-
/*
* Find our scsi_id structure by looking at the status fifo address
* written to by the sbp2 device.
@@ -2138,8 +2120,7 @@ static int sbp2_handle_status_write(stru
break;
}
}
-
- if (!scsi_id) {
+ if (unlikely(!scsi_id)) {
SBP2_ERR("scsi_id is NULL - device is gone?");
return RCODE_ADDRESS_ERROR;
}
@@ -2156,12 +2137,14 @@ static int sbp2_handle_status_write(stru
sbp2util_be32_to_cpu_buffer(sb, 8);
/*
- * Handle command ORB status here if necessary. First, need to match
- * status with command.
+ * Ignore unsolicited status. Handle command ORB status.
*/
- command = sbp2util_find_command_for_orb(scsi_id, sb->ORB_offset_lo);
+ if (unlikely(STATUS_GET_SRC(sb->ORB_offset_hi_misc) == 2))
+ command = NULL;
+ else
+ command = sbp2util_find_command_for_orb(scsi_id,
+ sb->ORB_offset_lo);
if (command) {
-
SBP2_DEBUG("Found status for command ORB");
pci_dma_sync_single_for_cpu(hi->host->pdev, command->command_orb_dma,
sizeof(struct sbp2_command_orb),
@@ -2177,16 +2160,23 @@ static int sbp2_handle_status_write(stru
* Matched status with command, now grab scsi command pointers
* and check status.
*/
+ /*
+ * FIXME: If the src field in the status is 1, the ORB DMA must
+ * not be reused until status for a subsequent ORB is received.
+ */
SCpnt = command->Current_SCpnt;
spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
sbp2util_mark_command_completed(scsi_id, command);
spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
if (SCpnt) {
+ if (STATUS_TEST_RS(sb->ORB_offset_hi_misc))
+ scsi_status =
+ SBP2_SCSI_STATUS_COMMAND_TERMINATED;
/*
* See if the target stored any scsi status information.
*/
- if (STATUS_GET_LENGTH(sb->ORB_offset_hi_misc) > 1) {
+ if (STATUS_GET_LEN(sb->ORB_offset_hi_misc) > 1) {
SBP2_DEBUG("CHECK CONDITION");
scsi_status = sbp2_status_to_sense_data(
(unchar *)sb, SCpnt->sense_buffer);
@@ -2196,7 +2186,7 @@ static int sbp2_handle_status_write(stru
* Check to see if the dead bit is set. If so, we'll
* have to initiate a fetch agent reset.
*/
- if (STATUS_GET_DEAD_BIT(sb->ORB_offset_hi_misc)) {
+ if (STATUS_TEST_D(sb->ORB_offset_hi_misc)) {
SBP2_DEBUG("Dead bit set - "
"initiating fetch agent reset");
sbp2_agent_reset(scsi_id, 0);
next prev parent reply other threads:[~2006-07-23 21:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-07-23 20:53 [PATCH 2.6.18-rc1-mm2 0/6] ieee1394: sbp2: misc updates Stefan Richter
2006-07-23 20:55 ` [PATCH 2.6.18-rc1-mm2 1/6] ieee1394: sbp2: safer last_orb and next_ORB handling Stefan Richter
2006-07-23 20:56 ` [PATCH 2.6.18-rc1-mm2 2/6] ieee1394: sbp2: discard return value of sbp2_link_orb_command Stefan Richter
2006-07-23 20:57 ` [PATCH 2.6.18-rc1-mm2 3/6] ieee1394: sbp2: optimize DMA direction of command ORBs Stefan Richter
2006-07-23 20:58 ` [PATCH 2.6.18-rc1-mm2 4/6] ieee1394: sbp2: safer initialization of status fifo Stefan Richter
2006-07-23 20:59 ` Stefan Richter [this message]
2006-07-23 21:00 ` [PATCH 2.6.18-rc1-mm2 6/6] ieee1394: sbp2: convert sbp2util_down_timeout to waitqueue Stefan Richter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=tkrat.1e07144d0042077e@s5r6.in-berlin.de \
--to=stefanr@s5r6.in-berlin.de \
--cc=bcollins@ubuntu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux1394-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®