Re: [patch] remove NPCDISPLAY from AMD64

2020-06-16 Thread johnc
Ok, I didn't know such things existed.

I would think that a coreboot framebuffer would be preferred over a
VGA console, unless there was a system that only had the framebuffer
in uncached-not-write-combined mapping.

 Original Message 
Subject: Re: [patch] remove NPCDISPLAY from AMD64
From: Jonathan Gray 
Date: Tue, June 16, 2020 9:39 pm
To: jo...@armadilloaerospace.com
Cc: "tech@openbsd.org" , j...@openbsd.org

On Tue, Jun 16, 2020 at 07:15:24PM -0700, jo...@armadilloaerospace.com
wrote:
> You can't put an ISA CGA/EGA/MGA in an AMD64 system, so these can
> go away.

While it is incredibly unlikely someone would try, amd64 capable
"industrial" motherboards with ISA exist. We don't build
pcdisplay(4) on amd64 by default however.

> 
> Does anyone know if there is an ordering reason that the coreboot
> efifb_cb_cnattach console is after the VGA attach? Things could be
> cleaned up a bit if the efifb entry points just checked for both
> efi and coreboot framebuffers in the same function.

https://marc.info/?l=openbsd-tech=146609528005661=2

"The cnattach path still has to be different to allow vga to
try to attach before doing the coreboot version"

It seems the coreboot framebuffer support was intended to be a last
resort if vga and efi gop were not available going by the initial mail
when it was proposed as a different driver:

https://marc.info/?l=openbsd-tech=14655876693=2

> 
> 
> Index: wscons_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/wscons_machdep.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 wscons_machdep.c
> --- wscons_machdep.c 14 Oct 2017 04:44:43 - 1.14
> +++ wscons_machdep.c 17 Jun 2020 02:05:34 -
> @@ -35,18 +35,12 @@
> #include 
> 
> #include "vga.h"
> -#include "pcdisplay.h"
> -#if (NVGA > 0) || (NPCDISPLAY > 0)
> +#if (NVGA > 0)
> #include 
> #include 
> -#if (NVGA > 0)
> #include 
> #include 
> #endif
> -#if (NPCDISPLAY > 0)
> -#include 
> -#endif
> -#endif
> 
> #include "wsdisplay.h"
> #if NWSDISPLAY > 0
> @@ -146,10 +140,6 @@ wscn_video_init(void)
> #endif
> #if (NEFIFB > 0)
> if (efifb_cb_cnattach() == 0)
> - return (0);
> -#endif
> -#if (NPCDISPLAY > 0)
> - if (pcdisplay_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM) == 0)
> return (0);
> #endif
> return (-1);
> 
> 
>



Re: [patch] remove NPCDISPLAY from AMD64

2020-06-16 Thread David Riley
On Jun 17, 2020, at 12:39 AM, Jonathan Gray  wrote:
> 
> On Tue, Jun 16, 2020 at 07:15:24PM -0700, jo...@armadilloaerospace.com wrote:
>> You can't put an ISA CGA/EGA/MGA in an AMD64 system, so these can
>> go away.
> 
> While it is incredibly unlikely someone would try, amd64 capable
> "industrial" motherboards with ISA exist.  We don't build
> pcdisplay(4) on amd64 by default however.

I know literally dozens of people (including myself) who would try this.  It 
is, however, extremely unlikely that it'll actually be *required* anywhere.

Is there a specific reason it doesn't work (i.e. architectural features that no 
longer exist on AMD64, like lack of third-party DMA on LPC bus, though I know 
that's not a CGA/EGA/MDA thing), or is it just a highly unlikely scenario?


- Dave



Re: [patch] remove NPCDISPLAY from AMD64

2020-06-16 Thread Jonathan Gray
On Tue, Jun 16, 2020 at 07:15:24PM -0700, jo...@armadilloaerospace.com wrote:
> You can't put an ISA CGA/EGA/MGA in an AMD64 system, so these can
> go away.

While it is incredibly unlikely someone would try, amd64 capable
"industrial" motherboards with ISA exist.  We don't build
pcdisplay(4) on amd64 by default however.

> 
> Does anyone know if there is an ordering reason that the coreboot
> efifb_cb_cnattach console is after the VGA attach? Things could be
> cleaned up a bit if the efifb entry points just checked for both
> efi and coreboot framebuffers in the same function.

https://marc.info/?l=openbsd-tech=146609528005661=2

"The cnattach path still has to be different to allow vga to
try to attach before doing the coreboot version"

It seems the coreboot framebuffer support was intended to be a last
resort if vga and efi gop were not available going by the initial mail
when it was proposed as a different driver:

https://marc.info/?l=openbsd-tech=14655876693=2

> 
> 
> Index: wscons_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/wscons_machdep.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 wscons_machdep.c
> --- wscons_machdep.c  14 Oct 2017 04:44:43 -  1.14
> +++ wscons_machdep.c  17 Jun 2020 02:05:34 -
> @@ -35,18 +35,12 @@
>  #include 
>  
>  #include "vga.h"
> -#include "pcdisplay.h"
> -#if (NVGA > 0) || (NPCDISPLAY > 0)
> +#if (NVGA > 0)
>  #include 
>  #include 
> -#if (NVGA > 0)
>  #include 
>  #include 
>  #endif
> -#if (NPCDISPLAY > 0)
> -#include 
> -#endif
> -#endif
>  
>  #include "wsdisplay.h"
>  #if NWSDISPLAY > 0
> @@ -146,10 +140,6 @@ wscn_video_init(void)
>  #endif
>  #if (NEFIFB > 0)
>   if (efifb_cb_cnattach() == 0)
> - return (0);
> -#endif
> -#if (NPCDISPLAY > 0)
> - if (pcdisplay_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM) == 0)
>   return (0);
>  #endif
>   return (-1);
> 
> 
> 



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread William Ahern
On Tue, Jun 16, 2020 at 06:18:13AM -0600, Todd C. Miller wrote:
> On Tue, 16 Jun 2020 12:48:58 +0200, Martin Pieuchot wrote:
> 
> > The diff below implements DragonFly's approach of adding a new kind of
> > filter, EVFILT_EXCEPT, to report such conditions.  This extends the
> > existing kqueue interface which is questionable.  On the one hand this
> > allows userland programs to use kevent(2) to check for this conditions.
> > One the other hand this is not supported by any other BSD and thus non
> > standard.
> 
> Actually, it looks like macOS uses EVFILT_EXCEPT too.  They were
> the first OS to implement poll in terms of kqueue as far as I know.
> I don't think there is a problem extended kqueue with EVFILT_EXCEPT.

macOS also has EV_OOBAND as an analog to EV_EOF, but the addition caught
people by surprise:

  https://bugs.chromium.org/p/chromium/issues/detail?id=437642
  https://nvd.nist.gov/vuln/detail/CVE-2015-1105

I would guess EV_OOBAND was intended to behave like POLLRDBAND, and
considering that POLLRDBAND is in POLLIN it made some sense to signal
EV_OOBAND by default. Likewis for EV_EOF, which is also always signaled,
though ignoring it is benign. It all seems logical (notwithstanding the
inability to mask it), but only if that was the behavior from day 1.
Surprising people with it doesn't seem like a good idea.



[patch] remove NPCDISPLAY from AMD64

2020-06-16 Thread johnc
You can't put an ISA CGA/EGA/MGA in an AMD64 system, so these can
go away.

Does anyone know if there is an ordering reason that the coreboot
efifb_cb_cnattach console is after the VGA attach? Things could be
cleaned up a bit if the efifb entry points just checked for both
efi and coreboot framebuffers in the same function.


Index: wscons_machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/wscons_machdep.c,v
retrieving revision 1.14
diff -u -p -r1.14 wscons_machdep.c
--- wscons_machdep.c14 Oct 2017 04:44:43 -  1.14
+++ wscons_machdep.c17 Jun 2020 02:05:34 -
@@ -35,18 +35,12 @@
 #include 
 
 #include "vga.h"
-#include "pcdisplay.h"
-#if (NVGA > 0) || (NPCDISPLAY > 0)
+#if (NVGA > 0)
 #include 
 #include 
-#if (NVGA > 0)
 #include 
 #include 
 #endif
-#if (NPCDISPLAY > 0)
-#include 
-#endif
-#endif
 
 #include "wsdisplay.h"
 #if NWSDISPLAY > 0
@@ -146,10 +140,6 @@ wscn_video_init(void)
 #endif
 #if (NEFIFB > 0)
if (efifb_cb_cnattach() == 0)
-   return (0);
-#endif
-#if (NPCDISPLAY > 0)
-   if (pcdisplay_cnattach(X86_BUS_SPACE_IO, X86_BUS_SPACE_MEM) == 0)
return (0);
 #endif
return (-1);




Re: simplify Toeplitz cache computation

2020-06-16 Thread David Gwynne



> On 17 Jun 2020, at 01:57, Theo Buehler  wrote:
> 
> The diff below removes some of the unnecessary complications in the
> calculation of the stoeplitz_cache and brings them into a form more
> suitable for mathematical reasoning. I added a somewhat dense comment
> which explains the full construction and which will help justifying
> upcoming diffs.
> 
> The observations for the code changes are quite simple:

Sure ;)

Actually, reading your new comments in the code makes things a lot clearer to 
me. Thank you.

> First, scache->bytes[val] is a uint16_t, and it's easy to see that we
> only need the lower 16 bits of res in the second nested pair of for
> loops.  The values of key[b] are only xored together, to compute res,
> so we only need the lower 16 bits of those, too.
> 
> Next, looking at the first nested for loop, we see that the values
> 0..15 of j only touch the top 16 bits of key[b], so we can skip them.
> For b = 0, the inner loop for j in 16..31 scans backwards through skey
> and sets the corresponding bits of key[b], so we can see key[0] = skey.
> A bit of pondering then leads to the general expression:
> 
>   key[b] = skey << b | skey >> (NBSK - b);
> 
> I renamed the key array into toeplitz_column since it stores columns of
> the Toeplitz matrix.  If key is considered better, I won't insist.

I'd call it column instead of toeplitz_column or key, but I also don't insist.

> It's not very expensive to brute-force verify that scache->bytes[val]
> remains the same for all values of val and all values of skey. I did
> this on amd64, sparc64 and powerpc.

OK by me.

> 
> Index: sys/net/toeplitz.c
> ===
> RCS file: /var/cvs/src/sys/net/toeplitz.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 toeplitz.c
> --- sys/net/toeplitz.c16 Jun 2020 04:46:49 -  1.1
> +++ sys/net/toeplitz.c16 Jun 2020 15:08:29 -
> @@ -76,40 +76,37 @@ stoeplitz_init(void)
> 
> #define NBSK (NBBY * sizeof(stoeplitz_key))
> 
> +/*
> + * The Toeplitz hash of a 16-bit number considered as a column vector over
> + * the field with two elements is calculated as a matrix multiplication with
> + * a 16x16 circulant Toeplitz matrix T generated by skey.
> + *
> + * The first eight columns H of T generate the remaining eight columns using
> + * the byteswap operation J = swap16:  T = [H JH].  Thus, the Toeplitz hash 
> of
> + * n = [hi lo] is computed via the formula T * n = (H * hi) ^ swap16(H * lo).
> + *
> + * Therefore the results H * val for all values of a byte are cached in 
> scache.
> + */
> void
> stoeplitz_cache_init(struct stoeplitz_cache *scache, stoeplitz_key skey)
> {
> - uint32_t key[NBBY];
> - unsigned int j, b, shift, val;
> + uint16_t toeplitz_column[NBBY];
> + unsigned int b, shift, val;
> 
> - bzero(key, sizeof(key));
> + bzero(toeplitz_column, sizeof(toeplitz_column));
> 
> - /*
> -  * Calculate 32bit keys for one byte; one key for each bit.
> -  */
> - for (b = 0; b < NBBY; ++b) {
> - for (j = 0; j < 32; ++j) {
> - unsigned int bit;
> + /* Calculate the first eight columns H of the Toeplitz matrix T. */
> + for (b = 0; b < NBBY; ++b)
> + toeplitz_column[b] = skey << b | skey >> (NBSK - b);
> 
> - bit = b + j;
> -
> - shift = NBSK - (bit % NBSK) - 1;
> - if (skey & (1 << shift))
> - key[b] |= 1 << (31 - j);
> - }
> - }
> -
> - /*
> -  * Cache the results of all possible bit combination of
> -  * one byte.
> -  */
> + /* Cache the results of H * val for all possible values of a byte. */
>   for (val = 0; val < 256; ++val) {
> - uint32_t res = 0;
> + uint16_t res = 0;
> 
>   for (b = 0; b < NBBY; ++b) {
>   shift = NBBY - b - 1;
>   if (val & (1 << shift))
> - res ^= key[b];
> + res ^= toeplitz_column[b];
>   }
>   scache->bytes[val] = res;
>   }
> 



Re: [PATCH] [xenocara] app/xenodm/config/Xsetup_0 - reduce the number of lines

2020-06-16 Thread Marc Espie
On Tue, Jun 16, 2020 at 08:43:02PM +0100, Raf Czlonka wrote:
> Ping.
> 
> CC'ing espie@ as he committed the initial code.
> 
> Cheers,
> 
> Raf
> 
> On Sun, Jun 07, 2020 at 07:30:39PM BST, Raf Czlonka wrote:
> > Hi all,
> > 
> > I've been running openbsd-backgrounds on all of my desktop machines and
> > thought this can be simplified a bit:
> > 
> > - fewer lines to uncomment
> > - easier to automate, i.e. via one liner, script, config management, etc.
> > - still under 80 columns wide
> > 
> > For your consideration.
> > 
> > Cheers,
> > 
> > Raf
> > 
> > Index: app/xenodm/config/Xsetup_0
> > ===
> > RCS file: /cvs/xenocara/app/xenodm/config/Xsetup_0,v
> > retrieving revision 1.6
> > diff -u -p -r1.6 Xsetup_0
> > --- app/xenodm/config/Xsetup_0  29 Jun 2019 13:33:06 -  1.6
> > +++ app/xenodm/config/Xsetup_0  7 Jun 2020 18:29:16 -
> > @@ -6,9 +6,6 @@ xconsole -geometry 480x130-0-0 -daemon -
> >  #  install package openbsd-backgrounds
> >  #  then uncomment:
> >  #
> > -# if test -x /usr/local/bin/openbsd-wallpaper
> > -# then
> > -#  /usr/local/bin/openbsd-wallpaper
> > -# fi
> > +# test -x /usr/local/bin/openbsd-wallpaper && 
> > /usr/local/bin/openbsd-wallpaper
> >  
> >  # sxpm OpenBSD.xpm &
> 
Even though it's longer, I actually prefer using if  for this kind of thing.



Re: [PATCH] [xenocara] app/xenodm/config/Xsetup_0 - reduce the number of lines

2020-06-16 Thread Raf Czlonka
Ping.

CC'ing espie@ as he committed the initial code.

Cheers,

Raf

On Sun, Jun 07, 2020 at 07:30:39PM BST, Raf Czlonka wrote:
> Hi all,
> 
> I've been running openbsd-backgrounds on all of my desktop machines and
> thought this can be simplified a bit:
> 
> - fewer lines to uncomment
> - easier to automate, i.e. via one liner, script, config management, etc.
> - still under 80 columns wide
> 
> For your consideration.
> 
> Cheers,
> 
> Raf
> 
> Index: app/xenodm/config/Xsetup_0
> ===
> RCS file: /cvs/xenocara/app/xenodm/config/Xsetup_0,v
> retrieving revision 1.6
> diff -u -p -r1.6 Xsetup_0
> --- app/xenodm/config/Xsetup_029 Jun 2019 13:33:06 -  1.6
> +++ app/xenodm/config/Xsetup_07 Jun 2020 18:29:16 -
> @@ -6,9 +6,6 @@ xconsole -geometry 480x130-0-0 -daemon -
>  #  install package openbsd-backgrounds
>  #  then uncomment:
>  #
> -# if test -x /usr/local/bin/openbsd-wallpaper
> -# then
> -#/usr/local/bin/openbsd-wallpaper
> -# fi
> +# test -x /usr/local/bin/openbsd-wallpaper && 
> /usr/local/bin/openbsd-wallpaper
>  
>  # sxpm OpenBSD.xpm &



pkg_info: recommend -aQ over -Q

2020-06-16 Thread Ali Farzanrad
Hi,

Today I was searching for postgresql-server package using pkg_info,
but I didn't found that (OpenBSD-6.7-STABLE)!

$ pkg_info -Q postgres
dovecot-postgresql-2.3.10.1v0

After a bit of research I found the problem in this file:

/usr/libdata/perl5/OpenBSD/PackageRepositoryList.pm:
...
sub match_locations
{
my ($self, @search) = @_;
my $result = [];
for my $repo (@{$self->{l}}) {
my $l = $repo->match_locations(@search);
if ($search[0]->{keep_all}) {
push(@$result, @$l);
} elsif (@$l > 0) {
return $l;
}
}
return $result;
}

which searches for matched packages in packages-stable before packages
url;  so I figured out that I should use -aQ options:

$ pkg_info -aQ postgres
check_postgres-2.25.0
debug-qt5-postgresql-5.13.2
dovecot-postgresql-2.3.10.1v0
dovecot-postgresql-2.3.10v0
kamailio-postgresql-5.0.6p0
orthanc-plugin-postgresql-2.0p2
postgresql-client-12.2
postgresql-contrib-12.2
postgresql-docs-12.2
postgresql-odbc-10.02.p0
postgresql-pg_upgrade-12.2
postgresql-pllua-2.0.4
postgresql-plpython-12.2
postgresql-plr-8.4
postgresql-previous-11.6
postgresql-server-12.2
postgresql_autodoc-1.40p1
qt3-postgresql-3.8p12
qt4-postgresql-4.8.7p7
qt5-postgresql-5.13.2
sope-postgres-4.3.0
tdbc-postgres-1.0.6

Anyway, I recommend following patch for www/faq/faq15.html:

cvs diff: Diffing .
Index: faq15.html
===
RCS file: /cvs/www/faq/faq15.html,v
retrieving revision 1.184
diff -u -p -r1.184 faq15.html
--- faq15.html  14 Aug 2019 13:07:46 -  1.184
+++ faq15.html  16 Jun 2020 19:29:53 -
@@ -148,10 +148,10 @@ To search for any given package name, us
 https://man.openbsd.org/pkg_info;>pkg_info(1).
 
 
-$ pkg_info -Q unzip
-lunzip-1.8
-unzip-6.0p9
-unzip-6.0p9-iconv
+$ pkg_info -aQ unzip
+lunzip-1.11
+unzip-6.0p13
+unzip-6.0p13-iconv
 
 
 Another way to find what you're looking for is with the pkglocate



pipe: reduce number of allocations

2020-06-16 Thread Anton Lindqvist
Hi,
Instead of performing three distinct allocations per created pipe,
reduce it to a single one. Not only should this be more performant, it
also solves a kqueue related issue found by visa@ who also requested
this change:

> If you attach an EVFILT_WRITE filter to a pipe fd, the knote gets added
> to the peer's klist. This is a problem for kqueue  because if you close
> the peer's fd, the knote is left in the list whose head is about to be
> freed. knote_fdclose() is not able to clear the knote because it is not
> registered with the peer's fd.

FreeBSD also takes a similar approach to pipe allocations.

Comments? OK?

Index: kern/sys_pipe.c
===
RCS file: /cvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.120
diff -u -p -r1.120 sys_pipe.c
--- kern/sys_pipe.c 15 Jun 2020 15:29:40 -  1.120
+++ kern/sys_pipe.c 16 Jun 2020 19:07:49 -
@@ -49,6 +49,12 @@
 
 #include 
 
+struct pipe_pair {
+   struct pipe pp_wpipe;
+   struct pipe pp_rpipe;
+   struct rwlock pp_lock;
+};
+
 /*
  * interfaces to the outside world
  */
@@ -103,13 +109,12 @@ const struct filterops pipe_wfiltops = {
 unsigned int nbigpipe;
 static unsigned int amountpipekva;
 
-struct pool pipe_pool;
-struct pool pipe_lock_pool;
+struct pool pipe_pair_pool;
 
 intdopipe(struct proc *, int *, int);
 void   pipeselwakeup(struct pipe *);
 
-struct pipe *pipe_create(void);
+intpipe_create(struct pipe *);
 void   pipe_destroy(struct pipe *);
 intpipe_rundown(struct pipe *);
 struct pipe *pipe_peer(struct pipe *);
@@ -120,6 +125,8 @@ int pipe_iolock(struct pipe *);
 void   pipe_iounlock(struct pipe *);
 intpipe_iosleep(struct pipe *, const char *);
 
+struct pipe_pair *pipe_pair_create(void);
+
 /*
  * The pipe system call for the DTYPE_PIPE type of pipes
  */
@@ -153,33 +160,17 @@ dopipe(struct proc *p, int *ufds, int fl
 {
struct filedesc *fdp = p->p_fd;
struct file *rf, *wf;
+   struct pipe_pair *pp;
struct pipe *rpipe, *wpipe = NULL;
-   struct rwlock *lock;
int fds[2], cloexec, error;
 
cloexec = (flags & O_CLOEXEC) ? UF_EXCLOSE : 0;
 
-   if ((rpipe = pipe_create()) == NULL) {
-   error = ENOMEM;
-   goto free1;
-   }
-
-   /*
-* One lock is used per pipe pair in order to obtain exclusive access to
-* the pipe pair.
-*/
-   lock = pool_get(_lock_pool, PR_WAITOK);
-   rw_init(lock, "pipelk");
-   rpipe->pipe_lock = lock;
-
-   if ((wpipe = pipe_create()) == NULL) {
-   error = ENOMEM;
-   goto free1;
-   }
-   wpipe->pipe_lock = lock;
-
-   rpipe->pipe_peer = wpipe;
-   wpipe->pipe_peer = rpipe;
+   pp = pipe_pair_create();
+   if (pp == NULL)
+   return (ENOMEM);
+   wpipe = >pp_wpipe;
+   rpipe = >pp_rpipe;
 
fdplock(fdp);
 
@@ -226,7 +217,6 @@ free3:
rpipe = NULL;
 free2:
fdpunlock(fdp);
-free1:
pipe_destroy(wpipe);
pipe_destroy(rpipe);
return (error);
@@ -272,19 +262,14 @@ pipe_buffer_realloc(struct pipe *cpipe, 
 /*
  * initialize and allocate VM and memory for pipe
  */
-struct pipe *
-pipe_create(void)
+int
+pipe_create(struct pipe *cpipe)
 {
-   struct pipe *cpipe;
int error;
 
-   cpipe = pool_get(_pool, PR_WAITOK | PR_ZERO);
-
error = pipe_buffer_realloc(cpipe, PIPE_SIZE);
-   if (error != 0) {
-   pool_put(_pool, cpipe);
-   return (NULL);
-   }
+   if (error != 0)
+   return (error);
 
sigio_init(>pipe_sigio);
 
@@ -292,7 +277,7 @@ pipe_create(void)
cpipe->pipe_atime = cpipe->pipe_ctime;
cpipe->pipe_mtime = cpipe->pipe_ctime;
 
-   return (cpipe);
+   return (0);
 }
 
 struct pipe *
@@ -834,7 +819,6 @@ void
 pipe_destroy(struct pipe *cpipe)
 {
struct pipe *ppipe;
-   struct rwlock *lock = NULL;
 
if (cpipe == NULL)
return;
@@ -862,20 +846,13 @@ pipe_destroy(struct pipe *cpipe)
ppipe->pipe_state |= PIPE_EOF;
wakeup(ppipe);
ppipe->pipe_peer = NULL;
-   } else {
-   /*
-* Peer already gone. This is last reference to the pipe lock
-* and it must therefore be freed below.
-*/
-   lock = cpipe->pipe_lock;
}
 
rw_exit_write(cpipe->pipe_lock);
 
pipe_buffer_free(cpipe);
-   if (lock != NULL)
-   pool_put(_lock_pool, lock);
-   pool_put(_pool, cpipe);
+   if (ppipe == NULL)
+   pool_put(_pair_pool, cpipe->pipe_pair);
 }
 
 /*
@@ -1008,8 +985,33 @@ filt_pipewrite(struct knote *kn, long hi
 void
 pipe_init(void)
 {
-   pool_init(_pool, sizeof(struct pipe), 0, IPL_MPFLOOR, PR_WAITOK,
-   "pipepl", NULL);
-   pool_init(_lock_pool, sizeof(struct rwlock), 0, IPL_MPFLOOR,

Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Vitaliy Makkoveev
On Tue, Jun 16, 2020 at 03:10:02PM +0200, Martin Pieuchot wrote:
> On 16/06/20(Tue) 06:18, Todd C. Miller wrote:
> > On Tue, 16 Jun 2020 12:48:58 +0200, Martin Pieuchot wrote:
> > 
> > > The diff below implements DragonFly's approach of adding a new kind of
> > > filter, EVFILT_EXCEPT, to report such conditions.  This extends the
> > > existing kqueue interface which is questionable.  On the one hand this
> > > allows userland programs to use kevent(2) to check for this conditions.
> > > One the other hand this is not supported by any other BSD and thus non
> > > standard.
> > 
> > Actually, it looks like macOS uses EVFILT_EXCEPT too.  They were
> > the first OS to implement poll in terms of kqueue as far as I know.
> > I don't think there is a problem extended kqueue with EVFILT_EXCEPT.
> 
> Interesting, is there any open source code from Apple that you could
> point me at?  I'd be interested to study their kqueue interface.
> 

https://github.com/apple/darwin-xnu



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Todd C . Miller
On Tue, 16 Jun 2020 10:13:11 -0600, "Theo de Raadt" wrote:

> Everytime someone did poll on select, or select on poll, or something
> on something, the emulation ended up being dangerously buggy in the
> first round.

Agreed.

> I hope such emulation isn't a goal in libc.  What happens in the kernel
> back side, tho, could do with some simplication.

No one has proposed emulating poll/select in libc.  That is something
I would object to quite strongly.

 - todd



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Theo de Raadt
Todd C. Miller  wrote:

> On Tue, 16 Jun 2020 16:21:14 +0300, Vitaliy Makkoveev wrote:
> 
> > https://github.com/apple/darwin-xnu
> 
> Note that the poll emulation in xnu was incomplete the last time I
> checked.  Granted, that was 10 years ago so it might be better now,
> but at the time you couldn't poll much more than sockets.  I got
> burned trying to poll tty devices which simply didn't work.  The
> select system call doesn't use kqueue as a backend, though.
> 
> According the the man pages this is still the case:
> 
> BUGS
>  The poll() system call currently does not support devices.

Everytime someone did poll on select, or select on poll, or something
on something, the emulation ended up being dangerously buggy in the
first round.

I hope such emulation isn't a goal in libc.  What happens in the kernel
back side, tho, could do with some simplication.



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Todd C . Miller
On Tue, 16 Jun 2020 16:21:14 +0300, Vitaliy Makkoveev wrote:

> https://github.com/apple/darwin-xnu

Note that the poll emulation in xnu was incomplete the last time I
checked.  Granted, that was 10 years ago so it might be better now,
but at the time you couldn't poll much more than sockets.  I got
burned trying to poll tty devices which simply didn't work.  The
select system call doesn't use kqueue as a backend, though.

According the the man pages this is still the case:

BUGS
 The poll() system call currently does not support devices.

 - todd



simplify Toeplitz cache computation

2020-06-16 Thread Theo Buehler
The diff below removes some of the unnecessary complications in the
calculation of the stoeplitz_cache and brings them into a form more
suitable for mathematical reasoning. I added a somewhat dense comment
which explains the full construction and which will help justifying
upcoming diffs.

The observations for the code changes are quite simple:

First, scache->bytes[val] is a uint16_t, and it's easy to see that we
only need the lower 16 bits of res in the second nested pair of for
loops.  The values of key[b] are only xored together, to compute res,
so we only need the lower 16 bits of those, too.

Next, looking at the first nested for loop, we see that the values
0..15 of j only touch the top 16 bits of key[b], so we can skip them.
For b = 0, the inner loop for j in 16..31 scans backwards through skey
and sets the corresponding bits of key[b], so we can see key[0] = skey.
A bit of pondering then leads to the general expression:

key[b] = skey << b | skey >> (NBSK - b);

I renamed the key array into toeplitz_column since it stores columns of
the Toeplitz matrix.  If key is considered better, I won't insist.

It's not very expensive to brute-force verify that scache->bytes[val]
remains the same for all values of val and all values of skey. I did
this on amd64, sparc64 and powerpc.

Index: sys/net/toeplitz.c
===
RCS file: /var/cvs/src/sys/net/toeplitz.c,v
retrieving revision 1.1
diff -u -p -r1.1 toeplitz.c
--- sys/net/toeplitz.c  16 Jun 2020 04:46:49 -  1.1
+++ sys/net/toeplitz.c  16 Jun 2020 15:08:29 -
@@ -76,40 +76,37 @@ stoeplitz_init(void)
 
 #define NBSK (NBBY * sizeof(stoeplitz_key))
 
+/*
+ * The Toeplitz hash of a 16-bit number considered as a column vector over
+ * the field with two elements is calculated as a matrix multiplication with
+ * a 16x16 circulant Toeplitz matrix T generated by skey.
+ *
+ * The first eight columns H of T generate the remaining eight columns using
+ * the byteswap operation J = swap16:  T = [H JH].  Thus, the Toeplitz hash of
+ * n = [hi lo] is computed via the formula T * n = (H * hi) ^ swap16(H * lo).
+ *
+ * Therefore the results H * val for all values of a byte are cached in scache.
+ */
 void
 stoeplitz_cache_init(struct stoeplitz_cache *scache, stoeplitz_key skey)
 {
-   uint32_t key[NBBY];
-   unsigned int j, b, shift, val;
+   uint16_t toeplitz_column[NBBY];
+   unsigned int b, shift, val;
 
-   bzero(key, sizeof(key));
+   bzero(toeplitz_column, sizeof(toeplitz_column));
 
-   /*
-* Calculate 32bit keys for one byte; one key for each bit.
-*/
-   for (b = 0; b < NBBY; ++b) {
-   for (j = 0; j < 32; ++j) {
-   unsigned int bit;
+   /* Calculate the first eight columns H of the Toeplitz matrix T. */
+   for (b = 0; b < NBBY; ++b)
+   toeplitz_column[b] = skey << b | skey >> (NBSK - b);
 
-   bit = b + j;
-
-   shift = NBSK - (bit % NBSK) - 1;
-   if (skey & (1 << shift))
-   key[b] |= 1 << (31 - j);
-   }
-   }
-
-   /*
-* Cache the results of all possible bit combination of
-* one byte.
-*/
+   /* Cache the results of H * val for all possible values of a byte. */
for (val = 0; val < 256; ++val) {
-   uint32_t res = 0;
+   uint16_t res = 0;
 
for (b = 0; b < NBBY; ++b) {
shift = NBBY - b - 1;
if (val & (1 << shift))
-   res ^= key[b];
+   res ^= toeplitz_column[b];
}
scache->bytes[val] = res;
}



add sosplice(9) regression test

2020-06-16 Thread Vitaliy Makkoveev
sosplice(9) should provide EPROTONOSUPPORT error while we are splicing
sockets with different domains but there is not test for this case. Diff
below adds test for the case while we are thying to splice inet and unix
sockets. We can's splice unix sockets so there is no reason to test the
case for splicing unix and inet sockets.

Index: regress/sys/kern/sosplice/error/args-inet-unix-EPROTONOSUPPORT.pl
===
RCS file: regress/sys/kern/sosplice/error/args-inet-unix-EPROTONOSUPPORT.pl
diff -N regress/sys/kern/sosplice/error/args-inet-unix-EPROTONOSUPPORT.pl
--- /dev/null   1 Jan 1970 00:00:00 -
+++ regress/sys/kern/sosplice/error/args-inet-unix-EPROTONOSUPPORT.pl   16 Jun 
2020 09:24:20 -
@@ -0,0 +1,24 @@
+# test EPROTONOSUPPORT for splicing inet and unix sockets
+
+use strict;
+use warnings;
+use IO::Socket;
+use BSD::Socket::Splice "SO_SPLICE";
+use IO::Socket::UNIX;
+
+our %args = (
+errno => 'EPROTONOSUPPORT',
+func => sub {
+   my $s = IO::Socket::INET->new(
+   Proto => "udp",
+   LocalAddr => "127.0.0.1",
+   ) or die "socket bind failed: $!";
+
+   my $ss = IO::Socket::UNIX->new(
+   Type => SOCK_STREAM,
+   ) or die "socket splice failed: $!";
+
+   $s->setsockopt(SOL_SOCKET, SO_SPLICE, pack('i', $ss->fileno()))
+   and die "splice inet and unix sockets succeeded";
+},
+);



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Martin Pieuchot
On 16/06/20(Tue) 06:18, Todd C. Miller wrote:
> On Tue, 16 Jun 2020 12:48:58 +0200, Martin Pieuchot wrote:
> 
> > The diff below implements DragonFly's approach of adding a new kind of
> > filter, EVFILT_EXCEPT, to report such conditions.  This extends the
> > existing kqueue interface which is questionable.  On the one hand this
> > allows userland programs to use kevent(2) to check for this conditions.
> > One the other hand this is not supported by any other BSD and thus non
> > standard.
> 
> Actually, it looks like macOS uses EVFILT_EXCEPT too.  They were
> the first OS to implement poll in terms of kqueue as far as I know.
> I don't think there is a problem extended kqueue with EVFILT_EXCEPT.

Interesting, is there any open source code from Apple that you could
point me at?  I'd be interested to study their kqueue interface.



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Todd C . Miller
On Tue, 16 Jun 2020 12:48:58 +0200, Martin Pieuchot wrote:

> The diff below implements DragonFly's approach of adding a new kind of
> filter, EVFILT_EXCEPT, to report such conditions.  This extends the
> existing kqueue interface which is questionable.  On the one hand this
> allows userland programs to use kevent(2) to check for this conditions.
> One the other hand this is not supported by any other BSD and thus non
> standard.

Actually, it looks like macOS uses EVFILT_EXCEPT too.  They were
the first OS to implement poll in terms of kqueue as far as I know.
I don't think there is a problem extended kqueue with EVFILT_EXCEPT.

> In the tree there's two poll handlers that set the POLLPRI & POLLRDBAND
> bits as illustrated by the diff below.
>
> Do we see value in this new type of filter?  Should I document it and
> put it in?  Or should I restrict it to the __EV_POLL for now?  In the
> latter case should we pick a different name and/or prefix it?

I think EVFILT_EXCEPT should be exposed to userland.  It is not our
own invention and two other OSes support it.

OK millert@

 - todd



[PATCH]: sysupgrade(8) don't create /home/_sysupgrade/keep

2020-06-16 Thread Martin Vahlensieck
Hi

In the last revision install.sub stopped using /home/_sysupgrade/keep,
so unless I miss something this line can be removed. 

Best,

Martin

Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.37
diff -u -p -r1.37 sysupgrade.sh
--- sysupgrade.sh   26 Jan 2020 22:08:36 -  1.37
+++ sysupgrade.sh   16 Jun 2020 10:40:25 -
@@ -178,8 +178,6 @@ if [[ -n ${DL} ]]; then
unpriv cksum -qC SHA256 ${DL}
 fi
 
-${KEEP} && > keep
-
 cat <<__EOT >/auto_upgrade.conf
 Location of sets = disk
 Pathname to the sets = /home/_sysupgrade/



New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread Martin Pieuchot
The kqueue subsystem has no mechanism to indicate "exceptional conditions",
that is the equivalent of poll(2)'s POLLPRI & POLLRDBAND and select(2)'s
`exceptfds'.

The diff below implements DragonFly's approach of adding a new kind of
filter, EVFILT_EXCEPT, to report such conditions.  This extends the
existing kqueue interface which is questionable.  On the one hand this
allows userland programs to use kevent(2) to check for this conditions.
One the other hand this is not supported by any other BSD and thus non
standard.

In the tree there's two poll handlers that set the POLLPRI & POLLRDBAND
bits as illustrated by the diff below.

Do we see value in this new type of filter?  Should I document it and
put it in?  Or should I restrict it to the __EV_POLL for now?  In the
latter case should we pick a different name and/or prefix it?

Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.139
diff -u -p -r1.139 kern_event.c
--- kern/kern_event.c   15 Jun 2020 15:42:11 -  1.139
+++ kern/kern_event.c   16 Jun 2020 10:32:45 -
@@ -158,6 +158,7 @@ const struct filterops *const sysfilt_op
_filtops,   /* EVFILT_SIGNAL */
_filtops, /* EVFILT_TIMER */
_filtops,  /* EVFILT_DEVICE */
+   _filtops,  /* EVFILT_EXCEPT */
 };
 
 void
Index: kern/tty_pty.c
===
RCS file: /cvs/src/sys/kern/tty_pty.c,v
retrieving revision 1.100
diff -u -p -r1.100 tty_pty.c
--- kern/tty_pty.c  15 Jun 2020 15:29:40 -  1.100
+++ kern/tty_pty.c  16 Jun 2020 10:32:45 -
@@ -107,6 +107,7 @@ voidfilt_ptcrdetach(struct knote *);
 intfilt_ptcread(struct knote *, long);
 void   filt_ptcwdetach(struct knote *);
 intfilt_ptcwrite(struct knote *, long);
+intfilt_ptcexcept(struct knote *, long);
 
 static struct pt_softc **ptyarralloc(int);
 static int check_pty(int);
@@ -719,6 +720,23 @@ filt_ptcwrite(struct knote *kn, long hin
return (kn->kn_data > 0);
 }
 
+int
+filt_ptcexcept(struct knote *kn, long hint)
+{
+   struct pt_softc *pti = (struct pt_softc *)kn->kn_hook;
+   struct tty *tp;
+
+   tp = pti->pt_tty;
+   kn->kn_data = 0;
+
+   /* If in packet or user control mode, check for data. */
+   if (((pti->pt_flags & PF_PKT) && pti->pt_send) ||
+   ((pti->pt_flags & PF_UCNTL) && pti->pt_ucntl))
+   kn->kn_data = 1;
+
+   return (kn->kn_data > 0);
+}
+
 const struct filterops ptcread_filtops = {
.f_flags= FILTEROP_ISFD,
.f_attach   = NULL,
@@ -733,6 +751,13 @@ const struct filterops ptcwrite_filtops 
.f_event= filt_ptcwrite,
 };
 
+const struct filterops ptcexcept_filtops = {
+   .f_flags= FILTEROP_ISFD,
+   .f_attach   = NULL,
+   .f_detach   = filt_ptcrdetach,
+   .f_event= filt_ptcexcept,
+};
+
 int
 ptckqfilter(dev_t dev, struct knote *kn)
 {
@@ -749,6 +774,9 @@ ptckqfilter(dev_t dev, struct knote *kn)
klist = >pt_selw.si_note;
kn->kn_fop = _filtops;
break;
+   case EVFILT_EXCEPT:
+   klist = >pt_selr.si_note;
+   kn->kn_fop = _filtops;
default:
return (EINVAL);
}
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.245
diff -u -p -r1.245 uipc_socket.c
--- kern/uipc_socket.c  15 Jun 2020 15:29:40 -  1.245
+++ kern/uipc_socket.c  16 Jun 2020 10:32:45 -
@@ -71,6 +71,7 @@ int   filt_soread(struct knote *kn, long h
 void   filt_sowdetach(struct knote *kn);
 intfilt_sowrite(struct knote *kn, long hint);
 intfilt_solisten(struct knote *kn, long hint);
+intfilt_soexcept(struct knote *kn, long hint);
 
 const struct filterops solisten_filtops = {
.f_flags= FILTEROP_ISFD,
@@ -93,6 +94,12 @@ const struct filterops sowrite_filtops =
.f_event= filt_sowrite,
 };
 
+const struct filterops soexcept_filtops = {
+   .f_flags= FILTEROP_ISFD,
+   .f_attach   = NULL,
+   .f_detach   = filt_sordetach,
+   .f_event= filt_soexcept,
+};
 
 #ifndef SOMINCONN
 #define SOMINCONN 80
@@ -2026,6 +2033,10 @@ soo_kqfilter(struct file *fp, struct kno
kn->kn_fop = _filtops;
sb = >so_snd;
break;
+   case EVFILT_EXCEPT:
+   kn->kn_fop = _filtops;
+   sb = >so_rcv;
+   break;
default:
return (EINVAL);
}
@@ -2137,6 +2148,21 @@ filt_solisten(struct knote *kn, long hin
if ((hint & NOTE_SUBMIT) == 0)
s = solock(so);
kn->kn_data = so->so_qlen;
+   if ((hint & NOTE_SUBMIT) == 0)
+   

Re: interrupt to cpu mapping API

2020-06-16 Thread Mark Kettenis
> Date: Tue, 16 Jun 2020 11:20:05 +1000
> From: David Gwynne 
> 
> there's been discussions for years (and even some diffs!) about how we
> should let drivers establish interrupts on multiple cpus.
> 
> the simple approach is to let every driver look at the number of cpus in
> a box and just pin an interrupt on it, which is what pretty much
> everyone else started with, but we have never seemed to get past
> bikeshedding about. from what i can tell, the principal objections to
> this are:
> 
> 1. interrupts will tend to land on low numbered cpus.
> 
> ie, if drivers try to establish n interrupts on m cpus, they'll
> start at cpu 0 and go to cpu n, which means cpu 0 will end up with more
> interrupts than cpu m-1. apparently this is terrible, even though
> currently we have all the interrupts on cpu0 anyway and the world
> hasnt ended.
> 
> 2. some cpus shouldn't be used for interrupts.
> 
> why a cpu should or shouldn't be used for interrupts can be pretty
> arbitrary, but in practical terms i'm going to borrow from the scheduler
> and say that we shouldn't run work on hyperthreads. discussions about
> big.little configs and so on can wait.

When hw.smt=0 we should defenotely not have interrupts on the parked
CPUs.

> 3. making all the drivers make the same decisions about the above is
> a lot of maintenance overhead.
> 
> either we will have a bunch of inconsistencies, or we'll have a lot
> of untested commits to keep everything the same.
> 
> my proposed solution to the above is this diff to provide the intrmap
> api. drivers that want to establish multiple interrupts ask the api for
> a set of cpus it can use, and the api considers the above issues when
> generating a set of cpus for the driver to use. drivers then establish
> interrupts on cpus with the info provided by the map.
> 
> it is based on the if_ringmap api in dragonflybsd, but generalised so it
> could be used by something like nvme(4) in the future.
> 
> jmatthew@ and i have been working on implementing a
> pci_intr_establish_cpu() api on a few archs, and tweaking some drivers
> to see if it works out well, and so far the conclusion is "yes, yes it
> does".
> 
> the best example so far is vmx hacked up to establish interrupts on
> multiple cps using the api. i changed the interrupt name string so it
> includes the ring and cpuid it is establishing on. each vmx is also
> limited to 8 rings overall, no matter how big the system is.
> 
> on a machine with 2 vmx interfaces, 16 "real" CPUs, and no hyperthreads,
> the mappings look like this:
> 
> dlg@kbuild ~$ vmstat -zi | grep vmx
> irq114/vmx0 00
> irq115/vmx0:0:0   2075
> irq116/vmx0:1:15   220
> irq117/vmx0:2:14   110
> irq118/vmx0:3:13   240
> irq119/vmx0:4:12   390
> irq120/vmx0:5:1110
> irq121/vmx0:6:10   120
> irq122/vmx0:7:9 70
> irq126/vmx1 00
> irq127/vmx1:0:8 00
> irq128/vmx1:1:7 00
> irq129/vmx1:2:6 00
> irq130/vmx1:3:5 00
> irq131/vmx1:4:4 00
> irq132/vmx1:5:3 00
> irq133/vmx1:6:2 00
> irq134/vmx1:7:1 00
> 
> if you move it to 8 cores and 16 threads:
> 
> dlg@kbuild ~$ sysctl hw.{ncpu,ncpufound,ncpuonline}
> hw.ncpu=16
> hw.ncpufound=16
> hw.ncpuonline=8
> dlg@kbuild ~$ vmstat -zi | grep vmx
> irq114/vmx0 00
> irq115/vmx0:0:0400
> irq116/vmx0:1:14   150
> irq117/vmx0:2:12   330
> irq118/vmx0:3:10   640
> irq119/vmx0:4:8230
> irq120/vmx0:5:6320
> irq121/vmx0:6:4   1371
> irq122/vmx0:7:2   2453
> irq126/vmx1 00
> irq127/vmx1:0:0 00
> irq128/vmx1:1:1400
> irq129/vmx1:2:1200
> irq130/vmx1:3:1000
> irq131/vmx1:4:8 00
> irq132/vmx1:5:6 00
> irq133/vmx1:6:4 00
> irq134/vmx1:7:2 00
> dlg@kbuild ~$ dmesg | grep smt
> cpu0: smt 0, core 0, package 0
> cpu1: smt 1, core 0, package 0
> cpu2: smt 0, core 1, package 0
> cpu3: smt 1, core 1, package 0
> cpu4: smt 0, core 2, package 0
> cpu5: smt 1, core 2, package 0
> cpu6: smt 0, core 3, package 0
> cpu7: smt 1, core 3, package 0
> cpu8: smt 0, core 4, package 0
> cpu9: smt 1, core 4, package 0
> cpu10: smt 0, core 5, package 0
> cpu11: smt 1, core 5, package 0
> cpu12: smt 0, core 6, 

xhci: zero length multi-TRB inbound xfer does not work

2020-06-16 Thread sc . dying
hi,

The function xhci_event_xfer_isoc() of sys/dev/usb/xhci.c at line 954
does not work with zero length multi-TRB inbound transfer.

   949  /*
   950   * If we queued two TRBs for a frame and this is the second TRB,
   951   * check if the first TRB needs accounting since it might not 
have
   952   * raised an interrupt in case of full data received.
   953   */
   954  if ((letoh32(xp->ring.trbs[trb_idx].trb_flags) & 
XHCI_TRB_TYPE_MASK) ==
   955  XHCI_TRB_TYPE_NORMAL) {
   956  frame_idx--;
   957  if (trb_idx == 0)
   958  trb0_idx = xp->ring.ntrb - 2;
   959  else
   960  trb0_idx = trb_idx - 1;
   961  if (xfer->frlengths[frame_idx] == 0) {
   962  xfer->frlengths[frame_idx] = 
XHCI_TRB_LEN(letoh32(
   963  xp->ring.trbs[trb0_idx].trb_status));
   964  }
   965  }
   966  
   967  xfer->frlengths[frame_idx] +=
   968  XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - 
remain;
   969  xfer->actlen += xfer->frlengths[frame_idx];

When a multi-TRB inbound transfer TD completes with transfer length = 0,
the HC should generate two events: 1st event for ISOCH TRB /w ISP|CHAIN
and 2nd event for NORMAL TRB w/ ISP|IOC.
Transfer Length field (it's remain length, actually) of each event is
same as requested length is, i.e., transferred length is 0.
So when the first event raises the frlengths is set to 0 at line 967.
It's correct.
On second event, as the comment describes, xhci.c tries to calculate
the 1st TRB xfer length at lines 954-965. The requested length of
1st TRB is stored into frlengths -- even though the xfer len is 0.

If frlengths = 0, we cannot distinguish the case the first event is
not raised from the case the transferred length is 0.
The frlengths is already 0 so the requested length of 1st TRB is stored.

For example, I applied debug printf [*1], and run
mplayer tv:// for my webcam.
I see...

#25 remain 1024 type 5 origlen 1024 frlengths[25] 0
#25 (omitted) frlen[25] 1024
#26 remain 2048 type 1 origlen 2048 frlengths[25] 1024

These console logs show a 3072 bytes frame is splitted into
two TRBs and got 0 bytes. The first TRB transfers 0 bytes and
the second TRB transfers 0, too, but it results 1024 bytes.

My proposal patch [*2] adds a flag to xhci_xfer that indicates the
TRB processed by xhci.c previously has CHAIN bit, and updates the
frlengths only when that flag is not set.



[*1]
debug printf.
It shows only splitted isochronous TDs.

--- sys/dev/usb/xhci.c.orig Sun Apr  5 10:12:37 2020
+++ sys/dev/usb/xhci.c  Fri May 29 04:13:36 2020
@@ -961,12 +961,23 @@ xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xh
if (xfer->frlengths[frame_idx] == 0) {
xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
xp->ring.trbs[trb0_idx].trb_status));
+   printf("#%d (omitted) frlen[%d] %u\n",
+   trb0_idx, frame_idx, xfer->frlengths[frame_idx]);
}
}
 
xfer->frlengths[frame_idx] +=
XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
xfer->actlen += xfer->frlengths[frame_idx];
+   uint32_t trb_flags = letoh32(xp->ring.trbs[trb_idx].trb_flags);
+   if ((trb_flags & XHCI_TRB_CHAIN) ||
+   (trb_flags & XHCI_TRB_TYPE_MASK) == XHCI_TRB_TYPE_NORMAL) {
+   printf("#%d remain %u type %u origlen %u frlengths[%d] %hu\n",
+   trb_idx, remain,
+   XHCI_TRB_TYPE(trb_flags),
+   XHCI_TRB_LEN(le32toh(xp->ring.trbs[trb_idx].trb_status)),
+   frame_idx, xfer->frlengths[frame_idx]);
+   }
 
if (xx->index != trb_idx)
return (1);

[*2]
patch

--- sys/dev/usb/xhcivar.h.orig  Sun Oct  6 21:19:28 2019
+++ sys/dev/usb/xhcivar.h   Fri May 22 04:19:57 2020
@@ -40,6 +40,7 @@ struct xhci_xfer {
struct usbd_xfer xfer;
int  index; /* Index of the last TRB */
size_t   ntrb;  /* Number of associated TRBs */
+   bool trb_chained;
 };
 
 struct xhci_ring {
--- sys/dev/usb/xhci.c.orig Sun Apr  5 10:12:37 2020
+++ sys/dev/usb/xhci.c  Thu Jun  4 05:39:03 2020
@@ -958,15 +958,23 @@ xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xh
trb0_idx = xp->ring.ntrb - 2;
else
trb0_idx = trb_idx - 1;
-   if (xfer->frlengths[frame_idx] == 0) {
+   if (xfer->frlengths[frame_idx] == 0 && !xx->trb_chained) {
xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
xp->ring.trbs[trb0_idx].trb_status));
}