Re: pci_sdhc: Intel eMMC controller fix
On 11/20/19, Patrick Wildt wrote: > > A bit late, but committed, thanks! By the way, now that we > have your glkgpio(4), does that make the SD controller work? > > Patrick > Thanks. The SD slot does not work yet. Needs ACPI gpio bits in pci frontend for card detect.
Re: iked(8): Don't change SA state before message is validated
On Thu, Nov 21, 2019 at 01:15:50PM +0100, Tobias Heider wrote:
> Hi,
>
> this is a somewhat bigger diff that tries to separate the
> state-changing message handling parts from the message parsing in iked.
>
> Touching the SA structure in the parser is generally a bad idea
> as it means we're changing global state before the message
> is fully validated (or decrypted/integrity checked).
> The main offenders in iked are the "Notify" and "Certificate Request"
> payloads.
> With the diff, the parsed message content is attached to the
> iked_message structure (temporarily) and the SA (global state)
> is modified only when the full message is considered valid.
>
> This fixes a few problems:
> Broken message could change global state, now they only change state
> when we consider them not broken.
> The evaluation of Notify/Certreq contents depended on the order
> of the payloads, which is problematic because IKEv2 does not put any
> restrictions on the order of payloads. With the diff applied,
> message contents are evaluated in a fixed order.
>
> Functionally there should be no difference, with or without the diff,
> in use-cases that have worked before and I haven't found any regressions
> in my tests. I would appreciate if people running iked could apply and
> test this diff in their setups, just to be sure.
>
> Tobias
sthen@ ran into a problem because I limited the number of CERTREQs
to 1 + ocsp (which apparently is not enough).
Here is an updated diff which allows multiple CERTREQs.
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index bc64f5b3a7b..60122a7f639 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -497,6 +497,13 @@ RB_HEAD(iked_sas, iked_sa);
RB_HEAD(iked_addrpool, iked_sa);
RB_HEAD(iked_addrpool6, iked_sa);
+struct iked_certreq {
+ struct ibuf *cr_data;
+ uint8_t cr_type;
+ SLIST_ENTRY(iked_certreq)cr_entry;
+};
+SLIST_HEAD(iked_certreqs, iked_certreq);
+
struct iked_message {
struct ibuf *msg_data;
size_t msg_offset;
@@ -529,6 +536,7 @@ struct iked_message {
/* Parsed information */
struct iked_proposalsmsg_proposals;
+ struct iked_certreqs msg_certreqs;
struct iked_spi msg_rekey;
struct ibuf *msg_nonce; /* dh NONCE */
uint16_t msg_dhgroup; /* dh group */
@@ -537,6 +545,10 @@ struct iked_message {
struct iked_id msg_id;
struct iked_id msg_cert;
struct ibuf *msg_cookie;
+ uint16_t msg_group;
+ uint16_t msg_cpi;
+ uint8_t msg_transform;
+ uint16_t msg_flags;
/* MOBIKE */
int msg_update_sa_addresses;
@@ -554,6 +566,19 @@ struct iked_message {
#define IKED_RETRANSMIT_TRIES 5 /* try 5 times */
};
+#define IKED_MSG_NAT_SRC_IP0x01
+#define IKED_MSG_NAT_DST_IP0x02
+
+#define IKED_MSG_FLAGS_FRAGMENTATION 0x0001
+#define IKED_MSG_FLAGS_MOBIKE 0x0002
+#define IKED_MSG_FLAGS_SIGSHA2 0x0004
+#define IKED_MSG_FLAGS_CHILD_SA_NOT_FOUND 0x0008
+#define IKED_MSG_FLAGS_NO_ADDITIONAL_SAS 0x0010
+#define IKED_MSG_FLAGS_AUTHENTICATION_FAILED 0x0020
+#define IKED_MSG_FLAGS_INVALID_KE 0x0040
+#define IKED_MSG_FLAGS_IPCOMP_SUPPORTED0x0080
+
+
struct iked_user {
char usr_name[LOGIN_NAME_MAX];
char usr_pass[IKED_PASSWORD_SIZE];
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index e4d37665368..90bedf91abd 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -82,6 +82,8 @@ intikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
int ikev2_send_init_error(struct iked *, struct iked_message *);
+int ikev2_handle_certreq(struct iked*, struct iked_message *);
+
int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
int ikev2_ikesa_enable(struct iked *, struct iked_sa *, struct iked_sa *);
@@ -118,6 +120,8 @@ int ikev2_match_proposals(struct iked_proposal *, struct
iked_proposal *,
int ikev2_valid_proposal(struct iked_proposal *,
struct iked_transform **, struct iked_transform **, int *);
+int ikev2_handle_notifies(struct iked *, struct iked_message *);
+
ssize_t ikev2_add_proposals(struct iked *, struct iked_sa *, struct
ibuf *,
struct iked_proposals *, uint8_t, int, int, int);
ssize_t ikev2_add_cp(struct iked *, struct iked_sa *, struct ibuf *);
@@ -845,6 +849,9 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
if (!ikev2_msg_frompeer(msg))
Re: ptrace(2) errnos
> Date: Wed, 20 Nov 2019 13:49:33 +0100
> From: Martin Pieuchot
>
> Two fixes to reduce difference with other BSDs and make NetBSD's
> t_ptrace.c regress pass:
>
> - Return EBUSY when calling PT_TRACE_ME more than once, both FreeBSD and
> NetBSD do that.
fine
> - Return EPERM instead of EINVAL when trying to trace a parent, this is
> what NetBSD does. Alternatively I could keep EINVAL, like FreeBSD, and
> adapt the regression test.
but I'd prefer to keep EINVAL here.
> Index: /sys/kern/sys_process.c
> ===
> RCS file: /cvs/src/sys/kern/sys_process.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 sys_process.c
> --- /sys/kern/sys_process.c 19 Feb 2018 09:25:13 - 1.80
> +++ /sys/kern/sys_process.c 20 Nov 2019 12:43:39 -
> @@ -282,6 +282,8 @@ ptrace_ctrl(struct proc *p, int req, pid
> case PT_TRACE_ME:
> /* Just set the trace flag. */
> tr = p->p_p;
> + if (ISSET(tr->ps_flags, PS_TRACED))
> + return EBUSY;
> atomic_setbits_int(&tr->ps_flags, PS_TRACED);
> tr->ps_oppid = tr->ps_pptr->ps_pid;
> if (tr->ps_ptstat == NULL)
> @@ -402,7 +404,7 @@ ptrace_ctrl(struct proc *p, int req, pid
>* the process graph).
>*/
> if (tr->ps_pid != 1 && inferior(p->p_p, tr)) {
> - error = EINVAL;
> + error = EPERM;
> goto fail;
> }
> }
>
>
Re: ldomctl, ldomd: Honour DEBUG in Makefile
> From: "Theo de Raadt"
> Date: Tue, 19 Nov 2019 22:32:39 -0700
>
> Seems accidentally left over from development. As a rule we don't
> ship with -g.
Indeed. I don't see why these should be different.
> Klemens Nanni wrote:
>
> > As of now, `export DEBUG=... ; make' will ignore my flags.
> >
> > I guess kettenis wanted symbols by default, so leave a default DEBUG
> > value that allows overwriting instead of omitting DEBUG as usual.
> >
> > OK?
> >
> >
> > Index: ldomctl/Makefile
> > ===
> > RCS file: /cvs/src/usr.sbin/ldomctl/Makefile,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 Makefile
> > --- ldomctl/Makefile28 Jul 2019 15:30:45 - 1.9
> > +++ ldomctl/Makefile19 Nov 2019 22:16:19 -
> > @@ -8,7 +8,7 @@ PROG= ldomctl
> > SRCS= ldomctl.c ds.c mdesc.c util.c mdstore.c pri.c config.c parse.y
> > CFLAGS+=-Wall
> > CFLAGS+=-I${.CURDIR}/../ldomd -I${.CURDIR}
> > -DEBUG= -g
> > +DEBUG?=-g
> >
> > .else
> >
> > Index: ldomd/Makefile
> > ===
> > RCS file: /cvs/src/usr.sbin/ldomd/Makefile,v
> > retrieving revision 1.5
> > diff -u -p -r1.5 Makefile
> > --- ldomd/Makefile 28 Jul 2019 15:30:45 - 1.5
> > +++ ldomd/Makefile 19 Nov 2019 22:16:23 -
> > @@ -8,7 +8,7 @@ PROG= ldomd
> > SRCS= ldomd.c ds.c mdesc.c util.c var-config.c
> > CFLAGS+=-Wall
> > CFLAGS+=-I${.CURDIR}/../ldomctl
> > -DEBUG= -g
> > +DEBUG?=-g
> >
> > .else
> >
> >
>
iked(8): Don't change SA state before message is validated
Hi,
this is a somewhat bigger diff that tries to separate the
state-changing message handling parts from the message parsing in iked.
Touching the SA structure in the parser is generally a bad idea
as it means we're changing global state before the message
is fully validated (or decrypted/integrity checked).
The main offenders in iked are the "Notify" and "Certificate Request"
payloads.
With the diff, the parsed message content is attached to the
iked_message structure (temporarily) and the SA (global state)
is modified only when the full message is considered valid.
This fixes a few problems:
Broken message could change global state, now they only change state
when we consider them not broken.
The evaluation of Notify/Certreq contents depended on the order
of the payloads, which is problematic because IKEv2 does not put any
restrictions on the order of payloads. With the diff applied,
message contents are evaluated in a fixed order.
Functionally there should be no difference, with or without the diff,
in use-cases that have worked before and I haven't found any regressions
in my tests. I would appreciate if people running iked could apply and
test this diff in their setups, just to be sure.
Tobias
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index bc64f5b3a7b..c2429daf7a3 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -537,6 +537,14 @@ struct iked_message {
struct iked_id msg_id;
struct iked_id msg_cert;
struct ibuf *msg_cookie;
+ uint16_t msg_group;
+ uint16_t msg_cpi;
+ uint8_t msg_transform;
+ uint16_t msg_flags;
+
+ uint8_t msg_certreq_type; /* CR Encoding*/
+ struct ibuf *msg_certreq_auth; /* CR Authority */
+ struct ibuf *msg_certreq_ocsp; /* CR OCSP values */
/* MOBIKE */
int msg_update_sa_addresses;
@@ -554,6 +562,19 @@ struct iked_message {
#define IKED_RETRANSMIT_TRIES 5 /* try 5 times */
};
+#define IKED_MSG_NAT_SRC_IP0x01
+#define IKED_MSG_NAT_DST_IP0x02
+
+#define IKED_MSG_FLAGS_FRAGMENTATION 0x0001
+#define IKED_MSG_FLAGS_MOBIKE 0x0002
+#define IKED_MSG_FLAGS_SIGSHA2 0x0004
+#define IKED_MSG_FLAGS_CHILD_SA_NOT_FOUND 0x0008
+#define IKED_MSG_FLAGS_NO_ADDITIONAL_SAS 0x0010
+#define IKED_MSG_FLAGS_AUTHENTICATION_FAILED 0x0020
+#define IKED_MSG_FLAGS_INVALID_KE 0x0040
+#define IKED_MSG_FLAGS_IPCOMP_SUPPORTED0x0080
+
+
struct iked_user {
char usr_name[LOGIN_NAME_MAX];
char usr_pass[IKED_PASSWORD_SIZE];
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index bd22bda0255..3b4300ca7b3 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -82,6 +82,8 @@ intikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
int ikev2_send_init_error(struct iked *, struct iked_message *);
+int ikev2_handle_certreq(struct iked*, struct iked_message *);
+
int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
int ikev2_ikesa_enable(struct iked *, struct iked_sa *, struct iked_sa *);
@@ -118,6 +120,8 @@ int ikev2_match_proposals(struct iked_proposal *, struct
iked_proposal *,
int ikev2_valid_proposal(struct iked_proposal *,
struct iked_transform **, struct iked_transform **, int *);
+int ikev2_handle_notifies(struct iked *, struct iked_message *);
+
ssize_t ikev2_add_proposals(struct iked *, struct iked_sa *, struct
ibuf *,
struct iked_proposals *, uint8_t, int, int, int);
ssize_t ikev2_add_cp(struct iked *, struct iked_sa *, struct ibuf *);
@@ -845,6 +849,9 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
if (!ikev2_msg_frompeer(msg))
return;
+ if (ikev2_handle_notifies(env, msg) != 0)
+ return;
+
if (sa && msg->msg_nat_detected && sa->sa_natt == 0 &&
(sock = ikev2_msg_getsocket(env,
sa->sa_local.addr_af, 1)) != NULL) {
@@ -881,6 +888,9 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
__func__));
break;
}
+ if (ikev2_handle_certreq(env, msg) != 0)
+ return;
+
(void)ikev2_init_auth(env, msg);
break;
case IKEV2_EXCHANGE_IKE_AUTH:
@@ -2354,6 +2364,9 @@ ikev2_resp_recv(struct iked *env, struct iked_message
*msg,
if (!ikev2_msg_frompeer(msg))
return;
+ if (ikev2_handle_notifies(env, msg) != 0)
+ return;
Re: sysupgrade: Allow to use another directory for the sets
On Wed, Nov 20, 2019 at 12:06:59PM +0100, Solene Rapenne wrote:
> On Tue, Nov 12, 2019 at 07:02:56PM +0100, Renaud Allard wrote:
> >
> >
> > On 12/11/2019 08:29, Theo de Raadt wrote:
> > >
> > > Renaud, please test it for me like this:
> > >
> > > sysupgrade -d /
> > >
> > > This interface is dangerously incorrect.
> > >
> >
> > What about this one?
>
> > Index: sysupgrade.8
> > ===
> > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> > retrieving revision 1.10
> > diff -u -p -r1.10 sysupgrade.8
> > --- sysupgrade.83 Oct 2019 12:43:58 - 1.10
> > +++ sysupgrade.812 Nov 2019 18:01:04 -
> > @@ -24,6 +24,7 @@
> > .Nm
> > .Op Fl fkn
> > .Op Fl r | s
> > +.Op Fl d Ar directory
> > .Op Ar installurl
> > .Sh DESCRIPTION
> > .Nm
> > @@ -48,6 +49,13 @@ triggering a one-shot upgrade using the
> > .Pp
> > The options are as follows:
> > .Bl -tag -width Ds
> > +.It Fl d Ar directory
> > +Choose the prefix of the
> > +.Ar directory
> > +in which the sets will be downloaded.
> > +_sysupgrade will be appended to that name.
> > +Default is
> > +.Pa /home .
> > .It Fl f
> > Force an already applied upgrade.
> > The default is to upgrade to latest snapshot only if available.
> > Index: sysupgrade.sh
> > ===
> > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> > retrieving revision 1.32
> > diff -u -p -r1.32 sysupgrade.sh
> > --- sysupgrade.sh 11 Nov 2019 18:26:52 - 1.32
> > +++ sysupgrade.sh 12 Nov 2019 18:01:04 -
> > @@ -25,7 +25,6 @@ umask 0022
> > export PATH=/usr/bin:/bin:/usr/sbin:/sbin
> >
> > ARCH=$(uname -m)
> > -SETSDIR=/home/_sysupgrade
> >
> > ug_err()
> > {
> > @@ -34,7 +33,7 @@ ug_err()
> >
> > usage()
> > {
> > - ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> > + ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-d directory] [installurl]"
> > }
> >
> > unpriv()
> > @@ -73,14 +72,16 @@ rmel() {
> > echo -n "$_c"
> > }
> >
> > +SETSDIR=/home/_sysupgrade
> > RELEASE=false
> > SNAP=false
> > FORCE=false
> > KEEP=false
> > REBOOT=true
> >
> > -while getopts fknrs arg; do
> > +while getopts d:fknrs arg; do
> > case ${arg} in
> > + d) SETSDIR=${OPTARG}/_sysupgrade;;
> > f) FORCE=true;;
> > k) KEEP=true;;
> > n) REBOOT=false;;
> > @@ -195,7 +196,7 @@ ${KEEP} && > keep
> >
> > cat <<__EOT >/auto_upgrade.conf
> > Location of sets = disk
> > -Pathname to the sets = /home/_sysupgrade/
> > +Pathname to the sets = ${SETSDIR}
> > Set name(s) = done
> > Directory does not contain SHA256.sig. Continue without verification = yes
> > __EOT
> > @@ -203,7 +204,7 @@ __EOT
> > if ! ${KEEP}; then
> > CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
> > cat <<__EOT > /etc/rc.firsttime
> > -rm -f /home/_sysupgrade/{${CLEAN}}
> > +rm -f ${SETSDIR}/{${CLEAN}}
> > __EOT
> > fi
> >
>
> I see no objection to this diff. Changes are minimal and it allows using
> another destination safely (_sysupgrade gets appended to the chosen base
> directory)
>
> ok solene@
I am for this feature, but think the original proposal to have the -d
argument define the directory and not the prefix was better,
if it could be made safe enough.
The recognized bad case as I understand it is to specify -d / which will
erase 3 essential kernels from the upgraded installation.
I suggest to use readlink -f to normalize the argument and check that it is
not / and the question is if that is a sufficient safety measure.
If so, here is Renaud Allard's original proposal with that added
argument check instead.
I also moved the creation of the download directory (mkdir -p ${SETSDIR})
to before checking the directory properties, since it feels safer since
if the target directory does not exist there is no check on the created
directory. If the parent directory has got the wrong owner, group,
or permissions, the created target directory might not pass the checks
for an existing traget directory.
Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.10
diff -u -u -r1.10 sysupgrade.8
--- sysupgrade.83 Oct 2019 12:43:58 - 1.10
+++ sysupgrade.821 Nov 2019 09:07:33 -
@@ -24,6 +24,7 @@
.Nm
.Op Fl fkn
.Op Fl r | s
+.Op Fl d Ar directory
.Op Ar installurl
.Sh DESCRIPTION
.Nm
@@ -32,9 +33,7 @@
to the next release or a new snapshot if available.
.Pp
.Nm
-downloads the necessary files to
-.Pa /home/_sysupgrade ,
-verifies them with
+downloads the necessary files, verifies them with
.Xr signify 1 ,
and copies bsd.rd to
.Pa /bsd.upgrade .
@@ -43,8 +42,7 @@
by default then reboots the system.
The bootloader will automatically choose
.Pa /bsd.upgrade ,
-triggering a one-shot upgrade using the files in
-.Pa /home/_sysupgrade .
+triggering
Re: XHCI support for bulk xfers >64k
On 2019/11/21 07:59, Patrick Wildt wrote: > On Thu, Nov 21, 2019 at 06:52:36AM +, sc.dy...@gmail.com wrote: >> Hi, >> >> On 2019/11/18 20:34, Marcus Glocker wrote: >>> On Sun, Nov 17, 2019 at 12:36:49PM +0100, Marcus Glocker wrote: >>> While recently testing UDL on XHCI I faced a lot of USB timeouts, and therefore screen rendering issues. The reason is that XHCI currently only supports single bulk transfers up to 64k, while UDL can schedule a bulk transfer up to 1m. The following diff adds XHCI bulk transfer support >64k and makes UDL work fine for me on XHCI. mpi@ already did an initial re-view, and I have adapted some of his comments already in this diff. More testing and comments welcome. >>> >>> After patrick@ did feedback that the current bulk transfer code in >>> XHCI should also work for larger transfers as is, we have found the >>> bug which resulted in a much smaller diff :-) >>> >>> Attached for your reference. It just has been committed. >>> >>> >>> Index: xhci.c >>> === >>> RCS file: /cvs/src/sys/dev/usb/xhci.c,v >>> retrieving revision 1.106 >>> diff -u -p -u -p -r1.106 xhci.c >>> --- xhci.c 6 Oct 2019 17:30:00 - 1.106 >>> +++ xhci.c 18 Nov 2019 19:32:30 - >>> @@ -2871,7 +2871,7 @@ xhci_device_generic_start(struct usbd_xf >>> /* If the buffer crosses a 64k boundary, we need one more. */ >>> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >>> if (len < xfer->length) >>> - ntrb++; >>> + ntrb = howmany(xfer->length - len, XHCI_TRB_MAXSIZE) + 1; >>> else >>> len = xfer->length; >>> >>> >> >> Should xhci_device_isoc_start also count up the chunks after boundary? >> > > We had discussed this as well, but since a frame cannot be bigger than > 64k, a hard boundary which is visible when you realize frlengths is a > uint16_t *, that change would essentially be a no-op. Oh, I missed it. Thank you for explanation. I see, xhci_event_xfer_isoc does not need fix. > > We could change it for the sake of having the same calculation in all > places. > > Thanks, > Patrick > >> >> --- sys/dev/usb/xhci.c.orig Mon Nov 18 22:47:25 2019 >> +++ sys/dev/usb/xhci.c Thu Nov 21 05:13:02 2019 >> @@ -3049,7 +3049,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) >> /* If the buffer crosses a 64k boundary, we need one more. */ >> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >> if (len < xfer->frlengths[i]) >> -ntrb++; >> +ntrb = howmany(xfer->frlengths[i] - len, >> +XHCI_TRB_MAXSIZE) + 1; >> >> paddr += xfer->frlengths[i]; >> } >> @@ -3066,7 +3067,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) >> /* If the buffer crosses a 64k boundary, we need one more. */ >> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >> if (len < xfer->frlengths[i]) >> -ntrb++; >> +ntrb = howmany(xfer->frlengths[i] - len, >> +XHCI_TRB_MAXSIZE) + 1; >> else >> len = xfer->frlengths[i]; >> >>
Re: XHCI support for bulk xfers >64k
On Thu, Nov 21, 2019 at 06:52:36AM +, sc.dy...@gmail.com wrote: > Hi, > > On 2019/11/18 20:34, Marcus Glocker wrote: > > On Sun, Nov 17, 2019 at 12:36:49PM +0100, Marcus Glocker wrote: > > > >> While recently testing UDL on XHCI I faced a lot of USB timeouts, and > >> therefore screen rendering issues. > >> The reason is that XHCI currently only supports single bulk transfers > >> up to 64k, while UDL can schedule a bulk transfer up to 1m. > >> > >> The following diff adds XHCI bulk transfer support >64k and makes UDL > >> work fine for me on XHCI. > >> > >> mpi@ already did an initial re-view, and I have adapted some of his > >> comments already in this diff. > >> > >> More testing and comments welcome. > > > > After patrick@ did feedback that the current bulk transfer code in > > XHCI should also work for larger transfers as is, we have found the > > bug which resulted in a much smaller diff :-) > > > > Attached for your reference. It just has been committed. > > > > > > Index: xhci.c > > === > > RCS file: /cvs/src/sys/dev/usb/xhci.c,v > > retrieving revision 1.106 > > diff -u -p -u -p -r1.106 xhci.c > > --- xhci.c 6 Oct 2019 17:30:00 - 1.106 > > +++ xhci.c 18 Nov 2019 19:32:30 - > > @@ -2871,7 +2871,7 @@ xhci_device_generic_start(struct usbd_xf > > /* If the buffer crosses a 64k boundary, we need one more. */ > > len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); > > if (len < xfer->length) > > - ntrb++; > > + ntrb = howmany(xfer->length - len, XHCI_TRB_MAXSIZE) + 1; > > else > > len = xfer->length; > > > > > > Should xhci_device_isoc_start also count up the chunks after boundary? > We had discussed this as well, but since a frame cannot be bigger than 64k, a hard boundary which is visible when you realize frlengths is a uint16_t *, that change would essentially be a no-op. We could change it for the sake of having the same calculation in all places. Thanks, Patrick > > --- sys/dev/usb/xhci.c.orig Mon Nov 18 22:47:25 2019 > +++ sys/dev/usb/xhci.cThu Nov 21 05:13:02 2019 > @@ -3049,7 +3049,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) > /* If the buffer crosses a 64k boundary, we need one more. */ > len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); > if (len < xfer->frlengths[i]) > - ntrb++; > + ntrb = howmany(xfer->frlengths[i] - len, > + XHCI_TRB_MAXSIZE) + 1; > > paddr += xfer->frlengths[i]; > } > @@ -3066,7 +3067,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) > /* If the buffer crosses a 64k boundary, we need one more. */ > len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); > if (len < xfer->frlengths[i]) > - ntrb++; > + ntrb = howmany(xfer->frlengths[i] - len, > + XHCI_TRB_MAXSIZE) + 1; > else > len = xfer->frlengths[i]; > >
