Give support to the 2019 Apple's MBP (13-inch)
Hello, I tried to install stable flavor from USB on the MBP 13-inch 2019, with no luck, and the following issues: 1. Booting kernel messages Centered on the screen, with a very small size, not working well for reading. 2. After booting, keyboard not working I had to connect an USB KB. 3. Couldn't install on disk I can't ensure I installed the OS on the SSD because I got some errors when choosing the to install device. But the font was very small... Questions 1. Does anybody tried this before? 2. Does anyone needs to help on the development? 3. Where can I start reading the kernel to fix the video issues? Thanks!
Re: userland clock_gettime proof of concept
Christian Weisgerber wrote: > Paul Irofti: > > > This iteration of the diff adds bounds checking for tk_user and moves > > the usertc.c stub to every arch in libc as recommanded by deraadt@. > > It also fixes a gettimeofday issue reported by cheloha@ and tb@. > > Additionally, it changes struct timekeep in an incompatible way. ;-) > A userland built before the addition of tk_nclocks is very unhappy > with a kernel built afterwards. There is no way to compile across > this. You have to (U)pgrade from boot media to install a ftp.openbsd.org > userland, and then you can re-compile with the new diff. > > Should we already bump major while the diff matures? See, I told everyone this shouldn't be commited, and then iterated in-tree! Imagine if this was in-tree. Such compatibility is a nightmare. I'd say the easiest way is to backtrack to a snapshot, then forward again. I want to see this diff support 3-4 architectures before commit.
Re: userland clock_gettime proof of concept
Paul Irofti: > This iteration of the diff adds bounds checking for tk_user and moves > the usertc.c stub to every arch in libc as recommanded by deraadt@. > It also fixes a gettimeofday issue reported by cheloha@ and tb@. Additionally, it changes struct timekeep in an incompatible way. ;-) A userland built before the addition of tk_nclocks is very unhappy with a kernel built afterwards. There is no way to compile across this. You have to (U)pgrade from boot media to install a ftp.openbsd.org userland, and then you can re-compile with the new diff. Should we already bump major while the diff matures? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: netstat -R: list rdomains with associated ifs and tables
It's useful information, I like it. (I preferred it with the route count, but I agree, it's hard on the system if there's a full DFZ table). One thing though - > twister ..in/netstat$ obj/netstat -R > Rdomain 0 > Interfaces: lo0 iwm0 re0 enc0 pflog0 > Routing tables: 0 6 7 77 When there are multiple tables it's clear that this is a list of table numbers, but when there's only one the output text is confusing: Rdomain 0 Interfaces: lo0 em1 enc0 tun2 vlan1 pflog0 Routing tables: 0 "there are zero routing tables?" Rdomain 100 Interfaces: vether100 lo100 vether101 [...] Routing tables: 100 "there are 100 tables?" This would be clearer if it used table/tables as appropriate e.g. Routing table: 0 Routing table: 100 Routing tables: 0 6 7 77 the code to handle this gets messy though, maybe someone can think of alternative wording that gets around this another way..
Re: userland clock_gettime proof of concept
Paul Irofti:
> > This iteration of the diff adds bounds checking for tk_user and moves
> > the usertc.c stub to every arch in libc as recommanded by deraadt@.
> > It also fixes a gettimeofday issue reported by cheloha@ and tb@.
>
> Forgot to add armv7 tk_nclock entries. Noticed by benno@, thanks!
One blemish I see is that tk_user is a magic number.
For example, sparc64 will have two timecounters: tick and stick.
They will be assigned magic numbers 1 and 2...
struct timecounter tick_timecounter = {
tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, 1
};
struct timecounter stick_timecounter = {
stick_get_timecount, NULL, ~0u, 0, "stick", 1000, NULL, 2
};
... and sparc64 usertc.c will need the corresponding magic array order:
static uint64_t (*get_tc[])(void) =
{
rdtick,
rdstick,
};
I don't know if we want to go through the effort to make this
prettier. We would need an MD header, say,
that gets picked up by , with something like
#define TC_TICK 1
#define TC_STICK2
The symbolic values could then be used in the kernel timecounter
definitions...
struct timecounter tick_timecounter = {
tick_get_timecount, NULL, ~0u, 0, "tick", 0, NULL, TC_TICK
};
struct timecounter stick_timecounter = {
stick_get_timecount, NULL, ~0u, 0, "stick", 1000, NULL, TC_STICK
};
... and in libc usertc.c:
static uint64_t (*get_tc[])(void) =
{
[TC_TICK] = rdtick,
[TC_STICK] = rdstick,
};
...
*tc = (*get_tc[tk_user])();
The cost would be yet another header file per arch.
Thoughts?
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: netstat -R: list rdomains with associated ifs and tables
Remi Locherer(remi.loche...@relo.ch) on 2020.06.10 22:16:36 +0200:
> On Tue, Jun 09, 2020 at 10:02:06AM +0200, Remi Locherer wrote:
> > On Tue, Jun 09, 2020 at 09:17:31AM +0200, Claudio Jeker wrote:
> > > On Tue, Jun 09, 2020 at 08:44:42AM +0200, Remi Locherer wrote:
> > > > On Mon, Jun 08, 2020 at 10:10:17PM +0200, Remi Locherer wrote:
> > > > > Hi,
> > > > >
> > > > > to my knowledge there is no easy way to list all active rdomains or
> > > > > routing tables. Other platforms have "show vrf" or similar commands
> > > > > for an overview.
> > > > >
> > > > > Here is my attempt at such a view for OpenBSD:
> > > >
> > > > Updated diff with small changes:
> > > > - Print inet instead of Internet (input deraadt)
> > > > - Removed padding before rdomain id.
> > > > - Changed man page wording.
> > > >
> > > > twister ..in/netstat$ obj/netstat -R
> > > > Rdomain 0
> > > > Interfaces: lo0 iwm0 re0 enc0 pflog0 mpe0
> > > > Routing tables:
> > > > 0: inet 8, inet6 45, mpls 1
> > > > 3: inet 1, inet6 0, mpls 0
> > > > 7: inet 130309, inet6 1, mpls 0
> > > >
> > > > Rdomain 77
> > > > Interfaces: vether77 lo77
> > > > Routing tables:
> > > > 77: inet 0, inet6 0, mpls 0
> > > >
> > > > Rdomain 122
> > > > Interfaces: vether122 lo122 pair122 vether1122 vether1123 vether1124
> > > > vether1125 vether1126 vether1127
> > > > Routing tables:
> > > > 122: inet 24, inet6 0, mpls 0
> > > >
> > > > Rdomain 255
> > > > Interfaces: vether255 lo255
> > > > Routing tables:
> > > > 255: inet 3, inet6 0, mpls 0
> > > >
> > > > twister ..in/netstat$
> > > >
> > > > OK?
> > >
> > > Why do you think the route counts are needed? You fetch all routing tables
> > > to count them in userland. The sysctl for doing that is expensive and
> > > especially on systems with full tables will make this command slow.
> > > If this is something we really want then the kernel should track and
> > > provide the count.
> >
> > These counters are of interest for operators. But I agree that counting
> > the routes in userland is unfortunate. But I don't know how bad it is.
> > Is a lock involved in the kernel when dumping the full table?
>
> I did some homework and figured out, that dumping a routing table takes the
> NET_LOCK. So it's not just inefficient counting all routes in userland it
> might have a negative impact on the system.
>
> Below my new proposal without the counters. I still think it would be good
> to have those counters. Maybe I'll try to find a solution for that.
Maybe sysctl NET_RT_STATS and struct rtstat could be expanded to cover this?
As for the diff, ok benno@. The number of routes is interesting but can be
added later.
/B.
> twister ..in/netstat$ obj/netstat -R
> Rdomain 0
> Interfaces: lo0 iwm0 re0 enc0 pflog0
> Routing tables: 0 6 7 77
>
> Rdomain 100
> Interfaces: vether100 lo100 vether101 vether102 vether103 vether104
> vether105 vether106 vether107 vether108 vether109
> Routing tables: 100
>
> Rdomain 255
> Interfaces: vether255 lo255
> Routing tables: 255
>
> twister ..in/netstat$
>
>
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/netstat/main.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 main.c
> --- main.c28 Apr 2019 17:59:51 - 1.116
> +++ main.c30 May 2020 17:59:33 -
> @@ -127,7 +127,7 @@ main(int argc, char *argv[])
> tableid = getrtable();
>
> while ((ch = getopt(argc, argv,
> - "AaBbc:deFf:ghI:iLlM:mN:np:P:qrsT:tuvW:w:")) != -1)
> + "AaBbc:deFf:ghI:iLlM:mN:np:P:qRrsT:tuvW:w:")) != -1)
> switch (ch) {
> case 'A':
> Aflag = 1;
> @@ -225,6 +225,9 @@ main(int argc, char *argv[])
> case 'q':
> qflag = 1;
> break;
> + case 'R':
> + Rflag = 1;
> + break;
> case 'r':
> rflag = 1;
> break;
> @@ -318,6 +321,11 @@ main(int argc, char *argv[])
> mroutepr();
> if (af == AF_INET6 || af == AF_UNSPEC)
> mroute6pr();
> + exit(0);
> + }
> +
> + if (Rflag) {
> + rdomainpr();
> exit(0);
> }
>
> Index: netstat.1
> ===
> RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
> retrieving revision 1.86
> diff -u -p -r1.86 netstat.1
> --- netstat.1 17 Apr 2019 20:34:21 - 1.86
> +++ netstat.1 8 Jun 2020 20:42:46 -
> @@ -74,6 +74,8 @@
> .Op Fl i | I Ar interface
> .Nm
> .Op Fl W Ar interface
> +.Nm
> +.Op Fl R
> .Sh DESCRIPTION
> The
> .Nm
> @@ -267,6 +269,8 @@ Otherwise the states of the matching soc
> Only show interfaces that have
Re: Call FRELE() without fdplock in dup*(2)
> On 10 Jun 2020, at 17:34, Visa Hankala wrote: > > A while ago, finishdup() was changed to release fdplock before calling > closef() to avoid potential lock order problems in the file close path. > However, the dup* code can still invoke that path with fdplock held > through FRELE(). That is fixed by the diff below. > > (In principle, the FRELE(fp, p) could be eliminated by getting fp while > the file descriptor table is locked. As long as the lock is held, other > threads are unable to close the file descriptor and hence the temporary > file reference is unnecessary.) > > OK? I agreed with your diff.
Re: netstat -R: list rdomains with associated ifs and tables
On Tue, Jun 09, 2020 at 10:02:06AM +0200, Remi Locherer wrote:
> On Tue, Jun 09, 2020 at 09:17:31AM +0200, Claudio Jeker wrote:
> > On Tue, Jun 09, 2020 at 08:44:42AM +0200, Remi Locherer wrote:
> > > On Mon, Jun 08, 2020 at 10:10:17PM +0200, Remi Locherer wrote:
> > > > Hi,
> > > >
> > > > to my knowledge there is no easy way to list all active rdomains or
> > > > routing tables. Other platforms have "show vrf" or similar commands
> > > > for an overview.
> > > >
> > > > Here is my attempt at such a view for OpenBSD:
> > >
> > > Updated diff with small changes:
> > > - Print inet instead of Internet (input deraadt)
> > > - Removed padding before rdomain id.
> > > - Changed man page wording.
> > >
> > > twister ..in/netstat$ obj/netstat -R
> > > Rdomain 0
> > > Interfaces: lo0 iwm0 re0 enc0 pflog0 mpe0
> > > Routing tables:
> > > 0: inet 8, inet6 45, mpls 1
> > > 3: inet 1, inet6 0, mpls 0
> > > 7: inet 130309, inet6 1, mpls 0
> > >
> > > Rdomain 77
> > > Interfaces: vether77 lo77
> > > Routing tables:
> > > 77: inet 0, inet6 0, mpls 0
> > >
> > > Rdomain 122
> > > Interfaces: vether122 lo122 pair122 vether1122 vether1123 vether1124
> > > vether1125 vether1126 vether1127
> > > Routing tables:
> > > 122: inet 24, inet6 0, mpls 0
> > >
> > > Rdomain 255
> > > Interfaces: vether255 lo255
> > > Routing tables:
> > > 255: inet 3, inet6 0, mpls 0
> > >
> > > twister ..in/netstat$
> > >
> > > OK?
> >
> > Why do you think the route counts are needed? You fetch all routing tables
> > to count them in userland. The sysctl for doing that is expensive and
> > especially on systems with full tables will make this command slow.
> > If this is something we really want then the kernel should track and
> > provide the count.
>
> These counters are of interest for operators. But I agree that counting
> the routes in userland is unfortunate. But I don't know how bad it is.
> Is a lock involved in the kernel when dumping the full table?
I did some homework and figured out, that dumping a routing table takes the
NET_LOCK. So it's not just inefficient counting all routes in userland it
might have a negative impact on the system.
Below my new proposal without the counters. I still think it would be good
to have those counters. Maybe I'll try to find a solution for that.
twister ..in/netstat$ obj/netstat -R
Rdomain 0
Interfaces: lo0 iwm0 re0 enc0 pflog0
Routing tables: 0 6 7 77
Rdomain 100
Interfaces: vether100 lo100 vether101 vether102 vether103 vether104 vether105
vether106 vether107 vether108 vether109
Routing tables: 100
Rdomain 255
Interfaces: vether255 lo255
Routing tables: 255
twister ..in/netstat$
Index: main.c
===
RCS file: /cvs/src/usr.bin/netstat/main.c,v
retrieving revision 1.116
diff -u -p -r1.116 main.c
--- main.c 28 Apr 2019 17:59:51 - 1.116
+++ main.c 30 May 2020 17:59:33 -
@@ -127,7 +127,7 @@ main(int argc, char *argv[])
tableid = getrtable();
while ((ch = getopt(argc, argv,
- "AaBbc:deFf:ghI:iLlM:mN:np:P:qrsT:tuvW:w:")) != -1)
+ "AaBbc:deFf:ghI:iLlM:mN:np:P:qRrsT:tuvW:w:")) != -1)
switch (ch) {
case 'A':
Aflag = 1;
@@ -225,6 +225,9 @@ main(int argc, char *argv[])
case 'q':
qflag = 1;
break;
+ case 'R':
+ Rflag = 1;
+ break;
case 'r':
rflag = 1;
break;
@@ -318,6 +321,11 @@ main(int argc, char *argv[])
mroutepr();
if (af == AF_INET6 || af == AF_UNSPEC)
mroute6pr();
+ exit(0);
+ }
+
+ if (Rflag) {
+ rdomainpr();
exit(0);
}
Index: netstat.1
===
RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
retrieving revision 1.86
diff -u -p -r1.86 netstat.1
--- netstat.1 17 Apr 2019 20:34:21 - 1.86
+++ netstat.1 8 Jun 2020 20:42:46 -
@@ -74,6 +74,8 @@
.Op Fl i | I Ar interface
.Nm
.Op Fl W Ar interface
+.Nm
+.Op Fl R
.Sh DESCRIPTION
The
.Nm
@@ -267,6 +269,8 @@ Otherwise the states of the matching soc
Only show interfaces that have seen packets (or bytes if
.Fl b
is specified).
+.It Fl R
+List all rdomains with associated interfaces and routing tables.
.It Fl r
Show the routing tables.
The output is explained in more detail below.
Index: netstat.h
===
RCS file: /cvs/src/usr.bin/netstat/netstat.h,v
retrieving revision 1.74
diff -u -p -r1.74 netstat.h
--- netstat.h 28 Apr 2019 17:59:51 - 1.74
+++ netst
Re: libkern: ffs() for arm64, powerpc, powerpc64
> Date: Wed, 10 Jun 2020 20:08:31 +0200 > From: Christian Weisgerber > > Next try. > Optimized versions for kernel ffs(3) on arm64, powerpc, powerpc64. > > I have tested arm64; cwen@ has tested powerpc in userland. > powerpc64 is copied from powerpc. > > ok? ok kettenis@ > Index: lib/libkern/arch/arm64/ffs.S > === > RCS file: lib/libkern/arch/arm64/ffs.S > diff -N lib/libkern/arch/arm64/ffs.S > --- /dev/null 1 Jan 1970 00:00:00 - > +++ lib/libkern/arch/arm64/ffs.S 10 Jun 2020 17:38:50 - > @@ -0,0 +1,17 @@ > +/* $OpenBSD$ */ > +/* > + * Written by Christian Weisgerber . > + * Public domain. > + */ > + > +#include > + > +ENTRY(ffs) > + RETGUARD_SETUP(ffs, x15) > + rbitw1, w0 > + clz w1, w1 > + cmp w0, wzr > + csinc w0, wzr, w1, eq > + RETGUARD_CHECK(ffs, x15) > + ret > +END(ffs) > Index: lib/libkern/arch/powerpc/ffs.S > === > RCS file: lib/libkern/arch/powerpc/ffs.S > diff -N lib/libkern/arch/powerpc/ffs.S > --- /dev/null 1 Jan 1970 00:00:00 - > +++ lib/libkern/arch/powerpc/ffs.S10 Jun 2020 17:39:02 - > @@ -0,0 +1,15 @@ > +/* $OpenBSD$ */ > +/* > + * Written by Christian Weisgerber . > + * Public domain. > + */ > + > +#include > + > +ENTRY(ffs) > + neg %r4, %r3 > + and %r3, %r3, %r4 > + cntlzw %r3, %r3 > + subfic %r3, %r3, 32 > + blr > +END(ffs) > Index: lib/libkern/arch/powerpc64/ffs.S > === > RCS file: lib/libkern/arch/powerpc64/ffs.S > diff -N lib/libkern/arch/powerpc64/ffs.S > --- /dev/null 1 Jan 1970 00:00:00 - > +++ lib/libkern/arch/powerpc64/ffs.S 10 Jun 2020 17:39:06 - > @@ -0,0 +1,15 @@ > +/* $OpenBSD$ */ > +/* > + * Written by Christian Weisgerber . > + * Public domain. > + */ > + > +#include > + > +ENTRY(ffs) > + neg %r4, %r3 > + and %r3, %r3, %r4 > + cntlzw %r3, %r3 > + subfic %r3, %r3, 32 > + blr > +END(ffs) > -- > Christian "naddy" Weisgerber na...@mips.inka.de > >
Re: Call FRELE() without fdplock in dup*(2)
On Wed, Jun 10, 2020 at 02:34:00PM +, Visa Hankala wrote: > A while ago, finishdup() was changed to release fdplock before calling > closef() to avoid potential lock order problems in the file close path. > However, the dup* code can still invoke that path with fdplock held > through FRELE(). That is fixed by the diff below. > > (In principle, the FRELE(fp, p) could be eliminated by getting fp while > the file descriptor table is locked. As long as the lock is held, other > threads are unable to close the file descriptor and hence the temporary > file reference is unnecessary.) > > OK? ok anton@
Re: libkern: ffs() for arm64, powerpc, powerpc64
Next try. Optimized versions for kernel ffs(3) on arm64, powerpc, powerpc64. I have tested arm64; cwen@ has tested powerpc in userland. powerpc64 is copied from powerpc. ok? Index: lib/libkern/arch/arm64/ffs.S === RCS file: lib/libkern/arch/arm64/ffs.S diff -N lib/libkern/arch/arm64/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/arm64/ffs.S10 Jun 2020 17:38:50 - @@ -0,0 +1,17 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + RETGUARD_SETUP(ffs, x15) + rbitw1, w0 + clz w1, w1 + cmp w0, wzr + csinc w0, wzr, w1, eq + RETGUARD_CHECK(ffs, x15) + ret +END(ffs) Index: lib/libkern/arch/powerpc/ffs.S === RCS file: lib/libkern/arch/powerpc/ffs.S diff -N lib/libkern/arch/powerpc/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/powerpc/ffs.S 10 Jun 2020 17:39:02 - @@ -0,0 +1,15 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END(ffs) Index: lib/libkern/arch/powerpc64/ffs.S === RCS file: lib/libkern/arch/powerpc64/ffs.S diff -N lib/libkern/arch/powerpc64/ffs.S --- /dev/null 1 Jan 1970 00:00:00 - +++ lib/libkern/arch/powerpc64/ffs.S10 Jun 2020 17:39:06 - @@ -0,0 +1,15 @@ +/* $OpenBSD$ */ +/* + * Written by Christian Weisgerber . + * Public domain. + */ + +#include + +ENTRY(ffs) + neg %r4, %r3 + and %r3, %r3, %r4 + cntlzw %r3, %r3 + subfic %r3, %r3, 32 + blr +END(ffs) -- Christian "naddy" Weisgerber na...@mips.inka.de
Call FRELE() without fdplock in dup*(2)
A while ago, finishdup() was changed to release fdplock before calling
closef() to avoid potential lock order problems in the file close path.
However, the dup* code can still invoke that path with fdplock held
through FRELE(). That is fixed by the diff below.
(In principle, the FRELE(fp, p) could be eliminated by getting fp while
the file descriptor table is locked. As long as the lock is held, other
threads are unable to close the file descriptor and hence the temporary
file reference is unnecessary.)
OK?
Index: kern/kern_descrip.c
===
RCS file: src/sys/kern/kern_descrip.c,v
retrieving revision 1.201
diff -u -p -r1.201 kern_descrip.c
--- kern/kern_descrip.c 13 Mar 2020 10:07:01 - 1.201
+++ kern/kern_descrip.c 10 Jun 2020 14:16:12 -
@@ -301,13 +301,14 @@ restart:
return (EBADF);
fdplock(fdp);
if ((error = fdalloc(p, 0, &new)) != 0) {
- FRELE(fp, p);
if (error == ENOSPC) {
fdexpand(p);
fdpunlock(fdp);
+ FRELE(fp, p);
goto restart;
}
fdpunlock(fdp);
+ FRELE(fp, p);
return (error);
}
/* No need for FRELE(), finishdup() uses current ref. */
@@ -373,13 +374,14 @@ restart:
fdplock(fdp);
if (new >= fdp->fd_nfiles) {
if ((error = fdalloc(p, new, &i)) != 0) {
- FRELE(fp, p);
if (error == ENOSPC) {
fdexpand(p);
fdpunlock(fdp);
+ FRELE(fp, p);
goto restart;
}
fdpunlock(fdp);
+ FRELE(fp, p);
return (error);
}
if (new != i)
@@ -433,13 +435,14 @@ restart:
}
fdplock(fdp);
if ((error = fdalloc(p, newmin, &i)) != 0) {
- FRELE(fp, p);
if (error == ENOSPC) {
fdexpand(p);
fdpunlock(fdp);
+ FRELE(fp, p);
goto restart;
}
fdpunlock(fdp);
+ FRELE(fp, p);
} else {
int dupflags = 0;
Re: code style questions
On Wed, Jun 10, 2020 at 05:38:36PM +1000, Jonathan Gray wrote: > On Wed, Jun 10, 2020 at 09:19:47AM +0200, Martin Pieuchot wrote: > > On 09/06/20(Tue) 20:19, jo...@armadilloaerospace.com wrote: > > > Looking for some guidance to avoid proposing any unpopular diffs. > > > > > > Style(9) says not to use static on file-local functions in the > > > kernel, because it interferes with the debugger. They still show up > > > on some functions today; is this still an issue? > > > > I don't know if it's an issue, but not using 'static' is still a > > convention. > > It is. Backtraces in drm code aren't as useful as they could be as > static is extensively used. > > One annoying thing in C is that keywords are grossly overloaded. If static and static were two different keywords, you could just #define static and be done with it. (are there any static variables in the drm code ?...)
iwx: increase command queue size
Increase iwx(4) firmware command queue size.
Otherwise, the firmware will eventually send spurious "command done"
notifications for commands which were completed successfully earlier
(which looks like a ring overflow) and then crash with a fatal error.
This is required to get -48 firmware to work without fatal firmware errors.
ok?
diff 593cf89bed2f9f27ccf55d4cebf8aa65c36c7bb1
047e3a7211742260b7a9097053380f87b72b3eae
blob - d6c823fe64ce7509e69c5b4ec20fdfa8681cfb2d
blob + 7781f264940a644deff624a05a3b9ce2b65d
--- sys/dev/pci/if_iwxreg.h
+++ sys/dev/pci/if_iwxreg.h
@@ -1402,7 +1402,7 @@ enum iwx_gen2_tx_fifo {
#define IWX_TX_QUEUE_CFG_TFD_SHORT_FORMAT (1 << 1)
#define IWX_DEFAULT_QUEUE_SIZE IWX_TFD_QUEUE_SIZE_MAX
-#define IWX_CMD_QUEUE_SIZE 32
+#define IWX_CMD_QUEUE_SIZE 64
/**
* struct iwx_tx_queue_cfg_cmd - txq hw scheduler config command
iwx: do not load the firmware twice
Firmware loading code inherited from iwm(4) loads firmware twice:
Once to load the 'INIT' (boot) device firmware, and a second time
to load the RT (run-time) device firmware. Both of these firmware
images are part of the single firmware file in /etc/firmware.
With iwx(4) devices only a single load is required. Double-loading the
firmware seems to be the reason for various fatal firmware errors.
The older -46 firmware we currently use works to some extent anyway,
which is why this problem wasn't obvious at all and took quite a
while to track down.
The -48 version of the firmware was extremely unhappy and just went
into fatal firmware error mode as soon as the driver tried to send
a frame. I have that firmware version working locally now with this
change and some additional fixes.
With this change regulatory domain updates start appearing on both
firmware versions. Which suggests that we never actually had the
firmware running in a fully operational state before.
I have already sent another patch which adds support for these
regulatory domain notifications.
ok?
diff 3387c0fc20f29323f58bb3abae4a685d2cca4969
593cf89bed2f9f27ccf55d4cebf8aa65c36c7bb1
blob - eaad5ad73abbd1d25eb5895de88cf48e731abfcc
blob + bb81a869c36b13d7bf5cb7c037a4c023d9baf6d0
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -6547,20 +6547,6 @@ iwx_init_hw(struct iwx_softc *sc)
if (err)
return err;
- /* Should stop and start HW since INIT image just loaded. */
- iwx_stop_device(sc);
- err = iwx_start_hw(sc);
- if (err) {
- printf("%s: could not initialize hardware\n", DEVNAME(sc));
- return err;
- }
-
- err = iwx_load_ucode_wait_alive(sc);
- if (err) {
- printf("%s: could not load firmware\n", DEVNAME(sc));
- goto err;
- }
-
if (!iwx_nic_lock(sc))
return EBUSY;
iwx: handle regulatory domain updates
(This patch depends on the iwx NVM parser patch which I just sent
out previously. This patch won't apply otherwise.)
iwx(4) devices have a hardware component that can detect which country
it is running in. The firmware will then inform the driver about the
auto-detected regulatory domain.
Currently we don't see this notification due to a bug in our driver.
I have a fix for that other bug which I will send out separately.
If we do not handle the regulatory domain notification, this other
fix would result in messages like:
iwx0: unhandled firmware response 0xc9/0x2008 rx ring 64[35]
For now, just print a message which shows the detected regulatory domain
if 'ifconfig iwx0 debug' is enabled. It is up to the driver whether it
wants to react to regulatory domain updates.
ok?
diff e883cbeb57aae26539e3cf9c73701c2e0d44bb50
3387c0fc20f29323f58bb3abae4a685d2cca4969
blob - f99f211301ac2da8cc350549e9fc029852c971fb
blob + eaad5ad73abbd1d25eb5895de88cf48e731abfcc
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -392,6 +392,7 @@ int iwx_rm_sta_cmd(struct iwx_softc *, struct iwx_node
intiwx_fill_probe_req(struct iwx_softc *, struct iwx_scan_probe_req *);
intiwx_config_umac_scan(struct iwx_softc *);
intiwx_umac_scan(struct iwx_softc *, int);
+void iwx_mcc_update(struct iwx_softc *, struct iwx_mcc_chub_notif *);
uint8_tiwx_ridx2rate(struct ieee80211_rateset *, int);
intiwx_rval2ridx(int);
void iwx_ack_rates(struct iwx_softc *, struct iwx_node *, int *, int *);
@@ -2709,12 +2710,15 @@ iwx_init_channel_map(struct iwx_softc *sc, uint16_t *c
if (is_5ghz && !data->sku_cap_band_52GHz_enable)
ch_flags &= ~IWX_NVM_CHANNEL_VALID;
- if (!(ch_flags & IWX_NVM_CHANNEL_VALID))
- continue;
-
hw_value = nvm_channels[ch_idx];
channel = &ic->ic_channels[hw_value];
+ if (!(ch_flags & IWX_NVM_CHANNEL_VALID)) {
+ channel->ic_freq = 0;
+ channel->ic_flags = 0;
+ continue;
+ }
+
if (!is_5ghz) {
flags = IEEE80211_CHAN_2GHZ;
channel->ic_flags
@@ -5245,6 +5249,24 @@ iwx_umac_scan(struct iwx_softc *sc, int bgscan)
return err;
}
+void
+iwx_mcc_update(struct iwx_softc *sc, struct iwx_mcc_chub_notif *notif)
+{
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ifnet *ifp = IC2IFP(ic);
+ char alpha2[3];
+
+ snprintf(alpha2, sizeof(alpha2), "%c%c",
+ (le16toh(notif->mcc) & 0xff00) >> 8, le16toh(notif->mcc) & 0xff);
+
+ if (ifp->if_flags & IFF_DEBUG) {
+ printf("%s: firmware has detected regulatory domain '%s' "
+ "(0x%x)\n", DEVNAME(sc), alpha2, le16toh(notif->mcc));
+ }
+
+ /* TODO: Schedule a task to send MCC_UPDATE_CMD? */
+}
+
uint8_t
iwx_ridx2rate(struct ieee80211_rateset *rs, int ridx)
{
@@ -6454,6 +6476,9 @@ iwx_send_update_mcc_cmd(struct iwx_softc *sc, const ch
.flags = IWX_CMD_WANT_RESP,
.data = { &mcc_cmd },
};
+ struct iwx_rx_packet *pkt;
+ struct iwx_mcc_update_resp *resp;
+ size_t resp_len;
int err;
memset(&mcc_cmd, 0, sizeof(mcc_cmd));
@@ -6465,16 +6490,41 @@ iwx_send_update_mcc_cmd(struct iwx_softc *sc, const ch
mcc_cmd.source_id = IWX_MCC_SOURCE_OLD_FW;
hcmd.len[0] = sizeof(struct iwx_mcc_update_cmd);
- hcmd.resp_pkt_len = sizeof(struct iwx_rx_packet) +
- sizeof(struct iwx_mcc_update_resp);
+ hcmd.resp_pkt_len = IWX_CMD_RESP_MAX;
err = iwx_send_cmd(sc, &hcmd);
if (err)
return err;
+ pkt = hcmd.resp_pkt;
+ if (!pkt || (pkt->hdr.flags & IWX_CMD_FAILED_MSK)) {
+ err = EIO;
+ goto out;
+ }
+
+ resp_len = iwx_rx_packet_payload_len(pkt);
+ if (resp_len < sizeof(*resp)) {
+ err = EIO;
+ goto out;
+ }
+
+ resp = (void *)pkt->data;
+ if (resp_len != sizeof(*resp) +
+ resp->n_channels * sizeof(resp->channels[0])) {
+ err = EIO;
+ goto out;
+ }
+
+ DPRINTF(("MCC status=0x%x mcc=0x%x cap=0x%x time=0x%x geo_info=0x%x
source_id=0x%d n_channels=%u\n",
+ resp->status, resp->mcc, resp->cap, resp->time, resp->geo_info,
resp->source_id, resp->n_channels));
+
+ /* Update channel map for net80211 and our scan configuration. */
+ iwx_init_channel_map(sc, NULL, resp->channels, resp->n_channels);
+
+out:
iwx_free_resp(sc, &hcmd);
- return 0;
+ return err;
}
int
@@ -7365,6 +7415,13 @@ iwx_rx_pkt(struct iwx_softc *sc, struct iwx_rx_data *d
break;
}
+ case IWX_MCC_CHUB_UPDATE_CMD: {
+ struct iwx_mcc_chub_notif *notif;
+
iwx: stop reading NVM directly
The iwx(4) firmware offers a command which reads data stored in non-volative
memory of the device. Currently the driver still parses this data directly
with a parser inherited from iwm(4). Use the command instead, which matches
what Linux does on this hardware and is future-proof.
The old parser is disabled with #if 0 in this diff. The diff becomes unreadable
if the old code gets removed at the same time. I can remove the old code later.
ok?
diff f1d28b72d0b54d89dd3bb820cb5c91a8709d1640
e883cbeb57aae26539e3cf9c73701c2e0d44bb50
blob - 2a770aa05771214b055de5c570fa155b9cb39433
blob + f99f211301ac2da8cc350549e9fc029852c971fb
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -160,6 +160,20 @@ const uint8_t iwx_nvm_channels_8000[] = {
149, 153, 157, 161, 165, 169, 173, 177, 181
};
+static const uint8_t iwx_nvm_channels_uhb[] = {
+ /* 2.4 GHz */
+ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14,
+ /* 5 GHz */
+ 36, 40, 44, 48, 52, 56, 60, 64, 68, 72, 76, 80, 84, 88, 92,
+ 96, 100, 104, 108, 112, 116, 120, 124, 128, 132, 136, 140, 144,
+ 149, 153, 157, 161, 165, 169, 173, 177, 181,
+ /* 6-7 GHz */
+ 1, 5, 9, 13, 17, 21, 25, 29, 33, 37, 41, 45, 49, 53, 57, 61, 65, 69,
+ 73, 77, 81, 85, 89, 93, 97, 101, 105, 109, 113, 117, 121, 125, 129,
+ 133, 137, 141, 145, 149, 153, 157, 161, 165, 169, 173, 177, 181, 185,
+ 189, 193, 197, 201, 205, 209, 213, 217, 221, 225, 229, 233
+};
+
#define IWX_NUM_2GHZ_CHANNELS 14
const struct iwx_rate {
@@ -291,8 +305,7 @@ int iwx_nvm_read_chunk(struct iwx_softc *, uint16_t, u
uint8_t *, uint16_t *);
intiwx_nvm_read_section(struct iwx_softc *, uint16_t, uint8_t *,
uint16_t *, size_t);
-void iwx_init_channel_map(struct iwx_softc *, const uint16_t * const,
- const uint8_t *nvm_channels, int nchan);
+void iwx_init_channel_map(struct iwx_softc *, uint16_t *, uint32_t *, int);
void iwx_setup_ht_rates(struct iwx_softc *);
intiwx_mimo_enabled(struct iwx_softc *);
void iwx_htprot_task(void *);
@@ -315,6 +328,9 @@ int iwx_parse_nvm_data(struct iwx_softc *, const uint1
const uint16_t *, const uint16_t *,
const uint16_t *, const uint16_t *,
const uint16_t *, int);
+intiwx_set_mac_addr_from_csr(struct iwx_softc *, struct iwx_nvm_data *);
+intiwx_is_valid_mac_addr(const uint8_t *);
+intiwx_nvm_get(struct iwx_softc *);
void iwx_set_hw_address_8000(struct iwx_softc *, struct iwx_nvm_data *,
const uint16_t *, const uint16_t *);
intiwx_parse_nvm_sections(struct iwx_softc *, struct iwx_nvm_section *);
@@ -2662,22 +2678,35 @@ iwx_fw_valid_rx_ant(struct iwx_softc *sc)
}
void
-iwx_init_channel_map(struct iwx_softc *sc, const uint16_t * const nvm_ch_flags,
-const uint8_t *nvm_channels, int nchan)
+iwx_init_channel_map(struct iwx_softc *sc, uint16_t *channel_profile_v3,
+uint32_t *channel_profile_v4, int nchan_profile)
{
struct ieee80211com *ic = &sc->sc_ic;
struct iwx_nvm_data *data = &sc->sc_nvm;
int ch_idx;
struct ieee80211_channel *channel;
- uint16_t ch_flags;
+ uint32_t ch_flags;
int is_5ghz;
int flags, hw_value;
+ int nchan;
+ const uint8_t *nvm_channels;
- for (ch_idx = 0; ch_idx < nchan; ch_idx++) {
- ch_flags = le16_to_cpup(nvm_ch_flags + ch_idx);
+ if (sc->sc_uhb_supported) {
+ nchan = nitems(iwx_nvm_channels_uhb);
+ nvm_channels = iwx_nvm_channels_uhb;
+ } else {
+ nchan = nitems(iwx_nvm_channels_8000);
+ nvm_channels = iwx_nvm_channels_8000;
+ }
- if (ch_idx >= IWX_NUM_2GHZ_CHANNELS &&
- !data->sku_cap_band_52GHz_enable)
+ for (ch_idx = 0; ch_idx < nchan && ch_idx < nchan_profile; ch_idx++) {
+ if (channel_profile_v4)
+ ch_flags = le32_to_cpup(channel_profile_v4 + ch_idx);
+ else
+ ch_flags = le16_to_cpup(channel_profile_v3 + ch_idx);
+
+ is_5ghz = ch_idx >= IWX_NUM_2GHZ_CHANNELS;
+ if (is_5ghz && !data->sku_cap_band_52GHz_enable)
ch_flags &= ~IWX_NVM_CHANNEL_VALID;
if (!(ch_flags & IWX_NVM_CHANNEL_VALID))
@@ -2686,7 +2715,6 @@ iwx_init_channel_map(struct iwx_softc *sc, const uint1
hw_value = nvm_channels[ch_idx];
channel = &ic->ic_channels[hw_value];
- is_5ghz = ch_idx >= IWX_NUM_2GHZ_CHANNELS;
if (!is_5ghz) {
flags = IEEE80211_CHAN_2GHZ;
channel->ic_flags
@@ -2889,6 +2917,134 @@ iwx_ampdu_rx_stop(struct ieee80211com *ic, struct ieee
iwx_add_task(sc, systq, &sc->ba_task);
}
+/* Read the mac address from WFMP registers. */
+int
+iwx_set_mac_addr_from_csr(struct iwx_softc *sc, struct iwx_nvm_data *data)
+{
+ cons
Re: code style questions
On Wed, Jun 10, 2020 at 09:19:47AM +0200, Martin Pieuchot wrote: > On 09/06/20(Tue) 20:19, jo...@armadilloaerospace.com wrote: > > Looking for some guidance to avoid proposing any unpopular diffs. > > > > Style(9) says not to use static on file-local functions in the > > kernel, because it interferes with the debugger. They still show up > > on some functions today; is this still an issue? > > I don't know if it's an issue, but not using 'static' is still a > convention. It is. Backtraces in drm code aren't as useful as they could be as static is extensively used.
Re: Diff for www:openssh/users.html
On Wed, Jun 10, 2020 at 3:23 AM wrote: > > Hi, > > Here a diff for www page: openssh/users.html > > Change URLs to those on archive.org (and others sites) > > Right? It says "The following operating systems and products are known to integrate OpenSSH into the base system.". If a system no longer exists, shouldn't it be removed from that list? I would not be impressed with a product if I found that half of their "recommendations" were dead projects just listed to fill out space.
Re: code style questions
On 09/06/20(Tue) 20:19, jo...@armadilloaerospace.com wrote:
> Looking for some guidance to avoid proposing any unpopular diffs.
>
> Style(9) says not to use static on file-local functions in the
> kernel, because it interferes with the debugger. They still show up
> on some functions today; is this still an issue?
I don't know if it's an issue, but not using 'static' is still a
convention.
> I usually advocate for directly inlining small functions that are only
> called from one place, because it removes any doubt about whether it
> is or ever should be called elsewhere. For instance, these functions:
>
> vaddr_t
> efifb_early_map(paddr_t pa)
> {
> return pmap_set_pml4_early(pa);
> }
>
> void
> efifb_early_cleanup(void)
> {
> pmap_clear_pml4_early();
> }
>
> Is that frowned on? I know I am on the longer-functions side of
> the spectrum.
It's a matter of taste/experience/amount of code read/etc. There's no
better way, all have pros and cons ;)
> Also, style(9) says that prototypes should not have variable names
> associated with the types. I try to use good names in the headers
> for documentation purposes; what is the thinking behind the rule?
There's a huge amount of code written this way. Changing this requires
effort and distracts from real changes. Trying to match the existing
style(9) helps when dealing with huge code base. Like everywhere
exceptions exist :)
Re: code style questions
On Tue, Jun 09, 2020 at 08:19:53PM -0700, jo...@armadilloaerospace.com wrote:
> Also, style(9) says that prototypes should not have variable names
> associated with the types. I try to use good names in the headers
> for documentation purposes; what is the thinking behind the rule?
function names are part of the API, variable names are not. Indeed they
are like comment, but deadly comments, because they are still part of
the namespace, hence susceptible to yield issues with macros.
In your example,
> int pcdisplay_copycols(void *id, int row, int srccol, int dstcol, int
> ncols);
> vs:
> int pcdisplay_copycols(void *, int, int, int,int);
>
first one won't compile if there is a
#define row (getenv("ROW"))
in effect.
Also, even with variable names, it's possible to mix-up parameters with
similar types.
That rule is somewhat controversial though.
I personally tend to prefer actually documenting what the function does in
a comment in the header, and comment how it does it in the source file.
