lsearch(3): implement with lfind(3)
lsearch(3) is just lfind(3) + append. If we write it that way we
shrink lsearch.c. The third function, linear_base(), is just added
complexity. The indirection buys us nothing.
I don't think we need to keep the historical comment, either.
lsearch(3) is pure computation, it does not set errno: The End.
The spec defines no errors, either.
libc housekeeping:
- lfind becomes PROTO_NORMAL because we're now using it internally.
- lfind then needs DEF_WEAK because it is not ISO C.
Unsure if I'm missing other libc housekeeping.
If not, ok?
Index: stdlib/lsearch.c
===
RCS file: /cvs/src/lib/libc/stdlib/lsearch.c,v
retrieving revision 1.6
diff -u -p -r1.6 lsearch.c
--- stdlib/lsearch.c7 Dec 2021 04:01:45 - 1.6
+++ stdlib/lsearch.c7 Dec 2021 04:32:34 -
@@ -37,53 +37,34 @@
#include
typedef int (*cmp_fn_t)(const void *, const void *);
-static void *linear_base(const void *, const void *, size_t *, size_t,
-cmp_fn_t, int);
void *
lsearch(const void *key, void *base, size_t *nelp, size_t width,
cmp_fn_t compar)
{
+ void *element = lfind(key, base, nelp, width, compar);
- return(linear_base(key, base, nelp, width, compar, 1));
+ /*
+* Use memmove(3) to ensure the key is copied cleanly into the
+* array, even if the key overlaps with the end of the array.
+*/
+ if (element == NULL) {
+ element = memmove((char *)base + *nelp * width, key, width);
+ *nelp += 1;
+ }
+ return element;
}
void *
lfind(const void *key, const void *base, size_t *nelp, size_t width,
cmp_fn_t compar)
{
- return(linear_base(key, base, nelp, width, compar, 0));
-}
-
-static void *
-linear_base(const void *key, const void *base, size_t *nelp, size_t width,
- cmp_fn_t compar, int add_flag)
-{
const char *element, *end;
end = (const char *)base + *nelp * width;
for (element = base; element < end; element += width)
if (!compar(key, element)) /* key found */
return((void *)element);
-
- if (!add_flag) /* key not found */
- return(NULL);
-
- /*
-* The UNIX System User's Manual, 1986 edition claims that
-* a NULL pointer is returned by lsearch with errno set
-* appropriately, if there is not enough room in the table
-* to add a new item. This can't be done as none of these
-* routines have any method of determining the size of the
-* table. This comment isn't in the 1986-87 System V
-* manual.
-*/
- ++*nelp;
-
- /*
-* Use memmove(3) to ensure the key is copied cleanly into the
-* array, even if the key overlaps with the end of the array.
-*/
- memmove((void *)end, key, width);
- return((void *)end);
+ return NULL;
}
+DEF_WEAK(lfind);
Index: hidden/search.h
===
RCS file: /cvs/src/lib/libc/hidden/search.h,v
retrieving revision 1.1
diff -u -p -r1.1 search.h
--- hidden/search.h 4 Oct 2015 08:36:57 - 1.1
+++ hidden/search.h 7 Dec 2021 04:32:34 -
@@ -24,7 +24,7 @@ PROTO_DEPRECATED(hcreate);
PROTO_DEPRECATED(hdestroy);
PROTO_DEPRECATED(hsearch);
PROTO_DEPRECATED(insque);
-PROTO_DEPRECATED(lfind);
+PROTO_NORMAL(lfind);
PROTO_DEPRECATED(lsearch);
PROTO_DEPRECATED(remque);
PROTO_DEPRECATED(tdelete);
Re: lsearch(3): memmove(3), not memcpy(3)
On Mon, 06 Dec 2021 19:29:55 -0600, Scott Cheloha wrote: > This? I think this is fine. OK millert@ - todd
Re: uvn_reference(): correct printf() argument order
On Mon, Dec 06, 2021 at 08:46:57PM -0600, Scott Cheloha wrote:
> The arguments to printf() here are backwards.
>
> ok?
ok jsg@
>
> Index: uvm_vnode.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 uvm_vnode.c
> --- uvm_vnode.c 23 Oct 2021 14:42:08 - 1.119
> +++ uvm_vnode.c 7 Dec 2021 02:45:09 -
> @@ -275,8 +275,8 @@ uvn_reference(struct uvm_object *uobj)
>
> #ifdef DEBUG
> if ((uvn->u_flags & UVM_VNODE_VALID) == 0) {
> - printf("uvn_reference: ref=%d, flags=0x%x\n", uvn->u_flags,
> - uobj->uo_refs);
> + printf("uvn_reference: ref=%d, flags=0x%x\n",
> + uobj->uo_refs, uvn->u_flags);
> panic("uvn_reference: invalid state");
> }
> #endif
>
>
uvn_reference(): correct printf() argument order
The arguments to printf() here are backwards.
ok?
Index: uvm_vnode.c
===
RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
retrieving revision 1.119
diff -u -p -r1.119 uvm_vnode.c
--- uvm_vnode.c 23 Oct 2021 14:42:08 - 1.119
+++ uvm_vnode.c 7 Dec 2021 02:45:09 -
@@ -275,8 +275,8 @@ uvn_reference(struct uvm_object *uobj)
#ifdef DEBUG
if ((uvn->u_flags & UVM_VNODE_VALID) == 0) {
- printf("uvn_reference: ref=%d, flags=0x%x\n", uvn->u_flags,
- uobj->uo_refs);
+ printf("uvn_reference: ref=%d, flags=0x%x\n",
+ uobj->uo_refs, uvn->u_flags);
panic("uvn_reference: invalid state");
}
#endif
Re: Rework UNIX sockets locking to be fine grained
Updated on top of latest source.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.269
diff -u -p -r1.269 uipc_socket.c
--- sys/kern/uipc_socket.c 11 Nov 2021 16:35:09 - 1.269
+++ sys/kern/uipc_socket.c 7 Dec 2021 01:40:10 -
@@ -52,6 +52,7 @@
#include
#include
#include
+#include
#ifdef DDB
#include
@@ -156,7 +157,9 @@ soalloc(int prflags)
so = pool_get(_pool, prflags);
if (so == NULL)
return (NULL);
- rw_init(>so_lock, "solock");
+ rw_init_flags(>so_lock, "solock", RWL_DUPOK);
+ refcnt_init(>so_refcnt);
+
return (so);
}
@@ -257,6 +260,8 @@ solisten(struct socket *so, int backlog)
void
sofree(struct socket *so, int s)
{
+ int persocket = solock_persocket(so);
+
soassertlocked(so);
if (so->so_pcb || (so->so_state & SS_NOFDREF) == 0) {
@@ -264,16 +269,53 @@ sofree(struct socket *so, int s)
return;
}
if (so->so_head) {
+ struct socket *head = so->so_head;
+
/*
* We must not decommission a socket that's on the accept(2)
* queue. If we do, then accept(2) may hang after select(2)
* indicated that the listening socket was ready.
*/
- if (!soqremque(so, 0)) {
+ if (so->so_onq == >so_q) {
sounlock(so, s);
return;
}
+
+ if (persocket) {
+ /*
+* Concurrent close of `head' could
+* abort `so' due to re-lock.
+*/
+ soref(so);
+ soref(head);
+ sounlock(so, SL_LOCKED);
+ solock(head);
+ solock(so);
+
+ if (so->so_onq != >so_q0) {
+ sounlock(head, SL_LOCKED);
+ sounlock(so, SL_LOCKED);
+ sorele(head);
+ sorele(so);
+ return;
+ }
+
+ sorele(head);
+ sorele(so);
+ }
+
+ soqremque(so, 0);
+
+ if (persocket)
+ sounlock(head, SL_LOCKED);
}
+
+ if (persocket) {
+ sounlock(so, SL_LOCKED);
+ refcnt_finalize(>so_refcnt, "sofinal");
+ solock(so);
+ }
+
sigio_free(>so_sigio);
klist_free(>so_rcv.sb_sel.si_note);
klist_free(>so_snd.sb_sel.si_note);
@@ -363,13 +405,36 @@ drop:
error = error2;
}
if (so->so_options & SO_ACCEPTCONN) {
+ int persocket = solock_persocket(so);
+
+ if (persocket) {
+ /* Wait concurrent sonewconn() threads. */
+ while (so->so_newconn > 0) {
+ so->so_state |= SS_NEWCONN_WAIT;
+ sosleep_nsec(so, >so_newconn, PSOCK,
+ "netlck", INFSLP);
+ }
+ }
+
while ((so2 = TAILQ_FIRST(>so_q0)) != NULL) {
+ if (persocket)
+ solock(so2);
(void) soqremque(so2, 0);
+ if (persocket)
+ sounlock(so, SL_LOCKED);
(void) soabort(so2);
+ if (persocket)
+ solock(so);
}
while ((so2 = TAILQ_FIRST(>so_q)) != NULL) {
+ if (persocket)
+ solock(so2);
(void) soqremque(so2, 1);
+ if (persocket)
+ sounlock(so, SL_LOCKED);
(void) soabort(so2);
+ if (persocket)
+ solock(so);
}
}
discard:
@@ -437,11 +502,19 @@ soconnect(struct socket *so, struct mbuf
int
soconnect2(struct socket *so1, struct socket *so2)
{
- int s, error;
+ int persocket, s, error;
+
+ if ((persocket = solock_persocket(so1))) {
+ solock_pair(so1, so2);
+ s = SL_LOCKED;
+ } else
+ s = solock(so1);
- s = solock(so1);
error = (*so1->so_proto->pr_usrreq)(so1, PRU_CONNECT2, NULL,
(struct mbuf *)so2, NULL, curproc);
+
+ if (persocket)
+ sounlock(so2, s);
sounlock(so1, s);
return (error);
}
Index: sys/kern/uipc_socket2.c
===
RCS file:
Re: lsearch(3): memmove(3), not memcpy(3)
Scott Cheloha wrote: > On Mon, Dec 06, 2021 at 03:09:05PM -0700, Theo de Raadt wrote: > > +* Use memmove(3) instead of memcpy(3), just in case key > > +* partially overlaps with the end of the array. > > > > It isn't a "just in case", as in a possibility. > > ? > > > It is gauranteed this condition will happen. > > I don't follow you. I would expect this to basically never happen. > It's user error. We're well outside of "defined behavior" here, I'm > just trying to make lsearch(3) do the best thing in a bad situation. Does the specification of this function say that a caller CANNOT lay out the objects that way? If it doesn't, they eventually will. > Like, I can write a program to demonstrate the problem, but this is > not something you would ever do intentionally. That is backwards again. Someone else will. By accident, even. > + > + /* > + * Use memmove(3) to ensure the key is copied cleanly into the > + * array, even if the key overlaps with the end of the array. > + */ > + memmove((void *)end, key, width); > return((void *)end); > } Yes that is better, it avoids calling the situation abstract or vague.
Re: lsearch(3): memmove(3), not memcpy(3)
On Mon, Dec 06, 2021 at 03:09:05PM -0700, Theo de Raadt wrote: > +* Use memmove(3) instead of memcpy(3), just in case key > +* partially overlaps with the end of the array. > > It isn't a "just in case", as in a possibility. ? > It is gauranteed this condition will happen. I don't follow you. I would expect this to basically never happen. It's user error. We're well outside of "defined behavior" here, I'm just trying to make lsearch(3) do the best thing in a bad situation. Like, I can write a program to demonstrate the problem, but this is not something you would ever do intentionally. > I don't like how these conditions are described as odd-cases, that > isn't how machines actually work, and I think it should be described > in a stronger sense. This? Index: lsearch.c === RCS file: /cvs/src/lib/libc/stdlib/lsearch.c,v retrieving revision 1.5 diff -u -p -r1.5 lsearch.c --- lsearch.c 18 Jul 2014 04:16:09 - 1.5 +++ lsearch.c 7 Dec 2021 01:28:58 - @@ -79,6 +79,11 @@ linear_base(const void *key, const void * manual. */ ++*nelp; - memcpy((void *)end, key, width); + + /* +* Use memmove(3) to ensure the key is copied cleanly into the +* array, even if the key overlaps with the end of the array. +*/ + memmove((void *)end, key, width); return((void *)end); }
Re: moncontrol(3): remove hertz()
On Mon, Dec 6, 2021 at 10:30 AM Scott Cheloha wrote: > In the moncontrol(3) code we have this fallback function, hertz(). > The idea is to use an undocumented side effect of setitimer(2) to > determine the width of the hardclock(9) tick. > > hertz() has a number of problems: > > So, I want to get rid of hertz() and replace it with nothing. This is > an extreme edge case. sysctl(2) has to fail. I do not think that is > possible here, outside of stack corruption. > I agree: sysctl(KERN_CLOCKRATE) isn't blocked by pledge(2), so how would they manage to break it? (I'm not even sure the failover from profhz to hz should be kept: the kernel does that itself in initclocks(9) so if profhz is 0 then hz is too, sysctl(2) failed, and your CPU is on fire.) ok guenther@
Re: lsearch(3): memmove(3), not memcpy(3)
+* Use memmove(3) instead of memcpy(3), just in case key +* partially overlaps with the end of the array. It isn't a "just in case", as in a possibility. It is gauranteed this condition will happen. I don't like how these conditions are described as odd-cases, that isn't how machines actually work, and I think it should be described in a stronger sense.
Re: lsearch(3): memmove(3), not memcpy(3)
On Mon, Dec 06, 2021 at 01:47:52PM -0700, Todd C. Miller wrote: > On Mon, 06 Dec 2021 09:03:43 -0600, Scott Cheloha wrote: > > > In lsearch(3), the key is allowed to overlap with the last element in > > base. We need to memmove(3), not memcpy(3), or we could corrupt the > > key in that edge case. > > OK, but does this deserve a comment so to that effect to prevent > someone from changing it back to memcpy() in the future? Probably. This is an obscure edge case. To be clear: If key partially overlaps base[*nelp] then lsearch(3) will mutate the key. We can't do anything about that. However, if we use memmove(3) we at least ensure that key is copied cleanly to base[*nelp]. So, the key will still be mutated but the original value of the key will be at base[*nelp] when we return. I think that's preferable to mutating key *and* copying a corrupted key to base[*nelp], no? Index: lsearch.c === RCS file: /cvs/src/lib/libc/stdlib/lsearch.c,v retrieving revision 1.5 diff -u -p -r1.5 lsearch.c --- lsearch.c 18 Jul 2014 04:16:09 - 1.5 +++ lsearch.c 6 Dec 2021 21:59:48 - @@ -79,6 +79,11 @@ linear_base(const void *key, const void * manual. */ ++*nelp; - memcpy((void *)end, key, width); + + /* +* Use memmove(3) instead of memcpy(3), just in case key +* partially overlaps with the end of the array. +*/ + memmove((void *)end, key, width); return((void *)end); }
Re: com(4) at acpi(4) on amd64
> Date: Mon, 6 Dec 2021 21:45:03 +0100
> From: Anton Lindqvist
>
> On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 6 Dec 2021 21:08:04 +0100
> > > From: Patrick Wildt
> > >
> > > Hi,
> > >
> > > On one machine I had the pleasure of having to try and use the
> > > Serial-over-LAN feature which shows up as just another com(4)
> > > device. Instead of having to manually add a com(4) at isa(4)
> > > I figured it would be nicer to have them attach via ACPI. At
> > > least on that machine, the SOL definitely shows up in the DSDT.
> > >
> > > Since I don't want to break any legacy machines, I figured I'd
> > > keep ignoring the isa(4) addresses specified in amd64's GENERIC.
> > >
> > > Right now this diff is more about putting it out there, not about
> > > asking for OKs, as amd64 isn't really my strong suit. If people
> > > are interested, I can definitely put in all the feedback there is.
> > >
> > > Patrick
> >
> > anton@ has a better diff he's working on
>
> Here's the diff in its current state. There's one thing I haven't had
> the time to figure out yet: in order to get interrupts working on my
> apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to
> acpi_intr_establish() which causes IST_EDGE to be passed to
> intr_establish(). Worth noting is that this matches what com at isa
> already does. This was however not necessary on my amd64 build machine
> where aaa_irq_flags also is zero.
Actually, it seems we're parsing the ACPI interrupt resource
descriptor wrong. The decompiled DSDTs I have here seem to use
IRQNoFlags() for the interrupts, which apparently implies the
interrupt is edge-triggered.
Let me see if I can cook a diff.
>
> Example attachment:
>
> com0 at acpi0 UAR2 addr 0x3f8/0x8 irq 3: ns16550a, 16 byte fifo
> com0: console
>
> diff --git share/man/man4/com.4 share/man/man4/com.4
> index 73b421f2ca7..e54255fe0d6 100644
> --- share/man/man4/com.4
> +++ share/man/man4/com.4
> @@ -61,11 +61,13 @@
> .Cd "com0 at isa? port 0x3f8 irq 4"
> .Cd "com1 at isa? port 0x2f8 irq 3"
> .Pp
> +.Cd "# arm64 and amd64"
> +.Cd "com* at acpi?"
> +.Pp
> .Cd "# armv7"
> .Cd "com* at fdt?"
> .Pp
> .Cd "# arm64"
> -.Cd "com* at acpi?"
> .Cd "com* at fdt?"
> .Pp
> .Cd "# hppa"
> diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC
> index ecccd1323d9..87fcaced306 100644
> --- sys/arch/amd64/conf/GENERIC
> +++ sys/arch/amd64/conf/GENERIC
> @@ -65,6 +65,11 @@ amdgpio* at acpi?
> aplgpio* at acpi?
> bytgpio* at acpi?
> chvgpio* at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com2 at acpi?
> +com3 at acpi?
> +com* at acpi?
> glkgpio* at acpi?
> pchgpio* at acpi?
> sdhc*at acpi?
> diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK
> index 6041293b287..a5f10b357dd 100644
> --- sys/arch/amd64/conf/RAMDISK
> +++ sys/arch/amd64/conf/RAMDISK
> @@ -34,6 +34,9 @@ acpipci*at acpi?
> acpiprt* at acpi?
> acpimadt0at acpi?
> #acpitz* at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com* at acpi?
>
> mpbios0 at bios0
>
> diff --git sys/arch/amd64/conf/RAMDISK_CD sys/arch/amd64/conf/RAMDISK_CD
> index 8bd646b4ea3..d007c0102d0 100644
> --- sys/arch/amd64/conf/RAMDISK_CD
> +++ sys/arch/amd64/conf/RAMDISK_CD
> @@ -51,6 +51,10 @@ bytgpio* at acpi?
> sdhc*at acpi?
> acpihve* at acpi?
> chvgpio*at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com2 at acpi?
> +com* at acpi?
> glkgpio* at acpi?
>
> mpbios0 at bios0
> diff --git sys/dev/acpi/acpi.c sys/dev/acpi/acpi.c
> index 7577424e8a2..e89869aedbd 100644
> --- sys/dev/acpi/acpi.c
> +++ sys/dev/acpi/acpi.c
> @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> - "PNP0501", /* 16550A-compatible COM Serial Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> "PNP0F03", /* Microsoft PS/2-style Mouse */
> "PNP0F13", /* PS/2 Mouse */
> diff --git sys/dev/acpi/com_acpi.c sys/dev/acpi/com_acpi.c
> index 852be6c71b3..be78e34a016 100644
> --- sys/dev/acpi/com_acpi.c
> +++ sys/dev/acpi/com_acpi.c
> @@ -49,10 +49,12 @@ struct cfattach com_acpi_ca = {
>
> const char *com_hids[] = {
> "HISI0031",
> + "PNP0501",
> NULL
> };
>
> int com_acpi_is_console(struct com_acpi_softc *);
> +int com_acpi_is_designware(const char *);
> int com_acpi_intr_designware(void *);
>
> int
> @@ -69,7 +71,7 @@ com_acpi_attach(struct device *parent, struct device *self,
> void *aux)
> {
> struct com_acpi_softc *sc = (struct com_acpi_softc *)self;
> struct acpi_attach_args *aaa = aux;
> - uint32_t freq;
> + int
Re: kbind(2): push kernel lock down as needed
On Mon, Dec 06, 2021 at 08:35:15PM +0100, Mark Kettenis wrote:
> > Date: Sun, 5 Dec 2021 18:01:04 -0600
> > From: Scott Cheloha
> >
> > Two things in sys_kbind() need an explicit kernel lock. First,
> > sigexit(). Second, uvm_map_extract(), because the following call
> > chain panics without it:
> >
> > [...]
> >
> > With this committed we can unlock kbind(2).
> >
> > Thoughts? ok?
>
> To be honest, I don't think this makes sense unless you can make the
> "normal" code path lock free. You're replacing a single
> KERNEL_LOCK/UNLOCK pair with (potentially) a bunch of them. That may
> actually make things worse. So I think we need to make
> uvm_map_extract() mpsafe first.
Unlocking uvm_map_extract() would improve things, yes.
I wrote a benchmark so we have something concrete to talk about.
Attached inline at the end. The benchmark spawns 1 or more threads
that repeatedly fork(2), execve(2), and waitpid(2). By default the
child executes /usr/bin/true, the simplest program in base that needs
ld.so. The benchmark stops when a given number of fork/execve/waitpid
cycles have completed. It then reports the elapsed time.
With this patch + kbind(2) unlocked the benchmark completes no faster
in any configuration.
> In case mpi@ disagrees, the unbalanced KERNEL_LOCK() before the
> sigexit() call is unfortunate. I'd add a /* NOTREACHED */ after the
> sigexit() call to signal that the unbalanced KERNEL_LOCK() is ok.
Thanks, wasn't sure about that. Updated.
--
Here's the benchmark. Try something like this:
# How quickly can four threads fork+exec true(1) a
# thousand times?
$ ./forkexecwait -n4 1000 /usr/bin/true
/*
* cc -o forkexecwait forkexecwait.c -lpthread
*/
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
pthread_t *thread;
pthread_barrier_t barrier;
char *default_argv[] = { "true", NULL };
char *default_envp[] = { NULL };
char *exec_path = "/usr/bin/true";
char **exec_argv = default_argv;
struct timespec start, stop;
unsigned long count, max;
volatile int done;
void *thread_func(void *);
void thread_join(pthread_t *, unsigned int);
void thread_spawn(pthread_t **, unsigned int, void *(*)(void *));
void usage(void);
int
main(int argc, char *argv[])
{
struct timespec elapsed;
const char *errstr;
unsigned int nthreads;
int ch, error;
nthreads = 1;
while ((ch = getopt(argc, argv, "n:")) != -1) {
switch (ch) {
case 'n':
nthreads = strtonum(optarg, 1, UINT_MAX, );
if (errstr != NULL)
errx(1, "nthreads is %s: %s", errstr, optarg);
break;
default:
usage();
}
}
argc -= optind;
argv += optind;
if (argc < 1)
usage();
max = strtonum(argv[0], 1, LONG_MAX, );
if (errstr != NULL)
errx(1, "count is %s: %s", errstr, argv[0]);
if (argc >= 2) {
exec_path = argv[1];
exec_argv = [1];
}
error = pthread_barrier_init(, NULL, nthreads);
if (error)
errc(1, error, "pthread_barrier_init");
thread_spawn(, nthreads, _func);
thread_join(thread, nthreads);
error = pthread_barrier_destroy();
if (error)
errc(1, error, "pthread_barrier_destroy");
timespecsub(, , );
printf("%lld.%09ld\n", (long long)elapsed.tv_sec, elapsed.tv_nsec);
return 0;
}
void *
thread_func(void *arg)
{
int error, status;
pid_t pid;
error = pthread_barrier_wait();
if (error) {
if (error != PTHREAD_BARRIER_SERIAL_THREAD)
errc(1, error, "pthread_barrier_wait");
clock_gettime(CLOCK_MONOTONIC, );
}
while (!done) {
switch (pid = fork()) {
case -1:
err(1, "fork");
case 0:
execve(exec_path, exec_argv, default_envp);
error = (errno == ENOENT) ? 127 : 126;
err(error, "execve %s", exec_path);
default:
while (waitpid(pid, , 0) == -1) {
if (errno != EINTR)
err(1, "waitpid %ld", (long)pid);
}
if (atomic_inc_long_nv() == max) {
clock_gettime(CLOCK_MONOTONIC, );
done = 1;
}
}
}
return NULL;
}
void
thread_join(pthread_t *thread, unsigned int nthreads)
{
unsigned int i;
int error;
for (i = 0; i < nthreads; i++) {
error = pthread_join(thread[i], NULL);
Re: com(4) at acpi(4) on amd64
These com devices are normally edge interrupt, which means they are sometimes shared. Shared isn't ideal, but it often works. So an acpi driver is now participating in edge shared devices. Let's imagine a com2 is mistakenly wired to irq 3, but not listed by acpi, while com0 is at irq 3 and listed by acpi. Will this actually work correctly?
Re: com(4) at acpi(4) on amd64
Mark Kettenis wrote: > What we really want is that these "com* at acpi?" ports replace the > "com* at isa?" ports. These are the "standard" COM1-4 ports after > all. So the solution we came up with was to add: > > com0 at acpi? > com1 at acpi? > com2 at acpi? > com3 at acpi? > com* at acpi? That is the only way to do it. A * entry may not choose a fixed position, but you can have multiple fixed position methods. Then later fixed position configuration will find the fixed position already utilized, and be skipped. The problem with this scheme is that the declerations above don't define the the port/irq positions. Maybe at a MIMIMUM those 4 should contain a comments like "#expected to be isa? port 0x3f8 irq 4". There is a weird thing though where an acpi driver might succeed configure, but knock out the ability of other isa drivers to configure, if the interrupts are wierd. Waiting for the diff, to see what it does.
Re: lsearch(3): memmove(3), not memcpy(3)
On Mon, 06 Dec 2021 09:03:43 -0600, Scott Cheloha wrote: > In lsearch(3), the key is allowed to overlap with the last element in > base. We need to memmove(3), not memcpy(3), or we could corrupt the > key in that edge case. OK, but does this deserve a comment so to that effect to prevent someone from changing it back to memcpy() in the future? - todd
Re: com(4) at acpi(4) on amd64
On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote:
> > Date: Mon, 6 Dec 2021 21:08:04 +0100
> > From: Patrick Wildt
> >
> > Hi,
> >
> > On one machine I had the pleasure of having to try and use the
> > Serial-over-LAN feature which shows up as just another com(4)
> > device. Instead of having to manually add a com(4) at isa(4)
> > I figured it would be nicer to have them attach via ACPI. At
> > least on that machine, the SOL definitely shows up in the DSDT.
> >
> > Since I don't want to break any legacy machines, I figured I'd
> > keep ignoring the isa(4) addresses specified in amd64's GENERIC.
> >
> > Right now this diff is more about putting it out there, not about
> > asking for OKs, as amd64 isn't really my strong suit. If people
> > are interested, I can definitely put in all the feedback there is.
> >
> > Patrick
>
> anton@ has a better diff he's working on
Here's the diff in its current state. There's one thing I haven't had
the time to figure out yet: in order to get interrupts working on my
apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to
acpi_intr_establish() which causes IST_EDGE to be passed to
intr_establish(). Worth noting is that this matches what com at isa
already does. This was however not necessary on my amd64 build machine
where aaa_irq_flags also is zero.
Example attachment:
com0 at acpi0 UAR2 addr 0x3f8/0x8 irq 3: ns16550a, 16 byte fifo
com0: console
diff --git share/man/man4/com.4 share/man/man4/com.4
index 73b421f2ca7..e54255fe0d6 100644
--- share/man/man4/com.4
+++ share/man/man4/com.4
@@ -61,11 +61,13 @@
.Cd "com0 at isa? port 0x3f8 irq 4"
.Cd "com1 at isa? port 0x2f8 irq 3"
.Pp
+.Cd "# arm64 and amd64"
+.Cd "com* at acpi?"
+.Pp
.Cd "# armv7"
.Cd "com* at fdt?"
.Pp
.Cd "# arm64"
-.Cd "com* at acpi?"
.Cd "com* at fdt?"
.Pp
.Cd "# hppa"
diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC
index ecccd1323d9..87fcaced306 100644
--- sys/arch/amd64/conf/GENERIC
+++ sys/arch/amd64/conf/GENERIC
@@ -65,6 +65,11 @@ amdgpio* at acpi?
aplgpio* at acpi?
bytgpio* at acpi?
chvgpio* at acpi?
+com0 at acpi?
+com1 at acpi?
+com2 at acpi?
+com3 at acpi?
+com* at acpi?
glkgpio* at acpi?
pchgpio* at acpi?
sdhc* at acpi?
diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK
index 6041293b287..a5f10b357dd 100644
--- sys/arch/amd64/conf/RAMDISK
+++ sys/arch/amd64/conf/RAMDISK
@@ -34,6 +34,9 @@ acpipci* at acpi?
acpiprt* at acpi?
acpimadt0 at acpi?
#acpitz* at acpi?
+com0 at acpi?
+com1 at acpi?
+com* at acpi?
mpbios0at bios0
diff --git sys/arch/amd64/conf/RAMDISK_CD sys/arch/amd64/conf/RAMDISK_CD
index 8bd646b4ea3..d007c0102d0 100644
--- sys/arch/amd64/conf/RAMDISK_CD
+++ sys/arch/amd64/conf/RAMDISK_CD
@@ -51,6 +51,10 @@ bytgpio* at acpi?
sdhc* at acpi?
acpihve* at acpi?
chvgpio*at acpi?
+com0 at acpi?
+com1 at acpi?
+com2 at acpi?
+com* at acpi?
glkgpio* at acpi?
mpbios0at bios0
diff --git sys/dev/acpi/acpi.c sys/dev/acpi/acpi.c
index 7577424e8a2..e89869aedbd 100644
--- sys/dev/acpi/acpi.c
+++ sys/dev/acpi/acpi.c
@@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
"PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
"PNP0400", /* Standard LPT Parallel Port */
"PNP0401", /* ECP Parallel Port */
- "PNP0501", /* 16550A-compatible COM Serial Port */
"PNP0700", /* PC-class Floppy Disk Controller */
"PNP0F03", /* Microsoft PS/2-style Mouse */
"PNP0F13", /* PS/2 Mouse */
diff --git sys/dev/acpi/com_acpi.c sys/dev/acpi/com_acpi.c
index 852be6c71b3..be78e34a016 100644
--- sys/dev/acpi/com_acpi.c
+++ sys/dev/acpi/com_acpi.c
@@ -49,10 +49,12 @@ struct cfattach com_acpi_ca = {
const char *com_hids[] = {
"HISI0031",
+ "PNP0501",
NULL
};
intcom_acpi_is_console(struct com_acpi_softc *);
+intcom_acpi_is_designware(const char *);
intcom_acpi_intr_designware(void *);
int
@@ -69,7 +71,7 @@ com_acpi_attach(struct device *parent, struct device *self,
void *aux)
{
struct com_acpi_softc *sc = (struct com_acpi_softc *)self;
struct acpi_attach_args *aaa = aux;
- uint32_t freq;
+ int (*intrfn)(void *) = comintr;
sc->sc_acpi = (struct acpi_softc *)parent;
sc->sc_node = aaa->aaa_node;
@@ -85,34 +87,44 @@ com_acpi_attach(struct device *parent, struct device *self,
void *aux)
return;
}
- printf(" addr 0x%llx/0x%llx", aaa->aaa_addr[0], aaa->aaa_size[0]);
+ printf(" addr 0x%llx/%llu", aaa->aaa_addr[0], aaa->aaa_size[0]);
printf(" irq %d", aaa->aaa_irq[0]);
- freq = acpi_getpropint(sc->sc_node, "clock-frequency", 0);
-
Re: com(4) at acpi(4) on amd64
> Date: Mon, 6 Dec 2021 21:26:11 +0100 > From: Patrick Wildt > > Am Mon, Dec 06, 2021 at 09:23:45PM +0100 schrieb Mark Kettenis: > > > Date: Mon, 6 Dec 2021 21:08:04 +0100 > > > From: Patrick Wildt > > > > > > Hi, > > > > > > On one machine I had the pleasure of having to try and use the > > > Serial-over-LAN feature which shows up as just another com(4) > > > device. Instead of having to manually add a com(4) at isa(4) > > > I figured it would be nicer to have them attach via ACPI. At > > > least on that machine, the SOL definitely shows up in the DSDT. > > > > > > Since I don't want to break any legacy machines, I figured I'd > > > keep ignoring the isa(4) addresses specified in amd64's GENERIC. > > > > > > Right now this diff is more about putting it out there, not about > > > asking for OKs, as amd64 isn't really my strong suit. If people > > > are interested, I can definitely put in all the feedback there is. > > > > > > Patrick > > > > anton@ has a better diff he's working on > > Great, please forget I even said anything. Well, you've obviously set yourself up as a guinea pig for his diff ;).
Re: com(4) at acpi(4) on amd64
> From: "Theo de Raadt"
> Date: Mon, 06 Dec 2021 13:17:37 -0700
>
> What comN does this attach as?
>
> The isa ones are are hard-wired:
>
> com0at isa? port 0x3f8 irq 4# standard PC serial ports
> com1at isa? port 0x2f8 irq 3
> com2at isa? port 0x3e8 irq 5
> com3at isa? disable port 0x2e8 irq 9 # (conflicts with some video cards)
>
> So they will consume com0, com1, com2, com3.
>
> This means the first dynamic ones will be com4, etc etc etc.
>
> When you add com* at acpi, acpi is processed earlier, so the devices on
> these busses will move.
>
> com*at cardbus?
> com*at pci?
> com*at pcmcia? # PCMCIA modems/serial ports
> com*at puc?
>
> There will be people affected by their puc device numbers moving, in
> particular people who have built console servers, which will be very
> specific to the machine.
>
> Anyone want to comment on that?
So anton@ is working on a diff as well (and obviously made the mistake
of not involving more developers). He ran into a related issue.
What we really want is that these "com* at acpi?" ports replace the
"com* at isa?" ports. These are the "standard" COM1-4 ports after
all. So the solution we came up with was to add:
com0 at acpi?
com1 at acpi?
com2 at acpi?
com3 at acpi?
com* at acpi?
Now this may still reorder the com0-3 devices among themselves. But
there may be ways to address that issue by looking at additional
properties and by using an additional locator.
This would avoid renumbering the com(4) ports provided by puc(4).
> Patrick Wildt wrote:
>
> > Hi,
> >
> > On one machine I had the pleasure of having to try and use the
> > Serial-over-LAN feature which shows up as just another com(4)
> > device. Instead of having to manually add a com(4) at isa(4)
> > I figured it would be nicer to have them attach via ACPI. At
> > least on that machine, the SOL definitely shows up in the DSDT.
> >
> > Since I don't want to break any legacy machines, I figured I'd
> > keep ignoring the isa(4) addresses specified in amd64's GENERIC.
> >
> > Right now this diff is more about putting it out there, not about
> > asking for OKs, as amd64 isn't really my strong suit. If people
> > are interested, I can definitely put in all the feedback there is.
> >
> > Patrick
> >
> > diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
> > index ecccd1323d9..edb0131d823 100644
> > --- a/sys/arch/amd64/conf/GENERIC
> > +++ b/sys/arch/amd64/conf/GENERIC
> > @@ -76,6 +76,7 @@ tpm* at acpi?
> > acpihve* at acpi?
> > acpisurface* at acpi?
> > acpihid* at acpi?
> > +com* at acpi?
> > ipmi0 at acpi? disable
> > ccpmic*at iic?
> > tipmic*at iic?
> > diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
> > index 7577424e8a2..e89869aedbd 100644
> > --- a/sys/dev/acpi/acpi.c
> > +++ b/sys/dev/acpi/acpi.c
> > @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> > "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> > "PNP0400", /* Standard LPT Parallel Port */
> > "PNP0401", /* ECP Parallel Port */
> > - "PNP0501", /* 16550A-compatible COM Serial Port */
> > "PNP0700", /* PC-class Floppy Disk Controller */
> > "PNP0F03", /* Microsoft PS/2-style Mouse */
> > "PNP0F13", /* PS/2 Mouse */
> > diff --git a/sys/dev/acpi/com_acpi.c b/sys/dev/acpi/com_acpi.c
> > index 852be6c71b3..9251b973372 100644
> > --- a/sys/dev/acpi/com_acpi.c
> > +++ b/sys/dev/acpi/com_acpi.c
> > @@ -49,6 +49,7 @@ struct cfattach com_acpi_ca = {
> >
> > const char *com_hids[] = {
> > "HISI0031",
> > + "PNP0501",
> > NULL
> > };
> >
> > @@ -61,6 +62,13 @@ com_acpi_match(struct device *parent, void *match, void
> > *aux)
> > struct acpi_attach_args *aaa = aux;
> > struct cfdata *cf = match;
> >
> > +#ifdef __amd64__
> > + /* Ignore com(4) at isa(4) */
> > + if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 ||
> > + aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8)
> > + return 0;
> > +#endif
> > +
> > return acpi_matchhids(aaa, com_hids, cf->cf_driver->cd_name);
> > }
> >
> > @@ -95,8 +103,10 @@ com_acpi_attach(struct device *parent, struct device
> > *self, void *aux)
> > sc->sc.sc_uarttype = COM_UART_16550;
> > sc->sc.sc_frequency = freq ? freq : COM_FREQ;
> >
> > +#ifndef __amd64__
> > sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
> > sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
> > +#endif
> >
> > if (com_acpi_is_console(sc)) {
> > SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> > @@ -105,7 +115,9 @@ com_acpi_attach(struct device *parent, struct device
> > *self, void *aux)
> > comconsrate = B115200;
> > }
> >
> > - if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> > + if (sc->sc.sc_iobase
OpenBSD Errata: December 7, 2021 (rpki-client)
Errata patch for rpki-client has been released for OpenBSD 6.9. Binary updates for the amd64, i386 and arm64 platform are available via the syspatch utility. Source code patches can be found on the respective errata page: https://www.openbsd.org/errata69.html
Re: com(4) at acpi(4) on amd64
Stuart Henderson wrote: > > +#ifdef __amd64__ > > + /* Ignore com(4) at isa(4) */ > > + if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 || > > + aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8) > > + return 0; > > +#endif > > One of the benefits of using ACPI to pick up UARTs is that it can cope > with vendors who have screwed up their interrupt assignments (the most > common mistake I've encountered is to have 0x3f8 irq3 / 0x2f8 irq4), > I think this part of the diff will prevent that from working. > > Also, it's more than just amd64 ;) This part of the diff makes the acpi attachment ignore any uart at the 4 major addresses, as such they remain on isa, where the problem will resurface.
Re: com(4) at acpi(4) on amd64
Am Mon, Dec 06, 2021 at 09:23:45PM +0100 schrieb Mark Kettenis: > > Date: Mon, 6 Dec 2021 21:08:04 +0100 > > From: Patrick Wildt > > > > Hi, > > > > On one machine I had the pleasure of having to try and use the > > Serial-over-LAN feature which shows up as just another com(4) > > device. Instead of having to manually add a com(4) at isa(4) > > I figured it would be nicer to have them attach via ACPI. At > > least on that machine, the SOL definitely shows up in the DSDT. > > > > Since I don't want to break any legacy machines, I figured I'd > > keep ignoring the isa(4) addresses specified in amd64's GENERIC. > > > > Right now this diff is more about putting it out there, not about > > asking for OKs, as amd64 isn't really my strong suit. If people > > are interested, I can definitely put in all the feedback there is. > > > > Patrick > > anton@ has a better diff he's working on Great, please forget I even said anything.
Re: com(4) at acpi(4) on amd64
On 2021/12/06 21:08, Patrick Wildt wrote:
> Hi,
>
> On one machine I had the pleasure of having to try and use the
> Serial-over-LAN feature which shows up as just another com(4)
> device. Instead of having to manually add a com(4) at isa(4)
> I figured it would be nicer to have them attach via ACPI. At
> least on that machine, the SOL definitely shows up in the DSDT.
>
> Since I don't want to break any legacy machines, I figured I'd
> keep ignoring the isa(4) addresses specified in amd64's GENERIC.
>
> Right now this diff is more about putting it out there, not about
> asking for OKs, as amd64 isn't really my strong suit. If people
> are interested, I can definitely put in all the feedback there is.
>
> Patrick
>
> diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
> index ecccd1323d9..edb0131d823 100644
> --- a/sys/arch/amd64/conf/GENERIC
> +++ b/sys/arch/amd64/conf/GENERIC
> @@ -76,6 +76,7 @@ tpm*at acpi?
> acpihve* at acpi?
> acpisurface* at acpi?
> acpihid* at acpi?
> +com* at acpi?
> ipmi0at acpi? disable
> ccpmic* at iic?
> tipmic* at iic?
> diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
> index 7577424e8a2..e89869aedbd 100644
> --- a/sys/dev/acpi/acpi.c
> +++ b/sys/dev/acpi/acpi.c
> @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> - "PNP0501", /* 16550A-compatible COM Serial Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> "PNP0F03", /* Microsoft PS/2-style Mouse */
> "PNP0F13", /* PS/2 Mouse */
> diff --git a/sys/dev/acpi/com_acpi.c b/sys/dev/acpi/com_acpi.c
> index 852be6c71b3..9251b973372 100644
> --- a/sys/dev/acpi/com_acpi.c
> +++ b/sys/dev/acpi/com_acpi.c
> @@ -49,6 +49,7 @@ struct cfattach com_acpi_ca = {
>
> const char *com_hids[] = {
> "HISI0031",
> + "PNP0501",
> NULL
> };
>
> @@ -61,6 +62,13 @@ com_acpi_match(struct device *parent, void *match, void
> *aux)
> struct acpi_attach_args *aaa = aux;
> struct cfdata *cf = match;
>
> +#ifdef __amd64__
> + /* Ignore com(4) at isa(4) */
> + if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 ||
> + aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8)
> + return 0;
> +#endif
One of the benefits of using ACPI to pick up UARTs is that it can cope
with vendors who have screwed up their interrupt assignments (the most
common mistake I've encountered is to have 0x3f8 irq3 / 0x2f8 irq4),
I think this part of the diff will prevent that from working.
Also, it's more than just amd64 ;)
> +
> return acpi_matchhids(aaa, com_hids, cf->cf_driver->cd_name);
> }
>
> @@ -95,8 +103,10 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> sc->sc.sc_uarttype = COM_UART_16550;
> sc->sc.sc_frequency = freq ? freq : COM_FREQ;
>
> +#ifndef __amd64__
> sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
> sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
> +#endif
>
> if (com_acpi_is_console(sc)) {
> SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> @@ -105,7 +115,9 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> comconsrate = B115200;
> }
>
> - if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> + if (sc->sc.sc_iobase == comconsaddr)
> + sc->sc.sc_ioh = comconsioh;
> + else if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0],
> aaa->aaa_size[0],
> 0, >sc.sc_ioh)) {
> printf(": can't map registers\n");
> return;
>
Re: com(4) at acpi(4) on amd64
> Date: Mon, 6 Dec 2021 21:08:04 +0100
> From: Patrick Wildt
>
> Hi,
>
> On one machine I had the pleasure of having to try and use the
> Serial-over-LAN feature which shows up as just another com(4)
> device. Instead of having to manually add a com(4) at isa(4)
> I figured it would be nicer to have them attach via ACPI. At
> least on that machine, the SOL definitely shows up in the DSDT.
>
> Since I don't want to break any legacy machines, I figured I'd
> keep ignoring the isa(4) addresses specified in amd64's GENERIC.
>
> Right now this diff is more about putting it out there, not about
> asking for OKs, as amd64 isn't really my strong suit. If people
> are interested, I can definitely put in all the feedback there is.
>
> Patrick
anton@ has a better diff he's working on
> diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
> index ecccd1323d9..edb0131d823 100644
> --- a/sys/arch/amd64/conf/GENERIC
> +++ b/sys/arch/amd64/conf/GENERIC
> @@ -76,6 +76,7 @@ tpm*at acpi?
> acpihve* at acpi?
> acpisurface* at acpi?
> acpihid* at acpi?
> +com* at acpi?
> ipmi0at acpi? disable
> ccpmic* at iic?
> tipmic* at iic?
> diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
> index 7577424e8a2..e89869aedbd 100644
> --- a/sys/dev/acpi/acpi.c
> +++ b/sys/dev/acpi/acpi.c
> @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> - "PNP0501", /* 16550A-compatible COM Serial Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> "PNP0F03", /* Microsoft PS/2-style Mouse */
> "PNP0F13", /* PS/2 Mouse */
> diff --git a/sys/dev/acpi/com_acpi.c b/sys/dev/acpi/com_acpi.c
> index 852be6c71b3..9251b973372 100644
> --- a/sys/dev/acpi/com_acpi.c
> +++ b/sys/dev/acpi/com_acpi.c
> @@ -49,6 +49,7 @@ struct cfattach com_acpi_ca = {
>
> const char *com_hids[] = {
> "HISI0031",
> + "PNP0501",
> NULL
> };
>
> @@ -61,6 +62,13 @@ com_acpi_match(struct device *parent, void *match, void
> *aux)
> struct acpi_attach_args *aaa = aux;
> struct cfdata *cf = match;
>
> +#ifdef __amd64__
> + /* Ignore com(4) at isa(4) */
> + if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 ||
> + aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8)
> + return 0;
> +#endif
> +
> return acpi_matchhids(aaa, com_hids, cf->cf_driver->cd_name);
> }
>
> @@ -95,8 +103,10 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> sc->sc.sc_uarttype = COM_UART_16550;
> sc->sc.sc_frequency = freq ? freq : COM_FREQ;
>
> +#ifndef __amd64__
> sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
> sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
> +#endif
>
> if (com_acpi_is_console(sc)) {
> SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> @@ -105,7 +115,9 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> comconsrate = B115200;
> }
>
> - if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> + if (sc->sc.sc_iobase == comconsaddr)
> + sc->sc.sc_ioh = comconsioh;
> + else if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0],
> aaa->aaa_size[0],
> 0, >sc.sc_ioh)) {
> printf(": can't map registers\n");
> return;
>
>
Re: com(4) at acpi(4) on amd64
What comN does this attach as?
The isa ones are are hard-wired:
com0at isa? port 0x3f8 irq 4# standard PC serial ports
com1at isa? port 0x2f8 irq 3
com2at isa? port 0x3e8 irq 5
com3at isa? disable port 0x2e8 irq 9 # (conflicts with some video cards)
So they will consume com0, com1, com2, com3.
This means the first dynamic ones will be com4, etc etc etc.
When you add com* at acpi, acpi is processed earlier, so the devices on
these busses will move.
com*at cardbus?
com*at pci?
com*at pcmcia? # PCMCIA modems/serial ports
com*at puc?
There will be people affected by their puc device numbers moving, in
particular people who have built console servers, which will be very
specific to the machine.
Anyone want to comment on that?
Patrick Wildt wrote:
> Hi,
>
> On one machine I had the pleasure of having to try and use the
> Serial-over-LAN feature which shows up as just another com(4)
> device. Instead of having to manually add a com(4) at isa(4)
> I figured it would be nicer to have them attach via ACPI. At
> least on that machine, the SOL definitely shows up in the DSDT.
>
> Since I don't want to break any legacy machines, I figured I'd
> keep ignoring the isa(4) addresses specified in amd64's GENERIC.
>
> Right now this diff is more about putting it out there, not about
> asking for OKs, as amd64 isn't really my strong suit. If people
> are interested, I can definitely put in all the feedback there is.
>
> Patrick
>
> diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
> index ecccd1323d9..edb0131d823 100644
> --- a/sys/arch/amd64/conf/GENERIC
> +++ b/sys/arch/amd64/conf/GENERIC
> @@ -76,6 +76,7 @@ tpm*at acpi?
> acpihve* at acpi?
> acpisurface* at acpi?
> acpihid* at acpi?
> +com* at acpi?
> ipmi0at acpi? disable
> ccpmic* at iic?
> tipmic* at iic?
> diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
> index 7577424e8a2..e89869aedbd 100644
> --- a/sys/dev/acpi/acpi.c
> +++ b/sys/dev/acpi/acpi.c
> @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> - "PNP0501", /* 16550A-compatible COM Serial Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> "PNP0F03", /* Microsoft PS/2-style Mouse */
> "PNP0F13", /* PS/2 Mouse */
> diff --git a/sys/dev/acpi/com_acpi.c b/sys/dev/acpi/com_acpi.c
> index 852be6c71b3..9251b973372 100644
> --- a/sys/dev/acpi/com_acpi.c
> +++ b/sys/dev/acpi/com_acpi.c
> @@ -49,6 +49,7 @@ struct cfattach com_acpi_ca = {
>
> const char *com_hids[] = {
> "HISI0031",
> + "PNP0501",
> NULL
> };
>
> @@ -61,6 +62,13 @@ com_acpi_match(struct device *parent, void *match, void
> *aux)
> struct acpi_attach_args *aaa = aux;
> struct cfdata *cf = match;
>
> +#ifdef __amd64__
> + /* Ignore com(4) at isa(4) */
> + if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 ||
> + aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8)
> + return 0;
> +#endif
> +
> return acpi_matchhids(aaa, com_hids, cf->cf_driver->cd_name);
> }
>
> @@ -95,8 +103,10 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> sc->sc.sc_uarttype = COM_UART_16550;
> sc->sc.sc_frequency = freq ? freq : COM_FREQ;
>
> +#ifndef __amd64__
> sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
> sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
> +#endif
>
> if (com_acpi_is_console(sc)) {
> SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> @@ -105,7 +115,9 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> comconsrate = B115200;
> }
>
> - if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> + if (sc->sc.sc_iobase == comconsaddr)
> + sc->sc.sc_ioh = comconsioh;
> + else if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0],
> aaa->aaa_size[0],
> 0, >sc.sc_ioh)) {
> printf(": can't map registers\n");
> return;
>
com(4) at acpi(4) on amd64
Hi,
On one machine I had the pleasure of having to try and use the
Serial-over-LAN feature which shows up as just another com(4)
device. Instead of having to manually add a com(4) at isa(4)
I figured it would be nicer to have them attach via ACPI. At
least on that machine, the SOL definitely shows up in the DSDT.
Since I don't want to break any legacy machines, I figured I'd
keep ignoring the isa(4) addresses specified in amd64's GENERIC.
Right now this diff is more about putting it out there, not about
asking for OKs, as amd64 isn't really my strong suit. If people
are interested, I can definitely put in all the feedback there is.
Patrick
diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
index ecccd1323d9..edb0131d823 100644
--- a/sys/arch/amd64/conf/GENERIC
+++ b/sys/arch/amd64/conf/GENERIC
@@ -76,6 +76,7 @@ tpm* at acpi?
acpihve* at acpi?
acpisurface* at acpi?
acpihid* at acpi?
+com* at acpi?
ipmi0 at acpi? disable
ccpmic*at iic?
tipmic*at iic?
diff --git a/sys/dev/acpi/acpi.c b/sys/dev/acpi/acpi.c
index 7577424e8a2..e89869aedbd 100644
--- a/sys/dev/acpi/acpi.c
+++ b/sys/dev/acpi/acpi.c
@@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
"PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
"PNP0400", /* Standard LPT Parallel Port */
"PNP0401", /* ECP Parallel Port */
- "PNP0501", /* 16550A-compatible COM Serial Port */
"PNP0700", /* PC-class Floppy Disk Controller */
"PNP0F03", /* Microsoft PS/2-style Mouse */
"PNP0F13", /* PS/2 Mouse */
diff --git a/sys/dev/acpi/com_acpi.c b/sys/dev/acpi/com_acpi.c
index 852be6c71b3..9251b973372 100644
--- a/sys/dev/acpi/com_acpi.c
+++ b/sys/dev/acpi/com_acpi.c
@@ -49,6 +49,7 @@ struct cfattach com_acpi_ca = {
const char *com_hids[] = {
"HISI0031",
+ "PNP0501",
NULL
};
@@ -61,6 +62,13 @@ com_acpi_match(struct device *parent, void *match, void *aux)
struct acpi_attach_args *aaa = aux;
struct cfdata *cf = match;
+#ifdef __amd64__
+ /* Ignore com(4) at isa(4) */
+ if (aaa->aaa_addr[0] == 0x3f8 || aaa->aaa_addr[0] == 0x2f8 ||
+ aaa->aaa_addr[0] == 0x3e8 || aaa->aaa_addr[0] == 0x2e8)
+ return 0;
+#endif
+
return acpi_matchhids(aaa, com_hids, cf->cf_driver->cd_name);
}
@@ -95,8 +103,10 @@ com_acpi_attach(struct device *parent, struct device *self,
void *aux)
sc->sc.sc_uarttype = COM_UART_16550;
sc->sc.sc_frequency = freq ? freq : COM_FREQ;
+#ifndef __amd64__
sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
+#endif
if (com_acpi_is_console(sc)) {
SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
@@ -105,7 +115,9 @@ com_acpi_attach(struct device *parent, struct device *self,
void *aux)
comconsrate = B115200;
}
- if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
+ if (sc->sc.sc_iobase == comconsaddr)
+ sc->sc.sc_ioh = comconsioh;
+ else if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0],
aaa->aaa_size[0],
0, >sc.sc_ioh)) {
printf(": can't map registers\n");
return;
Re: kbind(2): push kernel lock down as needed
> Date: Sun, 5 Dec 2021 18:01:04 -0600
> From: Scott Cheloha
>
> Two things in sys_kbind() need an explicit kernel lock. First,
> sigexit(). Second, uvm_map_extract(), because the following call
> chain panics without it:
>
> panic
> assert
> uvn_reference
> uvm_mapent_clone
> uum_map_extract
> sys_kbind
> syscall
> Xsyscall
>
>uvn_reference() does KERNEL_ASSERT_LOCKED().
>
> These are the other routines called from sys_kbind():
>
> copyin
> kcopy
> trunc_page
> vm_map_lock
> vm_map_unlock
> uvm_unmap_detach
> uvm_unmap_remove
>
> copyin/kcopy are safe, trunc_page is safe, vm_map_lock/vm_map_unlock
> are safe, uvm_unmap_detach takes the kernel lock as needed, and
> uvm_unmap_remove has callees that take the kernel lock as needed.
>
> With this committed we can unlock kbind(2).
>
> Thoughts? ok?
To be honest, I don't think this makes sense unless you can make the
"normal" code path lock free. You're replacing a single
KERNEL_LOCK/UNLOCK pair with (potentially) a bunch of them. That may
actually make things worse. So I think we need to make
uvm_map_extract() mpsafe first.
In case mpi@ disagrees, the unbalanced KERNEL_LOCK() before the
sigexit() call is unfortunate. I'd add a /* NOTREACHED */ after the
sigexit() call to signal that the unbalanced KERNEL_LOCK() is ok.
> Index: uvm_mmap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_mmap.c,v
> retrieving revision 1.165
> diff -u -p -r1.165 uvm_mmap.c
> --- uvm_mmap.c5 Dec 2021 22:00:42 - 1.165
> +++ uvm_mmap.c5 Dec 2021 23:57:28 -
> @@ -1112,10 +1112,12 @@ sys_kbind(struct proc *p, void *v, regis
> if (pr->ps_kbind_addr == 0) {
> pr->ps_kbind_addr = pc;
> pr->ps_kbind_cookie = SCARG(uap, proc_cookie);
> - } else if (pc != pr->ps_kbind_addr || pc == BOGO_PC)
> - sigexit(p, SIGILL);
> - else if (pr->ps_kbind_cookie != SCARG(uap, proc_cookie))
> + } else if (pc != pr->ps_kbind_addr || pc == BOGO_PC ||
> + pr->ps_kbind_cookie != SCARG(uap, proc_cookie)) {
> + KERNEL_LOCK();
> sigexit(p, SIGILL);
> + }
> +
> if (psize < sizeof(struct __kbind) || psize > sizeof(param))
> return EINVAL;
> if ((error = copyin(paramp, , psize)))
> @@ -1169,8 +1171,11 @@ sys_kbind(struct proc *p, void *v, regis
> vm_map_unlock(kernel_map);
> kva = 0;
> }
> - if ((error = uvm_map_extract(>p_vmspace->vm_map,
> - baseva, PAGE_SIZE, , UVM_EXTRACT_FIXPROT)))
> + KERNEL_LOCK();
> + error = uvm_map_extract(>p_vmspace->vm_map,
> + baseva, PAGE_SIZE, , UVM_EXTRACT_FIXPROT);
> + KERNEL_UNLOCK();
> + if (error)
> break;
> last_baseva = baseva;
> }
>
>
moncontrol(3): remove hertz()
In the moncontrol(3) code we have this fallback function, hertz().
The idea is to use an undocumented side effect of setitimer(2) to
determine the width of the hardclock(9) tick.
hertz() has a number of problems:
1. Per the setitimer(2) standard, the value of it_interval is ignored
if it_value is zero. So, I broke this code over a year ago. Whoops.
2. hertz() silently cancels any running ITIMER_REAL timer and does not
attempt to restart the timer when it is finished. This is not documented.
3. hertz() doesn't cancel the timer it starts after it's done computing
the hardclock(9) width. So the application gets a SIGALRM later.
We could fix (3). But (2) is a race, you can't fix it, not perfectly.
And I don't want to roll back (1) for the sake of this crummy code.
So, I want to get rid of hertz() and replace it with nothing. This is
an extreme edge case. sysctl(2) has to fail. I do not think that is
possible here, outside of stack corruption.
Objections?
Index: gmon.c
===
RCS file: /cvs/src/lib/libc/gmon/gmon.c,v
retrieving revision 1.32
diff -u -p -r1.32 gmon.c
--- gmon.c 12 Oct 2020 22:08:33 - 1.32
+++ gmon.c 6 Dec 2021 18:23:12 -
@@ -50,7 +50,6 @@ static ints_scale;
PROTO_NORMAL(moncontrol);
PROTO_DEPRECATED(monstartup);
-static int hertz(void);
void
monstartup(u_long lowpc, u_long highpc)
@@ -159,17 +158,15 @@ _mcleanup(void)
if (p->state == GMON_PROF_ERROR)
ERR("_mcleanup: tos overflow\n");
+ /*
+* There is nothing we can do if sysctl(2) fails or if
+* clockinfo.hz is unset.
+*/
size = sizeof(clockinfo);
if (sysctl(mib, 2, , , NULL, 0) == -1) {
- /*
-* Best guess
-*/
- clockinfo.profhz = hertz();
+ clockinfo.profhz = 0;
} else if (clockinfo.profhz == 0) {
- if (clockinfo.hz != 0)
- clockinfo.profhz = clockinfo.hz;
- else
- clockinfo.profhz = hertz();
+ clockinfo.profhz = clockinfo.hz;/* best guess */
}
moncontrol(0);
@@ -304,23 +301,3 @@ moncontrol(int mode)
}
}
DEF_WEAK(moncontrol);
-
-/*
- * discover the tick frequency of the machine
- * if something goes wrong, we return 0, an impossible hertz.
- */
-static int
-hertz(void)
-{
- struct itimerval tim;
-
- tim.it_interval.tv_sec = 0;
- tim.it_interval.tv_usec = 1;
- tim.it_value.tv_sec = 0;
- tim.it_value.tv_usec = 0;
- setitimer(ITIMER_REAL, , 0);
- setitimer(ITIMER_REAL, 0, );
- if (tim.it_interval.tv_usec < 2)
- return(0);
- return (100 / tim.it_interval.tv_usec);
-}
Adjust socket and FIFO's EVFILT_EXCEPT
This patch adjusts the EVFILT_EXCEPT code of sockets and FIFOs so that
it would raise the HUP condition only when the channel has been closed
from both sides. This should match better with the POLLHUP case of
soo_poll() and fifo_poll().
The "poll index ... unclaimed" error seen by some was related to this.
With pfd[i].events = 0, the EVFILT_EXCEPT filter triggered prematurely
when the socket was only half-closed.
When comparing the code, note that POLL_NOHUP was set by the old
select(2) code with writefds. The kqueue-based code achieves the same
effect with FIFOs by not raising HUP in filt_fifowrite().
The resulting code is possibly overly contrived. The SS_CANTRCVMORE
condition alone is irrelevant for poll and select. It could be removed
to simplify the code, but then socket and FIFO's EVFILT_EXCEPT would
behave inconsistently relative to EV_EOF. However, only NOTE_OOB has
been documented and it skips SS_CANTRCVMORE...
OK?
Index: kern/uipc_socket.c
===
RCS file: src/sys/kern/uipc_socket.c,v
retrieving revision 1.269
diff -u -p -r1.269 uipc_socket.c
--- kern/uipc_socket.c 11 Nov 2021 16:35:09 - 1.269
+++ kern/uipc_socket.c 6 Dec 2021 16:04:55 -
@@ -2265,12 +2265,17 @@ filt_soexcept_common(struct knote *kn, s
}
} else if (so->so_state & SS_CANTRCVMORE) {
kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL) {
- if (so->so_state & SS_ISDISCONNECTED)
- kn->kn_flags |= __EV_HUP;
- }
kn->kn_fflags = so->so_error;
- rv = 1;
+ if ((kn->kn_flags & __EV_POLL) == 0)
+ rv = 1;
+ }
+
+ if (kn->kn_flags & __EV_POLL) {
+ /* Indicate HUP only when the socket is fully closed. */
+ if (so->so_state & SS_ISDISCONNECTED) {
+ kn->kn_flags |= __EV_HUP;
+ rv = 1;
+ }
}
return rv;
Index: miscfs/fifofs/fifo_vnops.c
===
RCS file: src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.85
diff -u -p -r1.85 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 24 Oct 2021 11:23:22 - 1.85
+++ miscfs/fifofs/fifo_vnops.c 6 Dec 2021 16:04:55 -
@@ -702,11 +702,16 @@ filt_fifoexcept_common(struct knote *kn,
if (so->so_state & SS_CANTRCVMORE) {
kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL) {
- if (so->so_state & SS_ISDISCONNECTED)
- kn->kn_flags |= __EV_HUP;
+ if ((kn->kn_flags & __EV_POLL) == 0)
+ rv = 1;
+ }
+
+ if (kn->kn_flags & __EV_POLL) {
+ /* Indicate HUP only when the FIFO is fully closed. */
+ if (so->so_state & SS_ISDISCONNECTED) {
+ kn->kn_flags |= __EV_HUP;
+ rv = 1;
}
- rv = 1;
}
return (rv);
Add EVFILT_EXCEPT for pipes
This adds EVFILT_EXCEPT handler for pipes. It is a subset of
EVFILT_READ; it triggers on EOF only. This captures the POLLHUP
condition of pipe_poll(), used with select(2)'s exceptfds and when
poll(2) has events without (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM).
OK?
Index: kern/sys_pipe.c
===
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.129
diff -u -p -r1.129 sys_pipe.c
--- kern/sys_pipe.c 24 Oct 2021 06:59:54 - 1.129
+++ kern/sys_pipe.c 6 Dec 2021 16:04:55 -
@@ -85,6 +85,10 @@ int filt_pipewrite(struct knote *kn, lon
intfilt_pipewritemodify(struct kevent *kev, struct knote *kn);
intfilt_pipewriteprocess(struct knote *kn, struct kevent *kev);
intfilt_pipewrite_common(struct knote *kn, struct pipe *rpipe);
+intfilt_pipeexcept(struct knote *kn, long hint);
+intfilt_pipeexceptmodify(struct kevent *kev, struct knote *kn);
+intfilt_pipeexceptprocess(struct knote *kn, struct kevent *kev);
+intfilt_pipeexcept_common(struct knote *kn, struct pipe *rpipe);
const struct filterops pipe_rfiltops = {
.f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
@@ -104,6 +108,15 @@ const struct filterops pipe_wfiltops = {
.f_process = filt_pipewriteprocess,
};
+const struct filterops pipe_efiltops = {
+ .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
+ .f_attach = NULL,
+ .f_detach = filt_pipedetach,
+ .f_event= filt_pipeexcept,
+ .f_modify = filt_pipeexceptmodify,
+ .f_process = filt_pipeexceptprocess,
+};
+
/*
* Default pipe buffer size(s), this can be kind-of large now because pipe
* space is pageable. The pipe code will try to maintain locality of
@@ -908,6 +921,11 @@ pipe_kqfilter(struct file *fp, struct kn
kn->kn_hook = wpipe;
klist_insert_locked(>pipe_sel.si_note, kn);
break;
+ case EVFILT_EXCEPT:
+ kn->kn_fop = _efiltops;
+ kn->kn_hook = rpipe;
+ klist_insert_locked(>pipe_sel.si_note, kn);
+ break;
default:
error = EINVAL;
}
@@ -1047,6 +1065,65 @@ filt_pipewriteprocess(struct knote *kn,
return (active);
}
+int
+filt_pipeexcept_common(struct knote *kn, struct pipe *rpipe)
+{
+ struct pipe *wpipe;
+
+ rw_assert_wrlock(rpipe->pipe_lock);
+
+ wpipe = pipe_peer(rpipe);
+
+ if ((rpipe->pipe_state & PIPE_EOF) || wpipe == NULL) {
+ kn->kn_flags |= EV_EOF;
+ if (kn->kn_flags & __EV_POLL)
+ kn->kn_flags |= __EV_HUP;
+ return (1);
+ }
+
+ return (0);
+}
+
+int
+filt_pipeexcept(struct knote *kn, long hint)
+{
+ struct pipe *rpipe = kn->kn_fp->f_data;
+
+ return (filt_pipeexcept_common(kn, rpipe));
+}
+
+int
+filt_pipeexceptmodify(struct kevent *kev, struct knote *kn)
+{
+ struct pipe *rpipe = kn->kn_fp->f_data;
+ int active;
+
+ rw_enter_write(rpipe->pipe_lock);
+ knote_modify(kev, kn);
+ active = filt_pipeexcept_common(kn, rpipe);
+ rw_exit_write(rpipe->pipe_lock);
+
+ return (active);
+}
+
+int
+filt_pipeexceptprocess(struct knote *kn, struct kevent *kev)
+{
+ struct pipe *rpipe = kn->kn_fp->f_data;
+ int active;
+
+ rw_enter_write(rpipe->pipe_lock);
+ if (kev != NULL && (kn->kn_flags & EV_ONESHOT))
+ active = 1;
+ else
+ active = filt_pipeexcept_common(kn, rpipe);
+ if (active)
+ knote_submit(kn, kev);
+ rw_exit_write(rpipe->pipe_lock);
+
+ return (active);
+}
+
void
pipe_init(void)
{
Re: [patch] ehci: change explicit function names to __func__ in debugging printfs
Ping
On Sat, Nov 27, 2021 at 09:36:32PM +0300, Mikhail wrote:
> While fiddling with ehci.c and fighting with urndis I noticed that most
> of the debugging printf's use explicit names, which is inconvenient for
> grep'ing. Also, couple of items used wrong function names
> (ehci_check_intr instead of ehci_check_qh_intr).
>
> Compilation tested with DIAGNOSTIC and EHCI_DEBUG defines.
>
> diff --git sys/dev/usb/ehci.c sys/dev/usb/ehci.c
> index afc423dbbb6..37ff3beeb7a 100644
> --- sys/dev/usb/ehci.c
> +++ sys/dev/usb/ehci.c
> @@ -314,10 +314,10 @@ ehci_init(struct ehci_softc *sc)
> sc->sc_offs = EREAD1(sc, EHCI_CAPLENGTH);
>
> sparams = EREAD4(sc, EHCI_HCSPARAMS);
> - DPRINTF(("ehci_init: sparams=0x%x\n", sparams));
> + DPRINTF(("%s: sparams=0x%x\n", __func__, sparams));
> sc->sc_noport = EHCI_HCS_N_PORTS(sparams);
> cparams = EREAD4(sc, EHCI_HCCPARAMS);
> - DPRINTF(("ehci_init: cparams=0x%x\n", cparams));
> + DPRINTF(("%s: cparams=0x%x\n", __func__, cparams));
>
> /* MUST clear segment register if 64 bit capable. */
> if (EHCI_HCC_64BIT(cparams))
> @@ -521,7 +521,7 @@ ehci_intr1(struct ehci_softc *sc)
> /* In case the interrupt occurs before initialization has completed. */
> if (sc == NULL) {
> #ifdef DIAGNOSTIC
> - printf("ehci_intr1: sc == NULL\n");
> + printf("%s: sc == NULL\n", __func__);
> #endif
> return (0);
> }
> @@ -704,7 +704,7 @@ ehci_check_qh_intr(struct ehci_softc *sc, struct
> usbd_xfer *xfer)
>* short packet (SPD and not ACTIVE).
>*/
> if (letoh32(lsqtd->qtd.qtd_status) & EHCI_QTD_ACTIVE) {
> - DPRINTFN(12, ("ehci_check_intr: active ex=%p\n", ex));
> + DPRINTFN(12, ("%s: active ex=%p\n", __func__, ex));
> for (sqtd = ex->sqtdstart; sqtd != lsqtd; sqtd=sqtd->nextqtd) {
> usb_syncmem(>dma,
> sqtd->offs + offsetof(struct ehci_qtd, qtd_status),
> @@ -724,7 +724,7 @@ ehci_check_qh_intr(struct ehci_softc *sc, struct
> usbd_xfer *xfer)
> if (EHCI_QTD_GET_BYTES(status) != 0)
> goto done;
> }
> - DPRINTFN(12, ("ehci_check_intr: ex=%p std=%p still active\n",
> + DPRINTFN(12, ("%s: ex=%p std=%p still active\n", __func__,
> ex, ex->sqtdstart));
> usb_syncmem(>dma,
> lsqtd->offs + offsetof(struct ehci_qtd, qtd_status),
> @@ -876,7 +876,7 @@ ehci_idone(struct usbd_xfer *xfer)
> int s = splhigh();
> if (ex->isdone) {
> splx(s);
> - printf("ehci_idone: ex=%p is done!\n", ex);
> + printf("%s: ex=%p is done!\n", __func__, ex);
> return;
> }
> ex->isdone = 1;
> @@ -904,8 +904,8 @@ ehci_idone(struct usbd_xfer *xfer)
> }
>
> cerr = EHCI_QTD_GET_CERR(status);
> - DPRINTFN(/*10*/2, ("ehci_idone: len=%d, actlen=%d, cerr=%d, "
> - "status=0x%x\n", xfer->length, actlen, cerr, status));
> + DPRINTFN(/*10*/2, ("%s: len=%d, actlen=%d, cerr=%d, "
> + "status=0x%x\n", __func__, xfer->length, actlen, cerr, status));
> xfer->actlen = actlen;
> if ((status & EHCI_QTD_HALTED) != 0) {
> if ((status & EHCI_QTD_BABBLE) == 0 && cerr > 0)
> @@ -920,7 +920,7 @@ ehci_idone(struct usbd_xfer *xfer)
> usbd_xfer_isread(xfer) ?
> BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE);
> usb_transfer_complete(xfer);
> - DPRINTFN(/*12*/2, ("ehci_idone: ex=%p done\n", ex));
> + DPRINTFN(/*12*/2, ("%s: ex=%p done\n", __func__, ex));
> }
>
> void
> @@ -1367,7 +1367,7 @@ ehci_open(struct usbd_pipe *pipe)
> int ival, speed, naks;
> int hshubaddr, hshubport;
>
> - DPRINTFN(1, ("ehci_open: pipe=%p, addr=%d, endpt=%d\n",
> + DPRINTFN(1, ("%s: pipe=%p, addr=%d, endpt=%d\n", __func__,
> pipe, addr, ed->bEndpointAddress));
>
> if (sc->sc_bus.dying)
> @@ -1408,7 +1408,7 @@ ehci_open(struct usbd_pipe *pipe)
> speed = EHCI_QH_SPEED_HIGH;
> break;
> default:
> - panic("ehci_open: bad device speed %d", dev->speed);
> + panic("%s: bad device speed %d", __func__, dev->speed);
> }
>
> /*
> @@ -1512,18 +1512,18 @@ ehci_open(struct usbd_pipe *pipe)
> }
> /* Spec page 271 says intervals > 16 are invalid */
> if (ed->bInterval == 0 || ed->bInterval > 16) {
> - printf("ehci: opening pipe with invalid bInterval\n");
> + printf("%s: opening pipe with invalid bInterval\n",
> __func__);
> return (USBD_INVAL);
> }
> if (UGETW(ed->wMaxPacketSize) == 0) {
> - printf("ehci: zero length endpoint
lsearch(3): memmove(3), not memcpy(3)
In lsearch(3), the key is allowed to overlap with the last element in base. We need to memmove(3), not memcpy(3), or we could corrupt the key in that edge case. ok? Index: lsearch.c === RCS file: /cvs/src/lib/libc/stdlib/lsearch.c,v retrieving revision 1.5 diff -u -p -r1.5 lsearch.c --- lsearch.c 18 Jul 2014 04:16:09 - 1.5 +++ lsearch.c 6 Dec 2021 15:02:51 - @@ -79,6 +79,6 @@ linear_base(const void *key, const void * manual. */ ++*nelp; - memcpy((void *)end, key, width); + memmove((void *)end, key, width); return((void *)end); }
Re: parallel ip forwarding
On Sat, Dec 04, 2021 at 10:41:02AM +0100, Hrvoje Popovski wrote: > r620-2# uvm_fault(0x8229d4e0, 0x137, 0, 2) -> e > kernel: page fault trap, code=0 > Stopped at ipsp_spd_lookup+0xa2f: movq%rax,0(%rcx) > TIDPIDUID PRFLAGS PFLAGS CPU COMMAND > 419237 67407 0 0x14000 0x2000 softnet > *157694 94649 0 0x14000 0x2002K softnet > ipsp_spd_lookup(fd80a4139800,2,14,2,0,0,5b815d966b14b44b,fd80a4139800) > at ipsp_spd_lookup+0xa2f Thanks a lot for the test. It crashes here: /home/bluhm/openbsd/cvs/src/sys/netinet/ip_spd.c:414 cdc: 48 03 0aadd(%rdx),%rcx *cdf: 48 89 01mov%rax,(%rcx) ce2: 49 8b 80 30 01 00 00mov0x130(%r8),%rax ce9: 49 8b 88 38 01 00 00mov0x138(%r8),%rcx cf0: 48 89 01mov%rax,(%rcx) cf3: 49 c7 80 38 01 00 00movq $0x,0x138(%r8) cfa: ff ff ff ff cfe: 49 c7 80 30 01 00 00movq $0x,0x130(%r8) d05: ff ff ff ff /home/bluhm/openbsd/cvs/src/sys/netinet/ip_spd.c:416 nomatchout: /* Cached TDB was not good. */ * TAILQ_REMOVE(>ipo_tdb->tdb_policy_head, ipo, ipo_tdb_next); tdb_unref(ipo->ipo_tdb); ipo->ipo_tdb = NULL; ipo->ipo_last_searched = 0; So mvs@'s concerns are correct, my IPsec workaround is not sufficient. I want to avoid another rwlock in the input path. Maybe I can throw some mutexes into IPsec to make it work. bluhm > ip_output_ipsec_lookup(fd80a4139800,14,0,800022c60228,0) at > ip_output_ipsec_lookup+0x4c > ip_output(fd80a4139800,0,800022c603e8,1,0,0,3ada3367ffb43fe1) at > ip_output+0x39d > ip_forward(fd80a4139800,80087048,fd8394511078,0) at > ip_forward+0x26a > ip_input_if(800022c60528,800022c60534,4,0,80087048) at > ip_input_if+0x353 > ipv4_input(80087048,fd80a4139800) at ipv4_input+0x39 > ether_input(80087048,fd80a4139800) at ether_input+0x3aa > if_input_process(80087048,800022c60618) at if_input_process+0x92 > ifiq_process(80087458) at ifiq_process+0x69 > taskq_thread(8002f080) at taskq_thread+0x81 > end trace frame: 0x0, count: 5
Re: em(4) vlan tagging support for 82576
On Mon, Dec 06, 2021 at 10:07:47AM +, Stuart Henderson wrote: > On 2021/12/05 19:22, Yury Shefer wrote: > > Hi all, > > > > I have quad-port Intel ET2 NIC based on 82576[1] controller. The manual > > says that hardware VLAN tagging should be supported but ifconfig output > > shows VLAN_MTU only in hwfeatures on OpenBSD 7.0. How do I check if 802.1Q > > tagging is offloaded or not? And if it's not - does it matter at 1Gbps > > speeds on 3 Ghz CPU? > > > > $ dmesg | grep em0 > > em0 at pci11 dev 0 function 0 "Intel 82576" rev 0x01: msi, address > > 90:e2:ba:84:64:14 > > > > $ ifconfig em0 hwfeatures > > em0: flags=808843 mtu 1500 > > Not supported by the driver with 82576 - you would see VLAN_HWTAGGING here: > > > hwfeatures=10 hardmtu 9216 > ^ > > >From if_em.c: > > 1949 #if NVLAN > 0 > 1950 if (sc->hw.mac_type != em_82575 && sc->hw.mac_type != em_82580 > && 1951 sc->hw.mac_type != em_82576 && > 1952 sc->hw.mac_type != em_i210 && sc->hw.mac_type != em_i350) > 1953 ifp->if_capabilities |= > IFCAP_VLAN_HWTAGGING; > 1954 #endif > > >From commit log: > > > revision 1.242 > date: 2010/08/03 16:21:52; author: jsg; state: Exp; lines: +3 -2; > Disable hardware VLAN stripping/insertion on 8257[56] for > now. While > stripping works insertion seems to have trouble in certain conditions, > which needs to be fixed before we want to enable hardware > support for this. > > ok deraadt@ > I do not recall specifics but 82575/82576 introduce new style descriptors closer to ix. We use the old descriptor format. I would not be surprised if some of the offload features require using the newer descriptor format to work. Sometimes problems with offloading don't show until using things like ospf and nfs or forcing fragmentation with large mtu values. > > You are best placed to tell whether it matters for your system. Is it fast > enough already? Bandwidth doesn't matter all that much, packets-per-second is > more important. > >
Re: em(4) vlan tagging support for 82576
On 2021/12/05 19:22, Yury Shefer wrote: > Hi all, > > I have quad-port Intel ET2 NIC based on 82576[1] controller. The manual > says that hardware VLAN tagging should be supported but ifconfig output > shows VLAN_MTU only in hwfeatures on OpenBSD 7.0. How do I check if 802.1Q > tagging is offloaded or not? And if it's not - does it matter at 1Gbps > speeds on 3 Ghz CPU? > > $ dmesg | grep em0 > em0 at pci11 dev 0 function 0 "Intel 82576" rev 0x01: msi, address > 90:e2:ba:84:64:14 > > $ ifconfig em0 hwfeatures > em0: flags=808843 mtu 1500 Not supported by the driver with 82576 - you would see VLAN_HWTAGGING here: > hwfeatures=10 hardmtu 9216 ^ >From if_em.c: 1949 #if NVLAN > 0 1950 if (sc->hw.mac_type != em_82575 && sc->hw.mac_type != em_82580 && 1951 sc->hw.mac_type != em_82576 && 1952 sc->hw.mac_type != em_i210 && sc->hw.mac_type != em_i350) 1953 ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING; 1954 #endif >From commit log: revision 1.242 date: 2010/08/03 16:21:52; author: jsg; state: Exp; lines: +3 -2; Disable hardware VLAN stripping/insertion on 8257[56] for now. While stripping works insertion seems to have trouble in certain conditions, which needs to be fixed before we want to enable hardware support for this. ok deraadt@ You are best placed to tell whether it matters for your system. Is it fast enough already? Bandwidth doesn't matter all that much, packets-per-second is more important.
Re: em(4) vlan tagging support for 82576
On 6.12.2021. 4:22, Yury Shefer wrote: > Hi all, > > I have quad-port Intel ET2 NIC based on 82576[1] controller. The manual > says that hardware VLAN tagging should be supported but ifconfig output > shows VLAN_MTU only in hwfeatures on OpenBSD 7.0. How do I check if 802.1Q > tagging is offloaded or not? And if it's not - does it matter at 1Gbps > speeds on 3 Ghz CPU? > > $ dmesg | grep em0 > em0 at pci11 dev 0 function 0 "Intel 82576" rev 0x01: msi, address > 90:e2:ba:84:64:14 > > $ ifconfig em0 hwfeatures > em0: flags=808843 mtu 1500 > hwfeatures=10 hardmtu 9216 > lladdr 90:e2:ba:84:64:14 > index 1 priority 0 llprio 3 > media: Ethernet autoselect (1000baseT full-duplex,rxpause,txpause) > status: active > inet 192.168.0.143 netmask 0xff00 broadcast 192.168.0.255 > > > > [1] > https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/gigabit-et-et2-ef-multi-port-server-adapters-brief.pdf > > > Thanks, > Yury. > Hi, man ifconfig and search for hwfeatures hwfeatures Display the interface hardware features: CSUM_IPv4 The device supports IPv4 checksum offload. CSUM_TCPv4 As above, for TCP in IPv4 datagrams. CSUM_UDPv4 As above, for UDP. VLAN_MTUThe device can handle full sized frames, plus the size of the vlan(4) tag. VLAN_HWTAGGING On transmit, the device can add the vlan(4) tag. CSUM_TCPv6 As CSUM_TCPv4, but supports IPv6 datagrams. CSUM_UDPv6 As above, for UDP. WOL The device supports Wake on LAN (WoL). hardmtu The maximum MTU supported. so i would say that your card can offload vlan header ..
