CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: nat Date: Sun Sep 29 12:36:41 UTC 2024 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Add quirk for the standard PiSCSI cdrom emulation. To generate a diff of this commit: cvs rdiff -u -r1.304 -r1.305 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.304 src/sys/dev/scsipi/scsiconf.c:1.305 --- src/sys/dev/scsipi/scsiconf.c:1.304 Sat Jun 22 10:10:07 2024 +++ src/sys/dev/scsipi/scsiconf.c Sun Sep 29 12:36:40 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: scsiconf.c,v 1.304 2024/06/22 10:10:07 palle Exp $ */ +/* $NetBSD: scsiconf.c,v 1.305 2024/09/29 12:36:40 nat Exp $ */ /*- * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc. @@ -48,7 +48,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.304 2024/06/22 10:10:07 palle Exp $"); +__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.305 2024/09/29 12:36:40 nat Exp $"); #include #include @@ -738,6 +738,8 @@ static const struct scsi_quirk_inquiry_p "MST ", "SnapLink", ""}, PQUIRK_NOLUNS}, {{T_DIRECT, T_FIXED, "NEC ", "D3847 ", "0307"}, PQUIRK_NOLUNS}, + {{T_CDROM, T_REMOV, + "PiSCSI ", "SCSI CD-ROM ", ""}, PQUIRK_NOREADDISCINFO}, {{T_DIRECT, T_FIXED, "QUANTUM ", "ELS85S ", ""}, PQUIRK_AUTOSAVE}, {{T_DIRECT, T_FIXED,
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: nat Date: Sun Sep 29 12:36:41 UTC 2024 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Add quirk for the standard PiSCSI cdrom emulation. To generate a diff of this commit: cvs rdiff -u -r1.304 -r1.305 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
Module Name:src Committed By: mlelstv Date: Sat Sep 28 08:57:47 UTC 2024 Modified Files: src/sys/dev/scsipi: sd.c Log Message: Don't artificially limit block size to 4096 bytes, use MAXPHYS. To generate a diff of this commit: cvs rdiff -u -r1.336 -r1.337 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.336 src/sys/dev/scsipi/sd.c:1.337 --- src/sys/dev/scsipi/sd.c:1.336 Sat Feb 24 22:06:49 2024 +++ src/sys/dev/scsipi/sd.c Sat Sep 28 08:57:47 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: sd.c,v 1.336 2024/02/24 22:06:49 mlelstv Exp $ */ +/* $NetBSD: sd.c,v 1.337 2024/09/28 08:57:47 mlelstv Exp $ */ /*- * Copyright (c) 1998, 2003, 2004 The NetBSD Foundation, Inc. @@ -47,7 +47,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.336 2024/02/24 22:06:49 mlelstv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: sd.c,v 1.337 2024/09/28 08:57:47 mlelstv Exp $"); #ifdef _KERNEL_OPT #include "opt_scsi.h" @@ -1315,7 +1315,7 @@ static int sd_validate_blksize(struct scsipi_periph *periph, int len) { - if (len >= 256 && powerof2(len) && len <= 4096) { + if (len >= 256 && powerof2(len) && len <= MAXPHYS) { return 1; }
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: mlelstv Date: Sat Sep 28 08:57:47 UTC 2024 Modified Files: src/sys/dev/scsipi: sd.c Log Message: Don't artificially limit block size to 4096 bytes, use MAXPHYS. To generate a diff of this commit: cvs rdiff -u -r1.336 -r1.337 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
Module Name:src Committed By: nat Date: Sun Sep 8 04:42:49 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: Only input needs to be polled. Tested with PDMA on mac68k (on emulated hw). To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 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.7 src/sys/dev/scsipi/if_dse.c:1.8 --- src/sys/dev/scsipi/if_dse.c:1.7 Sun Sep 8 04:40:34 2024 +++ src/sys/dev/scsipi/if_dse.c Sun Sep 8 04:42:49 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: if_dse.c,v 1.7 2024/09/08 04:40:34 nat Exp $ */ +/* $NetBSD: if_dse.c,v 1.8 2024/09/08 04:42:49 nat Exp $ */ /* * Driver for DaynaPORT SCSI/Link SCSI-Ethernet @@ -578,8 +578,7 @@ dse_send_worker(struct work *wk, void *c error = dse_scsipi_cmd(sc->sc_periph, (void *)&cmd_send, sizeof(cmd_send), sc->sc_tbuf, len, DSE_RETRIES, - DSE_TIMEOUT, NULL, XS_CTL_NOSLEEP | XS_CTL_POLL | - XS_CTL_DATA_OUT); + DSE_TIMEOUT, NULL, XS_CTL_NOSLEEP | XS_CTL_DATA_OUT); if (error) { aprint_error_dev(sc->sc_dev, "not queued, error %d\n", error); @@ -1080,7 +1079,7 @@ dse_set_multi(struct dse_softc *sc) error = dse_scsipi_cmd(sc->sc_periph, (struct scsipi_generic*)&cmd_set_multi, sizeof(cmd_set_multi), - mybuf, len, DSE_RETRIES, DSE_TIMEOUT, NULL, XS_CTL_POLL | XS_CTL_DATA_OUT); + mybuf, len, DSE_RETRIES, DSE_TIMEOUT, NULL, XS_CTL_DATA_OUT); free(mybuf, M_DEVBUF);
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: nat Date: Sun Sep 8 04:42:49 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: Only input needs to be polled. Tested with PDMA on mac68k (on emulated hw). To generate a diff of this commit: cvs rdiff -u -r1.7 -r1.8 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
Module Name:src Committed By: nat Date: Sun Sep 8 04:40:34 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: Use aprint_normal_dev for ethernet address. NFC. To generate a diff of this commit: cvs rdiff -u -r1.6 -r1.7 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.6 src/sys/dev/scsipi/if_dse.c:1.7 --- src/sys/dev/scsipi/if_dse.c:1.6 Sat Jul 6 10:37:33 2024 +++ src/sys/dev/scsipi/if_dse.c Sun Sep 8 04:40:34 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: if_dse.c,v 1.6 2024/07/06 10:37:33 andvar Exp $ */ +/* $NetBSD: if_dse.c,v 1.7 2024/09/08 04:40:34 nat Exp $ */ /* * Driver for DaynaPORT SCSI/Link SCSI-Ethernet @@ -983,7 +983,7 @@ dse_get_addr(struct dse_softc *sc, uint8 if (error == 0) { memcpy(myaddr, &(tmpbuf[0]), ETHER_ADDR_LEN); - aprint_error_dev(sc->sc_dev, "ethernet address %s\n", + aprint_normal_dev(sc->sc_dev, "ethernet address %s\n", ether_sprintf(myaddr)); }
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: nat Date: Sun Sep 8 04:40:34 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: Use aprint_normal_dev for ethernet address. NFC. To generate a diff of this commit: cvs rdiff -u -r1.6 -r1.7 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
Module Name:src Committed By: andvar Date: Sat Jul 6 10:37:33 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: s/occurence/occurrence/ in comment. To generate a diff of this commit: cvs rdiff -u -r1.5 -r1.6 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.5 src/sys/dev/scsipi/if_dse.c:1.6 --- src/sys/dev/scsipi/if_dse.c:1.5 Mon Jan 1 22:29:48 2024 +++ src/sys/dev/scsipi/if_dse.c Sat Jul 6 10:37:33 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: if_dse.c,v 1.5 2024/01/01 22:29:48 gutteridge Exp $ */ +/* $NetBSD: if_dse.c,v 1.6 2024/07/06 10:37:33 andvar Exp $ */ /* * Driver for DaynaPORT SCSI/Link SCSI-Ethernet @@ -1364,7 +1364,7 @@ dseopen(dev_t dev, int flag, int fmt, st /* * close the device.. only called if we are the LAST - * occurence of an open device + * occurrence of an open device */ int dseclose(dev_t dev, int flag, int fmt, struct lwp *l)
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: andvar Date: Sat Jul 6 10:37:33 UTC 2024 Modified Files: src/sys/dev/scsipi: if_dse.c Log Message: s/occurence/occurrence/ in comment. To generate a diff of this commit: cvs rdiff -u -r1.5 -r1.6 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
Module Name:src Committed By: palle Date: Sat Jun 22 10:10:07 UTC 2024 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Add quirk for sparc64/sun4v ldom virtual cd devices To generate a diff of this commit: cvs rdiff -u -r1.303 -r1.304 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.303 src/sys/dev/scsipi/scsiconf.c:1.304 --- src/sys/dev/scsipi/scsiconf.c:1.303 Sat Oct 15 18:42:49 2022 +++ src/sys/dev/scsipi/scsiconf.c Sat Jun 22 10:10:07 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: scsiconf.c,v 1.303 2022/10/15 18:42:49 jmcneill Exp $ */ +/* $NetBSD: scsiconf.c,v 1.304 2024/06/22 10:10:07 palle Exp $ */ /*- * Copyright (c) 1998, 1999, 2004 The NetBSD Foundation, Inc. @@ -48,7 +48,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.303 2022/10/15 18:42:49 jmcneill Exp $"); +__KERNEL_RCSID(0, "$NetBSD: scsiconf.c,v 1.304 2024/06/22 10:10:07 palle Exp $"); #include #include @@ -864,6 +864,8 @@ static const struct scsi_quirk_inquiry_p "SONY", "CDL1100 ", ""}, PQUIRK_NOLUNS}, {{T_ENCLOSURE, T_FIXED, "SUN ", "SENA", ""}, PQUIRK_NOLUNS}, + {{T_CDROM, T_REMOV, + "SUN ", "Virtual CDROM ", ""}, PQUIRK_NOREADDISCINFO}, }; /*
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: palle Date: Sat Jun 22 10:10:07 UTC 2024 Modified Files: src/sys/dev/scsipi: scsiconf.c Log Message: Add quirk for sparc64/sun4v ldom virtual cd devices To generate a diff of this commit: cvs rdiff -u -r1.303 -r1.304 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
Module Name:src Committed By: palle Date: Sat Jun 22 10:07:46 UTC 2024 Modified Files: src/sys/dev/scsipi: cd.c scsipiconf.h Log Message: Add quirk for devices that does not handle READ_DISCINFO To generate a diff of this commit: cvs rdiff -u -r1.354 -r1.355 src/sys/dev/scsipi/cd.c cvs rdiff -u -r1.130 -r1.131 src/sys/dev/scsipi/scsipiconf.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
Module Name:src Committed By: palle Date: Sat Jun 22 10:07:46 UTC 2024 Modified Files: src/sys/dev/scsipi: cd.c scsipiconf.h Log Message: Add quirk for devices that does not handle READ_DISCINFO To generate a diff of this commit: cvs rdiff -u -r1.354 -r1.355 src/sys/dev/scsipi/cd.c cvs rdiff -u -r1.130 -r1.131 src/sys/dev/scsipi/scsipiconf.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/cd.c diff -u src/sys/dev/scsipi/cd.c:1.354 src/sys/dev/scsipi/cd.c:1.355 --- src/sys/dev/scsipi/cd.c:1.354 Sun Jun 26 21:00:28 2022 +++ src/sys/dev/scsipi/cd.c Sat Jun 22 10:07:46 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: cd.c,v 1.354 2022/06/26 21:00:28 andvar Exp $ */ +/* $NetBSD: cd.c,v 1.355 2024/06/22 10:07:46 palle 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.354 2022/06/26 21:00:28 andvar Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cd.c,v 1.355 2024/06/22 10:07:46 palle Exp $"); #include #include @@ -1545,6 +1545,11 @@ read_cd_capacity(struct scsipi_periph *p *blksize = 2048; /* some drives lie ! */ } + /* If the device doesn't handle READ_DISCINFO properly, */ + /* return the dummies */ + if (periph->periph_quirks & PQUIRK_NOREADDISCINFO) + return 0; + /* recordables have READ_DISCINFO implemented */ flags = XS_CTL_DATA_IN | XS_CTL_SILENT; memset(&di_cmd, 0, sizeof(di_cmd)); @@ -2414,11 +2419,12 @@ cd_setblksize(struct cd_softc *cd) } if (bsize == 0) { -printf("cd_setblksize: trying to change bsize, but no blk_desc\n"); + printf("cd_setblksize: trying to change bsize, but no blk_desc\n"); + return (EINVAL); } if (_3btol(bdesc->blklen) == 2048) { -printf("cd_setblksize: trying to change bsize, but blk_desc is correct\n"); + printf("cd_setblksize: trying to change bsize, but blk_desc is correct\n"); return (EINVAL); } Index: src/sys/dev/scsipi/scsipiconf.h diff -u src/sys/dev/scsipi/scsipiconf.h:1.130 src/sys/dev/scsipi/scsipiconf.h:1.131 --- src/sys/dev/scsipi/scsipiconf.h:1.130 Thu Mar 28 10:44:29 2019 +++ src/sys/dev/scsipi/scsipiconf.h Sat Jun 22 10:07:46 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: scsipiconf.h,v 1.130 2019/03/28 10:44:29 kardel Exp $ */ +/* $NetBSD: scsipiconf.h,v 1.131 2024/06/22 10:07:46 palle Exp $ */ /*- * Copyright (c) 1998, 1999, 2000, 2004 The NetBSD Foundation, Inc. @@ -504,6 +504,8 @@ struct scsipi_periph { #define PQUIRK_NOREPSUPPOPC 0x0100 /* does not grok REPORT SUPPORTED OPCODES to fetch device timeouts */ +#define PQUIRK_NOREADDISCINFO 0x0200 /* device doesn't do + READ_DISCINFO properly */ /* * Error values an adapter driver may return */
CVS commit: src/sys/dev/scsipi
Module Name:src Committed By: kardel Date: Fri Jun 14 18:44:18 UTC 2024 Modified Files: src/sys/dev/scsipi: scsipi_base.c Log Message: Ignore unit attention caused EIO errors when attempting to fetch supported op-codes and their timeout values during device attachment. To generate a diff of this commit: cvs rdiff -u -r1.189 -r1.190 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
Module Name:src Committed By: kardel Date: Fri Jun 14 18:44:18 UTC 2024 Modified Files: src/sys/dev/scsipi: scsipi_base.c Log Message: Ignore unit attention caused EIO errors when attempting to fetch supported op-codes and their timeout values during device attachment. To generate a diff of this commit: cvs rdiff -u -r1.189 -r1.190 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.189 src/sys/dev/scsipi/scsipi_base.c:1.190 --- src/sys/dev/scsipi/scsipi_base.c:1.189 Sat Apr 9 23:38:32 2022 +++ src/sys/dev/scsipi/scsipi_base.c Fri Jun 14 18:44:18 2024 @@ -1,4 +1,4 @@ -/* $NetBSD: scsipi_base.c,v 1.189 2022/04/09 23:38:32 riastradh Exp $ */ +/* $NetBSD: scsipi_base.c,v 1.190 2024/06/14 18:44:18 kardel 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.189 2022/04/09 23:38:32 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: scsipi_base.c,v 1.190 2024/06/14 18:44:18 kardel Exp $"); #ifdef _KERNEL_OPT #include "opt_scsi.h" @@ -1404,6 +1404,7 @@ scsipi_get_opcodeinfo(struct scsipi_peri u_int8_t *data; int len = 16*1024; int rc; + int retries; struct scsi_repsuppopcode cmd; /* refrain from asking for supported opcodes */ @@ -1420,7 +1421,6 @@ scsipi_get_opcodeinfo(struct scsipi_peri * enumerate all codes * if timeout exists insert maximum into opcode table */ - data = malloc(len, M_DEVBUF, M_WAITOK|M_ZERO); memset(&cmd, 0, sizeof(cmd)); @@ -1429,9 +1429,22 @@ scsipi_get_opcodeinfo(struct scsipi_peri cmd.repoption = RSOC_RCTD|RSOC_ALL; _lto4b(len, cmd.alloclen); - rc = scsipi_command(periph, (void *)&cmd, sizeof(cmd), - (void *)data, len, 0, 1000, NULL, - XS_CTL_DATA_IN|XS_CTL_SILENT); + /* loop to skip any UNIT ATTENTIONS at this point */ + retries = 3; + do { + rc = scsipi_command(periph, (void *)&cmd, sizeof(cmd), +(void *)data, len, 0, 6, NULL, +XS_CTL_DATA_IN|XS_CTL_SILENT); +#ifdef SCSIPI_DEBUG + if (rc != 0) { + SC_DEBUG(periph, SCSIPI_DB3, +("SCSI_MAINTENANCE_IN" + "[RSOC_REPORT_SUPPORTED_OPCODES] command" +" failed: rc=%d, retries=%d\n", +rc, retries)); + } +#endif +} while (rc == EIO && retries-- > 0); if (rc == 0) { int count;
CVS commit: src/sys/dev/scsipi
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
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(&dksc->sc_dkdev); + /* Discover wedges on this disk if it is online */ + if (result == SDGP_RESULT_OK) + dkwedge_discover(&dksc->sc_dkdev); /* * Establish a shutdown hook so that we can ensure that
CVS commit: src/sys/dev/scsipi
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
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
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
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
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
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
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
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->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
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
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
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
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
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
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
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(&ifp->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
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
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->sc_send_work, NULL); - } + } /* else: nothing to do - work is already queued */ mutex_exit(&sc->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->sc_recv_work, NULL); - } + } mutex_exit(&sc->sc_iflock); ifp->if_flags &= ~IFF_OACTIVE; mutex_enter(&sc->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->sc_send_work, NULL); - } + } mutex_exit(&sc->sc_iflock); } return (error);
CVS commit: src/sys/dev/scsipi
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
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
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
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 = &sd_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 = &sd_cd, + .d_devtounit = disklabel_dev_unit, .d_flag = D_DISK | D_MPSAFE };
CVS commit: src/sys/dev/scsipi
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
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(&replun, 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
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
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(&replun, 0, sizeof(replun)); + periph = scsipi_alloc_periph(M_WAITOK); periph->periph_channel = chan; periph->periph_switch = &scsi_probe_dev;
CVS commit: src/sys/dev/scsipi
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
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
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, &luns, &nluns) == 0) { for (size_t i = 0; i < nluns; i++)
CVS commit: src/sys/dev/scsipi
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, &luns, &nluns) == 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, &luns, &nluns) == 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
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
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
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
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
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
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
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 = &scsi_probe_dev; + + periph->periph_target = target; + periph->periph_lun = 0; + periph->periph_quirks = chan->chan_defquirks; + + if ((error = scsipi_inquire(periph, &inqbuf, + 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 *)&replun, 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 *)&rlr[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(&lunp->lun[0]) & 0x3FFF; + break; + case 0x00: + (*luns)[i] = _2btol(&lunp->lun[0]) & 0x00FF; + break; + } + } + + kheapsort(*luns, *nluns, sizeof(**luns), lun_compar, &tmp); + +end: + if (rlr) + kmem_free(rlr, rlrlen); +end2: + scsipi_fr
CVS commit: src/sys/dev/scsipi
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, &sa, CFARGS(.submatch = config_stdsubmatch, .locators = locs))) != NULL) { @@ -1034,9 +1035,11 @@ scsi_probe_device(struct scsibus_softc * */ config_attach(sc->sc_dev, cf, &sa, scsibusprint, CFARGS(.locators = locs)); + KERNEL_UNLOCK_ONE(NULL); } else { scsibusprint(&sa, device_xname(sc->sc_dev)); aprint_normal(" not configured\n"); + KERNEL_UNLOCK_ONE(NULL); goto bad; }
CVS commit: src/sys/dev/scsipi
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
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 = &chread_filtops; + kn->kn_hook = sc; + selrecord_knote(&sc->sc_selq, kn); break; case EVFILT_WRITE: - kn->kn_fop = &chwrite_filtops; + kn->kn_fop = &seltrue_filtops; break; default: return (EINVAL); } - kn->kn_hook = sc; - - selrecord_knote(&sc->sc_selq, kn); - return (0); }
CVS commit: src/sys/dev/scsipi
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
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
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
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
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
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
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
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
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
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
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
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
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, &xs->sense.scsi_sense, screq->senselen); + memcpy(screq->sense, &xs->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, &xs->sense.scsi_sense, screq->senselen); + memcpy(screq->sense, &xs->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
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
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
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.
CVS commit: src/sys/dev/scsipi
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(&cmd, 0, sizeof(cmd)); +memset(&posdata, 0, sizeof(posdata)); +cmd.opcode = READ_POSITION; +cmd.byte1 = 6; /* service action: LONG FORM */ + +error = scsipi_command(st->sc_periph, (void *)&cmd, sizeof(cmd), +(void *)&posdata, 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(&posdata[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
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
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
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
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
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
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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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(&st->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
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
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
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
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 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
Re: CVS commit: src/sys/dev/scsipi
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 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
Re: CVS commit: src/sys/dev/scsipi
On Aug 9, 2013, at 12:58 , Frank Kardel 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
Re: CVS commit: src/sys/dev/scsipi
On Aug 9, 2013, at 12:58 , Frank Kardel 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
Re: CVS commit: src/sys/dev/scsipi
On Fri, Apr 20, 2012 at 09:52:42AM +0200, Manuel Bouyer wrote: > > (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. Yup. Anyone want to make a list of such entities so we can parcel them out with an eye to getting them all fixed before -7? I stuck my neck out and said I'd deal with the tty code. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/dev/scsipi
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). >
re: CVS commit: src/sys/dev/scsipi
> > 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. > 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. > 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. 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. > > > > 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. these asserts aren't specific to autoconfig, and IIRC, most drivers do not call into scsipi during attach. > > > > 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. > > > 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. > > 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. we've actually run the interrupt part of autoconfig with multiple threads since sometime between netbsd 4.0 and 5.0. > > 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. :-) > > > 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. > > the contract has changed in a way th
Re: CVS commit: src/sys/dev/scsipi
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 NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/dev/scsipi
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
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 NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/dev/scsipi
On Fri, Apr 20, 2012 at 10:03:09AM +1000, matthew green wrote: > > > > 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. 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. > the thing that does work is that > IPL_VM using drivers will automatically get kernel locked in their > interrupt handlers. 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). > > > > 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 > 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.) 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. > > > 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. 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. > > > > 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. It's hard to drvctl -d the root device ... > > > 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. the contract has changed in a way that exposes all non-MPSAFE driver to a race condition. The problem shows up in scsipi, but it's not the only place where the problem exists. ata is not MP-safe, so the problem is also in IDE and SATA drivers (including some USB drivers). mii(4) and ifmedia is not MP-safe, so ethernet drivers have this problem too. I've not audit
Re: CVS commit: src/sys/dev/scsipi
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
> > 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
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. > >
re: CVS commit: src/sys/dev/scsipi
> > > 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
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 NetBSD: 26 ans d'experience feront toujours la difference --
re: CVS commit: src/sys/dev/scsipi
> > > 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.