CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sat Feb 24 22:06:50 UTC 2024

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
Don't try to discover wedges when the unit isn't online.


To generate a diff of this commit:
cvs rdiff -u -r1.335 -r1.336 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sat Feb 24 22:06:50 UTC 2024

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
Don't try to discover wedges when the unit isn't online.


To generate a diff of this commit:
cvs rdiff -u -r1.335 -r1.336 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/sd.c
diff -u src/sys/dev/scsipi/sd.c:1.335 src/sys/dev/scsipi/sd.c:1.336
--- src/sys/dev/scsipi/sd.c:1.335	Sun Aug 28 10:26:37 2022
+++ src/sys/dev/scsipi/sd.c	Sat Feb 24 22:06:49 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: sd.c,v 1.335 2022/08/28 10:26:37 mlelstv Exp $	*/
+/*	$NetBSD: sd.c,v 1.336 2024/02/24 22:06:49 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
@@ -47,7 +47,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.335 2022/08/28 10:26:37 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.336 2024/02/24 22:06:49 mlelstv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -351,8 +351,9 @@ sdattach(device_t parent, device_t self,
 	}
 	aprint_normal("\n");
 
-	/* Discover wedges on this disk. */
-	dkwedge_discover(>sc_dkdev);
+	/* Discover wedges on this disk if it is online */
+	if (result == SDGP_RESULT_OK)
+		dkwedge_discover(>sc_dkdev);
 
 	/*
 	 * Establish a shutdown hook so that we can ensure that

CVS commit: src/sys/dev/scsipi

[David H. Gutteridge]

Module Name:src
Committed By:   gutteridge
Date:   Mon Jan  1 22:29:49 UTC 2024

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
if_dse.c: s/addredses/addresses/ in comment


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[David H. Gutteridge]

Module Name:src
Committed By:   gutteridge
Date:   Mon Jan  1 22:29:49 UTC 2024

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
if_dse.c: s/addredses/addresses/ in comment


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_dse.c
diff -u src/sys/dev/scsipi/if_dse.c:1.4 src/sys/dev/scsipi/if_dse.c:1.5
--- src/sys/dev/scsipi/if_dse.c:1.4	Wed Dec 20 18:09:19 2023
+++ src/sys/dev/scsipi/if_dse.c	Mon Jan  1 22:29:48 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_dse.c,v 1.4 2023/12/20 18:09:19 skrll Exp $ */
+/*	$NetBSD: if_dse.c,v 1.5 2024/01/01 22:29:48 gutteridge Exp $ */
 
 /*
  * Driver for DaynaPORT SCSI/Link SCSI-Ethernet
@@ -300,7 +300,7 @@ static const scsi_dayna_ether_generic	so
 
 #if 0
 /*
- * Compare two Ether/802 addredses for equality, inlined and
+ * Compare two Ether/802 addresses for equality, inlined and
  * unrolled for speed.
  * Note: use this like memcmp()
  */

CVS commit: src/sys/dev/scsipi

[Andrius Varanavicius]

Module Name:src
Committed By:   andvar
Date:   Thu Dec  7 07:04:13 UTC 2023

Modified Files:
src/sys/dev/scsipi: scsi_disk.h

Log Message:
s/multiplcation/multiplication/ in comment.


To generate a diff of this commit:
cvs rdiff -u -r1.34 -r1.35 src/sys/dev/scsipi/scsi_disk.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Andrius Varanavicius]

Module Name:src
Committed By:   andvar
Date:   Thu Dec  7 07:04:13 UTC 2023

Modified Files:
src/sys/dev/scsipi: scsi_disk.h

Log Message:
s/multiplcation/multiplication/ in comment.


To generate a diff of this commit:
cvs rdiff -u -r1.34 -r1.35 src/sys/dev/scsipi/scsi_disk.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsi_disk.h
diff -u src/sys/dev/scsipi/scsi_disk.h:1.34 src/sys/dev/scsipi/scsi_disk.h:1.35
--- src/sys/dev/scsipi/scsi_disk.h:1.34	Wed Nov 10 16:17:34 2021
+++ src/sys/dev/scsipi/scsi_disk.h	Thu Dec  7 07:04:13 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsi_disk.h,v 1.34 2021/11/10 16:17:34 msaitoh Exp $	*/
+/*	$NetBSD: scsi_disk.h,v 1.35 2023/12/07 07:04:13 andvar Exp $	*/
 
 /*
  * SCSI-specific interface description
@@ -333,7 +333,7 @@ union scsi_disk_pages {
 		u_int8_t pg_length;	/* page length (should be 0x0a) */
 		u_int8_t flags;		/* cache parameter flags */
 #define	CACHING_RCD	0x01		/* read cache disable */
-#define	CACHING_MF	0x02		/* multiplcation factor */
+#define	CACHING_MF	0x02		/* multiplication factor */
 #define	CACHING_WCE	0x04		/* write cache enable (write-back) */
 #define	CACHING_SIZE	0x08		/* use CACHE SEGMENT SIZE */
 #define	CACHING_DISC	0x10		/* pftch across time discontinuities */

CVS commit: src/sys/dev/scsipi

[Nathanial Sloss]

Module Name:src
Committed By:   nat
Date:   Thu Dec 22 23:06:11 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
Fix condition for ending the pacet read loop.

len is unsigned 16 bit so testing for less than zero is not valid.


To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Nathanial Sloss]

Module Name:src
Committed By:   nat
Date:   Thu Dec 22 23:06:11 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
Fix condition for ending the pacet read loop.

len is unsigned 16 bit so testing for less than zero is not valid.


To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_dse.c
diff -u src/sys/dev/scsipi/if_dse.c:1.2 src/sys/dev/scsipi/if_dse.c:1.3
--- src/sys/dev/scsipi/if_dse.c:1.2	Thu Dec 22 22:39:20 2022
+++ src/sys/dev/scsipi/if_dse.c	Thu Dec 22 23:06:11 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_dse.c,v 1.2 2022/12/22 22:39:20 nat Exp $ */
+/*	$NetBSD: if_dse.c,v 1.3 2022/12/22 23:06:11 nat Exp $ */
 
 /*
  * Driver for DaynaPORT SCSI/Link SCSI-Ethernet
@@ -850,7 +850,7 @@ dse_read(struct dse_softc *sc, uint8_t *
 			len = peek_packet(data);
 		}
 #endif
-		if (len <=0)
+		if (len == 0)
 			break;
 
 #ifdef DSE_DEBUG

CVS commit: src/sys/dev/scsipi

[Nathanial Sloss]

Module Name:src
Committed By:   nat
Date:   Thu Dec 22 22:39:20 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
Remove unused commented out code.
Remove unintentional stray debug printfs.
Fix DSE_DEBUG build.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Nathanial Sloss]

Module Name:src
Committed By:   nat
Date:   Thu Dec 22 22:39:20 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_dse.c

Log Message:
Remove unused commented out code.
Remove unintentional stray debug printfs.
Fix DSE_DEBUG build.


To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.2 src/sys/dev/scsipi/if_dse.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_dse.c
diff -u src/sys/dev/scsipi/if_dse.c:1.1 src/sys/dev/scsipi/if_dse.c:1.2
--- src/sys/dev/scsipi/if_dse.c:1.1	Thu Dec 22 11:05:55 2022
+++ src/sys/dev/scsipi/if_dse.c	Thu Dec 22 22:39:20 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_dse.c,v 1.1 2022/12/22 11:05:55 nat Exp $ */
+/*	$NetBSD: if_dse.c,v 1.2 2022/12/22 22:39:20 nat Exp $ */
 
 /*
  * Driver for DaynaPORT SCSI/Link SCSI-Ethernet
@@ -561,11 +561,11 @@ dse_send_worker(struct work *wk, void *c
 			m = m0 = m_free(m);
 		}
 		if (len < DSE_MINSIZE) {
-#ifdef SEDEBUG
+#ifdef DSE_DEBUG
 			if (sc->sc_debug)
-aprnt_error_dev(sc->sc_dev,
+aprint_error_dev(sc->sc_dev,
 "packet size %d (%zu) < %d\n", len,
-cp - (u_char *)sc->sc_tbuf, SEMINSIZE);
+cp - (u_char *)sc->sc_tbuf, DSE_MINSIZE);
 #endif
 			memset(cp, 0, DSE_MINSIZE - len);
 			len = DSE_MINSIZE;
@@ -854,8 +854,8 @@ dse_read(struct dse_softc *sc, uint8_t *
 			break;
 
 #ifdef DSE_DEBUG
-		aprint_error_dev("dse_read: datalen = %d, packetlen = %d, "
-		"proto = 0x%04x\n", datalen, len,
+		aprint_error_dev(sc->sc_dev, "dse_read: datalen = %d, packetlen"
+		" = %d, proto = 0x%04x\n", datalen, len,
 		ntohs(((struct ether_header *)data)->ether_type));
 #endif
 		if ((len < (DSE_MINSIZE + ETHER_CRC_LEN)) ||
@@ -865,7 +865,6 @@ dse_read(struct dse_softc *sc, uint8_t *
 			"%d; dropping\n", len);
 #endif
 			if_statinc(ifp, if_ierrors);
-printf("LEN %d\n",len);
 			break;
 		}
 
@@ -874,10 +873,9 @@ printf("LEN %d\n",len);
 		if (m == NULL) {
 #ifdef DSE_DEBUG
 			if (sc->sc_debug)
-aprint_error_dev("dse_read: dse_get returned "
-"null\n");
+aprint_error_dev(sc->sc_dev, "dse_read: "
+"dse_get returned null\n");
 #endif
-printf("M null\n");
 			if_statinc(ifp, if_ierrors);
 			goto next_packet;
 		}
@@ -1146,7 +1144,6 @@ dse_ioctl(struct ifnet *ifp, u_long cmd,
 		switch (ifa->ifa_addr->sa_family) {
 #ifdef INET
 		case AF_INET:
-	//		sc->protos |= (PROTO_IP | PROTO_ARP | PROTO_REVARP);
 			if ((error = dse_init(sc)) != 0)
 break;
 			arp_ifinit(ifp, ifa);
@@ -1154,7 +1151,6 @@ dse_ioctl(struct ifnet *ifp, u_long cmd,
 #endif
 #ifdef NETATALK
 		case AF_APPLETALK:
-	//		sc->protos |= (PROTO_AT | PROTO_AARP);
 			if ((error = dse_init(sc)) != 0)
 break;
 			break;
@@ -1233,7 +1229,7 @@ dse_ioctl(struct ifnet *ifp, u_long cmd,
 			mutex_exit(>sc_iflock);
 			break;
 		}
-#ifdef SEDEBUG
+#ifdef DSE_DEBUG
 		if (ifp->if_flags & IFF_DEBUG)
 			sc->sc_debug = 1;
 		else

CVS commit: src/sys/dev/scsipi

[Jared D. McNeill]

Module Name:src
Committed By:   jmcneill
Date:   Sat Oct 15 18:42:49 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Add PQUIRK_ONLYBIG for Oracle OCI BlockVolumes.

Oracle cloud BlockVolumes do not appear to support SCSI READ6 or WRITE6
commands, so set PQUIRK_ONLYBIG to avoid it here.


To generate a diff of this commit:
cvs rdiff -u -r1.302 -r1.303 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.302 src/sys/dev/scsipi/scsiconf.c:1.303
--- src/sys/dev/scsipi/scsiconf.c:1.302	Thu Apr 14 16:50:26 2022
+++ src/sys/dev/scsipi/scsiconf.c	Sat Oct 15 18:42:49 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.302 2022/04/14 16:50:26 pgoyette Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.303 2022/10/15 18:42:49 jmcneill Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.302 2022/04/14 16:50:26 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.303 2022/10/15 18:42:49 jmcneill Exp $");
 
 #include 
 #include 
@@ -792,6 +792,8 @@ static const struct scsi_quirk_inquiry_p
 	 "SEAGATE ", "SX336704LC"   , ""}, PQUIRK_CAP_SYNC | PQUIRK_CAP_WIDE16},
 	{{T_DIRECT, T_FIXED,
 	 "SEAGATE ", "SX173404LC",   ""}, PQUIRK_CAP_SYNC | PQUIRK_CAP_WIDE16},
+	{{T_DIRECT, T_FIXED,
+	 "ORACLE",   "BlockVolume",	 ""},	  PQUIRK_ONLYBIG},
 
 	{{T_DIRECT, T_REMOV,
 	 "IOMEGA", "ZIP 100",		 "J.03"}, PQUIRK_NOLUNS|PQUIRK_NOSYNC},

CVS commit: src/sys/dev/scsipi

[Jared D. McNeill]

Module Name:src
Committed By:   jmcneill
Date:   Sat Oct 15 18:42:49 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Add PQUIRK_ONLYBIG for Oracle OCI BlockVolumes.

Oracle cloud BlockVolumes do not appear to support SCSI READ6 or WRITE6
commands, so set PQUIRK_ONLYBIG to avoid it here.


To generate a diff of this commit:
cvs rdiff -u -r1.302 -r1.303 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Mon Aug 29 07:32:46 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
Make this build again.  Sorry about that.


To generate a diff of this commit:
cvs rdiff -u -r1.117 -r1.118 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_se.c
diff -u src/sys/dev/scsipi/if_se.c:1.117 src/sys/dev/scsipi/if_se.c:1.118
--- src/sys/dev/scsipi/if_se.c:1.117	Sun Aug 28 09:48:12 2022
+++ src/sys/dev/scsipi/if_se.c	Mon Aug 29 07:32:46 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_se.c,v 1.117 2022/08/28 09:48:12 skrll Exp $	*/
+/*	$NetBSD: if_se.c,v 1.118 2022/08/29 07:32:46 skrll Exp $	*/
 
 /*
  * Copyright (c) 1997 Ian W. Dall 
@@ -59,7 +59,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.117 2022/08/28 09:48:12 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.118 2022/08/29 07:32:46 skrll Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -207,7 +207,9 @@ static void	se_ifstart(struct ifnet *);
 
 static void	sedone(struct scsipi_xfer *, int);
 static int	se_ioctl(struct ifnet *, u_long, void *);
+#if 0
 static void	sewatchdog(struct ifnet *);
+#endif
 
 #if 0
 static inline uint16_t ether_cmp(void *, void *);
@@ -217,7 +219,9 @@ static void	se_recv_worker(struct work *
 static void	se_recv(struct se_softc *);
 static struct mbuf *se_get(struct se_softc *, char *, int);
 static int	se_read(struct se_softc *, char *, int);
+#if 0
 static void	se_reset(struct se_softc *);
+#endif
 static int	se_add_proto(struct se_softc *, int);
 static int	se_get_addr(struct se_softc *, uint8_t *);
 static int	se_set_media(struct se_softc *, int);
@@ -778,7 +782,6 @@ sewatchdog(struct ifnet *ifp)
 
 	se_reset(sc);
 }
-#endif
 
 static void
 se_reset(struct se_softc *sc)
@@ -794,6 +797,7 @@ se_reset(struct se_softc *sc)
 #endif
 	se_init(sc);
 }
+#endif
 
 static int
 se_add_proto(struct se_softc *sc, int proto)

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Mon Aug 29 07:32:46 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
Make this build again.  Sorry about that.


To generate a diff of this commit:
cvs rdiff -u -r1.117 -r1.118 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sun Aug 28 10:26:37 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c sdvar.h

Log Message:
Don't fetch data beyond end of inquiry buffer, which, here, is not
NUL-terminated.

Reduce target buffer to needed size (product name + NUL terminator).


To generate a diff of this commit:
cvs rdiff -u -r1.334 -r1.335 src/sys/dev/scsipi/sd.c
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/scsipi/sdvar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/sd.c
diff -u src/sys/dev/scsipi/sd.c:1.334 src/sys/dev/scsipi/sd.c:1.335
--- src/sys/dev/scsipi/sd.c:1.334	Mon Mar 28 12:39:46 2022
+++ src/sys/dev/scsipi/sd.c	Sun Aug 28 10:26:37 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: sd.c,v 1.334 2022/03/28 12:39:46 riastradh Exp $	*/
+/*	$NetBSD: sd.c,v 1.335 2022/08/28 10:26:37 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
@@ -47,7 +47,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.334 2022/03/28 12:39:46 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.335 2022/08/28 10:26:37 mlelstv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -258,9 +258,8 @@ sdattach(device_t parent, device_t self,
 	SC_DEBUG(periph, SCSIPI_DB2, ("sdattach: "));
 
 	sd->type = (sa->sa_inqbuf.type & SID_TYPE);
-	strncpy(sd->name, sa->sa_inqbuf.product, sizeof(sd->name));
-
-	strncpy(sd->typename, sa->sa_inqbuf.product, sizeof(sd->typename));
+	memcpy(sd->name, sa->sa_inqbuf.product, uimin(16, sizeof(sd->name)));
+	memcpy(sd->typename, sa->sa_inqbuf.product, uimin(16, sizeof(sd->typename)));
 
 	if (sd->type == T_SIMPLE_DIRECT)
 		periph->periph_quirks |= PQUIRK_ONLYBIG | PQUIRK_NOBIGMODESENSE;

Index: src/sys/dev/scsipi/sdvar.h
diff -u src/sys/dev/scsipi/sdvar.h:1.39 src/sys/dev/scsipi/sdvar.h:1.40
--- src/sys/dev/scsipi/sdvar.h:1.39	Tue Mar 19 06:59:40 2019
+++ src/sys/dev/scsipi/sdvar.h	Sun Aug 28 10:26:37 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: sdvar.h,v 1.39 2019/03/19 06:59:40 mlelstv Exp $	*/
+/*	$NetBSD: sdvar.h,v 1.40 2022/08/28 10:26:37 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@@ -88,7 +88,7 @@ struct sd_softc {
 	callout_t sc_callout;
 	u_int8_t type;
 	char name[16]; /* product name, for default disklabel */
-	char typename[128+4+1]; /* stored in disk info */
+	char typename[16+1]; /* stored in disk info */
 };
 
 #define	SDGP_RESULT_OK		0	/* parameters obtained */

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sun Aug 28 10:26:37 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c sdvar.h

Log Message:
Don't fetch data beyond end of inquiry buffer, which, here, is not
NUL-terminated.

Reduce target buffer to needed size (product name + NUL terminator).


To generate a diff of this commit:
cvs rdiff -u -r1.334 -r1.335 src/sys/dev/scsipi/sd.c
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/scsipi/sdvar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Sun Aug 28 09:48:12 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
se(4): don't set if_watchdog as it's not used.

if_timer is never set in this driver and so if_watchdog will never be
called.


To generate a diff of this commit:
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_se.c
diff -u src/sys/dev/scsipi/if_se.c:1.116 src/sys/dev/scsipi/if_se.c:1.117
--- src/sys/dev/scsipi/if_se.c:1.116	Thu Jul  7 06:11:28 2022
+++ src/sys/dev/scsipi/if_se.c	Sun Aug 28 09:48:12 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_se.c,v 1.116 2022/07/07 06:11:28 skrll Exp $	*/
+/*	$NetBSD: if_se.c,v 1.117 2022/08/28 09:48:12 skrll Exp $	*/
 
 /*
  * Copyright (c) 1997 Ian W. Dall 
@@ -59,7 +59,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.116 2022/07/07 06:11:28 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.117 2022/08/28 09:48:12 skrll Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -359,7 +359,7 @@ seattach(device_t parent, device_t self,
 	ifp->if_softc = sc;
 	ifp->if_start = se_ifstart;
 	ifp->if_ioctl = se_ioctl;
-	ifp->if_watchdog = sewatchdog;
+	ifp->if_watchdog = NULL;
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
 	ifp->if_extflags = IFEF_MPSAFE;
 	IFQ_SET_READY(>if_snd);
@@ -767,7 +767,7 @@ se_read(struct se_softc *sc, char *data,
 	return (n);
 }
 
-
+#if 0
 static void
 sewatchdog(struct ifnet *ifp)
 {
@@ -778,6 +778,7 @@ sewatchdog(struct ifnet *ifp)
 
 	se_reset(sc);
 }
+#endif
 
 static void
 se_reset(struct se_softc *sc)

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Sun Aug 28 09:48:12 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
se(4): don't set if_watchdog as it's not used.

if_timer is never set in this driver and so if_watchdog will never be
called.


To generate a diff of this commit:
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Thu Jul  7 06:11:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
Trailing whitespace


To generate a diff of this commit:
cvs rdiff -u -r1.115 -r1.116 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/if_se.c
diff -u src/sys/dev/scsipi/if_se.c:1.115 src/sys/dev/scsipi/if_se.c:1.116
--- src/sys/dev/scsipi/if_se.c:1.115	Sat Jan  1 10:32:29 2022
+++ src/sys/dev/scsipi/if_se.c	Thu Jul  7 06:11:28 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_se.c,v 1.115 2022/01/01 10:32:29 msaitoh Exp $	*/
+/*	$NetBSD: if_se.c,v 1.116 2022/07/07 06:11:28 skrll Exp $	*/
 
 /*
  * Copyright (c) 1997 Ian W. Dall 
@@ -59,7 +59,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.115 2022/01/01 10:32:29 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_se.c,v 1.116 2022/07/07 06:11:28 skrll Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -465,7 +465,7 @@ se_ifstart(struct ifnet *ifp)
 	if (!sc->sc_send_work_pending)  {
 		sc->sc_send_work_pending = true;
 		workqueue_enqueue(sc->sc_send_wq, >sc_send_work, NULL);
-	} 
+	}
 	/* else: nothing to do - work is already queued */
 	mutex_exit(>sc_iflock);
 }
@@ -915,7 +915,7 @@ se_init(struct se_softc *sc)
 			sc->sc_recv_work_pending = true;
 			workqueue_enqueue(sc->sc_recv_wq, >sc_recv_work,
 			NULL);
-		} 
+		}
 		mutex_exit(>sc_iflock);
 		ifp->if_flags &= ~IFF_OACTIVE;
 		mutex_enter(>sc_iflock);
@@ -923,7 +923,7 @@ se_init(struct se_softc *sc)
 			sc->sc_send_work_pending = true;
 			workqueue_enqueue(sc->sc_send_wq, >sc_send_work,
 			NULL);
-		} 
+		}
 		mutex_exit(>sc_iflock);
 	}
 	return (error);

CVS commit: src/sys/dev/scsipi

[Nick Hudson]

Module Name:src
Committed By:   skrll
Date:   Thu Jul  7 06:11:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: if_se.c

Log Message:
Trailing whitespace


To generate a diff of this commit:
cvs rdiff -u -r1.115 -r1.116 src/sys/dev/scsipi/if_se.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Andrius Varanavicius]

Module Name:src
Committed By:   andvar
Date:   Sun Jun 26 21:00:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: cd.c

Log Message:
s/Ramdom/Random/ in comments.


To generate a diff of this commit:
cvs rdiff -u -r1.353 -r1.354 src/sys/dev/scsipi/cd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/cd.c
diff -u src/sys/dev/scsipi/cd.c:1.353 src/sys/dev/scsipi/cd.c:1.354
--- src/sys/dev/scsipi/cd.c:1.353	Tue Oct 12 08:36:29 2021
+++ src/sys/dev/scsipi/cd.c	Sun Jun 26 21:00:28 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: cd.c,v 1.353 2021/10/12 08:36:29 andvar Exp $	*/
+/*	$NetBSD: cd.c,v 1.354 2022/06/26 21:00:28 andvar Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2001, 2003, 2004, 2005, 2008 The NetBSD Foundation,
@@ -50,7 +50,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: cd.c,v 1.353 2021/10/12 08:36:29 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cd.c,v 1.354 2022/06/26 21:00:28 andvar Exp $");
 
 #include 
 #include 
@@ -2461,7 +2461,7 @@ mmc_profile2class(uint16_t mmc_profile)
 		return MMC_CLASS_DVD;
 	case 0x40 : /* BD-ROM  */
 	case 0x41 : /* BD-R Sequential recording (SRM) */
-	case 0x42 : /* BD-R Ramdom Recording (RRM) */
+	case 0x42 : /* BD-R Random Recording (RRM) */
 	case 0x43 : /* BD-RE */
 		return MMC_CLASS_BD;
 	}
@@ -3334,7 +3334,7 @@ mmc_do_closetrack(struct scsipi_periph *
 	case 0x12 : /* DVD-RAM */
 	case 0x1a : /* DVD+RW  */
 	case 0x2a : /* DVD+RW Dual layer */
-	case 0x42 : /* BD-R Ramdom Recording (RRM) */
+	case 0x42 : /* BD-R Random Recording (RRM) */
 	case 0x43 : /* BD-RE */
 	case 0x52 : /* HD DVD-RW ; DVD-RAM like */
 		return EINVAL;
@@ -3393,7 +3393,7 @@ mmc_do_close_or_finalise(struct scsipi_p
 	case 0x12 : /* DVD-RAM */
 	case 0x1a : /* DVD+RW  */
 	case 0x2a : /* DVD+RW Dual layer */
-	case 0x42 : /* BD-R Ramdom Recording (RRM) */
+	case 0x42 : /* BD-R Random Recording (RRM) */
 	case 0x43 : /* BD-RE */
 	case 0x52 : /* HD DVD-RW; DVD-RAM like */
 		return EINVAL;

CVS commit: src/sys/dev/scsipi

[Andrius Varanavicius]

Module Name:src
Committed By:   andvar
Date:   Sun Jun 26 21:00:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: cd.c

Log Message:
s/Ramdom/Random/ in comments.


To generate a diff of this commit:
cvs rdiff -u -r1.353 -r1.354 src/sys/dev/scsipi/cd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:39:47 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
sd(4): Use d_cfdriver/devtounit to avoid open/detach races.


To generate a diff of this commit:
cvs rdiff -u -r1.333 -r1.334 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/sd.c
diff -u src/sys/dev/scsipi/sd.c:1.333 src/sys/dev/scsipi/sd.c:1.334
--- src/sys/dev/scsipi/sd.c:1.333	Thu Jan 27 18:44:49 2022
+++ src/sys/dev/scsipi/sd.c	Mon Mar 28 12:39:46 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: sd.c,v 1.333 2022/01/27 18:44:49 jakllsch Exp $	*/
+/*	$NetBSD: sd.c,v 1.334 2022/03/28 12:39:46 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
@@ -47,7 +47,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.333 2022/01/27 18:44:49 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.334 2022/03/28 12:39:46 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -167,6 +167,8 @@ const struct bdevsw sd_bdevsw = {
 	.d_dump = sddump,
 	.d_psize = sdsize,
 	.d_discard = nodiscard,
+	.d_cfdriver = _cd,
+	.d_devtounit = disklabel_dev_unit,
 	.d_flag = D_DISK | D_MPSAFE
 };
 
@@ -182,6 +184,8 @@ const struct cdevsw sd_cdevsw = {
 	.d_mmap = nommap,
 	.d_kqfilter = nokqfilter,
 	.d_discard = nodiscard,
+	.d_cfdriver = _cd,
+	.d_devtounit = disklabel_dev_unit,
 	.d_flag = D_DISK | D_MPSAFE
 };
 

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Mon Mar 28 12:39:47 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
sd(4): Use d_cfdriver/devtounit to avoid open/detach races.


To generate a diff of this commit:
cvs rdiff -u -r1.333 -r1.334 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Mar 12 16:57:16 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
scsi(9): Handle bogus number of LUNs in SCSI_REPORT_LUNS.

Reported-by: syzbot+76ef9084533d4bcce...@syzkaller.appspotmail.com


To generate a diff of this commit:
cvs rdiff -u -r1.299 -r1.300 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.299 src/sys/dev/scsipi/scsiconf.c:1.300
--- src/sys/dev/scsipi/scsiconf.c:1.299	Sat Mar 12 15:32:32 2022
+++ src/sys/dev/scsipi/scsiconf.c	Sat Mar 12 16:57:15 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.299 2022/03/12 15:32:32 riastradh Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.300 2022/03/12 16:57:15 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.299 2022/03/12 15:32:32 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.300 2022/03/12 16:57:15 riastradh Exp $");
 
 #include 
 #include 
@@ -400,7 +400,7 @@ scsi_report_luns(struct scsibus_softc *s
 	uint16_t tmp;
 
 	int error;
-	size_t i, rlrlen;
+	size_t i, rlrlen, rlrlenmin;
 
 	memset(, 0, sizeof(replun));
 
@@ -421,7 +421,7 @@ scsi_report_luns(struct scsibus_softc *s
 		goto end2;
 	}
 
-	rlrlen = sizeof(*rlr) + sizeof(*lunp) * 1;
+	rlrlen = rlrlenmin = sizeof(*rlr) + sizeof(*lunp) * 1;
 
 again:
 	rlr = kmem_zalloc(rlrlen, KM_SLEEP);
@@ -443,6 +443,10 @@ again:
 		16383 * sizeof(*lunp));
 		kmem_free(rlr, old_rlrlen);
 		rlr = NULL;
+		if (rlrlen < rlrlenmin) {
+			error = EIO;
+			goto end;
+		}
 		goto again;
 	}
 

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Sat Mar 12 16:57:16 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
scsi(9): Handle bogus number of LUNs in SCSI_REPORT_LUNS.

Reported-by: syzbot+76ef9084533d4bcce...@syzkaller.appspotmail.com


To generate a diff of this commit:
cvs rdiff -u -r1.299 -r1.300 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Juergen Hannken-Illjes]

Module Name:src
Committed By:   hannken
Date:   Sat Feb  5 17:32:59 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Initialize "replun" -- found with KMSAN.


To generate a diff of this commit:
cvs rdiff -u -r1.297 -r1.298 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.297 src/sys/dev/scsipi/scsiconf.c:1.298
--- src/sys/dev/scsipi/scsiconf.c:1.297	Sat Jan 29 11:20:30 2022
+++ src/sys/dev/scsipi/scsiconf.c	Sat Feb  5 17:32:59 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.297 2022/01/29 11:20:30 martin Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.298 2022/02/05 17:32:59 hannken Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.297 2022/01/29 11:20:30 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.298 2022/02/05 17:32:59 hannken Exp $");
 
 #include 
 #include 
@@ -399,6 +399,8 @@ scsi_report_luns(struct scsibus_softc *s
 	int error;
 	size_t i, rlrlen;
 
+	memset(, 0, sizeof(replun));
+
 	periph = scsipi_alloc_periph(M_WAITOK);
 	periph->periph_channel = chan;
 	periph->periph_switch = _probe_dev;

CVS commit: src/sys/dev/scsipi

[Juergen Hannken-Illjes]

Module Name:src
Committed By:   hannken
Date:   Sat Feb  5 17:32:59 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Initialize "replun" -- found with KMSAN.


To generate a diff of this commit:
cvs rdiff -u -r1.297 -r1.298 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Martin Husemann]

Module Name:src
Committed By:   martin
Date:   Sat Jan 29 11:20:30 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
In some cases the gcc optimizer is not smart enough to figure out why
the luns and nluns variables are never actually used when they are not
initialized - so initialize them always.


To generate a diff of this commit:
cvs rdiff -u -r1.296 -r1.297 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Martin Husemann]

Module Name:src
Committed By:   martin
Date:   Sat Jan 29 11:20:30 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
In some cases the gcc optimizer is not smart enough to figure out why
the luns and nluns variables are never actually used when they are not
initialized - so initialize them always.


To generate a diff of this commit:
cvs rdiff -u -r1.296 -r1.297 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.296 src/sys/dev/scsipi/scsiconf.c:1.297
--- src/sys/dev/scsipi/scsiconf.c:1.296	Fri Jan 28 18:23:28 2022
+++ src/sys/dev/scsipi/scsiconf.c	Sat Jan 29 11:20:30 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.296 2022/01/28 18:23:28 christos Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.297 2022/01/29 11:20:30 martin Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.296 2022/01/28 18:23:28 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.297 2022/01/29 11:20:30 martin Exp $");
 
 #include 
 #include 
@@ -477,8 +477,8 @@ end2:
 static void
 scsi_discover_luns(struct scsibus_softc *sc, int target, int minlun, int maxlun)
 {
-	uint16_t *luns;
-	size_t nluns;
+	uint16_t *luns = NULL;	/* XXX gcc */
+	size_t nluns = 0;	/* XXX gcc */
 
 	if (scsi_report_luns(sc, target, , ) == 0) {
 		for (size_t i = 0; i < nluns; i++)

CVS commit: src/sys/dev/scsipi

[Christos Zoulas]

Module Name:src
Committed By:   christos
Date:   Fri Jan 28 18:23:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Factor out the lun detection code to simplify control flow.


To generate a diff of this commit:
cvs rdiff -u -r1.295 -r1.296 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.295 src/sys/dev/scsipi/scsiconf.c:1.296
--- src/sys/dev/scsipi/scsiconf.c:1.295	Fri Jan 28 09:02:45 2022
+++ src/sys/dev/scsipi/scsiconf.c	Fri Jan 28 13:23:28 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.295 2022/01/28 14:02:45 jakllsch Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.296 2022/01/28 18:23:28 christos Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.295 2022/01/28 14:02:45 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.296 2022/01/28 18:23:28 christos Exp $");
 
 #include 
 #include 
@@ -474,6 +474,30 @@ end2:
 	return error;
 }
 
+static void
+scsi_discover_luns(struct scsibus_softc *sc, int target, int minlun, int maxlun)
+{
+	uint16_t *luns;
+	size_t nluns;
+
+	if (scsi_report_luns(sc, target, , ) == 0) {
+		for (size_t i = 0; i < nluns; i++)
+			if (luns[i] >= minlun && luns[i] <= maxlun)
+scsi_probe_device(sc, target, luns[i]);
+		kmem_free(luns, sizeof(*luns) * nluns);
+		return;
+	}
+
+	for (int lun = minlun; lun <= maxlun; lun++) {
+		/*
+		 * See if there's a device present, and configure it.
+		 */
+		if (scsi_probe_device(sc, target, lun) == 0)
+			break;
+		/* otherwise something says we should look further */
+	}
+}
+
 /*
  * Probe the requested scsi bus. It must be already set up.
  * target and lun optionally narrow the search if not -1
@@ -482,8 +506,6 @@ int
 scsi_probe_bus(struct scsibus_softc *sc, int target, int lun)
 {
 	struct scsipi_channel *chan = sc->sc_channel;
-	uint16_t *luns;
-	size_t nluns = 0; /* XXXGCC */
 	int maxtarget, mintarget, maxlun, minlun;
 	int error;
 
@@ -516,25 +538,8 @@ scsi_probe_bus(struct scsibus_softc *sc,
 	for (target = mintarget; target <= maxtarget; target++) {
 		if (target == chan->chan_id)
 			continue;
-		if (scsi_report_luns(sc, target, , ) == 0) {
-			for (size_t i = 0; i < nluns; i++)
-if (luns[i] >= minlun && luns[i] <= maxlun)
-	scsi_probe_device(sc, target, luns[i]);
-		} else
-		for (lun = minlun; lun <= maxlun; lun++) {
-			/*
-			 * See if there's a device present, and configure it.
-			 */
-			if (scsi_probe_device(sc, target, lun) == 0)
-break;
-			/* otherwise something says we should look further */
-		}
 
-		if (luns != NULL) {
-			kmem_free(luns, sizeof(*luns) * nluns);
-			luns = NULL;
-			nluns = 0;
-		}
+		scsi_discover_luns(sc, target, minlun, maxlun);
 
 		/*
 		 * Now that we've discovered all of the LUNs on this

CVS commit: src/sys/dev/scsipi

[Christos Zoulas]

Module Name:src
Committed By:   christos
Date:   Fri Jan 28 18:23:28 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
Factor out the lun detection code to simplify control flow.


To generate a diff of this commit:
cvs rdiff -u -r1.295 -r1.296 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Fri Jan 28 14:02:45 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
shut up GCC about possibly-uninit; some KNF


To generate a diff of this commit:
cvs rdiff -u -r1.294 -r1.295 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.294 src/sys/dev/scsipi/scsiconf.c:1.295
--- src/sys/dev/scsipi/scsiconf.c:1.294	Thu Jan 27 18:37:02 2022
+++ src/sys/dev/scsipi/scsiconf.c	Fri Jan 28 14:02:45 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.294 2022/01/27 18:37:02 jakllsch Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.295 2022/01/28 14:02:45 jakllsch Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.294 2022/01/27 18:37:02 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.295 2022/01/28 14:02:45 jakllsch Exp $");
 
 #include 
 #include 
@@ -482,6 +482,8 @@ int
 scsi_probe_bus(struct scsibus_softc *sc, int target, int lun)
 {
 	struct scsipi_channel *chan = sc->sc_channel;
+	uint16_t *luns;
+	size_t nluns = 0; /* XXXGCC */
 	int maxtarget, mintarget, maxlun, minlun;
 	int error;
 
@@ -509,9 +511,6 @@ scsi_probe_bus(struct scsibus_softc *sc,
 	 */
 	scsipi_adapter_ioctl(chan, SCBUSIOLLSCAN, NULL, 0, curproc);
 
-	uint16_t *luns;
-	size_t nluns;
-
 	if ((error = scsipi_adapter_addref(chan->chan_adapter)) != 0)
 		goto ret;
 	for (target = mintarget; target <= maxtarget; target++) {

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Fri Jan 28 14:02:45 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
shut up GCC about possibly-uninit; some KNF


To generate a diff of this commit:
cvs rdiff -u -r1.294 -r1.295 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Thu Jan 27 18:44:49 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
use powerof2() in sd_validate_blksize()


To generate a diff of this commit:
cvs rdiff -u -r1.332 -r1.333 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/sd.c
diff -u src/sys/dev/scsipi/sd.c:1.332 src/sys/dev/scsipi/sd.c:1.333
--- src/sys/dev/scsipi/sd.c:1.332	Sun May 30 06:46:46 2021
+++ src/sys/dev/scsipi/sd.c	Thu Jan 27 18:44:49 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: sd.c,v 1.332 2021/05/30 06:46:46 dholland Exp $	*/
+/*	$NetBSD: sd.c,v 1.333 2022/01/27 18:44:49 jakllsch Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc.
@@ -47,7 +47,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.332 2021/05/30 06:46:46 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.333 2022/01/27 18:44:49 jakllsch Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -1311,19 +1311,14 @@ static int
 sd_validate_blksize(struct scsipi_periph *periph, int len)
 {
 
-	switch (len) {
-	case 256:
-	case 512:
-	case 1024:
-	case 2048:
-	case 4096:
+	if (len >= 256 && powerof2(len) && len <= 4096) {
 		return 1;
 	}
 
 	if (periph) {
 		scsipi_printaddr(periph);
 		printf("%s sector size: 0x%x.  Defaulting to %d bytes.\n",
-		(len ^ (1 << (ffs(len) - 1))) ?
+		!powerof2(len) ?
 		"preposterous" : "unsupported",
 		len, SD_DEFAULT_BLKSIZE);
 	}

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Thu Jan 27 18:44:49 UTC 2022

Modified Files:
src/sys/dev/scsipi: sd.c

Log Message:
use powerof2() in sd_validate_blksize()


To generate a diff of this commit:
cvs rdiff -u -r1.332 -r1.333 src/sys/dev/scsipi/sd.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Thu Jan 27 18:37:02 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsi_spc.h scsiconf.c

Log Message:
Try REPORT LUNS command to enumerate logical units.


To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/scsipi/scsi_spc.h
cvs rdiff -u -r1.293 -r1.294 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Jonathan A. Kollasch]

Module Name:src
Committed By:   jakllsch
Date:   Thu Jan 27 18:37:02 UTC 2022

Modified Files:
src/sys/dev/scsipi: scsi_spc.h scsiconf.c

Log Message:
Try REPORT LUNS command to enumerate logical units.


To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/sys/dev/scsipi/scsi_spc.h
cvs rdiff -u -r1.293 -r1.294 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsi_spc.h
diff -u src/sys/dev/scsipi/scsi_spc.h:1.6 src/sys/dev/scsipi/scsi_spc.h:1.7
--- src/sys/dev/scsipi/scsi_spc.h:1.6	Thu Mar 28 10:44:29 2019
+++ src/sys/dev/scsipi/scsi_spc.h	Thu Jan 27 18:37:02 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsi_spc.h,v 1.6 2019/03/28 10:44:29 kardel Exp $	*/
+/*	$NetBSD: scsi_spc.h,v 1.7 2022/01/27 18:37:02 jakllsch Exp $	*/
 
 /*-
  * Copyright (c) 2005 The NetBSD Foundation, Inc.
@@ -404,6 +404,30 @@ struct scsi_reserve_release_10_idparam {
 /*
  * REPORT LUNS
  */
+#define SCSI_REPORT_LUNS		0xa0
+
+struct scsi_report_luns {
+	uint8_t opcode;
+	uint8_t reserved1;
+	uint8_t selectreport;
+#define SELECTREPORT_NORMAL		0x00
+#define SELECTREPORT_WELLKNOWN		0x01
+#define SELECTREPORT_ALL		0x02
+	uint8_t reserved3[3];
+	uint8_t alloclen[4];
+	uint8_t reserved10;
+	uint8_t control;
+};
+
+struct scsi_report_luns_header {
+	uint8_t	length[4];		/* in bytes, not including header */
+	uint8_t _res4[4];
+	/* followed by array of: */
+};
+
+struct scsi_report_luns_lun {
+	uint8_t lun[8];
+};
 
 /*
  * MAINTENANCE_IN[REPORT SUPPORTED OPERATION CODES]

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.293 src/sys/dev/scsipi/scsiconf.c:1.294
--- src/sys/dev/scsipi/scsiconf.c:1.293	Tue Dec 21 22:53:21 2021
+++ src/sys/dev/scsipi/scsiconf.c	Thu Jan 27 18:37:02 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.294 2022/01/27 18:37:02 jakllsch Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.294 2022/01/27 18:37:02 jakllsch Exp $");
 
 #include 
 #include 
@@ -64,6 +64,7 @@ __KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -370,6 +371,109 @@ scsibusdetach(device_t self, int flags)
 	return 0;
 }
 
+static int
+lun_compar(const void *a, const void *b)
+{
+	const uint16_t * const la = a, * const lb = b;
+
+	if (*la < *lb)
+		return -1;
+	if (*la > *lb)
+		return 1;
+	return 0;
+}
+
+static int
+scsi_report_luns(struct scsibus_softc *sc, int target,
+uint16_t ** const luns, size_t *nluns)
+{
+	struct scsi_report_luns replun;
+	struct scsi_report_luns_header *rlr;
+	struct scsi_report_luns_lun *lunp;
+
+	struct scsipi_channel *chan = sc->sc_channel;
+	struct scsipi_inquiry_data inqbuf;
+	struct scsipi_periph *periph;
+	uint16_t tmp;
+
+	int error;
+	size_t i, rlrlen;
+
+	periph = scsipi_alloc_periph(M_WAITOK);
+	periph->periph_channel = chan;
+	periph->periph_switch = _probe_dev;
+
+	periph->periph_target = target;
+	periph->periph_lun = 0;
+	periph->periph_quirks = chan->chan_defquirks;
+
+	if ((error = scsipi_inquire(periph, ,
+	XS_CTL_DISCOVERY | XS_CTL_SILENT)))
+		goto end2;
+	periph->periph_version = inqbuf.version & SID_ANSII;
+	if (periph->periph_version < 3) {
+		error = ENOTSUP;
+		goto end2;
+	}
+
+	rlrlen = sizeof(*rlr) + sizeof(*lunp) * 1;
+
+again:
+	rlr = kmem_zalloc(rlrlen, KM_SLEEP);
+
+	replun.opcode = SCSI_REPORT_LUNS;
+	replun.selectreport = SELECTREPORT_NORMAL;
+	_lto4b(rlrlen, replun.alloclen);
+
+	error = scsipi_command(periph, (void *), sizeof(replun),
+	(void *)rlr, rlrlen, SCSIPIRETRIES, 1, NULL,
+	XS_CTL_DATA_IN | XS_CTL_DISCOVERY | XS_CTL_SILENT);
+	if (error)
+		goto end;
+
+	if (sizeof(*rlr) + _4btol(rlr->length) > rlrlen &&
+	sizeof(*rlr) + _4btol(rlr->length) <= 32) {
+		const size_t old_rlrlen = rlrlen;
+		rlrlen = sizeof(*rlr) + uimin(_4btol(rlr->length),
+		16383 * sizeof(*lunp));
+		kmem_free(rlr, old_rlrlen);
+		rlr = NULL;
+		goto again;
+	}
+
+	KASSERT(nluns != NULL);
+	*nluns = (rlrlen - sizeof(*rlr)) / sizeof(*lunp);
+
+	KASSERT(luns != NULL);
+	*luns = kmem_alloc(*nluns * sizeof(**luns), KM_SLEEP);
+
+	for (i = 0; i < *nluns; i++) {
+		lunp = &((struct scsi_report_luns_lun *)[1])[i];
+		switch (lunp->lun[0] & 0xC0) {
+		default:
+			scsi_print_addr(periph);
+			printf("LUN %016"PRIx64" ignored\n", _8btol(lunp->lun));
+			(*luns)[i] = 0;
+			break;
+		case 0x40:
+			(*luns)[i] = _2btol(>lun[0]) & 0x3FFF;
+			break;
+		case 0x00:
+			(*luns)[i] = _2btol(>lun[0]) & 0x00FF;
+			break;
+		}
+	}
+
+	kheapsort(*luns, *nluns, sizeof(**luns), lun_compar, );
+
+end:
+	if (rlr)
+		kmem_free(rlr, rlrlen);
+end2:
+	scsipi_free_periph(periph);
+	return error;
+}

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Tue Dec 21 22:53:21 UTC 2021

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
scsi(4): Take kernel lock around entry into autoconf.

This code paths is entered by kthreads marked MP-safe, not just from
autoconf.

I'm not sure this is sufficient -- it's not clear to me whether
anything prevents concurrently scanning the same target.  Someone with
a better understanding of scsi(4) locking will have to audit this.

(For example, maybe it is guaranteed only to happen only either (a)
in autoconf, or (b) in a thread that doesn't start until autoconf is
done.  But I don't know -- and if it is this, it should be asserted
so we can verify it.)


To generate a diff of this commit:
cvs rdiff -u -r1.292 -r1.293 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsiconf.c
diff -u src/sys/dev/scsipi/scsiconf.c:1.292 src/sys/dev/scsipi/scsiconf.c:1.293
--- src/sys/dev/scsipi/scsiconf.c:1.292	Sat Aug  7 16:19:16 2021
+++ src/sys/dev/scsipi/scsiconf.c	Tue Dec 21 22:53:21 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsiconf.c,v 1.292 2021/08/07 16:19:16 thorpej Exp $	*/
+/*	$NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -48,7 +48,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.292 2021/08/07 16:19:16 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.293 2021/12/21 22:53:21 riastradh Exp $");
 
 #include 
 #include 
@@ -1012,6 +1012,7 @@ scsi_probe_device(struct scsibus_softc *
 	locs[SCSIBUSCF_TARGET] = target;
 	locs[SCSIBUSCF_LUN] = lun;
 
+	KERNEL_LOCK(1, NULL);
 	if ((cf = config_search(sc->sc_dev, ,
 CFARGS(.submatch = config_stdsubmatch,
    .locators = locs))) != NULL) {
@@ -1034,9 +1035,11 @@ scsi_probe_device(struct scsibus_softc *
 		 */
 		config_attach(sc->sc_dev, cf, , scsibusprint,
 		CFARGS(.locators = locs));
+		KERNEL_UNLOCK_ONE(NULL);
 	} else {
 		scsibusprint(, device_xname(sc->sc_dev));
 		aprint_normal(" not configured\n");
+		KERNEL_UNLOCK_ONE(NULL);
 		goto bad;
 	}
 

CVS commit: src/sys/dev/scsipi

[Taylor R Campbell]

Module Name:src
Committed By:   riastradh
Date:   Tue Dec 21 22:53:21 UTC 2021

Modified Files:
src/sys/dev/scsipi: scsiconf.c

Log Message:
scsi(4): Take kernel lock around entry into autoconf.

This code paths is entered by kthreads marked MP-safe, not just from
autoconf.

I'm not sure this is sufficient -- it's not clear to me whether
anything prevents concurrently scanning the same target.  Someone with
a better understanding of scsi(4) locking will have to audit this.

(For example, maybe it is guaranteed only to happen only either (a)
in autoconf, or (b) in a thread that doesn't start until autoconf is
done.  But I don't know -- and if it is this, it should be asserted
so we can verify it.)


To generate a diff of this commit:
cvs rdiff -u -r1.292 -r1.293 src/sys/dev/scsipi/scsiconf.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Jason R Thorpe]

Module Name:src
Committed By:   thorpej
Date:   Sun Sep 26 14:57:19 UTC 2021

Modified Files:
src/sys/dev/scsipi: ch.c

Log Message:
Use seltrue_filtops rather than rolling our own with filt_seltrue.


To generate a diff of this commit:
cvs rdiff -u -r1.94 -r1.95 src/sys/dev/scsipi/ch.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/ch.c
diff -u src/sys/dev/scsipi/ch.c:1.94 src/sys/dev/scsipi/ch.c:1.95
--- src/sys/dev/scsipi/ch.c:1.94	Sun Sep 26 01:16:09 2021
+++ src/sys/dev/scsipi/ch.c	Sun Sep 26 14:57:19 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: ch.c,v 1.94 2021/09/26 01:16:09 thorpej Exp $	*/
+/*	$NetBSD: ch.c,v 1.95 2021/09/26 14:57:19 thorpej Exp $	*/
 
 /*-
  * Copyright (c) 1996, 1997, 1998, 1999, 2004 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: ch.c,v 1.94 2021/09/26 01:16:09 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ch.c,v 1.95 2021/09/26 14:57:19 thorpej Exp $");
 
 #include 
 #include 
@@ -494,13 +494,6 @@ static const struct filterops chread_fil
 	.f_event = filt_chread,
 };
 
-static const struct filterops chwrite_filtops = {
-	.f_flags = FILTEROP_ISFD,
-	.f_attach = NULL,
-	.f_detach = filt_chdetach,
-	.f_event = filt_seltrue,
-};
-
 static int
 chkqfilter(dev_t dev, struct knote *kn)
 {
@@ -509,20 +502,18 @@ chkqfilter(dev_t dev, struct knote *kn)
 	switch (kn->kn_filter) {
 	case EVFILT_READ:
 		kn->kn_fop = _filtops;
+		kn->kn_hook = sc;
+		selrecord_knote(>sc_selq, kn);
 		break;
 
 	case EVFILT_WRITE:
-		kn->kn_fop = _filtops;
+		kn->kn_fop = _filtops;
 		break;
 
 	default:
 		return (EINVAL);
 	}
 
-	kn->kn_hook = sc;
-
-	selrecord_knote(>sc_selq, kn);
-
 	return (0);
 }
 

CVS commit: src/sys/dev/scsipi

[Jason R Thorpe]

Module Name:src
Committed By:   thorpej
Date:   Sun Sep 26 14:57:19 UTC 2021

Modified Files:
src/sys/dev/scsipi: ch.c

Log Message:
Use seltrue_filtops rather than rolling our own with filt_seltrue.


To generate a diff of this commit:
cvs rdiff -u -r1.94 -r1.95 src/sys/dev/scsipi/ch.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/sys/dev/scsipi

[Kimmo Suominen]

I don't think this should be reverted, because LUN 0 must exist, but if
there is no device on it, it will report "NOT PRESENT". We do not want
the scan to stop in this case, but it should continue with other LUNs
(such as those found through REPORT LUNS in the future).

Kind regards,
+ Kimmo


On Fri, Sep 18, 2020 at 03:04:25PM +, Jonathan A. Kollasch wrote:
> Module Name:  src
> Committed By: jakllsch
> Date: Fri Sep 18 15:04:25 UTC 2020
> 
> Modified Files:
>   src/sys/dev/scsipi: scsiconf.c
> 
> Log Message:
> Revert scsiconf.c 1.288, it only worked for LUN 1.
> 
> vioscsi(4) now sets PQUIRK_FORCELUNS, which fixes the original issue for
> all LUNs.
> 
> To-do: should issue REPORT LUNS and use the information it returns to
> probe LUNs in an optimized way.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.289 -r1.290 src/sys/dev/scsipi/scsiconf.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 

Re: CVS commit: src/sys/dev/scsipi

[Kimmo Suominen]

On Sun, Jul 12, 2020 at 12:05:37AM +0700, Robert Elz wrote:
> Just to make things clear here, the LUN you're talking about is not
> the scsi unit number (which is what I think Martin was referring to)
> but a sub-device number within a single scsi ID.   Right?

Correct. I should have written "SCSI target" instead of "SCSI bus" in
my commit message to avoid confusion.

In both of the use cases I highlighted (Proxmox and Linode), the disks
are always on SCSI ID 0. However, the "problematic" disks are on LUN 1
instead of LUN 0.

Out of curiosity, I just added a "scsi2" disk to the Proxmox VM. It has
been placed on LUN 2, and NetBSD does not pick it up even with this
patch... FreeBSD does (although the earlier LUNs are clearly causing
unexpected output from the "pass" attachments), so I guess I might be
looking at this a bit more. It would be nice to have (at least debug)
output about the LUN that terminates the scan loop.

Will probably open a ticket with Proxmox, too, in an attempt to put
a stop to this unnecessary wasteful skipping of perfectly good LUN
numbers.

+ Kimmo

Re: CVS commit: src/sys/dev/scsipi

[Robert Elz]

Date:Sat, 11 Jul 2020 18:24:51 +0300
From:Kimmo Suominen 
Message-ID:  <20200711152451.ga1...@homeworld.netbsd.org>

  | On Sat, Jul 11, 2020 at 05:00:02PM +0200, Martin Husemann wrote:
  | > I don't understand the change. When was this broken? This has always 
worked
  | > for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7.
  |
  | I think all real SCSI hardware I've had has always just only had LUN 0,
  | and each disk has been on its own SCSI ID (target).

Just to make things clear here, the LUN you're talking about is not
the scsi unit number (which is what I think Martin was referring to)
but a sub-device number within a single scsi ID.   Right?

In real scsi hardware, the only place I think I've ever seen other than
LUN 0 is in a raid array device, where there is a single scsi bus
attachment, with a single ID, and then each raid volume created gets
a different LUN (not all scsi raids work that way I think, but some do).

kre

Re: CVS commit: src/sys/dev/scsipi

[Martin Husemann]

On Sat, Jul 11, 2020 at 06:24:51PM +0300, Kimmo Suominen wrote:
> I think all real SCSI hardware I've had has always just only had LUN 0,
> and each disk has been on its own SCSI ID (target).

Yes, I confused ID and LUN here - just ignore me.

Martin

Re: CVS commit: src/sys/dev/scsipi

[Kimmo Suominen]

On Sat, Jul 11, 2020 at 05:00:02PM +0200, Martin Husemann wrote:
> I don't understand the change. When was this broken? This has always worked
> for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7.

I think all real SCSI hardware I've had has always just only had LUN 0,
and each disk has been on its own SCSI ID (target).

> Or is there something special in your setup?

Well, this is how Proxmox and Linode do it, and with this change it is
possible to install and use NetBSD on those platforms.

Of course, anyone running their own Proxmox could just avoid the "Virtio
SCSI single" controller type (or SCSI disks altogether), but there is
no user or admin control for specifying the LUN ID of a disk.

With a platform like Linode, you are stuck with the configuration it
creates for you from a boot profile (or other UI object). Although Linode
is usually responsive to bug reports, here it is not clear if the bug is
in how they do things or how NetBSD behaves.

With this change it is possible to run the NetBSD installer just like
the FreeBSD one (or any other "custom distro") on Linode.

Kind regards,
+ Kimmo

Re: CVS commit: src/sys/dev/scsipi

[Martin Husemann]

On Sat, Jul 11, 2020 at 05:57:46PM +0300, Kimmo Suominen wrote:
> On Sat, Jul 11, 2020 at 05:47:34PM +0300, Jukka Ruohonen wrote:
> > I'd reckon a pullup to NetBSD 9 would be in order?
> 
> Yes, I was just waiting to be able to link to mail-index.  I had
> already checked that the patch applies cleanly to both netbsd-9
> and netbsd-8.

I don't understand the change. When was this broken? This has always worked
for me e.g. with the sd0 at LUN 3 and the controller at 6 or 7.

Or is there something special in your setup?

Martin

Re: CVS commit: src/sys/dev/scsipi

[Kimmo Suominen]

On Sat, Jul 11, 2020 at 05:47:34PM +0300, Jukka Ruohonen wrote:
> I'd reckon a pullup to NetBSD 9 would be in order?

Yes, I was just waiting to be able to link to mail-index.  I had
already checked that the patch applies cleanly to both netbsd-9
and netbsd-8.

http://releng.netbsd.org/cgi-bin/req-9.cgi?show=1000
http://releng.netbsd.org/cgi-bin/req-8.cgi?show=1571

Cheers,
+ Kimmo

Re: CVS commit: src/sys/dev/scsipi

[Jukka Ruohonen]

On Sat, Jul 11, 2020 at 02:31:46PM +, Kimmo Suominen wrote:
> Use case 2: A Linode boot profile with multiple disks results in
> the first disk ("sda") on LUN 1, while the second disk ("sdb") is
> on LUN 0, each on their own bus.

As Linode is quite popular, and supposedly uses a rather similar setup to
its competitors (e.g., Vultr), I'd reckon a pullup to NetBSD 9 would be in
order?  Some of these (e.g., netcup.eu in Europe) even offer off-the-shelf
NetBSD 9 images.

- Jukka

CVS commit: src/sys/dev/scsipi

[SAITOH Masanobu]

Module Name:src
Committed By:   msaitoh
Date:   Thu Sep 19 03:37:31 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_base.c

Log Message:
 Use unsigned to avoid undefined behavior in scsipi_{get,put}_tag().
Found by kUBSan.


To generate a diff of this commit:
cvs rdiff -u -r1.182 -r1.183 src/sys/dev/scsipi/scsipi_base.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[SAITOH Masanobu]

Module Name:src
Committed By:   msaitoh
Date:   Thu Sep 19 03:37:31 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_base.c

Log Message:
 Use unsigned to avoid undefined behavior in scsipi_{get,put}_tag().
Found by kUBSan.


To generate a diff of this commit:
cvs rdiff -u -r1.182 -r1.183 src/sys/dev/scsipi/scsipi_base.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsipi_base.c
diff -u src/sys/dev/scsipi/scsipi_base.c:1.182 src/sys/dev/scsipi/scsipi_base.c:1.183
--- src/sys/dev/scsipi/scsipi_base.c:1.182	Thu Mar 28 10:44:29 2019
+++ src/sys/dev/scsipi/scsipi_base.c	Thu Sep 19 03:37:31 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsipi_base.c,v 1.182 2019/03/28 10:44:29 kardel Exp $	*/
+/*	$NetBSD: scsipi_base.c,v 1.183 2019/09/19 03:37:31 msaitoh Exp $	*/
 
 /*-
  * Copyright (c) 1998, 1999, 2000, 2002, 2003, 2004 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsipi_base.c,v 1.182 2019/03/28 10:44:29 kardel Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsipi_base.c,v 1.183 2019/09/19 03:37:31 msaitoh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -367,7 +367,7 @@ scsipi_get_tag(struct scsipi_xfer *xs)
 #endif
 
 	bit -= 1;
-	periph->periph_freetags[word] &= ~(1 << bit);
+	periph->periph_freetags[word] &= ~(1U << bit);
 	tag = (word << 5) | bit;
 
 	/* XXX Should eventually disallow this completely. */
@@ -398,7 +398,7 @@ scsipi_put_tag(struct scsipi_xfer *xs)
 	word = xs->xs_tag_id >> 5;
 	bit = xs->xs_tag_id & 0x1f;
 
-	periph->periph_freetags[word] |= (1 << bit);
+	periph->periph_freetags[word] |= (1U << bit);
 }
 
 /*

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Thu May 30 16:57:39 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_ioctl.c

Log Message:
use correct size when copying outgoing sense data.


To generate a diff of this commit:
cvs rdiff -u -r1.71 -r1.72 src/sys/dev/scsipi/scsipi_ioctl.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Thu May 30 16:57:39 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_ioctl.c

Log Message:
use correct size when copying outgoing sense data.


To generate a diff of this commit:
cvs rdiff -u -r1.71 -r1.72 src/sys/dev/scsipi/scsipi_ioctl.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsipi_ioctl.c
diff -u src/sys/dev/scsipi/scsipi_ioctl.c:1.71 src/sys/dev/scsipi/scsipi_ioctl.c:1.72
--- src/sys/dev/scsipi/scsipi_ioctl.c:1.71	Sun May 26 08:12:41 2019
+++ src/sys/dev/scsipi/scsipi_ioctl.c	Thu May 30 16:57:39 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsipi_ioctl.c,v 1.71 2019/05/26 08:12:41 mlelstv Exp $	*/
+/*	$NetBSD: scsipi_ioctl.c,v 1.72 2019/05/30 16:57:39 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.71 2019/05/26 08:12:41 mlelstv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.72 2019/05/30 16:57:39 mlelstv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_freebsd.h"
@@ -168,14 +168,16 @@ scsipi_user_done(struct scsipi_xfer *xs)
 		SC_DEBUG(periph, SCSIPI_DB3, ("have sense\n"));
 		screq->senselen_used = uimin(sizeof(xs->sense.scsi_sense),
 		SENSEBUFLEN);
-		memcpy(screq->sense, >sense.scsi_sense, screq->senselen);
+		memcpy(screq->sense, >sense.scsi_sense,
+		screq->senselen_used);
 		screq->retsts = SCCMD_SENSE;
 		break;
 	case XS_SHORTSENSE:
 		SC_DEBUG(periph, SCSIPI_DB3, ("have short sense\n"));
 		screq->senselen_used = uimin(sizeof(xs->sense.atapi_sense),
 		SENSEBUFLEN);
-		memcpy(screq->sense, >sense.scsi_sense, screq->senselen);
+		memcpy(screq->sense, >sense.atapi_sense,
+		screq->senselen_used);
 		screq->retsts = SCCMD_UNKNOWN; /* XXX need a shortsense here */
 		break;
 	case XS_DRIVER_STUFFUP:

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sun May 26 08:12:41 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_ioctl.c

Log Message:
Add sanity checks to SCIOCCOMMAND, adapter drivers might be confused or trigger
assertions (e.g. umass).


To generate a diff of this commit:
cvs rdiff -u -r1.70 -r1.71 src/sys/dev/scsipi/scsipi_ioctl.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Michael van Elst]

Module Name:src
Committed By:   mlelstv
Date:   Sun May 26 08:12:41 UTC 2019

Modified Files:
src/sys/dev/scsipi: scsipi_ioctl.c

Log Message:
Add sanity checks to SCIOCCOMMAND, adapter drivers might be confused or trigger
assertions (e.g. umass).


To generate a diff of this commit:
cvs rdiff -u -r1.70 -r1.71 src/sys/dev/scsipi/scsipi_ioctl.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/scsipi_ioctl.c
diff -u src/sys/dev/scsipi/scsipi_ioctl.c:1.70 src/sys/dev/scsipi/scsipi_ioctl.c:1.71
--- src/sys/dev/scsipi/scsipi_ioctl.c:1.70	Mon Sep  3 16:29:33 2018
+++ src/sys/dev/scsipi/scsipi_ioctl.c	Sun May 26 08:12:41 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: scsipi_ioctl.c,v 1.70 2018/09/03 16:29:33 riastradh Exp $	*/
+/*	$NetBSD: scsipi_ioctl.c,v 1.71 2019/05/26 08:12:41 mlelstv Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.70 2018/09/03 16:29:33 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: scsipi_ioctl.c,v 1.71 2019/05/26 08:12:41 mlelstv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_compat_freebsd.h"
@@ -328,10 +328,18 @@ scsipi_do_ioctl(struct scsipi_periph *pe
 		struct scsi_ioctl *si;
 		int len;
 
+		len = screq->datalen;
+
+		/*
+		 * If there is data, there must be a data buffer and a direction specified
+		 */
+		if (len > 0 && (screq->databuf == NULL ||
+		(screq->flags & (SCCMD_READ|SCCMD_WRITE)) == 0))
+			return (EINVAL);
+
 		si = si_get();
 		si->si_screq = *screq;
 		si->si_periph = periph;
-		len = screq->datalen;
 		if (len) {
 			si->si_iov.iov_base = screq->databuf;
 			si->si_iov.iov_len = len;

CVS commit: src/sys/dev/scsipi

[Frank Kardel]

Module Name:src
Committed By:   kardel
Date:   Sun May 19 19:06:53 UTC 2019

Modified Files:
src/sys/dev/scsipi: st.c

Log Message:
Add simple position recovery when positioning to EOM by reading
the position with READ_POSITION.
this allows for
mt eom
mt st
to return the correct file position.


To generate a diff of this commit:
cvs rdiff -u -r1.237 -r1.238 src/sys/dev/scsipi/st.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/scsipi/st.c
diff -u src/sys/dev/scsipi/st.c:1.237 src/sys/dev/scsipi/st.c:1.238
--- src/sys/dev/scsipi/st.c:1.237	Sat Feb 23 11:57:41 2019
+++ src/sys/dev/scsipi/st.c	Sun May 19 19:06:53 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: st.c,v 1.237 2019/02/23 11:57:41 kamil Exp $ */
+/*	$NetBSD: st.c,v 1.238 2019/05/19 19:06:53 kardel Exp $ */
 
 /*-
  * Copyright (c) 1998, 2004 The NetBSD Foundation, Inc.
@@ -50,7 +50,7 @@
  */
 
 #include 
-__KERNEL_RCSID(0, "$NetBSD: st.c,v 1.237 2019/02/23 11:57:41 kamil Exp $");
+__KERNEL_RCSID(0, "$NetBSD: st.c,v 1.238 2019/05/19 19:06:53 kardel Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_scsi.h"
@@ -355,6 +355,7 @@ static int	st_rewind(struct st_softc *, 
 static int	st_interpret_sense(struct scsipi_xfer *);
 static int	st_touch_tape(struct st_softc *);
 static int	st_erase(struct st_softc *, int full, int flags);
+static void st_updatefilepos(struct st_softc *);
 static int	st_rdpos(struct st_softc *, int, uint32_t *);
 static int	st_setpos(struct st_softc *, int, uint32_t *);
 
@@ -1823,8 +1824,7 @@ st_space(struct st_softc *st, int number
 	st->blkno = -1;
 			}
 		} else if (what == SP_EOM) {
-			/* This loses us relative position. */
-			st->fileno = st->blkno = -1;
+st_updatefilepos(st);
 		}
 	}
 	return error;
@@ -1997,6 +1997,54 @@ st_rewind(struct st_softc *st, u_int imm
 	return error;
 }
 
+static void
+st_updatefilepos(struct st_softc *st)
+{
+int error;
+uint8_t posdata[32];
+struct scsi_tape_read_position cmd;
+
+memset(, 0, sizeof(cmd));
+memset(, 0, sizeof(posdata));
+cmd.opcode = READ_POSITION;
+cmd.byte1 = 6;  /* service action: LONG FORM */
+
+error = scsipi_command(st->sc_periph, (void *), sizeof(cmd),
+(void *), sizeof(posdata), ST_RETRIES, ST_CTL_TIME, NULL,
+XS_CTL_SILENT | XS_CTL_DATA_IN);
+
+if (error == 0) {
+#ifdef SCSIPI_DEBUG
+if (st->sc_periph->periph_dbflags & SCSIPI_DB3) {
+int hard;
+
+printf("posdata: ");
+for (hard = 0; hard < sizeof(posdata); hard++)
+printf("%02x ", posdata[hard] & 0xff);
+printf("\n");
+}
+#endif
+if (posdata[0] & 0xC) { /* Block|Mark Position Unknown */
+SC_DEBUG(st->sc_periph, SCSIPI_DB3,
+ ("st_updatefilepos block/mark position unknown (0x%02x)\n",
+  posdata[0]));
+} else {
+st->fileno = _8btol([16]);
+st->blkno = 0;
+SC_DEBUG(st->sc_periph, SCSIPI_DB3,
+ ("st_updatefilepos file position %"PRId64"\n",
+  st->fileno));
+return;
+}
+} else {
+SC_DEBUG(st->sc_periph, SCSIPI_DB3,
+ ("st_updatefilepos READ POSITION(LONG_FORM) failed (error=%d)\n",
+  error));
+}
+st->fileno = -1;
+st->blkno = -1;
+}
+
 static int
 st_rdpos(struct st_softc *st, int hard, uint32_t *blkptr)
 {

Re: CVS commit: src/sys/dev/scsipi

[Kamil Rytarowski]

On 24.03.2018 16:47, Martin Husemann wrote:
> On Sat, Mar 24, 2018 at 09:38:15AM +0100, Thomas Klausner wrote:
>> On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote:
>>> On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote:
>>>
 I had to revert this in order to unbreak build.
>>>
>>>
>>> Please never do that.
>>
>> I think it's appropriate if it's a small patch that breaks the build
>> and can easily be backed out without side effects.
> 
> IIRC the official way is:
> 
>  - ping the commiter and notify them of the problem
>  - give a bit of time (like 24h or so)
>  - invoke core to revert the commit
> 
> See commit guidelines point 11:
> 
>   Do not revert other developer's commits.
> 
>   If you do not agree with another developer's
>   commit, do not revert it on your own. Contact the
>   other developer and explain to him or her the
>   issues you have with the commit in question. Ask
>   the other developer to back out the changes
>   instead of doing it yourself. 
> 
> Martin
> 

Thanks! I was trying to reach developers yesterday twice and get
feedback what to do, but no echo was received. It was almost a whole-day
breakage.

Next time I will try with a mail.



signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys/dev/scsipi

[Martin Husemann]

On Sat, Mar 24, 2018 at 09:38:15AM +0100, Thomas Klausner wrote:
> On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote:
> > On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote:
> > 
> > > I had to revert this in order to unbreak build.
> > 
> > 
> > Please never do that.
> 
> I think it's appropriate if it's a small patch that breaks the build
> and can easily be backed out without side effects.

IIRC the official way is:

 - ping the commiter and notify them of the problem
 - give a bit of time (like 24h or so)
 - invoke core to revert the commit

See commit guidelines point 11:

Do not revert other developer's commits.

If you do not agree with another developer's
commit, do not revert it on your own. Contact the
other developer and explain to him or her the
issues you have with the commit in question. Ask
the other developer to back out the changes
instead of doing it yourself. 

Martin

Re: CVS commit: src/sys/dev/scsipi

[Thomas Klausner]

On Sat, Mar 24, 2018 at 09:06:25AM +0100, Michael van Elst wrote:
> On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote:
> 
> > I had to revert this in order to unbreak build.
> 
> 
> Please never do that.

I think it's appropriate if it's a small patch that breaks the build
and can easily be backed out without side effects.
 Thomas

Re: CVS commit: src/sys/dev/scsipi

[Michael van Elst]

On Sat, Mar 24, 2018 at 02:50:05AM +0100, Kamil Rytarowski wrote:

> I had to revert this in order to unbreak build.


Please never do that.



-- 
Michael van Elst
Internet: mlel...@serpens.de
"A potential Snark may lurk in every tree."

Re: CVS commit: src/sys/dev/scsipi

[Robert Elz]

Date:Sat, 24 Mar 2018 02:50:05 +0100
From:Kamil Rytarowski 
Message-ID:  <8b2f90cf-0c7f-1154-b3fb-c4c600d07...@gmx.com>


  | > To generate a diff of this commit:
  | > cvs rdiff -u -r1.231 -r1.232 src/sys/dev/scsipi/st.c
  |
  | I had to revert this in order to unbreak build.

It looks as if an accompanying .h file update missed getting committed...

kre

Re: CVS commit: src/sys/dev/scsipi

[Kamil Rytarowski]

On 23.03.2018 07:01, Michael van Elst wrote:
> Module Name:  src
> Committed By: mlelstv
> Date: Fri Mar 23 06:01:07 UTC 2018
> 
> Modified Files:
>   src/sys/dev/scsipi: st.c
> 
> Log Message:
> Use separate lock to protect internal state and release locks when
> calling biodone.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.231 -r1.232 src/sys/dev/scsipi/st.c
> 


I had to revert this in order to unbreak build.

/usr/src/sys/dev/scsipi/st.c: In function 'stattach':
/usr/src/sys/dev/scsipi/st.c:398:16: error: 'struct st_softc' has no
member named 'buf_defer'; did you mean 'buf_queue'?
  bufq_alloc(>buf_defer, "fcfs", 0);
^~
/usr/src/sys/dev/scsipi/st.c:400:16: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
  mutex_init(>sc_iolock, MUTEX_DEFAULT, IPL_VM);
^~
/usr/src/sys/dev/scsipi/st.c: In function 'stdetach':
/usr/src/sys/dev/scsipi/st.c:451:15: error: 'struct st_softc' has no
member named 'buf_defer'; did you mean 'buf_queue'?
  bufq_drain(st->buf_defer);
   ^~
/usr/src/sys/dev/scsipi/st.c:459:14: error: 'struct st_softc' has no
member named 'buf_defer'; did you mean 'buf_queue'?
  bufq_free(st->buf_defer);
  ^~
/usr/src/sys/dev/scsipi/st.c:461:19: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
  mutex_destroy(>sc_iolock);
   ^~
/usr/src/sys/dev/scsipi/st.c: In function 'ststart':
/usr/src/sys/dev/scsipi/st.c:1270:17: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
  mutex_enter(>sc_iolock);
 ^~
/usr/src/sys/dev/scsipi/st.c:1272:26: error: 'struct st_softc' has no
member named 'buf_defer'; did you mean 'buf_queue'?
  while ((bp = bufq_get(st->buf_defer)) != NULL
  ^~
/usr/src/sys/dev/scsipi/st.c:1276:17: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_exit(>sc_iolock);
 ^~
/usr/src/sys/dev/scsipi/st.c:1280:18: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_enter(>sc_iolock);
  ^~
/usr/src/sys/dev/scsipi/st.c:1285:15: error: 'struct st_softc' has no
member named 'buf_defer'; did you mean 'buf_queue'?
bufq_put(st->buf_defer, bp);
   ^~
/usr/src/sys/dev/scsipi/st.c:1288:17: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_exit(>sc_iolock);
 ^~
/usr/src/sys/dev/scsipi/st.c:1296:18: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_enter(>sc_iolock);
  ^~
/usr/src/sys/dev/scsipi/st.c:1299:16: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
  mutex_exit(>sc_iolock);
^~
/usr/src/sys/dev/scsipi/st.c: In function 'stdone':
/usr/src/sys/dev/scsipi/st.c:1331:18: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_enter(>sc_iolock);
  ^~
/usr/src/sys/dev/scsipi/st.c:1353:17: error: 'struct st_softc' has no
member named 'sc_iolock'; did you mean 'sc_callout'?
   mutex_exit(>sc_iolock);
 ^~
--- st.o ---
*** [st.o] Error code 1

nbmake: stopped in /public/netbsd-root/sys/arch/amd64/compile/GENERIC
1 error

nbmake: stopped in /public/netbsd-root/sys/arch/amd64/compile/GENERIC

ERROR: Failed to make all in
"/public/netbsd-root/sys/arch/amd64/compile/GENERIC"





signature.asc
Description: OpenPGP digital signature

Re: CVS commit: src/sys/dev/scsipi

[Kengo NAKAHARA]

Hi,

Today, I found my environment panic while rebooting. As bisecting, it
seems the cause is below commit.

On 2017/06/18 7:35, Michael van Elst wrote:
> Module Name:  src
> Committed By: mlelstv
> Date: Sat Jun 17 22:35:50 UTC 2017
> 
> Modified Files:
>   src/sys/dev/scsipi: atapiconf.c cd.c scsi_base.c scsiconf.c
>   scsipi_base.c sd.c ss.c st.c
> 
> Log Message:
> The atapibus detach path did hold the channel mutex while calling into 
> autoconf,
> which would trigger a panic when unplugging a USB ATAPI CDROM.
> 
> Align detach code for scsibus and atapibus to fix this.
> 
> Also avoid races when detaching devices by replacing callout_stop with
> callout_halt.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.90 -r1.91 src/sys/dev/scsipi/atapiconf.c
> cvs rdiff -u -r1.340 -r1.341 src/sys/dev/scsipi/cd.c
> cvs rdiff -u -r1.91 -r1.92 src/sys/dev/scsipi/scsi_base.c
> cvs rdiff -u -r1.279 -r1.280 src/sys/dev/scsipi/scsiconf.c
> cvs rdiff -u -r1.175 -r1.176 src/sys/dev/scsipi/scsipi_base.c
> cvs rdiff -u -r1.324 -r1.325 src/sys/dev/scsipi/sd.c
> cvs rdiff -u -r1.88 -r1.89 src/sys/dev/scsipi/ss.c
> cvs rdiff -u -r1.230 -r1.231 src/sys/dev/scsipi/st.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

The panic message is here.

// snip
Jun 19 22:48:28 syncing disks... done
config_detach: detached device scsibus0 has children sd0
Skipping crash dump on recursive panic
panic: config_detach
fatal breakpoint trap in supervisor mode
trap type 1 code 0 rip 0x802249f5 cs 0x8 rflags 0x246 cr2 
0x70c8884aa38f ilevel 0 rsp 0xe4011099cc80
curlwp 0xe4027de12140 pid 632.1 lowest kstack 0xe401109992c0
Stopped in pid 632.1 (reboot) atnetbsd:breakpoint+0x5:  leave
db{2}> bt
breakpoint() at netbsd:breakpoint+0x5
vpanic() at netbsd:vpanic+0x140
snprintf() at netbsd:snprintf
config_detach() at netbsd:config_detach+0x218
config_detach_all() at netbsd:config_detach_all+0x97
cpu_reboot() at netbsd:cpu_reboot+0x173
sys_reboot() at netbsd:sys_reboot+0x75
syscall() at netbsd:syscall+0x1d8
--- syscall (number 208) ---
70c88843e5ba:

The addr2line of config_detach()+0x218 is here.
https://nxr.netbsd.org/xref/src/sys/kern/subr_autoconf.c#1728

My environment is amd64 which use sd0(USB Flash) as root filesystem.

Could someone have any solution?


Thanks,

-- 
//
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA 

Re: CVS commit: src/sys/dev/scsipi

[Martin Husemann]

On Tue, Apr 11, 2017 at 02:13:42AM +, Christos Zoulas wrote:
> Can't we fix this a different way? What's the problem?

The log description might have been confusing.

It just makes no sense to try to autoload any module while / is not yet
available. It may make sense to move the check for this into the module
auto load code though.

Martin

Re: CVS commit: src/sys/dev/scsipi

[Christos Zoulas]

In article <20170410215338.12a45f...@cvs.netbsd.org>,
Jaromir Dolecek  wrote:
>-=-=-=-=-=-
>
>just do not autoload scsiverbose module, it causes deadlock if it happens
>while root fs is being mounted
>
>adresses second part of PR kern/52147 by Michael van Elst, thank you

Can't we fix this a different way? What's the problem?

christos

Re: CVS commit: src/sys/dev/scsipi

[Frank Kardel]

Hi Erik !

I agree that 5 minutes is a really long time. Actually the inventory 
command will
wait even longer (5 min per element + 10 minutes as safeguard - I didn't 
change that).
Generating a uprintf would mean to manage a separate callout and using, 
I believe,

the tprintf() call from the callout function.

I am not sure whether that is worth the trouble. What timeout should we 
take for the
uprintf ? People with a slow device with a waiting indicator every 10 
seconds will not gain
any additional information other that they happen to have a slow device. 
Mostly changers
are used in automated environments. I am not looking at night at the 
operations of
our changers, but I am really annoyed when work is being aborted because 
of the scsi

timeout being too short.

tl;dr
I'd prefer not to add 'interactive experience' support to the kernel, 
Maybe someone can wip
up some tools(commands) for interactive usage. Maybe a warning about 
possible long

operation times in the chio man page could help.

Frank

On 08/10/13 00:06, Erik Fair wrote:

On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote:


Module Name:src
Committed By:   kardel
Date:   Fri Aug  9 19:58:44 UTC 2013

Modified Files:
src/sys/dev/scsipi: ch.c

Log Message:
bump command timeout to 5 minutes. several
types of changers (Overland PowerLoader, Dell
PowerVault) have been exceeding the 100 sec
limit aborting a perfectly (slowly) progressing
operation.

I think the kernel should uprintf(9) a notice to the effect that it has 
exceeded some (sooner than the 5 minute timeout) threshold and that it's really 
waiting for the device. Five minutes is a very long time for a timeout 
involving nominally local I/O devices. Without some progress indicator, users 
are likely to begin beating the system up (power cycle, etc) absent some 
persuasion to be patient.

Erik f...@netbsd.org

Re: CVS commit: src/sys/dev/scsipi

[Frank Kardel]

Erik,

I agree with this very much. Actually I have been toying around with 
this idea
quite some time. My thoughts where to build a sysctl tree for all scsi 
commands
with their default values and another level of sysctl timeout nodes 
where the
vendor and device name or driver name is part of the hierarchy to 
override timeouts

for some commands. Solaris has, I believe some of those knobs.

So far I didn't manage to invest the time for a complete implementation.

But maybe, as we currently are only suffering with changers, it would be 
sufficient
to add a single timeout knob for the changers. Like 
dev.ch0.scsi_timeout=300 (s)


How about that?

Frank

On 08/10/13 00:14, Erik Fair wrote:

On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote:


Module Name:src
Committed By:   kardel
Date:   Fri Aug  9 19:58:44 UTC 2013

Modified Files:
src/sys/dev/scsipi: ch.c

Log Message:
bump command timeout to 5 minutes. several
types of changers (Overland PowerLoader, Dell
PowerVault) have been exceeding the 100 sec
limit aborting a perfectly (slowly) progressing
operation.

Upon further reflection, I believe that this timeout value should be a 
device-specific tunable parameter because there is such wide variation in 
changer behavior/performance; perhaps by kernel config, or by sysctl(8) on a 
per-device-node basis. Or some other mechanism you prefer.

I believe that baking it into the kernel as a constant is not good design 
because we'll just see another commit just like this one at some later date, 
continuing to patch around the design problem.

Erik f...@netbsd.org

Re: CVS commit: src/sys/dev/scsipi

[Erik Fair]

On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote:

 Module Name:  src
 Committed By: kardel
 Date: Fri Aug  9 19:58:44 UTC 2013
 
 Modified Files:
   src/sys/dev/scsipi: ch.c
 
 Log Message:
 bump command timeout to 5 minutes. several
 types of changers (Overland PowerLoader, Dell
 PowerVault) have been exceeding the 100 sec
 limit aborting a perfectly (slowly) progressing
 operation.

I think the kernel should uprintf(9) a notice to the effect that it has 
exceeded some (sooner than the 5 minute timeout) threshold and that it's really 
waiting for the device. Five minutes is a very long time for a timeout 
involving nominally local I/O devices. Without some progress indicator, users 
are likely to begin beating the system up (power cycle, etc) absent some 
persuasion to be patient.

Erik f...@netbsd.org

Re: CVS commit: src/sys/dev/scsipi

[Erik Fair]

On Aug 9, 2013, at 12:58 , Frank Kardel kar...@netbsd.org wrote:

 Module Name:  src
 Committed By: kardel
 Date: Fri Aug  9 19:58:44 UTC 2013
 
 Modified Files:
   src/sys/dev/scsipi: ch.c
 
 Log Message:
 bump command timeout to 5 minutes. several
 types of changers (Overland PowerLoader, Dell
 PowerVault) have been exceeding the 100 sec
 limit aborting a perfectly (slowly) progressing
 operation.

Upon further reflection, I believe that this timeout value should be a 
device-specific tunable parameter because there is such wide variation in 
changer behavior/performance; perhaps by kernel config, or by sysctl(8) on a 
per-device-node basis. Or some other mechanism you prefer.

I believe that baking it into the kernel as a constant is not good design 
because we'll just see another commit just like this one at some later date, 
continuing to patch around the design problem.

Erik f...@netbsd.org

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Thu, Apr 19, 2012 at 06:39:29PM -0600, Warner Losh wrote:
 FreeBSD started out with MPSAFE and then went to NEEDS_GIANT as flags for the 
 drivers.  I'm with Matthew: patch the drivers that are broken (or not known 
 to be safe) and then you have a convenient thing to grep for when you want to 
 expand the drivers that are safe.  Much better that way, and it turned out to 
 be a big win in FreeBSD when we went from opt-in MPSAFE to out-out...

But we're still in the MPSAFE world, not yet at NEEDS_GIANT. Very few drivers
are MPSAFE in NetBSD, so almost all of them need to be patched.

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

Re: CVS commit: src/sys/dev/scsipi

[David Holland]

On Fri, Apr 20, 2012 at 09:37:00AM +0200, Manuel Bouyer wrote:
   we've never had autoconfig run with the kernel lock AFAICT, so this
   assumption has never been true.
  
  So this is a bug. The contract was really that spl-locked drivers would
  continue to work as is when fine-grained locking was introduced.
  I don't think it's reasonable to require every (but the few
  already MP-safe) drivers to take KERNEL_LOCK in their attach
  routine.

So autoconf should check and take the kernel lock before calling the
attach routine of a non-MPSAFE driver. If it doesn't already do this,
it's wrong.

Meanwhile, since scsipi is not MPSAFE, whoever is calling into scsipi
without taking the kernel lock first is wrong, so the change at the
beginning of this thread should be reverted.

(Furthermore, scsipi is itself wrong. It is a core component; it
should be MPSAFE.)

-- 
David A. Holland
dholl...@netbsd.org

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Fri, Apr 20, 2012 at 07:46:35AM +, David Holland wrote:
 On Fri, Apr 20, 2012 at 09:37:00AM +0200, Manuel Bouyer wrote:
we've never had autoconfig run with the kernel lock AFAICT, so this
assumption has never been true.
   
   So this is a bug. The contract was really that spl-locked drivers would
   continue to work as is when fine-grained locking was introduced.
   I don't think it's reasonable to require every (but the few
   already MP-safe) drivers to take KERNEL_LOCK in their attach
   routine.
 
 So autoconf should check and take the kernel lock before calling the
 attach routine of a non-MPSAFE driver. If it doesn't already do this,
 it's wrong.

I agree :)

 
 Meanwhile, since scsipi is not MPSAFE, whoever is calling into scsipi
 without taking the kernel lock first is wrong, so the change at the
 beginning of this thread should be reverted.

I also agree, in light of this discussion (I though that KERNEL_LOCK
was not held while cold on purpose, but it seems that the issue is at
another level, and more annoying).

But I find it acceptable as an interim solution, it at last
allows attaching scsi drivers that are probed at boot.

 
 (Furthermore, scsipi is itself wrong. It is a core component; it
 should be MPSAFE.)

I also agree. So should be ata, if_ethersubr, etc ...
that's a lot of work.

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Fri, Apr 20, 2012 at 06:06:35PM +1000, matthew green wrote:
 
   we've never had autoconfig run with the kernel lock AFAICT, so this
   assumption has never been true.
  
  So this is a bug. The contract was really that spl-locked drivers would
  continue to work as is when fine-grained locking was introduced.
 
 there are no actual bugs or races here.  this is about cleanup
 and ensuring code runs how it expects.

And that's the point: all code that use spl-style locking expects
to be called with KERNEL_LOCK held. And this includes the attach
routine.

 
  I don't think it's reasonable to require every (but the few
  already MP-safe) drivers to take KERNEL_LOCK in their attach
  routine.
 
 i'm not saying that they should.  i'm saying that scsipi drivers,
 regardless of whether they are fully MPSAFE or not, should now
 arrange to have the kernel lock held when calling into scsipi at
 all times.  this really has nothing to do with attach, except that
 attach is problem to fix currently in some drivers.

Almost all scsipi drivers ...

 
  It's not only IPL_VM drivers, it'a all driver wich don't register
  and interrupt handler explicitely marked MPSAFE (for example,
  with PCI_INTR_MPSAFE for PCI drivers).
 
 interesting; i hadn't seen PCI_INTR_MPSAFE before.  however, it
 seems ignored / unused.  the x86 pci_intr_setattr() does do
 something but it's never called.  none of the rest do anything.

this just means there's no PCI MP-SAFE driver yet ?

 
 my point is that drivers get interrupt protection from asking for
 an IPL_VM interrupt.  that's what tells the MD code to make sure
 the kernel lock is held for these interrupt handlers.

Or explicitely declared MPSAFE. at last x86 allows this.

 
 these KASSERT()s have been in place for a long time now and, until
 your change, we've fixed it at the caller level not worked around it
 at the scsipi level.

AFAIK the only drivers that has been changed are ncr53c9x and USB, and 
they
are MP-safe (they already use mutex instead of spl locking).
   
   usb is not MP safe in -current, only on the usbmp branch.  all those
   drivers take care of taking the kernel lock when calling into the
   scsipi code since they almost *always* run when not cold anyway.
  
  So things were fixed at the wrong place. Any non-MPSAFE driver is at
  risk right now
 
 i don't understand.  eg, the usbmp code never calls into scsipi
 during autoconfig.

usb is special in this regard.

 these asserts aren't specific to autoconfig,
 and IIRC, most drivers do not call into scsipi during attach.

Most if not all regular scsi drivers do: they do a config_found()
in their attach routine to attach the scsibus.
ATA does it from a kernel thread.

 
 your change can't work with the drivers that are broken without the
 major regression of holding kernel lock over autoconfig, or, never
 ever having them be modules or detached/reattached.

Maybe non-MPSAFE drivers can't be modules or detached/reattached, 
indeed.
   
   that's not true.  eg, radeondrm works just fine being MP safe or not.
   (it actually is MP safe, though it doesn't announce this to any of the
   rest of the system currently.)
  
  Yes, of course, unless there's a KASSERT, things will run fine most of
  the time. That's the problem with race conditions: it doesn't reliably
  fail. 
 
 that's not what i said.  what i said is that it is very trivial to have
 a non MPSAFE-driver handle being a module, being detached, and being
 reattached.

The discussion is about KERNEL_LOCK, and possible race of running
non-MPSAFE code without it.


  
  No, it's not specific to scsipi. It happens that scsipi asserts that
  KERNEL_LOCK is held, but any non-MPSAFE drivers needs to be called with
  KERNEL_LOCK held.
 
 why?  we've never done this for several years now and there's no reason
 why we think there is a problem with it currently.

why did you add this KASSERT to scsipi then ?

 we've actually run
 the interrupt part of autoconfig with multiple threads since sometime
 between netbsd 4.0 and 5.0.

this doesn't mean the races don't exists, just that we don't run into
them most of the time. Maybe that in some common case there's no possible
race, but that would be by accident, not by design.

 
   as in aside, you should make drvctl -d work for any drivers
   you maintain.  everyone (tm) should.  irrespective of modules
   or otherwise, the end result of it working tends to be bug
   fixes.
  
  It's hard to drvctl -d the root device ...
 
 eh, pxe is your friend.  :-)  

I can pxe boot the kernel; having root on NFS is another things.

 
If we want this to work for non-MPSAFE driver, then this has to be fixed
in autoconf (if the problem really exists, which I've not looked at 
yet).
   
   i don't believe there is a problem in autoconfig.  there is a
   problem with some (probably most) scsi drivers here, but that
   is about it.  it is true that the contracts have changed, but
 

re: CVS commit: src/sys/dev/scsipi

[matthew green]

 Module Name:  src
 Committed By: bouyer
 Date: Wed Apr 18 20:37:49 UTC 2012
 
 Modified Files:
   src/sys/dev/scsipi: scsipi_base.c
 
 Log Message:
 Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK

this is true, but can you please fix it differently?  ie autoconf
isn't cold.  most of scsi autoconf runs after cold is gone, so
i'd rather make whatever callers deal with it so that they work
cold or not.

thanks.


.mrg.

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Thu, Apr 19, 2012 at 04:41:04PM +1000, matthew green wrote:
 
  Module Name:src
  Committed By:   bouyer
  Date:   Wed Apr 18 20:37:49 UTC 2012
  
  Modified Files:
  src/sys/dev/scsipi: scsipi_base.c
  
  Log Message:
  Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK
 
 this is true, but can you please fix it differently?  ie autoconf
 isn't cold.  most of scsi autoconf runs after cold is gone, so
 i'd rather make whatever callers deal with it so that they work
 cold or not.

But if we're not cold, interrupts are enabled so I hope whatever calls
into scsi autoconf runs under the big lock if the underlying driver
isn't SMP-safe (and if it is, it's its job to take the KERNEL_LOCK).

The problem I've seen is with a driver I'm working on, similar to mfi(4)
(and I suspect mfi(4) would have had the same problem):
the driver's attach calls config_found_sm_loc() to attach the scsibus
while cold, so the big lock isn't helt at this point.

If the driver's attach is called after cold (e.g. after a detach/rescan
of the pci bus), the driver's attach should be called with KERNEL_LOCK
held, or bad things may happen when interrupts are enabled for this driver.

What kind of senario do you have in mind ?

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

re: CVS commit: src/sys/dev/scsipi

[matthew green]

   Module Name:  src
   Committed By: bouyer
   Date: Wed Apr 18 20:37:49 UTC 2012
   
   Modified Files:
 src/sys/dev/scsipi: scsipi_base.c
   
   Log Message:
   Fix KASSERT(): autoconf doesn't run under the KERNEL_LOCK
  
  this is true, but can you please fix it differently?  ie autoconf
  isn't cold.  most of scsi autoconf runs after cold is gone, so
  i'd rather make whatever callers deal with it so that they work
  cold or not.
 
 But if we're not cold, interrupts are enabled so I hope whatever calls
 into scsi autoconf runs under the big lock if the underlying driver
 isn't SMP-safe (and if it is, it's its job to take the KERNEL_LOCK).

the point of the KASSERT()'s it to make sure that, until scsipi is
made smp safe, calls into scsi are all done with the kernel lock.

 The problem I've seen is with a driver I'm working on, similar to mfi(4)
 (and I suspect mfi(4) would have had the same problem):
 the driver's attach calls config_found_sm_loc() to attach the scsibus
 while cold, so the big lock isn't helt at this point.

yeah - i'm pretty sure i've seen this and fixed it in a few other
places rather than the above change.

 If the driver's attach is called after cold (e.g. after a detach/rescan
 of the pci bus), the driver's attach should be called with KERNEL_LOCK
 held, or bad things may happen when interrupts are enabled for this driver.

there should be no reliance on cold being set for normal driver
attach.  it might be a module loaded after boot.  how ever the
driver is loaded, it will need to work without cold being set.

in my mind, the scsi code should try to run regardless of the value
of cold and that's why i replied above.

 What kind of senario do you have in mind ?

modules, as above.  or simply drvctl -d / -r.  IMO, only platform
specific code really should depend upon cold.  scsipi, as a
relatively high level subsystem, should not.

thanks.


.mrg.

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Thu, Apr 19, 2012 at 06:25:54PM +1000, matthew green wrote:
 [...]
 
  If the driver's attach is called after cold (e.g. after a detach/rescan
  of the pci bus), the driver's attach should be called with KERNEL_LOCK
  held, or bad things may happen when interrupts are enabled for this driver.
 
 there should be no reliance on cold being set for normal driver
 attach.  it might be a module loaded after boot.  how ever the
 driver is loaded, it will need to work without cold being set.

If it's a module laoded after boot, and the driver is not SMP-safe,
its attach function has to be called with KERNEL_LOCK held. Or you may
have problems when this driver starts receiving interrupts before its
attach function has returned (which is typically the case, interrupt enable
occurs somewhere in _attach()).

 
 in my mind, the scsi code should try to run regardless of the value
 of cold and that's why i replied above.
 
  What kind of senario do you have in mind ?
 
 modules, as above.  or simply drvctl -d / -r.  IMO, only platform
 specific code really should depend upon cold.  scsipi, as a
 relatively high level subsystem, should not.

But I don't think it's the job of non-SMP-safe drivers to make sure scsipi
is called with KERNEL_LOCK held (and scsipi may be the only subsystem
to check for KERNEL_LOCK(), but others do rely on it as well - e.g.
sys/dev/ata). Either attach() functions should always be called with
KERNEL_LOCK(), or for checks as above we have to accept that when
cold, locking is not fully up yet and lock checks should be bypassed.

-- 
Manuel Bouyer bou...@antioche.eu.org
 NetBSD: 26 ans d'experience feront toujours la difference
--

re: CVS commit: src/sys/dev/scsipi

[matthew green]

   If the driver's attach is called after cold (e.g. after a detach/rescan
   of the pci bus), the driver's attach should be called with KERNEL_LOCK
   held, or bad things may happen when interrupts are enabled for this 
   driver.
  
  there should be no reliance on cold being set for normal driver
  attach.  it might be a module loaded after boot.  how ever the
  driver is loaded, it will need to work without cold being set.
 
 If it's a module laoded after boot, and the driver is not SMP-safe,
 its attach function has to be called with KERNEL_LOCK held. Or you may
 have problems when this driver starts receiving interrupts before its
 attach function has returned (which is typically the case, interrupt enable
 occurs somewhere in _attach()).

i don't see how this matters.  drivers should never enable
interrupts if they're not ready to service them.

if this is happening, then it is a driver bug.

  in my mind, the scsi code should try to run regardless of the value
  of cold and that's why i replied above.
  
   What kind of senario do you have in mind ?
  
  modules, as above.  or simply drvctl -d / -r.  IMO, only platform
  specific code really should depend upon cold.  scsipi, as a
  relatively high level subsystem, should not.
 
 But I don't think it's the job of non-SMP-safe drivers to make sure scsipi
 is called with KERNEL_LOCK held (and scsipi may be the only subsystem
 to check for KERNEL_LOCK(), but others do rely on it as well - e.g.
 sys/dev/ata). Either attach() functions should always be called with
 KERNEL_LOCK(), or for checks as above we have to accept that when
 cold, locking is not fully up yet and lock checks should be bypassed.

the lack of checks elsewhere is not the point here.  we should add
them to dev/ata (and more) as well, if they depend upon these.

currently, autoconf isn't run with the kernel lock held at any point
and i don't think that saying that we should hold it during autoconf
is a good change.  we should fix the cases that trigger problems when
they appear, and it sounds like you've hit one in your new driver.
holding the kernel lock here would be a major step backwards.

scsipi depends upon kernel lock.  thus, callers should arrange for it
to be held.  since these are drivers calling, it is upto each driver
to do this currently.  this is what we've done with other problems we
have hit as they've arrived.

it may be that we have this problem in a large number of drivers but
these KASSERT()s have been in place for a long time now and, until
your change, we've fixed it at the caller level not worked around it
at the scsipi level.

your change can't work with the drivers that are broken without the
major regression of holding kernel lock over autoconfig, or, never
ever having them be modules or detached/reattached.

please make drvctl -d and rescan work for your drivers.  this will
make the checks against cold go away.


.mrg.

Re: CVS commit: src/sys/dev/scsipi

[Manuel Bouyer]

On Thu, Apr 19, 2012 at 07:00:56PM +1000, matthew green wrote:
 
If the driver's attach is called after cold (e.g. after a detach/rescan
of the pci bus), the driver's attach should be called with KERNEL_LOCK
held, or bad things may happen when interrupts are enabled for this 
driver.
   
   there should be no reliance on cold being set for normal driver
   attach.  it might be a module loaded after boot.  how ever the
   driver is loaded, it will need to work without cold being set.
  
  If it's a module laoded after boot, and the driver is not SMP-safe,
  its attach function has to be called with KERNEL_LOCK held. Or you may
  have problems when this driver starts receiving interrupts before its
  attach function has returned (which is typically the case, interrupt enable
  occurs somewhere in _attach()).
 
 i don't see how this matters.  drivers should never enable
 interrupts if they're not ready to service them.
 
 if this is happening, then it is a driver bug.

It looks like we're talking about different things.
I'm talking about this senario, with a non-smp-safe driver.

foo_attach()
{
do software and hardware initialisation();

s = splbio();
enable_interrupt(foo_intr());
finish_init_and_attach_children();
splx(s);
return;
}

foo_intr()
{

}

In this context, the driver enables interrupt, but don't expect to have its
interrupt handler called before the splx(), because
finish_init_and_attach_children() is running at IPL_BIO. But if foo_attach()
is not called with KERNEL_LOCK held, another CPU may call foo_intr(),
breaking the driver's spl protection.

And I don't think it's the driver's business to make sure KERNEL_LOCK is
held in it's attach routine: it's a non-MPSAFE driver and it should not
even have to know about KERNEL_LOCK.

If attach routines can be called without KERNEL_LOCK held after cold,
then all non-MPSAFE drivers (i.e. almost all of them) needs to be fixed.


 
   in my mind, the scsi code should try to run regardless of the value
   of cold and that's why i replied above.
   
What kind of senario do you have in mind ?
   
   modules, as above.  or simply drvctl -d / -r.  IMO, only platform
   specific code really should depend upon cold.  scsipi, as a
   relatively high level subsystem, should not.
  
  But I don't think it's the job of non-SMP-safe drivers to make sure scsipi
  is called with KERNEL_LOCK held (and scsipi may be the only subsystem
  to check for KERNEL_LOCK(), but others do rely on it as well - e.g.
  sys/dev/ata). Either attach() functions should always be called with
  KERNEL_LOCK(), or for checks as above we have to accept that when
  cold, locking is not fully up yet and lock checks should be bypassed.
 
 the lack of checks elsewhere is not the point here.  we should add
 them to dev/ata (and more) as well, if they depend upon these.
 
 currently, autoconf isn't run with the kernel lock held at any point
 and i don't think that saying that we should hold it during autoconf
 is a good change.  we should fix the cases that trigger problems when
 they appear, and it sounds like you've hit one in your new driver.
 holding the kernel lock here would be a major step backwards.
 
 scsipi depends upon kernel lock.  thus, callers should arrange for it
 to be held.  since these are drivers calling, it is upto each driver
 to do this currently.  this is what we've done with other problems we
 have hit as they've arrived.

if the caller is MPSAFE, I agree. Non-MPSAFE driver should not have to care
about KERNEL_LOCK (precisely because they are not MPSAFE).
The basic assumption about kernel locking was that that driver and subsystems
using spl locking would continue to work as it, without changes. If we
change this assumption a lot of code needs to be changed (that is, almost
all our drivers).

 
 it may be that we have this problem in a large number of drivers but
 these KASSERT()s have been in place for a long time now and, until
 your change, we've fixed it at the caller level not worked around it
 at the scsipi level.

AFAIK the only drivers that has been changed are ncr53c9x and USB, and they
are MP-safe (they already use mutex instead of spl locking).

 
 your change can't work with the drivers that are broken without the
 major regression of holding kernel lock over autoconfig, or, never
 ever having them be modules or detached/reattached.

Maybe non-MPSAFE drivers can't be modules or detached/reattached, indeed.
But that's a problem with autoconf not dealing with non-MPSAFE drivers,
not the driver themselve (they were working before the MP changes, they
should continue to work as-is). If you want autoconf to not run
under the KERNEL_LOCK when not needed, then is has to be extended so that
MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE
drivers are still called with KERNEL_LOCK.

 
 please make drvctl -d and rescan work for your drivers.  this will
 make the checks against cold go away.

I don't 

re: CVS commit: src/sys/dev/scsipi

[matthew green]

  scsipi depends upon kernel lock.  thus, callers should arrange for it
  to be held.  since these are drivers calling, it is upto each driver
  to do this currently.  this is what we've done with other problems we
  have hit as they've arrived.
 
 if the caller is MPSAFE, I agree. Non-MPSAFE driver should not have to care
 about KERNEL_LOCK (precisely because they are not MPSAFE).
 The basic assumption about kernel locking was that that driver and subsystems
 using spl locking would continue to work as it, without changes. If we
 change this assumption a lot of code needs to be changed (that is, almost
 all our drivers).

we've never had autoconfig run with the kernel lock AFAICT, so this
assumption has never been true.  the thing that does work is that
IPL_VM using drivers will automatically get kernel locked in their
interrupt handlers.

  these KASSERT()s have been in place for a long time now and, until
  your change, we've fixed it at the caller level not worked around it
  at the scsipi level.
 
 AFAIK the only drivers that has been changed are ncr53c9x and USB, and they
 are MP-safe (they already use mutex instead of spl locking).

usb is not MP safe in -current, only on the usbmp branch.  all those
drivers take care of taking the kernel lock when calling into the
scsipi code since they almost *always* run when not cold anyway.

infact, the asserts were added in -current due to the usbmp work
and the understanding that they would advance the rest of the system.

  your change can't work with the drivers that are broken without the
  major regression of holding kernel lock over autoconfig, or, never
  ever having them be modules or detached/reattached.
 
 Maybe non-MPSAFE drivers can't be modules or detached/reattached, indeed.

that's not true.  eg, radeondrm works just fine being MP safe or not.
(it actually is MP safe, though it doesn't announce this to any of the
rest of the system currently.)

 But that's a problem with autoconf not dealing with non-MPSAFE drivers,
 not the driver themselve (they were working before the MP changes, they
 should continue to work as-is). If you want autoconf to not run
 under the KERNEL_LOCK when not needed, then is has to be extended so that
 MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE
 drivers are still called with KERNEL_LOCK.

it seems the wrong answer to me.  instead of fixing the one or
two drivers you've found problems in, you want to add more
infrastructure (IMO clutter) to the system and patch all the
drivers that *are* ok.  considering that this problem is
very specific to scsipi, i don't think that is a viable
solution.

  please make drvctl -d and rescan work for your drivers.  this will
  make the checks against cold go away.
 
 I don't want to touch each driver in our tree for drvctl -d/rescan to work.

as in aside, you should make drvctl -d work for any drivers
you maintain.  everyone (tm) should.  irrespective of modules
or otherwise, the end result of it working tends to be bug
fixes.

 If we want this to work for non-MPSAFE driver, then this has to be fixed
 in autoconf (if the problem really exists, which I've not looked at yet).

i don't believe there is a problem in autoconfig.  there is a
problem with some (probably most) scsi drivers here, but that
is about it.  it is true that the contracts have changed, but
this is progress after all.  the assert exists solely to find
places that are wrong and fix them.  your change hides this.

this isn't about SMPifing a whole driver, but adding a call to
taken/release the kernel lock around a few (often just one)
calls in a driver.  it has never been a difficult change to
make.  if you have drivers that needs it, i am happy to send
patches for testing.


.mrg.

Re: CVS commit: src/sys/dev/scsipi

[Warner Losh]

On Apr 19, 2012, at 6:03 PM, matthew green wrote:
 But that's a problem with autoconf not dealing with non-MPSAFE drivers,
 not the driver themselve (they were working before the MP changes, they
 should continue to work as-is). If you want autoconf to not run
 under the KERNEL_LOCK when not needed, then is has to be extended so that
 MPSAFE drivers can register themselves as MPSAFE, so that non-MPSAFE
 drivers are still called with KERNEL_LOCK.
 
 it seems the wrong answer to me.  instead of fixing the one or
 two drivers you've found problems in, you want to add more
 infrastructure (IMO clutter) to the system and patch all the
 drivers that *are* ok.  considering that this problem is
 very specific to scsipi, i don't think that is a viable
 solution.

FreeBSD started out with MPSAFE and then went to NEEDS_GIANT as flags for the 
drivers.  I'm with Matthew: patch the drivers that are broken (or not known to 
be safe) and then you have a convenient thing to grep for when you want to 
expand the drivers that are safe.  Much better that way, and it turned out to 
be a big win in FreeBSD when we went from opt-in MPSAFE to out-out...

Warner

Re: CVS commit: src/sys/dev/scsipi

[David Young]

On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote:
 Module Name:  src
 Committed By: hannken
 Date: Mon Apr 25 14:14:22 UTC 2011
 
 Modified Files:
   src/sys/dev/scsipi: scsiconf.c
 
 Log Message:
 Don't kill outstanding requests when detaching a scsibus on shutdown.
 Both the controller and tyhe targets are still running.

I don't think you have fixed the actual bug.  It sounds like the
detachment routine is broken in every case where the controller was not
abruptly detached, but the driver relinquishes control of the controller
in an orderly fashion because of an operator command.

Dave

-- 
David Young OJC Technologies
dyo...@ojctech.com  Urbana, IL * (217) 344-0444 x24

Re: CVS commit: src/sys/dev/scsipi

[Juergen Hannken-Illjes]

On Mon, Apr 25, 2011 at 10:33:14AM -0500, David Young wrote:
 On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote:
  Module Name:src
  Committed By:   hannken
  Date:   Mon Apr 25 14:14:22 UTC 2011
  
  Modified Files:
  src/sys/dev/scsipi: scsiconf.c
  
  Log Message:
  Don't kill outstanding requests when detaching a scsibus on shutdown.
  Both the controller and tyhe targets are still running.
 
 I don't think you have fixed the actual bug.  It sounds like the
 detachment routine is broken in every case where the controller was not
 abruptly detached, but the driver relinquishes control of the controller
 in an orderly fashion because of an operator command.

The actual bug was i386 shutdown, a loop of vfs_unmountall1, config_detach_all
and vfs_unmount_forceone.  Here config_detach_all() detaches devices from
the leafs up.
For me it sometimes happend that the (scsi) root disk had outstanding xfers
when it came to config_detach_all().  The disk would not detach but the
bus detach would kill all outstanding operations.  I don't want these xfers to
abort but the disk continue operations until all xfers are done.

And yes, this detach routine looks bogus at least ...

-- 
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)

Re: CVS commit: src/sys/dev/scsipi

[David Young]

On Mon, Apr 25, 2011 at 06:26:09PM +0200, Juergen Hannken-Illjes wrote:
 On Mon, Apr 25, 2011 at 10:33:14AM -0500, David Young wrote:
  On Mon, Apr 25, 2011 at 02:14:23PM +, Juergen Hannken-Illjes wrote:
   Module Name:  src
   Committed By: hannken
   Date: Mon Apr 25 14:14:22 UTC 2011
   
   Modified Files:
 src/sys/dev/scsipi: scsiconf.c
   
   Log Message:
   Don't kill outstanding requests when detaching a scsibus on shutdown.
   Both the controller and tyhe targets are still running.
  
  I don't think you have fixed the actual bug.  It sounds like the
  detachment routine is broken in every case where the controller was not
  abruptly detached, but the driver relinquishes control of the controller
  in an orderly fashion because of an operator command.
 
 The actual bug was i386 shutdown, a loop of vfs_unmountall1, config_detach_all
 and vfs_unmount_forceone.  Here config_detach_all() detaches devices from
 the leafs up.
 For me it sometimes happend that the (scsi) root disk had outstanding xfers
 when it came to config_detach_all().  The disk would not detach but the
 bus detach would kill all outstanding operations.  I don't want these xfers to
 abort but the disk continue operations until all xfers are done.
 
 And yes, this detach routine looks bogus at least ...

The bug was in scsibusdetach(), which is not doing things in the proper
order: it has to detach its children and check for error.  If no error,
then it can release the resources that the children were using.  See
attached patch.

Dave

-- 
David Young OJC Technologies
dyo...@ojctech.com  Urbana, IL * (217) 344-0444 x24
Index: scsiconf.c
===
RCS file: /cvsroot/src/sys/dev/scsipi/scsiconf.c,v
retrieving revision 1.261
diff -u -p -r1.261 scsiconf.c
--- scsiconf.c  25 Apr 2011 14:14:22 -  1.261
+++ scsiconf.c  25 Apr 2011 17:30:31 -
@@ -256,11 +256,20 @@ scsibusdetach(device_t self, int flags)
struct scsipi_xfer *xs;
int error;
 
+   /*
+* Detach all of the periphs.
+*/
+   if ((error = scsipi_target_detach(chan, -1, -1, flags)) != 0)
+   return error;
+
pmf_device_deregister(self);
 
/*
 * Process outstanding commands (which will never complete as the
 * controller is gone).
+*
+* XXX Surely this is redundant?  If we get this far, the
+* XXX peripherals have all been detached.
 */
for (ctarget = 0; ctarget  chan-chan_ntargets; ctarget++) {
if (ctarget == chan-chan_id)
@@ -269,8 +278,6 @@ scsibusdetach(device_t self, int flags)
periph = scsipi_lookup_periph(chan, ctarget, clun);
if (periph == NULL)
continue;
-   if ((flags  DETACH_SHUTDOWN) != 0)
-   return EBUSY;
TAILQ_FOREACH(xs, periph-periph_xferq, device_q) {
callout_stop(xs-xs_callout);
xs-error = XS_DRIVER_STUFFUP;
@@ -280,16 +287,10 @@ scsibusdetach(device_t self, int flags)
}
 
/*
-* Detach all of the periphs.
-*/
-   error = scsipi_target_detach(chan, -1, -1, flags);
-
-   /*
 * Now shut down the channel.
-* XXX only if no errors ?
 */
scsipi_channel_shutdown(chan);
-   return (error);
+   return 0;
 }
 
 /*

CVS commit: src/sys/dev/scsipi

[Antti Kantee]

Module Name:src
Committed By:   pooka
Date:   Fri Feb 12 11:39:33 UTC 2010

Modified Files:
src/sys/dev/scsipi: scsipi_base.c

Log Message:
delay() is used here, so need to include machine/param.h.


To generate a diff of this commit:
cvs rdiff -u -r1.150 -r1.151 src/sys/dev/scsipi/scsipi_base.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

CVS commit: src/sys/dev/scsipi

[Christoph Egger]

Module Name:src
Committed By:   cegger
Date:   Sat Feb  6 23:13:59 UTC 2010

Modified Files:
src/sys/dev/scsipi: scsi_spc.h

Log Message:
scsi_sense_data: add comments taken from src/sys/dev/ieee1394/sbp.h


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/sys/dev/scsipi/scsi_spc.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Re: CVS commit: src/sys/dev/scsipi

[Christos Zoulas]

On May 13,  7:56am, christoph_eg...@gmx.de (Christoph Egger) wrote:
-- Subject: Re: CVS commit: src/sys/dev/scsipi

| Christos Zoulas wrote:
|  Module Name:src
|  Committed By:   christos
|  Date:   Wed May 13 02:35:25 UTC 2009
|  
|  Modified Files:
|  src/sys/dev/scsipi: scsipiconf.h
|  
|  Log Message:
|  sprinkle #ifdef _KERNEL to make scsictl compile.
|  
| 
| Ah, the device_t change exposed something that shouldn't
| be visible in userspace, it seems.
| 
| Thanks for fixing it and sorry for build breakage.
| I should have also compile-tested userland of one arch at least,
| not just many kernels.

Not a problem. I think that the parts of this file that are needed from
scsictl don't really belong there, since none of the structures seem to
be useable from userland.

christos