Subject: port-i386/875: Problems with aic6360 driver and disconnecting devices
To: None <gnats-admin@NetBSD.ORG>
From: None <jarle@idt.unit.no>
List: netbsd-bugs
Date: 03/14/1995 15:20:04
>Number:         875
>Category:       port-i386
>Synopsis:       Problems with aic6360 driver and disconnecting devices
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 14 15:20:02 1995
>Originator:     jarle@idt.unit.no
>Organization:
Hackers Lost in Reality, Inc.
>Release:        1.0A
>Environment:
	
System: NetBSD darling.idt.unit.no 1.0A NetBSD 1.0A (DARLING) #13: Tue Mar 14 21:18:37 1995 jarle@darling.idt.unit.no:/usr/src/sys/arch/i386/compile/DARLING i386

>Description:
There is a disagreement between the way /sys/scsi/scsi_base.c and
/sys/dev/isa/aic6360.c handle residues.  The previous versions (e.g. in
1.0) of sc_err1 just zero'ed the returned residue if the returned error
code from the driver was XS_NOERROR.  In the current version the residue
value is copied to bp->bp_b_resid even if the command was executed
successfully.  This is a problem because the aic driver may return a
non-zero residue value *and* an error code of XS_NOERROR under certain
circumstances.  One of my disks, <QUANTUM, PD1800S, 3162> SCSI2, is fond of
disconnecting without saving the data pointer when it has received all its
data for a write op.  After a while, presumably when the bits have hit the
platter, it reconnects, returns a GOOD status, a CMD_COMPLETE message and
disconnects.  This leads the aic driver to load an old set of data_pointer
and data_length values from the acb because of the implied RESTORE POINTER
during reconnect, again leading to a non-zero residue value.  The included
diff fixes this by adding an extra boolean value telling whether the
sc_dleft has ever been zero; if true we zero the sc_dleft field on
CMD_COMPLETE.  An alternativ (maybe better) solution might be to just
believe the device, and do the zeroing in aic_done() if the command was
successfully executed with a GOOD status code.

On a related string, isn't the assignment at line 1462 a bit dangerous?
	acb->xs->resid = acb->data_length = sc->sc_dleft;
If this is for a command that has returned a CHECK SENSE status byte, and
this is the CMD_COMPLETE for the REQUEST SENSE command we will overwrite
the original residue value.  (If this value isn't used when sense data is
returned it doesn't really matter, as for several of the device classes,
although not for disks, the actual residue is returned as part of the sense
data). 
	
>How-To-Repeat:
Perform a long string of write ops (tar or dd) on a scsi drive that will
disconnect at the end of the data phase without sending a save pointer
message.
	
>Fix:
The diff has been tested on my own system, and seems to work without any
problems.  (Note that for the time being my system hasn't got any other
scsi devices than its two disks, so you should probably check for possible
problems with devices such as tapes and cd-roms.  Nah, forget it.  Of
course it works :-)
						-jarle
----
"C Code.  C code run.  Run, code, run...  PLEASE!!!"
				-- Barbara Tongue
============================ diff below ==================================
	
*** aic6360.c	Fri Feb  3 12:23:56 1995
--- aic6360.c.new	Tue Mar 14 23:01:30 1995
***************
*** 462,465 ****
--- 462,466 ----
  #define ACB_CHKSENSE	2
  #define	ACB_ABORTED	3
+ 	u_char null_left;
  };
  
***************
*** 987,990 ****
--- 988,992 ----
  	acb->data_length = xs->datalen;
  	acb->target_stat = 0;
+ 	acb->null_left = 0;
  	
  	s = splbio();
***************
*** 1460,1463 ****
--- 1462,1467 ----
  				acb->data_length = 0;
  			}
+ 			if (acb->null_left)
+ 				sc->sc_dleft = 0;
  			acb->xs->resid = acb->data_length = sc->sc_dleft;
  			sc->sc_state = AIC_CMDCOMPLETE;
***************
*** 1506,1509 ****
--- 1510,1514 ----
  			ti->dconns++;
  			sc->sc_state = AIC_DISCONNECT;
+ 			acb->null_left = sc->sc_dleft == 0;
  			break;
  
>Audit-Trail:
>Unformatted: