sparc64: magma(4): timeout_add(9) -> timeout_add_msec(9)
Another sbus(4) device driver. This code converts between milliseconds and ticks. If we strip out the conversions we can just use milliseconds directly with timeout_add_msec(9) and delete the conversion code, too. This is only built for sparc64. It ought to compile but I'm not sure. Assuming it compiles, ok? Index: magma.c === RCS file: /cvs/src/sys/dev/sbus/magma.c,v retrieving revision 1.30 diff -u -p -r1.30 magma.c --- magma.c 31 Dec 2019 10:05:33 - 1.30 +++ magma.c 10 Feb 2020 23:34:49 - @@ -1410,8 +1410,8 @@ mbppopen(dev_t dev, int flags, int mode, /* set defaults */ mp->mp_burst = BPP_BURST; - mp->mp_timeout = mbpp_mstohz(BPP_TIMEOUT); - mp->mp_delay = mbpp_mstohz(BPP_DELAY); + mp->mp_timeout = BPP_TIMEOUT; + mp->mp_delay = BPP_DELAY; /* init chips */ if (mp->mp_cd1400) {/* CD1400 */ @@ -1482,15 +1482,15 @@ mbppioctl(dev_t dev, u_long cmd, caddr_t error = EINVAL; } else { mp->mp_burst = bp->bp_burst; - mp->mp_timeout = mbpp_mstohz(bp->bp_timeout); - mp->mp_delay = mbpp_mstohz(bp->bp_delay); + mp->mp_timeout = bp->bp_timeout; + mp->mp_delay = bp->bp_delay; } break; case BPPIOCGPARAM: bp = (struct bpp_param *)data; bp->bp_burst = mp->mp_burst; - bp->bp_timeout = mbpp_hztoms(mp->mp_timeout); - bp->bp_delay = mbpp_hztoms(mp->mp_delay); + bp->bp_timeout = mp->mp_timeout; + bp->bp_delay = mp->mp_delay; break; case BPPIOCGSTAT: /* XXX make this more generic */ @@ -1540,7 +1540,7 @@ mbpp_rw(dev_t dev, struct uio *uio) */ if (mp->mp_timeout > 0) { SET(mp->mp_flags, MBPPF_TIMEOUT); - timeout_add(&mp->mp_timeout_tmo, mp->mp_timeout); + timeout_add_msec(&mp->mp_timeout_tmo, mp->mp_timeout); } len = cnt = 0; @@ -1588,7 +1588,7 @@ mbpp_rw(dev_t dev, struct uio *uio) if (mp->mp_delay > 0) { s = spltty(); /* XXX */ SET(mp->mp_flags, MBPPF_DELAY); - timeout_add(&mp->mp_start_tmo, mp->mp_delay); + timeout_add_msec(&mp->mp_start_tmo, mp->mp_delay); error = tsleep_nsec(mp, PCATCH | PZERO, "mbppdelay", INFSLP); splx(s); @@ -1739,27 +1739,4 @@ mbpp_recv(struct mbpp_port *mp, caddr_t /* return number of chars received */ return (len - mp->mp_cnt); -} - -int -mbpp_hztoms(int h) -{ - int m = h; - - if (m > 0) - m = m * 1000 / hz; - return (m); -} - -int -mbpp_mstohz(int m) -{ - int h = m; - - if (h > 0) { - h = h * hz / 1000; - if (h == 0) - h = 1000 / hz; - } - return (h); } Index: magmareg.h === RCS file: /cvs/src/sys/dev/sbus/magmareg.h,v retrieving revision 1.9 diff -u -p -r1.9 magmareg.h --- magmareg.h 29 Nov 2008 01:55:06 - 1.9 +++ magmareg.h 10 Feb 2020 23:34:49 - @@ -220,8 +220,6 @@ void mbpp_timeout(void *); void mbpp_start(void *); int mbpp_send(struct mbpp_port *, caddr_t, int); int mbpp_recv(struct mbpp_port *, caddr_t, int); -int mbpp_hztoms(int); -int mbpp_mstohz(int); #defineCD1400_REGMAPSIZE 0x80 #defineCD1190_REGMAPSIZE 0x20
sparc64: stp(4): tsleep(9) -> tsleep_nsec(9)
Ticks to microseconds. The parameter name "ms" is misleading. All the calling code uses microseconds. I think the author's thought process in choosing that name was roughly: "microseconds" -> "Micro Seconds" -> "ms". In any case, I've renamed the parameter to the less misleading "usecs". This driver is only built on sparc64. I think this ought to compile. If someone could confirm that it compiles that would help. If someone has one of these things that would be even better, though I have my doubts. ok? Index: stp4020.c === RCS file: /cvs/src/sys/dev/sbus/stp4020.c,v retrieving revision 1.21 diff -u -p -r1.21 stp4020.c --- stp4020.c 31 Dec 2019 10:05:33 - 1.21 +++ stp4020.c 10 Feb 2020 23:20:59 - @@ -832,23 +832,20 @@ stp4020_chip_intr_string(pcmcia_chipset_ * XXX - assumes a context */ void -stp4020_delay(unsigned int ms) +stp4020_delay(unsigned int usecs) { - unsigned int nticks; + int chan; - /* Convert to nticks */ - nticks = (ms * hz) / 100; - - if (cold || nticks == 0) { - delay(ms); + if (cold || usecs < tick) { + delay(usecs); return; } #ifdef DEBUG - if (nticks > 60 * hz) - panic("stp4020: preposterous delay: %u", nticks); + if (usecs > 60 * 100) + panic("stp4020: preposterous delay: %uus", usecs); #endif - tsleep(&nticks, 0, "stp4020_delay", nticks); + tsleep_nsec(&chan, 0, "stp4020_delay", USEC_TO_NSEC(usecs)); } #ifdef STP4020_DEBUG
Re: don't try to signal with newsyslog -r
On Feb 10 14:53:33, mill...@openbsd.org wrote: > On Mon, 10 Feb 2020 17:12:53 +0100, Jan Stary wrote: > > > The -r option of newsyslog(8) removes the requirement > > that newsyslog runs as root. Would it also make sense > > to not try to send the SIGHUP to syslogd in that case? > > This seems wrong to me. You are disabling more than just sending > a signal to SIGHUP, this will cause newsyslog to ignore *all* pid > files. Yes; it is wrong. A better way to avoid trying to send the signal seems to be to simply specify an empty command in the config file. Jan
Re: don't try to signal with newsyslog -r
Hi Ingo, On Feb 10 22:40:20, schwa...@usta.de wrote: > > The -r option of newsyslog(8) removes the requirement > > that newsyslog runs as root. Would it also make sense > > to not try to send the SIGHUP to syslogd in that case? > > While i'm not sure that i want to take care of this patch, > given that i'm not quite sure what the point of the -r option > even is in the first place, for example, I use it to rotate ~/battery/battery.log on my laptop upon boot, obtaining a concise history of the battery's life. Without -r, newsyslog would refuse to run. It is my impression that this is the inteded use: people rotating their own logs, not just system logs. I don't know why newsyslog refuses to run without root, creating a "need" for -r in the first place, > i'd like to point out that you > are also removing the warning. Is that intentional? Yes. > Naively, getting a warning when files are rotated but the > daemon isn't notified seems useful to me, even if you kind > of requested it with -r. > > In that case, wouldn't it make more sense to say something > like (untested, and no code auditing done) > > else if (needroot == 0 || kill(pid, signal)) > > ? > > That would also avoid the hopeless attempt to send a signal, > but would still print the warning. Sorry for being unclear. The log I am rotating is not written by syslog; it is written by my user's cronjob (basicaly saving the voltage and capacity from hw.sensors.acpibat0 every cron minutes). Given that, and the fact that I am not running newsyslog as root, I don't need or expect to signal syslog; the warning then only informs me about the hopeless attempt to send the signal. Jan > > Index: newsyslog.8 > > === > > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v > > retrieving revision 1.54 > > diff -u -p -r1.54 newsyslog.8 > > --- newsyslog.8 20 Jul 2017 18:39:16 - 1.54 > > +++ newsyslog.8 10 Feb 2020 16:08:51 - > > @@ -124,7 +124,7 @@ Removes the restriction that > > must be running as root. > > Note that in this mode > > .Nm > > -will not be able to send a > > +will not try to send a > > .Dv SIGHUP > > signal to > > .Xr syslogd 8 . > > Index: newsyslog.c > > === > > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v > > retrieving revision 1.112 > > diff -u -p -r1.112 newsyslog.c > > --- newsyslog.c 28 Jun 2019 13:35:02 - 1.112 > > +++ newsyslog.c 10 Feb 2020 16:08:51 - > > @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal) > > warnx("%s pid file: %s", err, pidfile); > > else if (noaction) > > (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid); > > - else if (kill(pid, signal)) > > + else if (needroot && kill(pid, signal)) > > warnx("warning - could not send SIG%s to PID from pid file %s", > > sys_signame[signal], pidfile); > > } > >
Re: don't try to signal with newsyslog -r
On Mon, 10 Feb 2020 17:12:53 +0100, Jan Stary wrote: > The -r option of newsyslog(8) removes the requirement > that newsyslog runs as root. Would it also make sense > to not try to send the SIGHUP to syslogd in that case? This seems wrong to me. You are disabling more than just sending a signal to SIGHUP, this will cause newsyslog to ignore *all* pid files. A better approach would be to avoid initializing working->pidfile to PIDFILE if the euid is non-zero. That way, user-specified pid files are still honored. - todd
Re: setlocale() in cron
On Mon, 10 Feb 2020 22:16:04 +0100, Ingo Schwarze wrote: > Even if there were something locale-dependent in cron(8), and even > if we consider somebody using these program in a portable way on a > non-OpenBSD system, i believe a daemon started as root should not be > locale-dependent, so i'd like to commit the patch to cron.c in any > case. > > While crontab.c does not seem quite as important, i see no possible > harm in cleaning this up, so i'd like to commit that one, too. I think these are both fine. In theory the date in the second line of the crontab comment block could be locale-dependent but that is not supposed to be user-visible anyway. - todd
Re: don't try to signal with newsyslog -r
Hi, Jan Stary wrote on Mon, Feb 10, 2020 at 05:12:53PM +0100: > The -r option of newsyslog(8) removes the requirement > that newsyslog runs as root. Would it also make sense > to not try to send the SIGHUP to syslogd in that case? While i'm not sure that i want to take care of this patch, given that i'm not quite sure what the point of the -r option even is in the first place, i'd like to point out that you are also removing the warning. Is that intentional? Naively, getting a warning when files are rotated but the daemon isn't notified seems useful to me, even if you kind of requested it with -r. In that case, wouldn't it make more sense to say something like (untested, and no code auditing done) else if (needroot == 0 || kill(pid, signal)) ? That would also avoid the hopeless attempt to send a signal, but would still print the warning. Yours, Ingo > Index: newsyslog.8 > === > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v > retrieving revision 1.54 > diff -u -p -r1.54 newsyslog.8 > --- newsyslog.8 20 Jul 2017 18:39:16 - 1.54 > +++ newsyslog.8 10 Feb 2020 16:08:51 - > @@ -124,7 +124,7 @@ Removes the restriction that > must be running as root. > Note that in this mode > .Nm > -will not be able to send a > +will not try to send a > .Dv SIGHUP > signal to > .Xr syslogd 8 . > Index: newsyslog.c > === > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v > retrieving revision 1.112 > diff -u -p -r1.112 newsyslog.c > --- newsyslog.c 28 Jun 2019 13:35:02 - 1.112 > +++ newsyslog.c 10 Feb 2020 16:08:51 - > @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal) > warnx("%s pid file: %s", err, pidfile); > else if (noaction) > (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid); > - else if (kill(pid, signal)) > + else if (needroot && kill(pid, signal)) > warnx("warning - could not send SIG%s to PID from pid file %s", > sys_signame[signal], pidfile); > }
Re: syslogd closing all udp is a tiny bit aggressiv
On Sun, Feb 9, 2020 at 4:12 PM Alexander Bluhm wrote: > On Thu, Feb 06, 2020 at 05:57:15PM -0500, sven falempin wrote: > > > Your DNS lookup fails at startup, sockets are closed. > > > Later at SIGHUP you DNS works again. Now the sockets are needed. > > > So do not close them if DNS for udp fails. > > I thought again about this problem. The fix can be more specific. > - if user requested udp4 or udp6, close the other af socket. > - after SIGHUP, when DNS works, close the unneeded af socket. > > ok? > > Index: usr.sbin/syslogd/syslogd.c > === > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v > retrieving revision 1.262 > diff -u -p -r1.262 syslogd.c > --- usr.sbin/syslogd/syslogd.c 5 Jul 2019 13:23:27 - 1.262 > +++ usr.sbin/syslogd/syslogd.c 9 Feb 2020 20:25:20 - > @@ -853,20 +853,6 @@ main(int argc, char *argv[]) > event_add(ev_udp, NULL); > if (fd_udp6 != -1) > event_add(ev_udp6, NULL); > - } else { > - /* > -* If generic UDP file descriptors are used neither > -* for receiving nor for sending, close them. Then > -* there is no useless *.514 in netstat. > -*/ > - if (fd_udp != -1 && !send_udp) { > - close(fd_udp); > - fd_udp = -1; > - } > - if (fd_udp6 != -1 && !send_udp6) { > - close(fd_udp6); > - fd_udp6 = -1; > - } > } > for (i = 0; i < nbind; i++) > if (fd_bind[i] != -1) > @@ -2416,6 +2402,7 @@ init(void) > s = 0; > strlcpy(progblock, "*", sizeof(progblock)); > strlcpy(hostblock, "*", sizeof(hostblock)); > + send_udp = send_udp6 = 0; > while (getline(&cline, &s, cf) != -1) { > /* > * check for end-of-section, comments, strip off trailing > @@ -2508,6 +2495,22 @@ init(void) > Initialized = 1; > dropped_warn(&init_dropped, "during initialization"); > > + if (SecureMode) { > + /* > +* If generic UDP file descriptors are used neither > +* for receiving nor for sending, close them. Then > +* there is no useless *.514 in netstat. > +*/ > + if (fd_udp != -1 && !send_udp) { > + close(fd_udp); > + fd_udp = -1; > + } > + if (fd_udp6 != -1 && !send_udp6) { > + close(fd_udp6); > + fd_udp6 = -1; > + } > + } > + > if (Debug) { > SIMPLEQ_FOREACH(f, &Files, f_next) { > for (i = 0; i <= LOG_NFACILITIES; i++) > @@ -2755,6 +2758,13 @@ cfline(char *line, char *progblock, char > sizeof(f->f_un.f_forw.f_addr)) != 0) { > log_warnx("bad hostname \"%s\"", > f->f_un.f_forw.f_loghost); > + /* DNS lookup may work after SIGHUP, keep sockets > */ > + if (strcmp(proto, "udp") == 0) > + send_udp = send_udp6 = 1; > + else if (strcmp(proto, "udp4") == 0) > + send_udp = 1; > + else if (strcmp(proto, "udp6") == 0) > + send_udp6 = 1; > break; > } > f->f_file = -1; > @ok here -- -- - Knowing is not enough; we must apply. Willing is not enough; we must do
Re: setlocale() in cron
Hi, Jan Stary wrote on Mon, Feb 10, 2020 at 01:13:58PM +0100: > Why does cron(8) and crontab(1) need to setlocale()? I looked through the *.c files in /usr/src/usr.sbin/cron/ and found the following locale-dependent functions: atrun.c: isalpha(3), isupper(3) cron.c: strtod(3) do_command.c: isalnum(3), isprint(3), stravis(3) entry.c: isalpha(3), isdigit(3) env.c:isspace(3) Neither strftime(3) nor strptime(3) appear to be used, which would be common traps in this respect, in particular in programs doing something with dates and times. Either way, on OpenBSD, none of these functions actually depend on the locale. Even if there were something locale-dependent in cron(8), and even if we consider somebody using these program in a portable way on a non-OpenBSD system, i believe a daemon started as root should not be locale-dependent, so i'd like to commit the patch to cron.c in any case. While crontab.c does not seem quite as important, i see no possible harm in cleaning this up, so i'd like to commit that one, too. OK? Ingo > Index: cron.c > === > RCS file: /cvs/src/usr.sbin/cron/cron.c,v > retrieving revision 1.77 > diff -u -p -r1.77 cron.c > --- cron.c23 Oct 2017 15:15:22 - 1.77 > +++ cron.c10 Feb 2020 12:12:13 - > @@ -28,7 +28,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -83,8 +82,6 @@ main(int argc, char *argv[]) > struct sigaction sact; > sigset_t blocked, omask; > struct group *grp; > - > - setlocale(LC_ALL, ""); > > setvbuf(stdout, NULL, _IOLBF, 0); > setvbuf(stderr, NULL, _IOLBF, 0); > Index: crontab.c > === > RCS file: /cvs/src/usr.sbin/cron/crontab.c,v > retrieving revision 1.93 > diff -u -p -r1.93 crontab.c > --- crontab.c 28 Jun 2019 13:32:47 - 1.93 > +++ crontab.c 10 Feb 2020 12:12:13 - > @@ -26,7 +26,6 @@ > #include > #include > #include > -#include > #include > #include > #include > @@ -92,7 +91,6 @@ main(int argc, char *argv[]) > user_gid = getgid(); > crontab_gid = getegid(); > > - setlocale(LC_ALL, ""); > openlog(__progname, LOG_PID, LOG_CRON); > > setvbuf(stderr, NULL, _IOLBF, 0); >
Re: Audio control API, part 2: add new sndioctl(1) utility
On Mon, Feb 10, 2020 at 09:59:09AM +, Raf Czlonka wrote: > On Sun, Feb 09, 2020 at 12:14:47PM GMT, Alexandre Ratchov wrote: > > Here's a new sndioctl utility similar to mixerctl(1) but using the new > > sndio API. Example: > > > > $ sndioctl > > output.level=127 > > app/aucat0.level=127 > > app/firefox0.level=127 > > app/firefox1.level=12 > > app/midisyn0.level=127 > > app/mpv0.level=127 > > app/prog5.level=127 > > app/prog6.level=127 > > app/prog7.level=127 > > hw/input.level=62 > > hw/input.mute=0 > > hw/output.level=63 > > hw/output.mute=0 > > > > Hi Alexandre, > > Just a quick question. > > Is there a good reason to have the above using "slash" ('/') as the > first separator instead of the, more familiar, "dot" ('.') known > from sysctl(8)'s MIB (Management Information Base) style names or > even the "pseudo" MIB know from mixerctl(1)? Hi, I don't know if the following qualifies as a "good reason". The first part (the group) is a prefix of the control identifier. The identifier itself has a strict "[channel]." format. The prefix is not always present, examples: output.level<- sndiod volume knob hw/output.level <- underlying hardware volume knob I tried to avoid the group part, but as mixers may be nested it seems necessary to avoid name clashes. In the sndioctl syntax, we could replace '/' by '.' but this looks confusing as the syntax doesn't map directly to the underlying model. But maybe we should hide such developer-centric details and just use only dots to make this look as a MIB. Another option I've considered is to drop the group concept in the API and simply prefix the stream name to make it unique; in turn we obtain a flat control list. It's uglier and seems to complicate GUIs task. For instance the group part could be used to represent controls of different groups in different sections or to filter-out certain groups).
Re: hidmt(4): only allow hid_input kind
Hi, Mark Kettenis wrote: > > Since we don't filter out hid_(end)collection, we add this item to the > > list. So, by making sure we only add hid_input items, the Pinebook > > Pro's trackpad works. > > > > Now I'm not sure if the hid code is supposed to re-use the Y-coordi- > > nates usage on the next collection. If someone knows how that should > > work, please let me down. > > > > ok? > > Both hidms and hiskbd do something similar, so ok kettenis@ I'm new to this code, having looked at src/sys/ for the first time at the u2k20 hackathon. Anyway, it looks to me like hidmt is a little bit simplistic (that's both good and bad) and with that in mind I think this change is fine. While it might be desirable/neccessary to have a more comprehensive software model of the connected HID in the future, FWIW right now this is ok pe...@stuge.se //Peter
Re: nep(4) rx error interrupts
On Mon, Feb 10, 2020 at 02:04:06PM +0100, Mark Kettenis wrote: > Jonathan Matthew schreef op 2020-02-10 13:19: > > I'm trying to use aggr on top of nep(4), which has turned up a few bugs. > > Firstly, the nic appears to report rx errors in the second interrupt > > state vector, which the driver doesn't expect, so I get this: > > > > nep3: nep_intr 0 8 0 > > > > and then, since the interrupt wasn't rearmed, the interface stops. > > > > Then, the driver doesn't clear rx error interrupt sources, so rearming > > results in an interrupt storm. The illumos nxge driver masks off > > read-only > > bits in the rx status register and writes it back, clearing all active > > interrupt sources, so I've done that here too. > > > > ok? > > > > > > Index: if_nep.c > > === > > RCS file: /cvs/src/sys/dev/pci/if_nep.c,v > > retrieving revision 1.31 > > diff -u -p -r1.31 if_nep.c > > --- if_nep.c9 Nov 2018 14:14:31 - 1.31 > > +++ if_nep.c10 Feb 2020 11:57:44 - > > @@ -333,11 +333,24 @@ > > #define RX_DMA_ENT_MSK(chan) (DMC + 0x00068 + (chan) * 0x00200) > > #define RX_DMA_ENT_MSK_RBR_EMPTY (1ULL << 3) > > #define RX_DMA_CTL_STAT(chan) (DMC + 0x00070 + (chan) * 0x00200) > > +#define RX_DMA_CTL_STAT_DC_FIFO_ERR (1ULL << 48) > > #define RX_DMA_CTL_STAT_MEX (1ULL << 47) > > #define RX_DMA_CTL_STAT_RCRTHRES (1ULL << 46) > > #define RX_DMA_CTL_STAT_RCRTO (1ULL << 45) > > +#define RX_DMA_CTL_STAT_PORT_DROP_PKT (1ULL << 42) > > +#define RX_DMA_CTL_STAT_WRED_DROP (1ULL << 41) > > +#define RX_DMA_CTL_STAT_RBR_PRE_EMTY (1ULL << 40) > > +#define RX_DMA_CTL_STAT_RCR_SHDW_FULL (1ULL << 39) > > #define RX_DMA_CTL_STAT_RBR_EMPTY (1ULL << 35) > > #define RX_DMA_CTL_STAT_PTRREAD_SHIFT 16 > > +#define RX_DMA_CTL_STAT_WR1C_BITS (RX_DMA_CTL_STAT_RBR_EMPTY | \ > > +RX_DMA_CTL_STAT_RCR_SHDW_FULL | \ > > +RX_DMA_CTL_STAT_RBR_PRE_EMTY | \ > > +RX_DMA_CTL_STAT_WRED_DROP | \ > > +RX_DMA_CTL_STAT_PORT_DROP_PKT | \ > > +RX_DMA_CTL_STAT_RCRTO | \ > > +RX_DMA_CTL_STAT_RCRTHRES | \ > > +RX_DMA_CTL_STAT_DC_FIFO_ERR) > > #define RX_DMA_CTL_STAT_DBG(chan) (DMC + 0x00098 + (chan) * 0x00200) > > > > #define TX_LOG_PAGE_VLD(chan) (FZC_DMC + 0x4 + (chan) * 0x00200) > > @@ -959,7 +972,7 @@ nep_intr(void *arg) > > rearm = 1; > > } > > > > - if (sv0 & (1ULL << LDN_RXDMA(sc->sc_port))) { > > + if ((sv0 | sv1) & (1ULL << LDN_RXDMA(sc->sc_port))) { > > This doesn't make sense to me. > > I suppose you see a bit getting set in sv1. Which bit is that? The same bit that gets set in sv0 under normal rx conditions. For nep3, sv1 is 8, on nep1 it's 2. You can see the illumos driver doing the same thing here: https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/io/nxge/nxge_hw.c#L274 > > > nep_rx_proc(sc); > > rearm = 1; > > } > > @@ -990,8 +1003,8 @@ nep_rx_proc(struct nep_softc *sc) > > int idx, len, i; > > > > val = nep_read(sc, RX_DMA_CTL_STAT(sc->sc_port)); > > - nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), > > - RX_DMA_CTL_STAT_RCRTHRES | RX_DMA_CTL_STAT_RCRTO); > > + val &= RX_DMA_CTL_STAT_WR1C_BITS; > > + nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), val); > > > > bus_dmamap_sync(sc->sc_dmat, NEP_DMA_MAP(sc->sc_rcring), 0, > > NEP_DMA_LEN(sc->sc_rcring), BUS_DMASYNC_POSTREAD);
Re: mention /etc/examples/ in bgpf.conf(5)/bgpd(8)
On Sun, Feb 09, 2020 at 01:37:33PM +, Jason McIntyre wrote: > On Sun, Feb 09, 2020 at 02:27:23PM +0100, Marc Espie wrote: > > On Sun, Feb 09, 2020 at 12:41:43AM +0100, Sebastian Benoit wrote: > > > Ingo Schwarze(schwa...@usta.de) on 2020.02.09 00:33:06 +0100: > > > > Hi, > > > > > > > > Jason McIntyre wrote on Sat, Feb 08, 2020 at 10:15:08PM +: > > > > > > > > > - i'm ok with adding the path to these files to a FILES section > > > > > > > > So, here is a specific patch for bgpf.conf(5) and bgpd(8) such > > > > that we can agree on a general direction for one case where > > > > the example file is particularly important. > > > > > > > > Once the direction is agreed, applying it to the relatively > > > > small number of other cases is likely not difficult. > > > > > > > > OK? > > > > Ingo > > > > > > I'm not against this as it only adds to the manpage, but an > > > alternative could be to just tell users (in afterboot(8)?) that there is > > > /etc/examples for them to look at and see whats there? > > > > > > Otherwise ok. > > > > I still think it's a good idea to put it in afterboot(8). > > That was the patch I sent to a few people that obviously triggered Ingo. > > > > As for /etc/examples, I still think that directory has merits. > > It's WAY easier to copy an example file and uncomment/tweak the parts you > > need while skimming through the manpage than creating one from scratch. > > > > Also, cut&paste from an EXAMPLE in the manpage is distasteful. > > > > Yeah, it's more info that has to be kept in synch. So what ? It's still > > useful, so it should stay. > > > > Here's an updated version, taking Theo's remarks into account. > > > > hi. i'm fine with your text, but it would be better placed in the > section "Daemons" (yes i'm aware not all the files are for daemons) > or a new section in "FURTHER CHANGES". > > jmc Sorry, I saw your message a bit late. I don't think it's a good idea. Specifically, that text should come in as early as possible. "FURTHER CHANGES" sound like a section near the end. "Daemons" also comes very late in the game (after ntpd, after sysctl.conf, which do have /etc/examples scripts) also, you should update, as your email setup is broken. any direct email to you have come back with : 553 ORCPT address syntax error which according to Stuart, has been fixed in recent snaps.
Re: gpio(4) support for APU2
Mark Kettenis wrote: > Nathanael Rensen schreef op 2020-01-29 15:47: > > The diff below adds gpio(4) support to wbsio(4) for Nuvoton NCT5104D > > (pcengines APU2). It is based on Matt Dainty's diff posted to this list > > in November 2018: > > > > https://marc.info/?l=openbsd-tech&m=154134941027009&w=2 > > > > A key difference from Matt Dainty's original diff is the use of > > config_search() rather than config_found() to avoid problems with > > the lm78 driver mentioned in his post. > > But as before this diff does nothing to make sure it is actually > safe to touch these gpio pins. Other machines might have the same > chip but use the pins internally to switch power to on-board > components. gpio devices, sigh. It's like wrapping a scarf around your head and crawling under a porch or deck. You never know what surprises you'll put your hands onto. spiders and pet poo, maybe some broken glass...
Re: Add note about example dhclient.conf
Hi Kyle, Kyle Isom wrote on Mon, Feb 10, 2020 at 07:34:25AM -0800: > On Sat, Feb 8, 2020, at 14:15, Jason McIntyre wrote: >> - i'm ok with adding the path to these files to a FILES section > Done. I already committed a comprehensive diff doing that in a simpler way earlier today: https://marc.info/?l=openbsd-cvs&m=158134072405241 But thanks for bringig up the issue, anyway. Yours, Ingo > Index: sbin/dhclient/dhclient.conf.5 > === > RCS file: /cvs/src/sbin/dhclient/dhclient.conf.5,v > retrieving revision 1.49 > diff -u -p -u -p -r1.49 dhclient.conf.5 > --- sbin/dhclient/dhclient.conf.5 17 Dec 2019 14:21:54 - 1.49 > +++ sbin/dhclient/dhclient.conf.5 10 Feb 2020 15:31:41 - > @@ -288,6 +288,11 @@ instead of the > .Ic sname > field of the DHCP offer when binding a lease. > .El > +.Sh FILES > +An example configuration for > +.Pa dhclient.conf > +is provided in > +.Pa /etc/examples/dhclient.conf . > .Sh SEE ALSO > .Xr dhclient.leases 5 , > .Xr dhcp-options 5 , > >
don't try to signal with newsyslog -r
The -r option of newsyslog(8) removes the requirement that newsyslog runs as root. Would it also make sense to not try to send the SIGHUP to syslogd in that case? Jan Index: newsyslog.8 === RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v retrieving revision 1.54 diff -u -p -r1.54 newsyslog.8 --- newsyslog.8 20 Jul 2017 18:39:16 - 1.54 +++ newsyslog.8 10 Feb 2020 16:08:51 - @@ -124,7 +124,7 @@ Removes the restriction that must be running as root. Note that in this mode .Nm -will not be able to send a +will not try to send a .Dv SIGHUP signal to .Xr syslogd 8 . Index: newsyslog.c === RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v retrieving revision 1.112 diff -u -p -r1.112 newsyslog.c --- newsyslog.c 28 Jun 2019 13:35:02 - 1.112 +++ newsyslog.c 10 Feb 2020 16:08:51 - @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal) warnx("%s pid file: %s", err, pidfile); else if (noaction) (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid); - else if (kill(pid, signal)) + else if (needroot && kill(pid, signal)) warnx("warning - could not send SIG%s to PID from pid file %s", sys_signame[signal], pidfile); }
Re: Add note about example dhclient.conf
On Sat, Feb 8, 2020, at 14:15, Jason McIntyre wrote: > - i'm ok with adding the path to these files to a FILES section Done. - Kyle Index: sbin/dhclient/dhclient.conf.5 === RCS file: /cvs/src/sbin/dhclient/dhclient.conf.5,v retrieving revision 1.49 diff -u -p -u -p -r1.49 dhclient.conf.5 --- sbin/dhclient/dhclient.conf.5 17 Dec 2019 14:21:54 - 1.49 +++ sbin/dhclient/dhclient.conf.5 10 Feb 2020 15:31:41 - @@ -288,6 +288,11 @@ instead of the .Ic sname field of the DHCP offer when binding a lease. .El +.Sh FILES +An example configuration for +.Pa dhclient.conf +is provided in +.Pa /etc/examples/dhclient.conf . .Sh SEE ALSO .Xr dhclient.leases 5 , .Xr dhcp-options 5 ,
Re: lcd(4/hppa): timeout_add(9) -> timeout_add_usec(9)
On Mon, Feb 10, 2020 at 11:48:34AM +0100, Mark Kettenis wrote: > On 2020-02-08 20:15, Scott Cheloha wrote: > > On Sun, Dec 22, 2019 at 01:17:56PM -0600, Scott Cheloha wrote: > > > lcd_softc.sc_delay's unit appears to be microseconds. > > > > > > ok? > > > > 1 month bump. > > While the diff looks ok to me, I can't test this until I have physical > access to my hppa machines. Alright, I'll wait for your test.
Re: hidmt(4): only allow hid_input kind
Patrick Wildt schreef op 2020-01-26 06:50: Hi, on the Pinebook Pro the trackpad isn't working. The reason is that the Y-coordinate is extracted twice. The first location has thevalue correctly, the second location has it zeroed or garbage. This is because when we iterate over the report, the Y-coordinate usage appears twice. This shouldn't happen, so why does it? Here's an excerpt of the report descriptor: Item(Global): Logical Maximum, data= [ 0x92 0x03 ] 914 Item(Global): Physical Maximum, data= [ 0x40 0x01 ] 320 Item(Local ): Usage, data= [ 0x31 ] 49 Direction-Y Item(Main ): Input, data= [ 0x02 ] 2 Data Variable Absolute No_Wrap Linear Preferred_State No_Null_Position Non_Volatile Bitfield Item(Main ): End Collection, data=none Item(Main ): Collection, data= [ 0x02 ] 2 Logical Directly after the Y-coordinate a collection ends and another starts. The end of the collection will make hid_get_item() return with an item where h.kind is hid_endcollection and h.usage is zero. The start of the new collection will make hid_get_item() return, and it will return with h.kind set to hid_collection and h.usage set to the last usage, which in this case is "Direction-Y". Since we don't filter out hid_(end)collection, we add this item to the list. So, by making sure we only add hid_input items, the Pinebook Pro's trackpad works. Now I'm not sure if the hid code is supposed to re-use the Y-coordi- nates usage on the next collection. If someone knows how that should work, please let me down. ok? Both hidms and hiskbd do something similar, so ok kettenis@ diff --git a/sys/dev/hid/hidmt.c b/sys/dev/hid/hidmt.c index ee8dcadcc31..67eea46f0c0 100644 --- a/sys/dev/hid/hidmt.c +++ b/sys/dev/hid/hidmt.c @@ -176,6 +176,8 @@ hidmt_setup(struct device *self, struct hidmt *mt, void *desc, int dlen) if (h.report_ID != mt->sc_rep_input) continue; + if (h.kind != hid_input) + continue; switch (h.usage) { /* contact level usages */
Re: nep(4) rx error interrupts
Jonathan Matthew schreef op 2020-02-10 13:19: I'm trying to use aggr on top of nep(4), which has turned up a few bugs. Firstly, the nic appears to report rx errors in the second interrupt state vector, which the driver doesn't expect, so I get this: nep3: nep_intr 0 8 0 and then, since the interrupt wasn't rearmed, the interface stops. Then, the driver doesn't clear rx error interrupt sources, so rearming results in an interrupt storm. The illumos nxge driver masks off read-only bits in the rx status register and writes it back, clearing all active interrupt sources, so I've done that here too. ok? Index: if_nep.c === RCS file: /cvs/src/sys/dev/pci/if_nep.c,v retrieving revision 1.31 diff -u -p -r1.31 if_nep.c --- if_nep.c9 Nov 2018 14:14:31 - 1.31 +++ if_nep.c10 Feb 2020 11:57:44 - @@ -333,11 +333,24 @@ #define RX_DMA_ENT_MSK(chan) (DMC + 0x00068 + (chan) * 0x00200) #define RX_DMA_ENT_MSK_RBR_EMPTY (1ULL << 3) #define RX_DMA_CTL_STAT(chan) (DMC + 0x00070 + (chan) * 0x00200) +#define RX_DMA_CTL_STAT_DC_FIFO_ERR (1ULL << 48) #define RX_DMA_CTL_STAT_MEX (1ULL << 47) #define RX_DMA_CTL_STAT_RCRTHRES (1ULL << 46) #define RX_DMA_CTL_STAT_RCRTO (1ULL << 45) +#define RX_DMA_CTL_STAT_PORT_DROP_PKT (1ULL << 42) +#define RX_DMA_CTL_STAT_WRED_DROP (1ULL << 41) +#define RX_DMA_CTL_STAT_RBR_PRE_EMTY (1ULL << 40) +#define RX_DMA_CTL_STAT_RCR_SHDW_FULL (1ULL << 39) #define RX_DMA_CTL_STAT_RBR_EMPTY (1ULL << 35) #define RX_DMA_CTL_STAT_PTRREAD_SHIFT 16 +#define RX_DMA_CTL_STAT_WR1C_BITS (RX_DMA_CTL_STAT_RBR_EMPTY | \ +RX_DMA_CTL_STAT_RCR_SHDW_FULL | \ +RX_DMA_CTL_STAT_RBR_PRE_EMTY | \ +RX_DMA_CTL_STAT_WRED_DROP | \ +RX_DMA_CTL_STAT_PORT_DROP_PKT | \ +RX_DMA_CTL_STAT_RCRTO | \ +RX_DMA_CTL_STAT_RCRTHRES | \ +RX_DMA_CTL_STAT_DC_FIFO_ERR) #define RX_DMA_CTL_STAT_DBG(chan) (DMC + 0x00098 + (chan) * 0x00200) #define TX_LOG_PAGE_VLD(chan) (FZC_DMC + 0x4 + (chan) * 0x00200) @@ -959,7 +972,7 @@ nep_intr(void *arg) rearm = 1; } - if (sv0 & (1ULL << LDN_RXDMA(sc->sc_port))) { + if ((sv0 | sv1) & (1ULL << LDN_RXDMA(sc->sc_port))) { This doesn't make sense to me. I suppose you see a bit getting set in sv1. Which bit is that? nep_rx_proc(sc); rearm = 1; } @@ -990,8 +1003,8 @@ nep_rx_proc(struct nep_softc *sc) int idx, len, i; val = nep_read(sc, RX_DMA_CTL_STAT(sc->sc_port)); - nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), - RX_DMA_CTL_STAT_RCRTHRES | RX_DMA_CTL_STAT_RCRTO); + val &= RX_DMA_CTL_STAT_WR1C_BITS; + nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), val); bus_dmamap_sync(sc->sc_dmat, NEP_DMA_MAP(sc->sc_rcring), 0, NEP_DMA_LEN(sc->sc_rcring), BUS_DMASYNC_POSTREAD);
Re: ldom.conf: Support devlias keyword for vdisk
Klemens Nanni schreef op 2020-02-05 21:35: Straight forward diff to allow calling disks names: $ cat ldom.conf domain guest { vcpu 4 memory 8G vnet vdisk "/var/ldom/guest.img" vdisk "/var/ldom/miniroot.fs" devalias=miniroot } $ doas ldomctl start -c guest {ok} devalias ... miniroot /virtual-devices@100/channel-devices@200/disk@1 disk1 /virtual-devices@100/channel-devices@200/disk@1 disk0 /virtual-devices@100/channel-devices@200/disk@0 disk /virtual-devices@100/channel-devices@200/disk@0 ... {ok} boot miniroot Boot device: /virtual-devices@100/channel-devices@200/disk@1 File and args: OpenBSD IEEE 1275 Bootblock 1.4 ... This helps me to navigate inside guests as I no longer have to remember in which order I specified the `vdisk' lines in ldom.conf(5). parse.y code is simply copied from existing vnet code and adapted with s/vnet/vdisk/. It would need the previously sent ldom.conf.5 vnet, although for correctness and consistency only. Feedback? OK? Sounds reasonable. Maybe get some feedback from a parse.y expert as well? Index: config.c === RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v retrieving revision 1.32 diff -u -p -r1.32 config.c --- config.c22 Jan 2020 07:52:38 - 1.32 +++ config.c5 Feb 2020 20:24:12 - @@ -2557,7 +2557,8 @@ guest_add_memory(struct guest *guest, ui } void -guest_add_vdisk(struct guest *guest, uint64_t id, const char *path) +guest_add_vdisk(struct guest *guest, uint64_t id, const char *path, +const char *user_devalias) { struct guest *primary; struct ldc_channel *lc; @@ -2577,6 +2578,8 @@ guest_add_vdisk(struct guest *guest, uin if (id == 0) guest_add_devalias(guest, "disk", devpath); guest_add_devalias(guest, devalias, devpath); + if (user_devalias != NULL) + guest_add_devalias(guest, user_devalias, devpath); free(devalias); free(devpath); } @@ -2849,7 +2852,8 @@ build_config(const char *filename, int n guest_add_memory(guest, -1, domain->memory); i = 0; SIMPLEQ_FOREACH(vdisk, &domain->vdisk_list, entry) - guest_add_vdisk(guest, i++, vdisk->path); + guest_add_vdisk(guest, i++, vdisk->path, + vdisk->devalias); i = 0; SIMPLEQ_FOREACH(vnet, &domain->vnet_list, entry) guest_add_vnetwork(guest, i++, vnet->mac_addr, Index: ldom.conf.5 === RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v retrieving revision 1.10 diff -u -p -r1.10 ldom.conf.5 --- ldom.conf.5 13 Jan 2020 09:29:41 - 1.10 +++ ldom.conf.5 5 Feb 2020 20:20:21 - @@ -56,7 +56,7 @@ See .Xr eeprom 8 for a list of OpenPROM variables. This keyword can be used multiple times. -.It Ic vdisk Ar file +.It Ic vdisk Ar file Op Ar keyword Ns = Ns Ar value ... The specified file is used to back a virtual disk of the guest domain. .Ar file @@ -64,6 +64,12 @@ can be a block device node or a disk ima .Cm create-vdisk command. This keyword can be used multiple times. +Valid options are: +.Bl -tag -width Ds +.It Ic devalias Ns = Ns Ar name +Alias the virtual disk as +.Ar name . +.El .It Ic vnet Op Brq Ar keyword Ns = Ns Ar value ... Assign a .Xr vnet 4 Index: ldomctl.h === RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.h,v retrieving revision 1.12 diff -u -p -r1.12 ldomctl.h --- ldomctl.h 4 Jan 2020 15:45:46 - 1.12 +++ ldomctl.h 5 Feb 2020 20:18:43 - @@ -158,6 +158,7 @@ extern uint64_t hv_memsize; struct vdisk { SIMPLEQ_ENTRY(vdisk)entry; const char *path; + const char *devalias; }; struct vnet { Index: parse.y === RCS file: /cvs/src/usr.sbin/ldomctl/parse.y,v retrieving revision 1.15 diff -u -p -r1.15 parse.y --- parse.y 9 Jan 2020 22:06:23 - 1.15 +++ parse.y 5 Feb 2020 20:18:48 - @@ -70,12 +70,17 @@ struct vcpu_opts { uint64_tstride; } vcpu_opts; +struct vdisk_opts { + const char *devalias; +} vdisk_opts; + struct vnet_opts { uint64_tmac_addr; uint64_tmtu; } vnet_opts; void vcput_opts_default(void); +void vdisk_opts_default(void); void vnet_opts_default(void); typedef struct { @@ -83,6 +88,7 @@ typedef struct { int64_t number; char*string; struct vcpu_opts
Re: gpio(4) support for APU2
Nathanael Rensen schreef op 2020-01-29 15:47: The diff below adds gpio(4) support to wbsio(4) for Nuvoton NCT5104D (pcengines APU2). It is based on Matt Dainty's diff posted to this list in November 2018: https://marc.info/?l=openbsd-tech&m=154134941027009&w=2 A key difference from Matt Dainty's original diff is the use of config_search() rather than config_found() to avoid problems with the lm78 driver mentioned in his post. But as before this diff does nothing to make sure it is actually safe to touch these gpio pins. Other machines might have the same chip but use the pins internally to switch power to on-board components. Index: sys/dev/isa/wbsioreg.h === RCS file: /cvs/src/sys/dev/isa/wbsioreg.h,v retrieving revision 1.5 diff -u -p -r1.5 wbsioreg.h --- sys/dev/isa/wbsioreg.h 17 Dec 2019 01:34:59 - 1.5 +++ sys/dev/isa/wbsioreg.h 29 Jan 2020 14:30:28 - @@ -30,8 +30,15 @@ /* Configuration Space Registers */ #define WBSIO_LDN 0x07/* Logical Device Number */ +#define WBSIO_MF 0x1c/* Multi Function Selection */ +#define WBSIO_MF_UARTD (1 << 2) +#define WBSIO_MF_UARTC (1 << 3) +#define WBSIO_MF_GP67 (1 << 4) #define WBSIO_ID 0x20/* Device ID */ #define WBSIO_REV 0x21/* Device Revision */ +#define WBSIO_CR27 0x27/* Global Option */ +#define WBSIO_SF 0x2f/* Strapping Function */ +#define WBSIO_LDN_EN 0x30/* Logical Device Enable */ #define WBSIO_ID_W83627HF 0x52 #define WBSIO_ID_W83627THF 0x82 @@ -52,8 +59,38 @@ #define WBSIO_ID_NCT6795D 0xd3 /* Logical Device Number (LDN) Assignments */ +#define WBSIO_LDN_GPIO10x07 +#define WBSIO_LDN_WDT 0x08 +#define WBSIO_LDN_GPIO20x09/* Not used */ #define WBSIO_LDN_HM 0x0b +#define WBSIO_LDN_GPIO30x0f /* Hardware Monitor Control Registers (LDN B) */ #define WBSIO_HM_ADDR_MSB 0x60/* Address [15:8] */ #define WBSIO_HM_ADDR_LSB 0x61/* Address [7:0] */ + +/* GPIO Control Registers (LDN 7) */ +/* GPIOn registers are offset by n*4 bytes */ +#define WBSIO_GPIO_IO 0xe0/* GPIO Direction */ +#define WBSIO_GPIO_DATA0xe1/* GPIO Data */ +#define WBSIO_GPIO_INVERT 0xe2/* GPIO Invert */ +#define WBSIO_GPIO_STATUS 0xe3/* GPIO Status */ +#define WBSIO_GPIO0_EN (1 << 0) +#define WBSIO_GPIO1_EN (1 << 1) +#define WBSIO_GPIO6_EN (1 << 6) +#define WBSIO_GPIO0_NPINS 8 +#define WBSIO_GPIO1_NPINS 8 +#define WBSIO_GPIO6_NPINS 1 +#define WBSIO_GPIO_NPINS (WBSIO_GPIO0_NPINS + WBSIO_GPIO1_NPINS + \ +WBSIO_GPIO6_NPINS) + +/* WDT Control Registers (LDN 8) */ +#define WBSIO_WDT_ENABLE 0x30 +#define WBSIO_WDT_CONTROL 0xf0 +#define WBSIO_WDT_COUNTER 0xf1 +#define WBSIO_WDT_MINUTE (1 << 3) +#define WBSIO_WDT_FAST (1 << 4) + +/* GPIO Control Registers (LDN F) */ +/* GPIOn register is offset by n bytes */ +#define WBSIO_GPIO_PP_OD 0xe0/* GPIO Push-Pull/Open Drain */ Index: sys/dev/isa/wbsio.c === RCS file: /cvs/src/sys/dev/isa/wbsio.c,v retrieving revision 1.11 diff -u -p -r1.11 wbsio.c --- sys/dev/isa/wbsio.c 17 Dec 2019 01:34:59 - 1.11 +++ sys/dev/isa/wbsio.c 29 Jan 2020 14:30:28 - @@ -23,11 +23,13 @@ #include #include #include +#include #include #include #include +#include #ifdef WBSIO_DEBUG #define DPRINTF(x) printf x @@ -40,12 +42,22 @@ struct wbsio_softc { bus_space_tag_t sc_iot; bus_space_handle_t sc_ioh; + + struct gpio_chipset_tag sc_gpio_gc; + gpio_pin_t sc_gpio_pins[WBSIO_GPIO_NPINS]; }; intwbsio_probe(struct device *, void *, void *); void wbsio_attach(struct device *, struct device *, void *); +intwbsio_search_cb(struct device *, void *, void *); intwbsio_print(void *, const char *); +intwbsio_gpio_pin_to_group(struct wbsio_softc *, int); +intwbsio_gpio_pin_read(struct wbsio_softc *, int); +intwbsio_gpio_read(void *, int); +void wbsio_gpio_write(void *, int, int); +void wbsio_gpio_ctl(void *, int, int); + struct cfattach wbsio_ca = { sizeof(struct wbsio_softc), wbsio_probe, @@ -56,6 +68,11 @@ struct cfdriver wbsio_cd = { NULL, "wbsio", DV_DULL }; +struct wbsio_aux { + struct isa_attach_args ia; + struct gpiobus_attach_args gba; +}; + static __inline void wbsio_conf_enable(bus_space_tag_t iot, bus_space_handle_t ioh) { @@ -132,9 +149,10 @@ wbsio_attach(struct device *parent, stru { struct wbsio_softc *sc = (void *)self; struct isa_attach_args *ia = aux; - struct isa_attach_args nia; - u_int8_t devid, reg, reg0, reg1; +
nep(4) rx error interrupts
I'm trying to use aggr on top of nep(4), which has turned up a few bugs. Firstly, the nic appears to report rx errors in the second interrupt state vector, which the driver doesn't expect, so I get this: nep3: nep_intr 0 8 0 and then, since the interrupt wasn't rearmed, the interface stops. Then, the driver doesn't clear rx error interrupt sources, so rearming results in an interrupt storm. The illumos nxge driver masks off read-only bits in the rx status register and writes it back, clearing all active interrupt sources, so I've done that here too. ok? Index: if_nep.c === RCS file: /cvs/src/sys/dev/pci/if_nep.c,v retrieving revision 1.31 diff -u -p -r1.31 if_nep.c --- if_nep.c9 Nov 2018 14:14:31 - 1.31 +++ if_nep.c10 Feb 2020 11:57:44 - @@ -333,11 +333,24 @@ #define RX_DMA_ENT_MSK(chan) (DMC + 0x00068 + (chan) * 0x00200) #define RX_DMA_ENT_MSK_RBR_EMPTY (1ULL << 3) #define RX_DMA_CTL_STAT(chan) (DMC + 0x00070 + (chan) * 0x00200) +#define RX_DMA_CTL_STAT_DC_FIFO_ERR (1ULL << 48) #define RX_DMA_CTL_STAT_MEX (1ULL << 47) #define RX_DMA_CTL_STAT_RCRTHRES (1ULL << 46) #define RX_DMA_CTL_STAT_RCRTO (1ULL << 45) +#define RX_DMA_CTL_STAT_PORT_DROP_PKT (1ULL << 42) +#define RX_DMA_CTL_STAT_WRED_DROP (1ULL << 41) +#define RX_DMA_CTL_STAT_RBR_PRE_EMTY (1ULL << 40) +#define RX_DMA_CTL_STAT_RCR_SHDW_FULL (1ULL << 39) #define RX_DMA_CTL_STAT_RBR_EMPTY (1ULL << 35) #define RX_DMA_CTL_STAT_PTRREAD_SHIFT 16 +#define RX_DMA_CTL_STAT_WR1C_BITS (RX_DMA_CTL_STAT_RBR_EMPTY | \ +RX_DMA_CTL_STAT_RCR_SHDW_FULL | \ +RX_DMA_CTL_STAT_RBR_PRE_EMTY | \ +RX_DMA_CTL_STAT_WRED_DROP | \ +RX_DMA_CTL_STAT_PORT_DROP_PKT | \ +RX_DMA_CTL_STAT_RCRTO | \ +RX_DMA_CTL_STAT_RCRTHRES | \ +RX_DMA_CTL_STAT_DC_FIFO_ERR) #define RX_DMA_CTL_STAT_DBG(chan) (DMC + 0x00098 + (chan) * 0x00200) #define TX_LOG_PAGE_VLD(chan) (FZC_DMC + 0x4 + (chan) * 0x00200) @@ -959,7 +972,7 @@ nep_intr(void *arg) rearm = 1; } - if (sv0 & (1ULL << LDN_RXDMA(sc->sc_port))) { + if ((sv0 | sv1) & (1ULL << LDN_RXDMA(sc->sc_port))) { nep_rx_proc(sc); rearm = 1; } @@ -990,8 +1003,8 @@ nep_rx_proc(struct nep_softc *sc) int idx, len, i; val = nep_read(sc, RX_DMA_CTL_STAT(sc->sc_port)); - nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), - RX_DMA_CTL_STAT_RCRTHRES | RX_DMA_CTL_STAT_RCRTO); + val &= RX_DMA_CTL_STAT_WR1C_BITS; + nep_write(sc, RX_DMA_CTL_STAT(sc->sc_port), val); bus_dmamap_sync(sc->sc_dmat, NEP_DMA_MAP(sc->sc_rcring), 0, NEP_DMA_LEN(sc->sc_rcring), BUS_DMASYNC_POSTREAD);
Re: iwm: work around missing Tx completion interrupts
Stefan Sperling schreef op 2020-02-06 12:45: At 36c3 I noticed roaming failures with iwm(4) where we would get stuck trying to roam to a different AP. Debugging this with bluhm@ we found that the reason it gets stuck is a non-zero refcount on the ic_bss node. When roaming, we wait for this reference count to hit zero before switching the new AP. A non-zero reference count implies that the driver still has outstanding frames destined for the old AP queued to hardware. What we observed was that the reference count never went back to zero so roaming never completed and no further data frames could be sent. ifconfig iwm0 down/up was required to get the interface working again. iwm(4) decrements the refcount whenever hardware signals Tx completion for a frame at Tx queue index 'N'. We observed that we sometimes get an interrupt for frame 'N - 2' followed by an interrupt for frame 'N', with no interrupt being received for frame 'N - 1'. I don't really know how the rings work on this hardware, but coalescing interrupts like that doesn't sound unreasonable. Whenever this had occurred a later decision to roam to another AP would fail as described above. A side-effect of this is that an mbuf gets leaked. This diff implements a workaround in iwm's interrupt handler. It's not pretty but I don't know the root cause. Given that this happens very rarely, and all we lose is success/failure information for the affected frames, this workaround seems acceptable to me. So far I only managed to trigger the problem at 36c3. If you want to see a printf when it happens, compile with 'option IWM_DEBUG'. ok? diff refs/heads/master refs/heads/iwm-txintr blob - 348771350037a5a281b12647bf95b5c2173bf57f (mode 644) blob + bdc3cebd73f392a5c1bfab702e6e8685298a6648 (mode 600) --- sys/dev/pci/if_iwm.c +++ sys/dev/pci/if_iwm.c @@ -1276,6 +1276,7 @@ iwm_alloc_tx_ring(struct iwm_softc *sc, struct iwm_tx_ ring->qid = qid; ring->queued = 0; ring->cur = 0; + ring->tail = 0; /* Allocate TX descriptors (256-byte aligned). */ size = IWM_TX_RING_COUNT * sizeof (struct iwm_tfd); @@ -1377,6 +1378,7 @@ iwm_reset_tx_ring(struct iwm_softc *sc, struct iwm_tx_ } ring->queued = 0; ring->cur = 0; + ring->tail = 0; } void @@ -4193,6 +4195,25 @@ iwm_rx_tx_cmd_single(struct iwm_softc *sc, struct iwm_ } void +iwm_txd_done(struct iwm_softc *sc, struct iwm_tx_data *txd) +{ + struct ieee80211com *ic = &sc->sc_ic; + + bus_dmamap_sync(sc->sc_dmat, txd->map, 0, txd->map->dm_mapsize, + BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(sc->sc_dmat, txd->map); + m_freem(txd->m); + txd->m = NULL; + + KASSERT(txd->in); + ieee80211_release_node(ic, &txd->in->in_ni); + txd->in = NULL; + + KASSERT(txd->done == 0); + txd->done = 1; +} + +void iwm_rx_tx_cmd(struct iwm_softc *sc, struct iwm_rx_packet *pkt, struct iwm_rx_data *data) { @@ -4202,31 +4223,35 @@ iwm_rx_tx_cmd(struct iwm_softc *sc, struct iwm_rx_pack int idx = cmd_hdr->idx; int qid = cmd_hdr->qid; struct iwm_tx_ring *ring = &sc->txq[qid]; - struct iwm_tx_data *txd = &ring->data[idx]; - struct iwm_node *in = txd->in; + struct iwm_tx_data *txd; - if (txd->done) - return; - bus_dmamap_sync(sc->sc_dmat, data->map, 0, IWM_RBUF_SIZE, BUS_DMASYNC_POSTREAD); sc->sc_tx_timer = 0; - iwm_rx_tx_cmd_single(sc, pkt, in); + txd = &ring->data[idx]; + if (txd->done) + return; - bus_dmamap_sync(sc->sc_dmat, txd->map, 0, txd->map->dm_mapsize, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(sc->sc_dmat, txd->map); - m_freem(txd->m); + iwm_rx_tx_cmd_single(sc, pkt, txd->in); + iwm_txd_done(sc, txd); - KASSERT(txd->done == 0); - txd->done = 1; - KASSERT(txd->in); - - txd->m = NULL; - txd->in = NULL; - ieee80211_release_node(ic, &in->in_ni); + /* +* XXX Sometimes we miss Tx completion interrupts. +* We cannot check Tx success/failure for affected frames; just free +* the associated mbuf and release the associated node reference. +*/ + while (ring->tail != idx) { + txd = &ring->data[ring->tail]; + if (!txd->done) { + DPRINTF(("%s: missed Tx completion: tail=%d idx=%d\n", + __func__, ring->tail, idx)); + iwm_txd_done(sc, txd); + ring->queued--; + } + ring->tail = (ring->tail + 1) % IWM_TX_RING_COUNT; + } if (--ring->queued < IWM_TX_RING_LOMARK) { sc->qfullmsk &= ~(1 << ring->qid); blob - 5206d9ac72a829d4bc2fd307a1d1a79e6e14627e blob + ad0975b69dd3d3888f584dd759f5ebcf5dd8e940 --- sys/dev/pci/if_iwmvar.h +++ sys/dev/pci/if_iwmvar.h @@ -270,6 +270,7
setlocale() in cron
Why does cron(8) and crontab(1) need to setlocale()? Jan Index: cron.c === RCS file: /cvs/src/usr.sbin/cron/cron.c,v retrieving revision 1.77 diff -u -p -r1.77 cron.c --- cron.c 23 Oct 2017 15:15:22 - 1.77 +++ cron.c 10 Feb 2020 12:12:13 - @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -83,8 +82,6 @@ main(int argc, char *argv[]) struct sigaction sact; sigset_t blocked, omask; struct group *grp; - - setlocale(LC_ALL, ""); setvbuf(stdout, NULL, _IOLBF, 0); setvbuf(stderr, NULL, _IOLBF, 0); Index: crontab.c === RCS file: /cvs/src/usr.sbin/cron/crontab.c,v retrieving revision 1.93 diff -u -p -r1.93 crontab.c --- crontab.c 28 Jun 2019 13:32:47 - 1.93 +++ crontab.c 10 Feb 2020 12:12:13 - @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -92,7 +91,6 @@ main(int argc, char *argv[]) user_gid = getgid(); crontab_gid = getegid(); - setlocale(LC_ALL, ""); openlog(__progname, LOG_PID, LOG_CRON); setvbuf(stderr, NULL, _IOLBF, 0);
cp.c and rm.c embedded in mv(1)
mv code contains copies of cp.c and rm.c - is that so that mv can avoid the fork+exec (and call the relevant cp/rm code itself)? If so, is it so that mv can be pledged? It isn't. There must be something worth the duplication ... Jan
Re: lcd(4/hppa): timeout_add(9) -> timeout_add_usec(9)
On 2020-02-08 20:15, Scott Cheloha wrote: On Sun, Dec 22, 2019 at 01:17:56PM -0600, Scott Cheloha wrote: lcd_softc.sc_delay's unit appears to be microseconds. ok? 1 month bump. While the diff looks ok to me, I can't test this until I have physical access to my hppa machines. Index: dev/lcd.c === RCS file: /cvs/src/sys/arch/hppa/dev/lcd.c,v retrieving revision 1.4 diff -u -p -r1.4 lcd.c --- dev/lcd.c 11 Dec 2015 16:07:01 - 1.4 +++ dev/lcd.c 8 Feb 2020 19:14:08 - @@ -138,7 +138,7 @@ lcd_blink(void *v, int on) sc->sc_on = on; bus_space_write_1(sc->sc_iot, sc->sc_cmdh, 0, sc->sc_heartbeat[0]); - timeout_add(&sc->sc_to, max(1, (sc->sc_delay * hz) / 100)); + timeout_add_usec(&sc->sc_to, sc->sc_delay); } void
Re: Audio control API, part 2: add new sndioctl(1) utility
On Sun, Feb 09, 2020 at 12:14:47PM GMT, Alexandre Ratchov wrote: > Here's a new sndioctl utility similar to mixerctl(1) but using the new > sndio API. Example: > > $ sndioctl > output.level=127 > app/aucat0.level=127 > app/firefox0.level=127 > app/firefox1.level=12 > app/midisyn0.level=127 > app/mpv0.level=127 > app/prog5.level=127 > app/prog6.level=127 > app/prog7.level=127 > hw/input.level=62 > hw/input.mute=0 > hw/output.level=63 > hw/output.mute=0 > Hi Alexandre, Just a quick question. Is there a good reason to have the above using "slash" ('/') as the first separator instead of the, more familiar, "dot" ('.') known from sysctl(8)'s MIB (Management Information Base) style names or even the "pseudo" MIB know from mixerctl(1)? Regards, Raf > Configuration parameters that are not exposed by sndiod will be > handled by audioctl(1), including the /etc/mixerctl.conf file at > system startup. > > Originally the program was designed to handle modern many-channel > devices by presenting many-channel knobs on a single line; this > feature isn't used yet as the corresponding kernel bits are missing. > > Index: usr.bin/Makefile > === > RCS file: /cvs/src/usr.bin/Makefile,v > retrieving revision 1.161 > diff -u -p -u -p -r1.161 Makefile > --- usr.bin/Makefile 9 Aug 2019 06:18:25 - 1.161 > +++ usr.bin/Makefile 9 Feb 2020 11:05:02 - > @@ -22,7 +22,7 @@ SUBDIR= apply arch at aucat audioctl awk > pr printenv printf quota radioctl rcs rdist rdistd \ > readlink renice rev rpcgen rpcinfo rs rsync rup rusers rwall \ > sdiff script sed sendbug shar showmount signify skey \ > - skeyaudit skeyinfo skeyinit sndiod snmp \ > + skeyaudit skeyinfo skeyinit sndioctl sndiod snmp \ > sort spell split ssh stat su systat \ > tail talk tcpbench tee telnet tftp tic time \ > tmux top touch tput tr true tset tsort tty usbhidaction usbhidctl \ > Index: usr.bin/sndioctl/Makefile > === > RCS file: usr.bin/sndioctl/Makefile > diff -N usr.bin/sndioctl/Makefile > --- /dev/null 1 Jan 1970 00:00:00 - > +++ usr.bin/sndioctl/Makefile 9 Feb 2020 11:05:02 - > @@ -0,0 +1,5 @@ > +#$OpenBSD$ > + > +PROG=sndioctl > +LDADD+= -lsndio > +.include > Index: usr.bin/sndioctl/sndioctl.1 > === > RCS file: usr.bin/sndioctl/sndioctl.1 > diff -N usr.bin/sndioctl/sndioctl.1 > --- /dev/null 1 Jan 1970 00:00:00 - > +++ usr.bin/sndioctl/sndioctl.1 9 Feb 2020 11:05:02 - > @@ -0,0 +1,148 @@ > +.\" $OpenBSD$ > +.\" > +.\" Copyright (c) 2007 Alexandre Ratchov > +.\" > +.\" Permission to use, copy, modify, and distribute this software for any > +.\" purpose with or without fee is hereby granted, provided that the above > +.\" copyright notice and this permission notice appear in all copies. > +.\" > +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > +.\" > +.Dd $Mdocdate: April 8 2011 $ > +.Dt SNDIOCTL 1 > +.Os > +.Sh NAME > +.Nm sndioctl > +.Nd control audio parameters > +.Sh SYNOPSIS > +.Nm > +.Bk -words > +.Op Fl iv > +.Op Fl f Ar device > +.Op Ar command ... > +.Ek > +.Nm > +.Bk -words > +.Fl d > +.Ek > +.Sh DESCRIPTION > +The > +.Nm > +utility can display or change parameters of > +.Xr sndio 7 > +audio devices. > +The options are as follows: > +.Bl -tag -width Ds > +.It Fl d > +Dump the raw list of available parameters and exit. > +Useful as a debug tool. > +.It Fl f Ar device > +Use this > +.Xr sndio 7 > +audio device. > +.It Fl m > +Monitor and display audio parameters changes. > +.It Fl i > +Display characteristics of requested parameters > +instead of their values. > +.It Fl v > +Enable verbose mode, a.k.a. multi-channel mode. > +By default parameters affecting different channels > +of the same stream are disguised as a single mono > +parameter to hide details that are not essential. > +.El > +.Pp > +If no commands are specified all valid parameters are displayed on > +.Em stdout . > +Unless > +.Fl d , > +.Fl m , > +or > +.Fl i > +are used, displayed parameters are valid commands. > +The set of available controls depends on the control device. > +.Pp > +Commands use the following two formats to display and set > +parameters respectively: > +.Pp > +.Dl group/stream[channel].function > +.Dl group/stream[channel].function=value > +.Pp > +On the