sparc64: magma(4): timeout_add(9) -> timeout_add_msec(9)

2020-02-10 Thread Scott Cheloha
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)

2020-02-10 Thread Scott Cheloha
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

2020-02-10 Thread Jan Stary
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

2020-02-10 Thread Jan Stary
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

2020-02-10 Thread Todd C . Miller
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

2020-02-10 Thread Todd C . Miller
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

2020-02-10 Thread Ingo Schwarze
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

2020-02-10 Thread sven falempin
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

2020-02-10 Thread Ingo Schwarze
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

2020-02-10 Thread Alexandre Ratchov
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

2020-02-10 Thread Peter Stuge
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

2020-02-10 Thread Jonathan Matthew
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)

2020-02-10 Thread Marc Espie
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

2020-02-10 Thread Theo de Raadt
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

2020-02-10 Thread Ingo Schwarze
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

2020-02-10 Thread Jan Stary
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

2020-02-10 Thread Kyle Isom
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)

2020-02-10 Thread Scott Cheloha
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

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Jonathan Matthew
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

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Jan Stary
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)

2020-02-10 Thread Jan Stary
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)

2020-02-10 Thread Mark Kettenis

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

2020-02-10 Thread Raf Czlonka
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