ieee1394: sbp2: safer last_orb and next_ORB handling

The sbp2 initiator has two ways to tell a target's fetch agent about new
command ORBs:
 - Write the ORB's address to the ORB_POINTER register.  This must not
   be done while the fetch agent is active.
 - Put the ORB's address into the previously submitted ORB's next_ORB
   field and write to the DOORBELL register.  This may be done while the
   fetch agent is active or suspended.  It must not be done while the
   fetch agent is in reset state.
Sbp2 has a last_orb pointer which indicates in what way a new command
should be announced.  That pointer is concurrently accessed at various
occasions.  Furthermore, initiator and target are accessing the next_ORB
field of ORBs concurrently and asynchronously.

This patch does:
 - Protect all initiator accesses to last_orb by sbp2_command_orb_lock.
 - Add pci_dma_sync_single_for_device before a previously submitted
   ORB's next_ORB field is overwritten.
 - Insert a memory barrier between when next_ORB_lo and next_ORB_hi are
   overwritten.  Next_ORB_hi must not be updated before next_ORB_lo.
 - Remove the rather unspecific and now superfluous qualifier "volatile"
   from the next_ORB fields.
 - Add comments on how last_orb is connected with what is known about
   the target's fetch agent's state.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
This commit is contained in:
Stefan Richter 2006-07-23 22:12:00 +02:00
parent 9154df538f
commit cc07818912
2 changed files with 50 additions and 55 deletions

View file

@ -1705,6 +1705,7 @@ static int sbp2_agent_reset(struct scsi_id_instance_data *scsi_id, int wait)
quadlet_t data; quadlet_t data;
u64 addr; u64 addr;
int retval; int retval;
unsigned long flags;
SBP2_DEBUG_ENTER(); SBP2_DEBUG_ENTER();
@ -1724,7 +1725,9 @@ static int sbp2_agent_reset(struct scsi_id_instance_data *scsi_id, int wait)
/* /*
* Need to make sure orb pointer is written on next command * Need to make sure orb pointer is written on next command
*/ */
spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
scsi_id->last_orb = NULL; scsi_id->last_orb = NULL;
spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
return 0; return 0;
} }
@ -1966,8 +1969,12 @@ static int sbp2_link_orb_command(struct scsi_id_instance_data *scsi_id,
{ {
struct sbp2scsi_host_info *hi = scsi_id->hi; struct sbp2scsi_host_info *hi = scsi_id->hi;
struct sbp2_command_orb *command_orb = &command->command_orb; struct sbp2_command_orb *command_orb = &command->command_orb;
struct node_entry *ne = scsi_id->ne; struct sbp2_command_orb *last_orb;
u64 addr; dma_addr_t last_orb_dma;
u64 addr = scsi_id->sbp2_command_block_agent_addr;
quadlet_t data[2];
size_t length;
unsigned long flags;
outstanding_orb_incr; outstanding_orb_incr;
SBP2_ORB_DEBUG("sending command orb %p, total orbs = %x", SBP2_ORB_DEBUG("sending command orb %p, total orbs = %x",
@ -1982,64 +1989,50 @@ static int sbp2_link_orb_command(struct scsi_id_instance_data *scsi_id,
/* /*
* Check to see if there are any previous orbs to use * Check to see if there are any previous orbs to use
*/ */
if (scsi_id->last_orb == NULL) { spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
quadlet_t data[2]; last_orb = scsi_id->last_orb;
last_orb_dma = scsi_id->last_orb_dma;
if (!last_orb) {
/* /*
* Ok, let's write to the target's management agent register * last_orb == NULL means: We know that the target's fetch agent
* is not active right now.
*/ */
addr = scsi_id->sbp2_command_block_agent_addr + SBP2_ORB_POINTER_OFFSET; addr += SBP2_ORB_POINTER_OFFSET;
data[0] = ORB_SET_NODE_ID(hi->host->node_id); data[0] = ORB_SET_NODE_ID(hi->host->node_id);
data[1] = command->command_orb_dma; data[1] = command->command_orb_dma;
sbp2util_cpu_to_be32_buffer(data, 8); sbp2util_cpu_to_be32_buffer(data, 8);
length = 8;
SBP2_ORB_DEBUG("write command agent, command orb %p", command_orb);
if (sbp2util_node_write_no_wait(ne, addr, data, 8) < 0) {
SBP2_ERR("sbp2util_node_write_no_wait failed.\n");
return -EIO;
}
SBP2_ORB_DEBUG("write command agent complete");
scsi_id->last_orb = command_orb;
scsi_id->last_orb_dma = command->command_orb_dma;
} else { } else {
quadlet_t data;
/* /*
* We have an orb already sent (maybe or maybe not * last_orb != NULL means: We know that the target's fetch agent
* processed) that we can append this orb to. So do so, * is (very probably) not dead or in reset state right now.
* and ring the doorbell. Have to be very careful * We have an ORB already sent that we can append a new one to.
* modifying these next orb pointers, as they are accessed * The target's fetch agent may or may not have read this
* both by the sbp2 device and us. * previous ORB yet.
*/ */
scsi_id->last_orb->next_ORB_lo = pci_dma_sync_single_for_cpu(hi->host->pdev, last_orb_dma,
cpu_to_be32(command->command_orb_dma); sizeof(struct sbp2_command_orb),
PCI_DMA_BIDIRECTIONAL);
last_orb->next_ORB_lo = cpu_to_be32(command->command_orb_dma);
wmb();
/* Tells hardware that this pointer is valid */ /* Tells hardware that this pointer is valid */
scsi_id->last_orb->next_ORB_hi = 0x0; last_orb->next_ORB_hi = 0;
pci_dma_sync_single_for_device(hi->host->pdev, pci_dma_sync_single_for_device(hi->host->pdev, last_orb_dma,
scsi_id->last_orb_dma,
sizeof(struct sbp2_command_orb), sizeof(struct sbp2_command_orb),
PCI_DMA_BIDIRECTIONAL); PCI_DMA_BIDIRECTIONAL);
addr += SBP2_DOORBELL_OFFSET;
data[0] = 0;
length = 4;
}
scsi_id->last_orb = command_orb;
scsi_id->last_orb_dma = command->command_orb_dma;
spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
/* SBP2_ORB_DEBUG("write to %s register, command orb %p",
* Ring the doorbell last_orb ? "DOORBELL" : "ORB_POINTER", command_orb);
*/ if (sbp2util_node_write_no_wait(scsi_id->ne, addr, data, length) < 0) {
data = cpu_to_be32(command->command_orb_dma); SBP2_ERR("sbp2util_node_write_no_wait failed.\n");
addr = scsi_id->sbp2_command_block_agent_addr + SBP2_DOORBELL_OFFSET; return -EIO;
SBP2_ORB_DEBUG("ring doorbell, command orb %p", command_orb);
if (sbp2util_node_write_no_wait(ne, addr, &data, 4) < 0) {
SBP2_ERR("sbp2util_node_write_no_wait failed");
return -EIO;
}
scsi_id->last_orb = command_orb;
scsi_id->last_orb_dma = command->command_orb_dma;
} }
return 0; return 0;
} }
@ -2231,14 +2224,16 @@ static int sbp2_handle_status_write(struct hpsb_host *host, int nodeid, int dest
} }
/* /*
* Check here to see if there are no commands in-use. If there are none, we can * Check here to see if there are no commands in-use. If there
* null out last orb so that next time around we write directly to the orb pointer... * are none, we know that the fetch agent left the active state
* Quick start saves one 1394 bus transaction. * _and_ that we did not reactivate it yet. Therefore clear
* last_orb so that next time we write directly to the
* ORB_POINTER register. That way the fetch agent does not need
* to refetch the next_ORB.
*/ */
spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags); spin_lock_irqsave(&scsi_id->sbp2_command_orb_lock, flags);
if (list_empty(&scsi_id->sbp2_command_orb_inuse)) { if (list_empty(&scsi_id->sbp2_command_orb_inuse))
scsi_id->last_orb = NULL; scsi_id->last_orb = NULL;
}
spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags); spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
} else { } else {

View file

@ -46,8 +46,8 @@
#define ORB_SET_DIRECTION(value) ((value & 0x1) << 27) #define ORB_SET_DIRECTION(value) ((value & 0x1) << 27)
struct sbp2_command_orb { struct sbp2_command_orb {
volatile u32 next_ORB_hi; u32 next_ORB_hi;
volatile u32 next_ORB_lo; u32 next_ORB_lo;
u32 data_descriptor_hi; u32 data_descriptor_hi;
u32 data_descriptor_lo; u32 data_descriptor_lo;
u32 misc; u32 misc;