Re: iwm: support dynamic queue allocation (DQA)
On Mon, Oct 14, 2019 at 01:51:02PM +0200, Stefan Sperling wrote: > > Newer iwm firmware requires the driver to use a feature known as > dynamic queue allocation (DQA). What matters is that the command queue > index was changed. Newer firmware images have stopped responding to > commands sent with the old command queue index, and this is preventing > us from dropping newer firmware versions into /etc/firmware without > breaking things. > > Some of our existing firmware images already support DQA (*-22 image > files), and some do not (*-16 image files), so we need to support both > modes of operation for now. (Linux has already removed non-DQA code > paths from their iwlwifi driver). > > I have successfully tested this diff on 8265 with our current firmware > image (22.361476.0) as well as a newer -22 firmware image (22.391740.0, > which is *not* in fw_update yet). I have also tested 7265 successfully. > > Tests on 7260 and 8260 devices are still required. > Nothing should change. At this point I am just looking for potential > regressions when using this diff against our current firmware images. > > Reviews and OKs are also welcome. Tested on 3165 with current firmware (16.242414.0), seems to be working fine. iwm0 at pci3 dev 0 function 0 "Intel Dual Band Wireless AC 3165" rev 0x81, msi iwm0: hw rev 0x210, fw ver 16.242414.0, address 08:d4:0c:xx:xx:xx
sysupgrade(8) download directory
Prompted by a discussion on irc.
While /home/_sysupgrade is a sensible default, it's not always possible
to use (for instance if /home is encrypted and mounted via rc.local or
even manually over ssh). Previous attempts have had sysupgrade(8)
automagically figure out a sensible location based on available space, but
this has issues too (a sufficiently large tmp on mfs), and breaks in the
case I mentioned. All this diff does is add an '-l path' argument to
sysupgrade allowing the user to specify where the sets get downloaded
to, with /home/_sysupgrade remaining the default if nothing is passed.
For example, if the only partition with sufficient space is /usr/local a
user might pass:
sysupgrade -l /usr/local/sets
Cheers,
Joe
Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.10
diff -u -p -r1.10 sysupgrade.8
--- sysupgrade.83 Oct 2019 12:43:58 - 1.10
+++ sysupgrade.815 Oct 2019 20:07:50 -
@@ -23,6 +23,7 @@
.Sh SYNOPSIS
.Nm
.Op Fl fkn
+.Op Fl l Ar path
.Op Fl r | s
.Op Ar installurl
.Sh DESCRIPTION
@@ -53,9 +54,13 @@ Force an already applied upgrade.
The default is to upgrade to latest snapshot only if available.
This option has no effect on releases.
.It Fl k
-Keep the files in
-.Pa /home/_sysupgrade .
+Keep the sets.
By default they will be deleted after the upgrade.
+.It Fl l Ar path
+Specify the
+.Ar path
+the upgrade will be downloaded to.
+The default is /home/_sysupgrade.
.It Fl n
Fetch and verify the files and create
.Pa /bsd.upgrade
@@ -77,7 +82,7 @@ The ramdisk kernel to trigger an unatten
.Ox
mirror top-level URL for fetching an upgrade.
.It Pa /home/_sysupgrade
-Directory the upgrade is downloaded to.
+Default directory the upgrade is downloaded to.
.El
.Sh SEE ALSO
.Xr signify 1 ,
Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.26
diff -u -p -r1.26 sysupgrade.sh
--- sysupgrade.sh 14 Oct 2019 06:58:53 - 1.26
+++ sysupgrade.sh 15 Oct 2019 20:07:50 -
@@ -34,7 +34,7 @@ ug_err()
usage()
{
- ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
+ ug_err "usage: ${0##*/} [-fkn] [-l path] [-r | -s] [installurl]"
}
unpriv()
@@ -79,10 +79,11 @@ FORCE=false
KEEP=false
REBOOT=true
-while getopts fknrs arg; do
+while getopts fkl:nrs arg; do
case ${arg} in
f) FORCE=true;;
k) KEEP=true;;
+ l) SETSDIR=${OPTARG};;
n) REBOOT=false;;
r) RELEASE=true;;
s) SNAP=true;;
@@ -188,7 +189,7 @@ ${KEEP} && > keep
cat <<__EOT >/auto_upgrade.conf
Location of sets = disk
-Pathname to the sets = /home/_sysupgrade/
+Pathname to the sets = ${SETSDIR}
Set name(s) = done
Directory does not contain SHA256.sig. Continue without verification = yes
__EOT
@@ -196,7 +197,7 @@ __EOT
if ! ${KEEP}; then
CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
cat <<__EOT > /etc/rc.firsttime
-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}
__EOT
fi
Re: random safety for pbkdf
Makes sense -- but perhaps justify the arc4random with a comment, explaining what is being done, so that people don't need to look in the commitlog? Ted Unangst wrote: > In the event that a program uses invalid parameters, I think we should > overwrite the key with random data. Otherwise, there's a chance the program > will continue with a zero key. It may even appear to work, encrypting and > decrypting data, but with a weak key. Random data means it fails closed, and > should also make it easier to detect such errors since it no longer > interoperates. > > > Index: bcrypt_pbkdf.c > === > RCS file: /home/cvs/src/lib/libutil/bcrypt_pbkdf.c,v > retrieving revision 1.13 > diff -u -p -r1.13 bcrypt_pbkdf.c > --- bcrypt_pbkdf.c12 Jan 2015 03:20:04 - 1.13 > +++ bcrypt_pbkdf.c15 Oct 2019 19:14:12 - > @@ -110,10 +110,10 @@ bcrypt_pbkdf(const char *pass, size_t pa > > /* nothing crazy */ > if (rounds < 1) > - return -1; > + goto bad; > if (passlen == 0 || saltlen == 0 || keylen == 0 || > keylen > sizeof(out) * sizeof(out)) > - return -1; > + goto bad; > stride = (keylen + sizeof(out) - 1) / sizeof(out); > amt = (keylen + stride - 1) / stride; > > @@ -166,4 +166,8 @@ bcrypt_pbkdf(const char *pass, size_t pa > explicit_bzero(out, sizeof(out)); > > return 0; > + > +bad: > + arc4random_buf(key, keylen); > + return -1; > } > Index: pkcs5_pbkdf2.c > === > RCS file: /home/cvs/src/lib/libutil/pkcs5_pbkdf2.c,v > retrieving revision 1.10 > diff -u -p -r1.10 pkcs5_pbkdf2.c > --- pkcs5_pbkdf2.c18 Apr 2017 04:06:21 - 1.10 > +++ pkcs5_pbkdf2.c15 Oct 2019 19:17:08 - > @@ -84,11 +84,11 @@ pkcs5_pbkdf2(const char *pass, size_t pa > size_t r; > > if (rounds < 1 || key_len == 0) > - return -1; > + goto bad; > if (salt_len == 0 || salt_len > SIZE_MAX - 4) > - return -1; > + goto bad; > if ((asalt = malloc(salt_len + 4)) == NULL) > - return -1; > + goto bad; > > memcpy(asalt, salt, salt_len); > > @@ -118,4 +118,8 @@ pkcs5_pbkdf2(const char *pass, size_t pa > explicit_bzero(obuf, sizeof(obuf)); > > return 0; > + > +bad: > + arc4random_buf(key, key_len); > + return -1; > } >
random safety for pbkdf
In the event that a program uses invalid parameters, I think we should overwrite the key with random data. Otherwise, there's a chance the program will continue with a zero key. It may even appear to work, encrypting and decrypting data, but with a weak key. Random data means it fails closed, and should also make it easier to detect such errors since it no longer interoperates. Index: bcrypt_pbkdf.c === RCS file: /home/cvs/src/lib/libutil/bcrypt_pbkdf.c,v retrieving revision 1.13 diff -u -p -r1.13 bcrypt_pbkdf.c --- bcrypt_pbkdf.c 12 Jan 2015 03:20:04 - 1.13 +++ bcrypt_pbkdf.c 15 Oct 2019 19:14:12 - @@ -110,10 +110,10 @@ bcrypt_pbkdf(const char *pass, size_t pa /* nothing crazy */ if (rounds < 1) - return -1; + goto bad; if (passlen == 0 || saltlen == 0 || keylen == 0 || keylen > sizeof(out) * sizeof(out)) - return -1; + goto bad; stride = (keylen + sizeof(out) - 1) / sizeof(out); amt = (keylen + stride - 1) / stride; @@ -166,4 +166,8 @@ bcrypt_pbkdf(const char *pass, size_t pa explicit_bzero(out, sizeof(out)); return 0; + +bad: + arc4random_buf(key, keylen); + return -1; } Index: pkcs5_pbkdf2.c === RCS file: /home/cvs/src/lib/libutil/pkcs5_pbkdf2.c,v retrieving revision 1.10 diff -u -p -r1.10 pkcs5_pbkdf2.c --- pkcs5_pbkdf2.c 18 Apr 2017 04:06:21 - 1.10 +++ pkcs5_pbkdf2.c 15 Oct 2019 19:17:08 - @@ -84,11 +84,11 @@ pkcs5_pbkdf2(const char *pass, size_t pa size_t r; if (rounds < 1 || key_len == 0) - return -1; + goto bad; if (salt_len == 0 || salt_len > SIZE_MAX - 4) - return -1; + goto bad; if ((asalt = malloc(salt_len + 4)) == NULL) - return -1; + goto bad; memcpy(asalt, salt, salt_len); @@ -118,4 +118,8 @@ pkcs5_pbkdf2(const char *pass, size_t pa explicit_bzero(obuf, sizeof(obuf)); return 0; + +bad: + arc4random_buf(key, key_len); + return -1; }
Re: iwm: support dynamic queue allocation (DQA)
seems to be working on Intel Dual Band Wireless AC 7260 -- I'm not entirely sure you are real.
kill scheduler_fork_hook()
The only raison d'etre of scheduler_fork_hook() is because `p_estcpu' is
not at the right place in "struct proc". Diff below fixes that. I
reordered the fields to not have implicit padding increase the size of
the structure.
ok?
Index: sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.273
diff -u -p -r1.273 proc.h
--- sys/proc.h 2 Aug 2019 02:17:35 - 1.273
+++ sys/proc.h 15 Oct 2019 14:50:40 -
@@ -349,10 +349,7 @@ struct proc {
#definep_startzero p_dupfd
int p_dupfd; /* Sideways return value from filedescopen.
XXX */
- longp_thrslpid; /* for thrsleep syscall */
-
/* scheduling */
- u_int p_estcpu; /* [s] Time averaged val of p_cpticks */
int p_cpticks; /* Ticks of cpu time. */
const volatile void *p_wchan; /* [s] Sleep address. */
struct timeout p_sleep_to;/* timeout for tsleep() */
@@ -364,6 +361,8 @@ struct proc {
u_int p_iticks; /* Statclock hits processing intr. */
struct cpu_info * volatile p_cpu; /* [s] CPU we're running on. */
+ longp_thrslpid; /* for thrsleep syscall */
+
struct rusage p_ru;/* Statistics */
struct tusage p_tu;/* accumulated times. */
struct timespec p_rtime; /* Real time. */
@@ -379,6 +378,7 @@ struct proc {
u_char p_priority; /* [s] Process priority. */
u_char p_usrpri; /* [s] User-prio based on p_estcpu & ps_nice. */
+ u_int p_estcpu; /* [s] Time averaged val of p_cpticks */
int p_pledge_syscall; /* Cache of current syscall */
struct ucred *p_ucred; /* cached credentials */
Index: sys/sched.h
===
RCS file: /cvs/src/sys/sys/sched.h,v
retrieving revision 1.55
diff -u -p -r1.55 sched.h
--- sys/sched.h 15 Oct 2019 10:05:43 - 1.55
+++ sys/sched.h 15 Oct 2019 14:48:45 -
@@ -182,11 +182,6 @@ void sched_init_runqueues(void);
void setrunqueue(struct cpu_info *, struct proc *, uint8_t);
void remrunqueue(struct proc *);
-/* Inherit the parent's scheduler history */
-#define scheduler_fork_hook(parent, child) do {
\
- (child)->p_estcpu = (parent)->p_estcpu; \
-} while (0)
-
/* Chargeback parents for the sins of their children. */
#define scheduler_wait_hook(parent, child) do {
\
(parent)->p_estcpu = ESTCPULIM((parent)->p_estcpu + (child)->p_estcpu);\
Index: kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.214
diff -u -p -r1.214 kern_fork.c
--- kern/kern_fork.c15 Oct 2019 10:05:43 - 1.214
+++ kern/kern_fork.c15 Oct 2019 14:48:37 -
@@ -170,13 +170,6 @@ thread_new(struct proc *parent, vaddr_t
*/
timeout_set(>p_sleep_to, endtsleep, p);
- /*
-* set priority of child to be that of parent
-* XXX should move p_estcpu into the region of struct proc which gets
-* copied.
-*/
- scheduler_fork_hook(parent, p);
-
#ifdef WITNESS
p->p_sleeplocks = NULL;
#endif
tun(4) & switch(4) -> tsleep_nsec(9)
Convert the two pseudo-drivers to use tsleep_nsec(9) in their read(2)
routine, both drivers block indefinitely.
ok?
Index: net/if_tun.c
===
RCS file: /cvs/src/sys/net/if_tun.c,v
retrieving revision 1.190
diff -u -p -r1.190 if_tun.c
--- net/if_tun.c13 Sep 2019 01:31:24 - 1.190
+++ net/if_tun.c7 Oct 2019 12:49:04 -
@@ -751,8 +751,8 @@ tun_dev_read(struct tun_softc *tp, struc
int destroyed;
while ((tp->tun_flags & TUN_READY) != TUN_READY) {
- if ((error = tsleep((caddr_t)tp,
- (PZERO + 1)|PCATCH, "tunread", 0)) != 0)
+ if ((error = tsleep_nsec(tp,
+ (PZERO + 1)|PCATCH, "tunread", INFSLP)) != 0)
return (error);
/* Make sure the interface still exists. */
ifp1 = if_get(ifidx);
@@ -766,8 +766,8 @@ tun_dev_read(struct tun_softc *tp, struc
if (tp->tun_flags & TUN_NBIO && ioflag & IO_NDELAY)
return (EWOULDBLOCK);
tp->tun_flags |= TUN_RWAIT;
- if ((error = tsleep((caddr_t)tp,
- (PZERO + 1)|PCATCH, "tunread", 0)) != 0)
+ if ((error = tsleep_nsec(tp,
+ (PZERO + 1)|PCATCH, "tunread", INFSLP)) != 0)
return (error);
/* Make sure the interface still exists. */
ifp1 = if_get(ifidx);
Index: net/switchctl.c
===
RCS file: /cvs/src/sys/net/switchctl.c,v
retrieving revision 1.15
diff -u -p -r1.15 switchctl.c
--- net/switchctl.c 12 May 2019 16:38:02 - 1.15
+++ net/switchctl.c 7 Oct 2019 12:49:17 -
@@ -148,7 +148,7 @@ switchread(dev_t dev, struct uio *uio, i
goto failed;
}
sc->sc_swdev->swdev_waiting = 1;
- error = tsleep(sc, (PZERO + 1)|PCATCH, "switchread", 0);
+ error = tsleep_nsec(sc, (PZERO + 1)|PCATCH, "switchread",
INFSLP);
if (error != 0)
goto failed;
/* sc might be deleted while sleeping */
tsleep_nsec(9) for revarp
The hz/2 idiom means 500ms, so use that. ok?
Index: netinet/if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.240
diff -u -p -r1.240 if_ether.c
--- netinet/if_ether.c 17 Jul 2019 16:46:18 - 1.240
+++ netinet/if_ether.c 7 Oct 2019 12:49:44 -
@@ -914,7 +914,7 @@ revarpwhoarewe(struct ifnet *ifp, struct
revarp_ifidx = ifp->if_index;
while (count--) {
revarprequest(ifp);
- result = tsleep((caddr_t)_myip, PSOCK, "revarp", hz/2);
+ result = tsleep_nsec(_myip, PSOCK, "revarp",
MSEC_TO_NSEC(500));
if (result != EWOULDBLOCK)
break;
}
Re: acpithinkpad: don't take over ws_[gs]et_param on version 2 devices
On Mon, Oct 14, 2019 at 11:00:57PM +0200, Mark Kettenis wrote: > > Date: Sun, 13 Oct 2019 21:37:53 -0500 > > From: joshua stein > > > > Newer ThinkPads have ACPI goo to allow acpivout to control screen > > backlight, so don't take over ws_[gs]et_param from it. This allows > > for 100 levels of backlight control rather than the 10 or 15 that > > are supported through acpithinkpad using its proprietary ACPI or > > CMOS interfaces. > > > > You can see the difference with and without this patch by doing: > > > > xbacklight -set 1 -steps 100 > > xbacklight -set 100 -steps 100 > > > > Apparently this will also be needed for newer AMD ThinkPads that use > > radeondrm. > > > > "Newer" here is being defined as anything not reporting version 1 > > (THINKPAD_HKEY_VERSION1) of the ThinkPad ACPI interface. > > Note that -current reports the version number in dmesg. For now > you'll need to compile your own kernel though, since there won't be > any new snapshots until 6.6 is released. > > > For responding to hardware brightness keys, you'll want to test with > > the acpivout patch I posted since otherwise the keys will be > > adjusting the backlight by 1% each time, and it may seem like it's > > not doing anything. That patch makes it properly adjust by 5% each > > time (but you still get fine-grained changes through wsconsctl or > > xbacklight). > > Wo what we need to get tested is whether backlight control (stil) > works on machines that report: > > acpithinkpad0 at acpi0: version 2.0 > > My x1c3 reports version 1.0 so I suspect the x250/t450/t550 reports > that as well (but it would be goo to get that confirmed). So we're > primarily interested in later generations. My x280 reports version 2.0 and this feels like a great improvement.
