Re: occasional SSIGSEGV on C++ exception handling
Den tis 23 feb. 2021 kl 08:48 skrev Theo de Raadt : > > > > > > No problem, real-life often takes precedence. > > > > > > > > > > No way! operator(7) would need an update! > > > > > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > > > > We document it! > > > > > > real life is not a C operator > > > > Well I guess that depends on how one lives their life, you may need to > > C an operator now and then. > > Operator, I need an exit fast. Might not be that simple, as seen in the last picture of this archived image parody of Matrix from looong ago: https://attrition.org/archive/www.detonate.net/matrix-orig/11.htm -- May the most significant bit of your life be positive.
Re: occasional SSIGSEGV on C++ exception handling
Bryan Steele wrote: > On Tue, Feb 23, 2021 at 06:23:22PM +1100, Jonathan Gray wrote: > > On Tue, Feb 23, 2021 at 08:10:54AM +0100, Otto Moerbeek wrote: > > > On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > > > > > > > > > > No problem, real-life often takes precedence. > > > > > > > > No way! operator(7) would need an update! > > > > > > > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > > > We document it! > > > > > > -Otto > > > > real life is not a C operator > > Well I guess that depends on how one lives their life, you may need to > C an operator now and then. Operator, I need an exit fast.
Re: occasional SSIGSEGV on C++ exception handling
On Tue, Feb 23, 2021 at 06:23:22PM +1100, Jonathan Gray wrote: > On Tue, Feb 23, 2021 at 08:10:54AM +0100, Otto Moerbeek wrote: > > On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > > > > > > > No problem, real-life often takes precedence. > > > > > > No way! operator(7) would need an update! > > > > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > > We document it! > > > > -Otto > > real life is not a C operator That's the bug I'm describing. > > > > > Index: operator.7 > > === > > RCS file: /cvs/src/share/man/man7/operator.7,v > > retrieving revision 1.11 > > diff -u -p -r1.11 operator.7 > > --- operator.7 21 Jun 2019 02:28:34 - 1.11 > > +++ operator.7 23 Feb 2021 07:09:10 - > > @@ -57,3 +57,5 @@ > > .It "\&," Ta "left to right" > > .El > > .Ed > > +.Sh BUGS > > +Often real life takes precedence. > > > > >
Re: occasional SSIGSEGV on C++ exception handling
On Tue, Feb 23, 2021 at 06:23:22PM +1100, Jonathan Gray wrote: > On Tue, Feb 23, 2021 at 08:10:54AM +0100, Otto Moerbeek wrote: > > On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > > > > > > > No problem, real-life often takes precedence. > > > > > > No way! operator(7) would need an update! > > > > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > > We document it! > > > > -Otto > > real life is not a C operator Well I guess that depends on how one lives their life, you may need to C an operator now and then. > > > > Index: operator.7 > > === > > RCS file: /cvs/src/share/man/man7/operator.7,v > > retrieving revision 1.11 > > diff -u -p -r1.11 operator.7 > > --- operator.7 21 Jun 2019 02:28:34 - 1.11 > > +++ operator.7 23 Feb 2021 07:09:10 - > > @@ -57,3 +57,5 @@ > > .It "\&," Ta "left to right" > > .El > > .Ed > > +.Sh BUGS > > +Often real life takes precedence. > > > > > >
Re: occasional SSIGSEGV on C++ exception handling
On Tue, Feb 23, 2021 at 08:10:54AM +0100, Otto Moerbeek wrote: > On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > > > > No problem, real-life often takes precedence. > > > > No way! operator(7) would need an update! > > > > What do we do when we see a bug? We fix it! What if it is not fixable? > We document it! > > -Otto real life is not a C operator > > Index: operator.7 > === > RCS file: /cvs/src/share/man/man7/operator.7,v > retrieving revision 1.11 > diff -u -p -r1.11 operator.7 > --- operator.721 Jun 2019 02:28:34 - 1.11 > +++ operator.723 Feb 2021 07:09:10 - > @@ -57,3 +57,5 @@ > .It "\&," Ta "left to right" > .El > .Ed > +.Sh BUGS > +Often real life takes precedence. > >
Re: occasional SSIGSEGV on C++ exception handling
On Mon, Feb 22, 2021 at 08:58:07PM -, Miod Vallat wrote: > > > No problem, real-life often takes precedence. > > No way! operator(7) would need an update! > What do we do when we see a bug? We fix it! What if it is not fixable? We document it! -Otto Index: operator.7 === RCS file: /cvs/src/share/man/man7/operator.7,v retrieving revision 1.11 diff -u -p -r1.11 operator.7 --- operator.7 21 Jun 2019 02:28:34 - 1.11 +++ operator.7 23 Feb 2021 07:09:10 - @@ -57,3 +57,5 @@ .It "\&," Ta "left to right" .El .Ed +.Sh BUGS +Often real life takes precedence.
Re: /bin/cp: overwrite symlink with file pointed to by symlink
> Hi Quentin, Hi Cristopher, > Thank you for having a look. You're welcome! > I know I'm very late, but I still like your patch. So ok chrisz@ if you > want to commit it. It didn't raise much passion at the time, we can just hope it gets a bit more attention now!
Re: uvm_fault_lower refactoring
> Date: Mon, 22 Feb 2021 10:10:21 +0100
> From: Martin Pieuchot
>
> On 16/02/21(Tue) 11:20, Martin Pieuchot wrote:
> > Start by moving `pgo_fault' handler outside of uvm_fault_lower().
> >
> > If a page has a backing object that prefer to handler to fault itself
> > the locking will be different, so keep it under KERNEL_LOCK() for the
> > moment and make it separate from the rest of uvm_fault_lower().
> >
> > ok?
>
> Anyone?
This diverges from NetBSD; you think that's a good idea?
> > Index: uvm/uvm_fault.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> > retrieving revision 1.115
> > diff -u -p -r1.115 uvm_fault.c
> > --- uvm/uvm_fault.c 16 Feb 2021 09:10:17 - 1.115
> > +++ uvm/uvm_fault.c 16 Feb 2021 10:13:58 -
> > @@ -598,10 +598,37 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
> > /* case 1: fault on an anon in our amap */
> > error = uvm_fault_upper(, , anons, fault_type);
> > } else {
> > - /* case 2: fault on backing object or zero fill */
> > - KERNEL_LOCK();
> > - error = uvm_fault_lower(, , pages, fault_type);
> > - KERNEL_UNLOCK();
> > + struct uvm_object *uobj = ufi.entry->object.uvm_obj;
> > +
> > + /*
> > +* if the desired page is not shadowed by the amap and
> > +* we have a backing object, then we check to see if
> > +* the backing object would prefer to handle the fault
> > +* itself (rather than letting us do it with the usual
> > +* pgo_get hook). the backing object signals this by
> > +* providing a pgo_fault routine.
> > +*/
> > + if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
> > + KERNEL_LOCK();
> > + error = uobj->pgops->pgo_fault(,
> > + flt.startva, pages, flt.npages,
> > + flt.centeridx, fault_type, flt.access_type,
> > + PGO_LOCKED);
> > + KERNEL_UNLOCK();
> > +
> > + if (error == VM_PAGER_OK)
> > + error = 0;
> > + else if (error == VM_PAGER_REFAULT)
> > + error = ERESTART;
> > + else
> > + error = EACCES;
> > + } else {
> > + /* case 2: fault on backing obj or zero fill */
> > + KERNEL_LOCK();
> > + error = uvm_fault_lower(, , pages,
> > + fault_type);
> > + KERNEL_UNLOCK();
> > + }
> > }
> > }
> >
> > @@ -1054,26 +1081,6 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> > struct vm_anon *anon = NULL;
> > vaddr_t currva;
> > voff_t uoff;
> > -
> > - /*
> > -* if the desired page is not shadowed by the amap and we have a
> > -* backing object, then we check to see if the backing object would
> > -* prefer to handle the fault itself (rather than letting us do it
> > -* with the usual pgo_get hook). the backing object signals this by
> > -* providing a pgo_fault routine.
> > -*/
> > - if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
> > - result = uobj->pgops->pgo_fault(ufi, flt->startva, pages,
> > - flt->npages, flt->centeridx, fault_type, flt->access_type,
> > - PGO_LOCKED);
> > -
> > - if (result == VM_PAGER_OK)
> > - return (0); /* pgo_fault did pmap enter */
> > - else if (result == VM_PAGER_REFAULT)
> > - return ERESTART;/* try again! */
> > - else
> > - return (EACCES);
> > - }
> >
> > /*
> > * now, if the desired page is not shadowed by the amap and we have
> >
>
>
Re: hppa: terminate backtrace of secondary processors
> Date: Sun, 7 Feb 2021 15:42:27 + > From: Miod Vallat > > When asking for the backtrace of a secondary processor in ddb, if that > backtrace reaches the secondary cpu startup code before the > switch_trampoline call, it will trust uninitialized stack data and is > likely to panic with an unaligned access at db_stack_trace_print+0x1d0. > (this was found the hard way by landry@ many years ago due to another > bug which got quickly fixed) > > The following diff makes sure the secondary processor stack is correctly > set up to hint the backtrace code that it should not attempt to go > further. Thanks; committed. > Index: locore.S > === > RCS file: /OpenBSD/src/sys/arch/hppa/hppa/locore.S,v > retrieving revision 1.193 > diff -u -p -r1.193 locore.S > --- locore.S 23 Oct 2014 16:57:45 - 1.193 > +++ locore.S 2 Feb 2021 17:08:57 - > @@ -2970,6 +2970,9 @@ ENTRY(hw_cpu_spinup_trampoline, 0) > stw r0, HPPA_FRAME_CRP(sp) > stw r0, HPPA_FRAME_PSP(sp) > > + ldilL%TFF_LAST, t1 > + stw t1, TF_FLAGS-TRAPFRAME_SIZEOF(sp) > + > /* Provide CPU with page tables. */ > ldilL%hppa_vtop, t1 > ldw R%hppa_vtop(t1), t1 > >
Re: ober_get_string.3: s/byte/character
On Mon, Feb 22, 2021 at 11:12:32PM +0100, Martijn van Duren wrote:
> As pointed out by claudio@, it makes more sense to talk about characters
> for fmt instead of bytes.
>
> The .Bl line already was >80 columns, but I don't know how to remedy
> this situation, so this diff makes that a little worse.
>
hi.
you can just use a "\", but i think it makes the source look worse, so i
would just leave it as-is (especially since, as you say, the 80 point is
already passed).
there is trailing whitespace at eol 74, so please zap that when/if you
commit.
one comment inline about the Bl line:
> martijn@
>
> Index: ober_get_string.3
> ===
> RCS file: /cvs/src/lib/libutil/ober_get_string.3,v
> retrieving revision 1.4
> diff -u -p -r1.4 ober_get_string.3
> --- ober_get_string.3 22 Feb 2021 17:15:02 - 1.4
> +++ ober_get_string.3 22 Feb 2021 22:08:32 -
> @@ -87,15 +87,15 @@ to return a valid
> .Fn ober_scanf_elements
> retrieves the values from zero or more elements starting at
> .Fa root .
> -For each byte in
> +For each character in
> .Fa fmt ,
> arguments of the types given in the following table are consumed
> and passed to the function listed, processing one
> .Vt ber_element
> -per byte.
> -The following bytes are valid:
> -.Bl -column -offset indent bytes ober_get_enumerated() "1: struct
> ber_element **"
> -.It Sy byte Ta Sy function Ta Sy arguments
> +per character.
> +The following characters are valid:
> +.Bl -column -offset indent characters ober_get_enumerated() "1: struct
> ber_element **"
i would rewrite this as:
.Bl -column "( or {" "ober_get_enumerated()" "1: struct ber_element **"
-offset indent
it just seems clearer to specify the column widths after the "-column"
marker, and finish with the offset.
column lists are horrible.
jmc
> +.It Sy character Ta Sy function Ta Sy arguments
> .It $ Ta see below Ta 0
> .It B Ta Fn ober_get_bitstring Ta 2: Vt void ** , size_t *
> .It b Ta Fn ober_get_booleanTa 1: Vt int *
>
>
quiz: Fix multi-line questions (trailing newline)
Hi all,
I was thrilled to find that the quiz for ed(1) commands, `function
ed-command` (as referenced in The UNIX Programming Enviroment), existed
in OpenBSD, but found that there was a bug where ~half of my correct
answers were not accepted.
Looking deeper, there is a bug in quiz(6) for datfiles with multi-line
answers.
Specifically, the following quiz.db line:
foo:\
bar
Is parsed into "foo:bar\n", which made it impossible to get right (since
the comparison was expecting a newline). This is a result of a bug in
quiz.c's appdstr(qp->q_text, lp, len), which copies the trailing newline
after "bar" (in lp) into q_text.
In appdstr, there was logic to strip the newline in the case of multiple
backslashes:
if (*(mp - 2) == '\\')
mp--;
*mp = '\0';
For example (foo:\\\nbar\\\nbaz\n) would have the second newline stripped
when appending bar to foo; since bar\\\n has a \\ before the newline, the
above code would roll back mp and truncate the \n. However, when baz is
appended, the check fails to find a \\ and the \n is kept, resulting in
"foo:barbaz\n".
The simplest fix for the bug is to modify this check to strip the newline
unconditionally:
if (*(mp - 1) == '\n')
mp--;
*mp = '\0';
That said, it seemed cleaner to modify the code to use getline(3) over
fgetln(3), which guarantees each iteration of the loop is a single line
and null-terminated, enabling the newline truncation in the loop and
allowing the use of strlcpy(3). This patch implements that fix.
One can verify the bug(fix) by:
$ cat < /tmp/qtest
foo1:bar
foo2:\\
bar
foo3:\\
ba\\
r
EOF
$ echo "/tmp/qtest:test:lines" > /tmp/qindex
# answer 'bar' to each question
$ quiz -i /tmp/qindex test lines # fails
$ /usr/obj/games/quiz/quiz -i /tmp/qindex test lines # succeeds
Any and all feedback welcome!
Thanks,
Alex
diff --git games/quiz/quiz.c games/quiz/quiz.c
index 073c1700719..c70dabe617a 100644
--- games/quiz/quiz.c
+++ games/quiz/quiz.c
@@ -110,7 +110,8 @@ get_file(const char *file)
{
FILE *fp;
QE *qp;
- size_t len;
+ ssize_t len;
+ size_t size;
char *lp;
if ((fp = fopen(file, "r")) == NULL)
@@ -123,9 +124,11 @@ get_file(const char *file)
*/
qp =
qsize = 0;
- while ((lp = fgetln(fp, )) != NULL) {
+ lp = NULL;
+ size = 0;
+ while ((len = getline(, , fp)) != -1) {
if (lp[len - 1] == '\n')
- --len;
+ lp[len - 1] = '\0';
if (qp->q_text && qp->q_text[0] != '\0' &&
qp->q_text[strlen(qp->q_text) - 1] == '\\')
qp->q_text = appdstr(qp->q_text, lp, len);
@@ -135,14 +138,15 @@ get_file(const char *file)
qp = qp->q_next;
if ((qp->q_text = malloc(len + 1)) == NULL)
errx(1, "malloc");
- /* lp may not be zero-terminated; cannot use strlcpy */
- strncpy(qp->q_text, lp, len);
- qp->q_text[len] = '\0';
+ strlcpy(qp->q_text, lp, len + 1);
qp->q_asked = qp->q_answered = FALSE;
qp->q_next = NULL;
++qsize;
}
}
+ free(lp);
+ if (ferror(fp))
+ err(1, "getline");
(void)fclose(fp);
}
@@ -334,11 +338,9 @@ appdstr(char *s, const char *tp, size_t len)
if (*(mp - 1) == '\\')
--mp;
- while ((ch = *mp++ = *tp++) && ch != '\n')
+ /* tp guaranteed null-terminated, copy in full */
+ while ((ch = *mp++ = *tp++) != '\0')
;
- if (*(mp - 2) == '\\')
- mp--;
- *mp = '\0';
free(s);
return (m);
ober_get_string.3: s/byte/character
As pointed out by claudio@, it makes more sense to talk about characters for fmt instead of bytes. The .Bl line already was >80 columns, but I don't know how to remedy this situation, so this diff makes that a little worse. martijn@ Index: ober_get_string.3 === RCS file: /cvs/src/lib/libutil/ober_get_string.3,v retrieving revision 1.4 diff -u -p -r1.4 ober_get_string.3 --- ober_get_string.3 22 Feb 2021 17:15:02 - 1.4 +++ ober_get_string.3 22 Feb 2021 22:08:32 - @@ -87,15 +87,15 @@ to return a valid .Fn ober_scanf_elements retrieves the values from zero or more elements starting at .Fa root . -For each byte in +For each character in .Fa fmt , arguments of the types given in the following table are consumed and passed to the function listed, processing one .Vt ber_element -per byte. -The following bytes are valid: -.Bl -column -offset indent bytes ober_get_enumerated() "1: struct ber_element **" -.It Sy byte Ta Sy function Ta Sy arguments +per character. +The following characters are valid: +.Bl -column -offset indent characters ober_get_enumerated() "1: struct ber_element **" +.It Sy character Ta Sy function Ta Sy arguments .It $ Ta see below Ta 0 .It B Ta Fn ober_get_bitstring Ta 2: Vt void ** , size_t * .It b Ta Fn ober_get_booleanTa 1: Vt int *
Re: mbuf leak ip_insertoptions
ok mvs@
> On 22 Feb 2021, at 18:51, Alexander Bluhm wrote:
>
> Hi,
>
> ip_insertoptions() may prepend a mbuf. In this case "goto bad" has
> to free the new chain. Currently we leak the new mbuf in front of
> the old chain. NetBSD has fixed this bug here:
>
>
> revision 1.33
> date: 1996-10-11 18:19:08 +; author: is; state: Exp; lines: +2 -2;
> Fix a mbuf leak in ip_output().
>
> Scenario: If ip_insertoptions() prepends a new mbuf to the chain, the
> bad: label's m_freem(m0) still would free only the original mbuf chain
> if the transmission failed for, e.g., no route to host; resulting in
> one lost mbuf per failed packet. (The original posting included a
> demonstration program).
>
> Original report of this bug was by jin...@isl.rdc.toshiba.co.jp
> (JINMEI Tatuya) on comp.bugs.4bsd.
>
>
> Free m instead of m0 in the bad case. This allows to simplify a
> bunch of goto done.
>
> ok?
>
> bluhm
>
> Index: netinet/ip_output.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.365
> diff -u -p -r1.365 ip_output.c
> --- netinet/ip_output.c 10 Feb 2021 18:28:06 - 1.365
> +++ netinet/ip_output.c 22 Feb 2021 15:48:38 -
> @@ -111,9 +111,6 @@ ip_output(struct mbuf *m0, struct mbuf *
> #if NPF > 0
> u_int orig_rtableid;
> #endif
> -#ifdef MROUTING
> - int rv;
> -#endif
>
> NET_ASSERT_LOCKED();
>
> @@ -250,8 +247,7 @@ reroute:
> /* Should silently drop packet */
> if (error == -EINVAL)
> error = 0;
> - m_freem(m);
> - goto done;
> + goto bad;
> }
> if (tdb != NULL) {
> /*
> @@ -348,13 +344,13 @@ reroute:
>*/
> if (ipmforwarding && ip_mrouter[ifp->if_rdomain] &&
> (flags & IP_FORWARDING) == 0) {
> + int rv;
> +
> KERNEL_LOCK();
> rv = ip_mforward(m, ifp);
> KERNEL_UNLOCK();
> - if (rv != 0) {
> - m_freem(m);
> - goto done;
> - }
> + if (rv != 0)
> + goto bad;
> }
> }
> #endif
> @@ -366,10 +362,8 @@ reroute:
>* loop back a copy if this host actually belongs to the
>* destination group on the loopback interface.
>*/
> - if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0) {
> - m_freem(m);
> - goto done;
> - }
> + if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0)
> + goto bad;
>
> goto sendit;
> }
> @@ -427,8 +421,7 @@ sendit:
> if (pf_test(AF_INET, (flags & IP_FORWARDING) ? PF_FWD : PF_OUT,
> ifp, ) != PF_PASS) {
> error = EACCES;
> - m_freem(m);
> - goto done;
> + goto bad;
> }
> if (m == NULL)
> goto done;
> @@ -453,8 +446,7 @@ sendit:
> if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&
> (m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
> error = EHOSTUNREACH;
> - m_freem(m);
> - goto done;
> + goto bad;
> }
> #endif
>
> @@ -534,7 +526,7 @@ done:
> if_put(ifp);
> return (error);
> bad:
> - m_freem(m0);
> + m_freem(m);
> goto done;
> }
>
>
Re: occasional SSIGSEGV on C++ exception handling
> No problem, real-life often takes precedence. No way! operator(7) would need an update!
Re: XBox One gamecontroller support
On Fri, Jan 15, 2021 at 06:32:04AM -0700, Thomas Frohwein wrote:
> On Sat, Jan 09, 2021 at 10:50:35AM -0700, Thomas Frohwein wrote:
> > Hi,
> >
> > This diff adds support for the XBox One gamecontroller in a similar way
> > to what we have for the (older) XBox 360 controller [1][2]. This diff
> > is based on the pertinent code in NetBSD's uhidev.c.
> >
> > Similarities include that the device doesn't provide a report
> > descriptor, so this diff adds one to uhid_rdesc.h.
> >
> > The biggest difference (that held this up for a while) is that the
> > controller needs an init byte sequence to "wake up."
> >
> > The original report descriptor from NetBSD just maps the buttons and
> > axes in the order that they appear in the report. I modified the report
> > descriptor to assign the buttons/axes in the most logical order for
> > compatibility, which results in the same layout that we already have
> > for the XBox 360 controller.
> >
> > The addition of the member sc_flags to struct uhidev_softc follows the
> > NetBSD example.
> >
> > I have tested this with usbhidctl(1), sdl2-jstest (from sdl-jstest
> > package), and a couple of SDL2 games (Owlboy, Cryptark).
> >
> > If you have an XBox One controller, the easiest way to test is with:
> >
> > $ usbhidctl -f /dev/uhidX -l
> >
> > Make sure to pick the right uhid device and that you have read
> > permissions for it.
> >
> > Of course, this works only with the controller connected via USB, and
> > doesn't magically add wireless support.
> >
> > If you test actual SDL2 applications, the button layout will likely
> > default to an odd configuration. I am simultaneously preparing a diff
> > for sdl2-2.0.14p0 that improves recognition and automatic mapping and
> > solves any issues with XBox One default button layout.
>
> I have an update to this diff. 2 testers used a more recent XBox One
> controller model (model number 1708 in both cases) and ran into
> problems with the original diff. Below is a new diff that uses a longer
> 5-byte init sequence taken from Linux' xpad.c [1], compared to the
> 2-byte sequence from NetBSD's uhidev.c that I offered in the initial
> diff. This fixed the issue for the 2 testers.
>
> My own XBox One controller is model number 1537 and still works with
> this diff.
>
> [1] https://github.com/paroj/xpad/blob/master/xpad.c#L479
solene@ has tested this and is ok with it, but I was hoping to get an
opinion from someone with more mileage with the USB internals.
The items of note to direct anyone wanting to give a quick opinion
inlined below.
>
> Index: uhid_rdesc.h
> ===
> RCS file: /cvs/src/sys/dev/usb/uhid_rdesc.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 uhid_rdesc.h
> --- uhid_rdesc.h 25 Oct 2013 03:09:59 - 1.1
> +++ uhid_rdesc.h 9 Jan 2021 15:11:30 -
> @@ -272,4 +272,94 @@ static const uByte uhid_xb360gp_report_d
> 0x95, 0x01, /* REPORT COUNT (1)*/
> 0x81, 0x01, /* INPUT (Constant)*/
> 0xc0,/* END COLLECTION */
> +};
> +
> +static const uByte uhid_xbonegp_report_descr[] = {
> +0x05, 0x01, /* USAGE PAGE (Generic Desktop)
> */
> +0x09, 0x05, /* USAGE (Gamepad)
> */
> +0xa1, 0x01, /* COLLECTION (Application)
> */
> +/* Button packet */
> +0xa1, 0x00, /* COLLECTION (Physical)
> */
> +0x85, 0x20, /* REPORT ID (0x20)
> */
> +/* Skip unknown field and counter */
> +0x09, 0x00, /* USAGE (Undefined)
> */
> +0x75, 0x08, /* REPORT SIZE (8)
> */
> +0x95, 0x02, /* REPORT COUNT (2)
> */
> +0x81, 0x03, /* INPUT (Constant, Variable,
> Absolute) */
> +/* Payload size */
> +0x09, 0x3b, /* USAGE (Byte Count)
> */
> +0x95, 0x01, /* REPORT COUNT (1)
> */
> +0x81, 0x02, /* INPUT (Data, Variable, Absolute)
> */
> +/* 16 Buttons: 1-2=Unknown, 3=Start, 4=Back, 5-8=ABXY,
> + * 9-12=D-Pad(Up,Dn,Lt,Rt), 13=LB, 14=RB, 15=LS, 16=RS
> + */
> +/* Skip the first 2 as they are not used */
> +0x75, 0x01, /* REPORT SIZE (1)
> */
> +0x95, 0x02, /* REPORT COUNT (2)
> */
> +0x81, 0x01, /* INPUT (Constant)
> */
> +/* Assign buttons Start(7), Back(8), ABXY(1-4) */
> +
sdmmc(4): check and retry bus width change
Hi,
it seems like some eMMCs are not capable of doing 8-bit operation,
even if the controller supports it. I was questioning our drivers
first, but it looks like it's the same on Linux. In the case that
8-bit doesn't work, they seem to fall back to lower values to make
that HW work.
This diff implements a mechanism that tries 8-bit, if available,
then 4-bit and in the end falls back to 1-bit. This makes my HW
work, but I would like to have this tested by a broader audience.
Apparently there's a "bus test" command, but it isn't implemented
on all host controllers. Hence I simply try to read the EXT_CSD
to make sure the transfer works.
For testing, a print like
printf("%s: using %u-bit width\n", DEVNAME(sc), width);
could be added at line 928.
What could possible regressions be? The width could become smaller
then previously. This would reduce the read/write transfer speed.
Also it's possible that eMMCs are not recognized/initialized anymore.
What could possible improvements be? eMMCs that previously didn't
work now work, with at least 1-bit or 4-bit wide transfers.
Please note that this only works for eMMCs. SD cards are *not* using
this code path. SD cards have a different initialization code path.
Please report any changes or non-changes. If nothing changes, that's
perfect.
Patrick
diff --git a/sys/dev/sdmmc/sdmmc_mem.c b/sys/dev/sdmmc/sdmmc_mem.c
index 59bcb1b4a11..5856b9bb1b3 100644
--- a/sys/dev/sdmmc/sdmmc_mem.c
+++ b/sys/dev/sdmmc/sdmmc_mem.c
@@ -56,6 +56,8 @@ int sdmmc_mem_signal_voltage(struct sdmmc_softc *, int);
intsdmmc_mem_sd_init(struct sdmmc_softc *, struct sdmmc_function *);
intsdmmc_mem_mmc_init(struct sdmmc_softc *, struct sdmmc_function *);
+intsdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *,
+ struct sdmmc_function *, int);
intsdmmc_mem_single_read_block(struct sdmmc_function *, int, u_char *,
size_t);
intsdmmc_mem_read_block_subr(struct sdmmc_function *, bus_dmamap_t,
@@ -908,31 +910,20 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
sdmmc_function *sf)
ext_csd[EXT_CSD_CARD_TYPE]);
}
- if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE)) {
+ if (ISSET(sc->sc_caps, SMC_CAPS_8BIT_MODE) &&
+ sdmmc_mem_mmc_select_bus_width(sc, sf, 8) == 0)
width = 8;
- value = EXT_CSD_BUS_WIDTH_8;
- } else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE)) {
+ else if (ISSET(sc->sc_caps, SMC_CAPS_4BIT_MODE) &&
+ sdmmc_mem_mmc_select_bus_width(sc, sf, 4) == 0)
width = 4;
- value = EXT_CSD_BUS_WIDTH_4;
- } else {
- width = 1;
- value = EXT_CSD_BUS_WIDTH_1;
- }
-
- if (width != 1) {
- error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BUS_WIDTH, value);
- if (error == 0)
- error = sdmmc_chip_bus_width(sc->sct,
- sc->sch, width);
- else {
+ else {
+ error = sdmmc_mem_mmc_select_bus_width(sc, sf, 1);
+ if (error != 0) {
DPRINTF(("%s: can't change bus width"
" (%d bit)\n", DEVNAME(sc), width));
return error;
}
-
- /* : need bus test? (using by CMD14 & CMD19) */
- sdmmc_delay(1);
+ width = 1;
}
if (timing != SDMMC_TIMING_LEGACY) {
@@ -1041,6 +1032,59 @@ sdmmc_mem_mmc_init(struct sdmmc_softc *sc, struct
sdmmc_function *sf)
return error;
}
+int
+sdmmc_mem_mmc_select_bus_width(struct sdmmc_softc *sc, struct sdmmc_function
*sf,
+int width)
+{
+ u_int8_t ext_csd[512];
+ int error, value;
+
+ switch (width) {
+ case 8:
+ value = EXT_CSD_BUS_WIDTH_8;
+ break;
+ case 4:
+ value = EXT_CSD_BUS_WIDTH_4;
+ break;
+ case 1:
+ value = EXT_CSD_BUS_WIDTH_1;
+ break;
+ default:
+ printf("%s: invalid bus width\n", DEVNAME(sc));
+ return EINVAL;
+ }
+
+ error = sdmmc_mem_mmc_switch(sf, EXT_CSD_CMD_SET_NORMAL,
+ EXT_CSD_BUS_WIDTH, value);
+ if (error != 0) {
+ DPRINTF(("%s: can't change card bus width"
+ " (%d bit)\n", DEVNAME(sc), width));
+ return error;
+ }
+
+ error = sdmmc_chip_bus_width(sc->sct,
+ sc->sch, width);
+ if (error != 0) {
+ DPRINTF(("%s: can't change host bus width"
+ " (%d bit)\n",
Re: uvm_fault_lower refactoring
Martin Pieuchot [m...@openbsd.org] wrote: > On 16/02/21(Tue) 11:20, Martin Pieuchot wrote: > > Start by moving `pgo_fault' handler outside of uvm_fault_lower(). > > > > If a page has a backing object that prefer to handler to fault itself > > the locking will be different, so keep it under KERNEL_LOCK() for the > > moment and make it separate from the rest of uvm_fault_lower(). > > > > ok? > > Anyone? > It makes sense, and works on my workstation...
Re: occasional SSIGSEGV on C++ exception handling
On 22.02.2021 15:56, Otto Moerbeek wrote: On Mon, Feb 22, 2021 at 11:09:41AM +0200, Paul Irofti wrote: - investigate the commit you mention above. Sadly I cannot remember the original case that prompted for the caching code to be added. Sorry I could not reply earlier. No problem, real-life often takes precedence. The caching code was added by me to make libreoffice work with non-toy spreadsheets. Apparently referencing external cells is done through exception handling in office suites and this lead to waiting for whole minutes for libreoffice to load. The gcc toolchain has this optimization, but clang one does not. So I added a simple caching mechanism a few years ago that make this bearable. See revision 1.8 of AddressSpace.hpp. Of course it moved in the meantime and CVS lost all its history. https://cvsweb.openbsd.org/src/lib/libunwind/src/Attic/AddressSpace.hpp In the meantime I made a an committed a fix. The root cause was that the cache isn't thread safe. My change fixes that by making the cache object tread_local: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/gnu/llvm/libunwind/src/UnwindCursor.hpp.diff?r1=1.2=1.3 Yes, I saw that. Thank you very much for taking care of it!
Re: mbuf leak ip_insertoptions
On Mon, Feb 22, 2021 at 04:51:09PM +0100, Alexander Bluhm wrote: > Free m instead of m0 in the bad case. This allows to simplify a > bunch of goto done. OK kn
mbuf leak ip_insertoptions
Hi,
ip_insertoptions() may prepend a mbuf. In this case "goto bad" has
to free the new chain. Currently we leak the new mbuf in front of
the old chain. NetBSD has fixed this bug here:
revision 1.33
date: 1996-10-11 18:19:08 +; author: is; state: Exp; lines: +2 -2;
Fix a mbuf leak in ip_output().
Scenario: If ip_insertoptions() prepends a new mbuf to the chain, the
bad: label's m_freem(m0) still would free only the original mbuf chain
if the transmission failed for, e.g., no route to host; resulting in
one lost mbuf per failed packet. (The original posting included a
demonstration program).
Original report of this bug was by jin...@isl.rdc.toshiba.co.jp
(JINMEI Tatuya) on comp.bugs.4bsd.
Free m instead of m0 in the bad case. This allows to simplify a
bunch of goto done.
ok?
bluhm
Index: netinet/ip_output.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.365
diff -u -p -r1.365 ip_output.c
--- netinet/ip_output.c 10 Feb 2021 18:28:06 - 1.365
+++ netinet/ip_output.c 22 Feb 2021 15:48:38 -
@@ -111,9 +111,6 @@ ip_output(struct mbuf *m0, struct mbuf *
#if NPF > 0
u_int orig_rtableid;
#endif
-#ifdef MROUTING
- int rv;
-#endif
NET_ASSERT_LOCKED();
@@ -250,8 +247,7 @@ reroute:
/* Should silently drop packet */
if (error == -EINVAL)
error = 0;
- m_freem(m);
- goto done;
+ goto bad;
}
if (tdb != NULL) {
/*
@@ -348,13 +344,13 @@ reroute:
*/
if (ipmforwarding && ip_mrouter[ifp->if_rdomain] &&
(flags & IP_FORWARDING) == 0) {
+ int rv;
+
KERNEL_LOCK();
rv = ip_mforward(m, ifp);
KERNEL_UNLOCK();
- if (rv != 0) {
- m_freem(m);
- goto done;
- }
+ if (rv != 0)
+ goto bad;
}
}
#endif
@@ -366,10 +362,8 @@ reroute:
* loop back a copy if this host actually belongs to the
* destination group on the loopback interface.
*/
- if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0) {
- m_freem(m);
- goto done;
- }
+ if (ip->ip_ttl == 0 || (ifp->if_flags & IFF_LOOPBACK) != 0)
+ goto bad;
goto sendit;
}
@@ -427,8 +421,7 @@ sendit:
if (pf_test(AF_INET, (flags & IP_FORWARDING) ? PF_FWD : PF_OUT,
ifp, ) != PF_PASS) {
error = EACCES;
- m_freem(m);
- goto done;
+ goto bad;
}
if (m == NULL)
goto done;
@@ -453,8 +446,7 @@ sendit:
if (ipsec_in_use && (flags & IP_FORWARDING) && (ipforwarding == 2) &&
(m_tag_find(m, PACKET_TAG_IPSEC_IN_DONE, NULL) == NULL)) {
error = EHOSTUNREACH;
- m_freem(m);
- goto done;
+ goto bad;
}
#endif
@@ -534,7 +526,7 @@ done:
if_put(ifp);
return (error);
bad:
- m_freem(m0);
+ m_freem(m);
goto done;
}
Re: occasional SSIGSEGV on C++ exception handling
On Mon, Feb 22, 2021 at 11:09:41AM +0200, Paul Irofti wrote: > > - investigate the commit you mention above. Sadly I cannot > >remember the original case that prompted for the caching code to > > be > >added. > > Sorry I could not reply earlier. No problem, real-life often takes precedence. > > The caching code was added by me to make libreoffice work with non-toy > spreadsheets. Apparently referencing external cells is done through > exception handling in office suites and this lead to waiting for whole > minutes for libreoffice to load. > > The gcc toolchain has this optimization, but clang one does not. So I added > a simple caching mechanism a few years ago that make this bearable. > > See revision 1.8 of AddressSpace.hpp. Of course it moved in the meantime and > CVS lost all its history. > > https://cvsweb.openbsd.org/src/lib/libunwind/src/Attic/AddressSpace.hpp > In the meantime I made a an committed a fix. The root cause was that the cache isn't thread safe. My change fixes that by making the cache object tread_local: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/gnu/llvm/libunwind/src/UnwindCursor.hpp.diff?r1=1.2=1.3 -Otto
Re: veb(4), a virtual ethernet bridge (that could replace bridge(4)?)
> On 22 Feb 2021, at 12:46 am, Vitaliy Makkoveev wrote:
>
> Hello.
>
>
>> +ifp->if_ioctl = veb_ioctl;
>> +ifp->if_input = veb_input;
>> +//ifp->if_rtrequest = veb_rtrequest;
>> +ifp->if_output = veb_output;
>> +ifp->if_enqueue = veb_enqueue;
>
> Could you replace c++ style comment in veb_clone_create()?
yep.
>
>> +veb_clone_destroy(struct ifnet *ifp)
>> +{
>> +struct veb_softc *sc = ifp->if_softc;
>> +struct veb_port *p, *np;
>> +
>> +NET_LOCK();
>> +sc->sc_dead = 1;
>> +
>> +if (ISSET(ifp->if_flags, IFF_RUNNING))
>> +veb_down(sc);
>> +NET_UNLOCK();
>> +
>> +if_detach(ifp);
>
>
> Also veb_down() looks strange here. I guess it is no reason to
> play with `if_flags' here and smr_barrier() could be called after
> if_detach(). This makes `sc_dead’ unnecessary.
i need to think about sc_dead again. i do it in a bunch of different drivers
and you're pretty confident it's not needed anymore.
technically the flags don't need to be cleared, but i like having the flow
right in case i made veb_down do more in the future.
uvm_fault: uvm_fault_lower_lookup() refactoring
Similar refactoring to what has been done for the upper part of the fault
handler, ok?
Index: uvm/uvm_fault.c
===
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.115
diff -u -p -r1.115 uvm_fault.c
--- uvm/uvm_fault.c 16 Feb 2021 09:10:17 - 1.115
+++ uvm/uvm_fault.c 22 Feb 2021 09:11:53 -
@@ -1039,6 +1039,98 @@ uvm_fault_upper(struct uvm_faultinfo *uf
}
/*
+ * uvm_fault_lower_lookup: look up on-memory uobj pages.
+ *
+ * 1. get on-memory pages.
+ * 2. if failed, give up (get only center page later).
+ * 3. if succeeded, enter h/w mapping of neighbor pages.
+ */
+
+struct vm_page *
+uvm_fault_lower_lookup(
+ struct uvm_faultinfo *ufi, const struct uvm_faultctx *flt,
+ struct vm_page **pages)
+{
+ struct uvm_object *uobj = ufi->entry->object.uvm_obj;
+ struct vm_page *uobjpage = NULL;
+ int lcv, gotpages;
+ vaddr_t currva;
+
+ counters_inc(uvmexp_counters, flt_lget);
+ gotpages = flt->npages;
+ (void) uobj->pgops->pgo_get(uobj,
+ ufi->entry->offset + (flt->startva - ufi->entry->start),
+ pages, , flt->centeridx,
+ flt->access_type & MASK(ufi->entry), ufi->entry->advice,
+ PGO_LOCKED);
+
+ /*
+* check for pages to map, if we got any
+*/
+ if (gotpages == 0) {
+ return NULL;
+ }
+
+ currva = flt->startva;
+ for (lcv = 0; lcv < flt->npages; lcv++, currva += PAGE_SIZE) {
+ if (pages[lcv] == NULL ||
+ pages[lcv] == PGO_DONTCARE)
+ continue;
+
+ KASSERT((pages[lcv]->pg_flags & PG_RELEASED) == 0);
+
+ /*
+* if center page is resident and not
+* PG_BUSY, then pgo_get made it PG_BUSY
+* for us and gave us a handle to it.
+* remember this page as "uobjpage."
+* (for later use).
+*/
+ if (lcv == flt->centeridx) {
+ uobjpage = pages[lcv];
+ continue;
+ }
+
+ /*
+* note: calling pgo_get with locked data
+* structures returns us pages which are
+* neither busy nor released, so we don't
+* need to check for this. we can just
+* directly enter the page (after moving it
+* to the head of the active queue [useful?]).
+*/
+
+ uvm_lock_pageq();
+ uvm_pageactivate(pages[lcv]); /* reactivate */
+ uvm_unlock_pageq();
+ counters_inc(uvmexp_counters, flt_nomap);
+
+ /*
+* Since this page isn't the page that's
+* actually faulting, ignore pmap_enter()
+* failures; it's not critical that we
+* enter these right now.
+*/
+ (void) pmap_enter(ufi->orig_map->pmap, currva,
+ VM_PAGE_TO_PHYS(pages[lcv]) | flt->pa_flags,
+ flt->enter_prot & MASK(ufi->entry),
+ PMAP_CANFAIL |
+(flt->wired ? PMAP_WIRED : 0));
+
+ /*
+* NOTE: page can't be PG_WANTED because
+* we've held the lock the whole time
+* we've had the handle.
+*/
+ atomic_clearbits_int([lcv]->pg_flags, PG_BUSY);
+ UVM_PAGE_OWN(pages[lcv], NULL);
+ }
+ pmap_update(ufi->orig_map->pmap);
+
+ return uobjpage;
+}
+
+/*
* uvm_fault_lower: handle lower fault.
*
*/
@@ -1049,10 +1141,9 @@ uvm_fault_lower(struct uvm_faultinfo *uf
struct vm_amap *amap = ufi->entry->aref.ar_amap;
struct uvm_object *uobj = ufi->entry->object.uvm_obj;
boolean_t promote, locked;
- int result, lcv, gotpages;
+ int result;
struct vm_page *uobjpage, *pg = NULL;
struct vm_anon *anon = NULL;
- vaddr_t currva;
voff_t uoff;
/*
@@ -1081,82 +1172,11 @@ uvm_fault_lower(struct uvm_faultinfo *uf
* we ask (with pgo_get) the object for resident pages that we care
* about and attempt to map them in. we do not let pgo_get block
* (PGO_LOCKED).
-*
-* ("get" has the option of doing a pmap_enter for us)
*/
- if (uobj != NULL) {
- counters_inc(uvmexp_counters, flt_lget);
- gotpages = flt->npages;
- (void) uobj->pgops->pgo_get(uobj, ufi->entry->offset +
- (flt->startva - ufi->entry->start),
- pages, , flt->centeridx,
- flt->access_type & MASK(ufi->entry),
- ufi->entry->advice, PGO_LOCKED);
-
- /* check for pages to map, if we got
Re: uvm_fault_lower refactoring
On 16/02/21(Tue) 11:20, Martin Pieuchot wrote:
> Start by moving `pgo_fault' handler outside of uvm_fault_lower().
>
> If a page has a backing object that prefer to handler to fault itself
> the locking will be different, so keep it under KERNEL_LOCK() for the
> moment and make it separate from the rest of uvm_fault_lower().
>
> ok?
Anyone?
>
> Index: uvm/uvm_fault.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 uvm_fault.c
> --- uvm/uvm_fault.c 16 Feb 2021 09:10:17 - 1.115
> +++ uvm/uvm_fault.c 16 Feb 2021 10:13:58 -
> @@ -598,10 +598,37 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
> /* case 1: fault on an anon in our amap */
> error = uvm_fault_upper(, , anons, fault_type);
> } else {
> - /* case 2: fault on backing object or zero fill */
> - KERNEL_LOCK();
> - error = uvm_fault_lower(, , pages, fault_type);
> - KERNEL_UNLOCK();
> + struct uvm_object *uobj = ufi.entry->object.uvm_obj;
> +
> + /*
> + * if the desired page is not shadowed by the amap and
> + * we have a backing object, then we check to see if
> + * the backing object would prefer to handle the fault
> + * itself (rather than letting us do it with the usual
> + * pgo_get hook). the backing object signals this by
> + * providing a pgo_fault routine.
> + */
> + if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
> + KERNEL_LOCK();
> + error = uobj->pgops->pgo_fault(,
> + flt.startva, pages, flt.npages,
> + flt.centeridx, fault_type, flt.access_type,
> + PGO_LOCKED);
> + KERNEL_UNLOCK();
> +
> + if (error == VM_PAGER_OK)
> + error = 0;
> + else if (error == VM_PAGER_REFAULT)
> + error = ERESTART;
> + else
> + error = EACCES;
> + } else {
> + /* case 2: fault on backing obj or zero fill */
> + KERNEL_LOCK();
> + error = uvm_fault_lower(, , pages,
> + fault_type);
> + KERNEL_UNLOCK();
> + }
> }
> }
>
> @@ -1054,26 +1081,6 @@ uvm_fault_lower(struct uvm_faultinfo *uf
> struct vm_anon *anon = NULL;
> vaddr_t currva;
> voff_t uoff;
> -
> - /*
> - * if the desired page is not shadowed by the amap and we have a
> - * backing object, then we check to see if the backing object would
> - * prefer to handle the fault itself (rather than letting us do it
> - * with the usual pgo_get hook). the backing object signals this by
> - * providing a pgo_fault routine.
> - */
> - if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
> - result = uobj->pgops->pgo_fault(ufi, flt->startva, pages,
> - flt->npages, flt->centeridx, fault_type, flt->access_type,
> - PGO_LOCKED);
> -
> - if (result == VM_PAGER_OK)
> - return (0); /* pgo_fault did pmap enter */
> - else if (result == VM_PAGER_REFAULT)
> - return ERESTART;/* try again! */
> - else
> - return (EACCES);
> - }
>
> /*
>* now, if the desired page is not shadowed by the amap and we have
>
Re: occasional SSIGSEGV on C++ exception handling
- investigate the commit you mention above. Sadly I cannot remember the original case that prompted for the caching code to be added. Sorry I could not reply earlier. The caching code was added by me to make libreoffice work with non-toy spreadsheets. Apparently referencing external cells is done through exception handling in office suites and this lead to waiting for whole minutes for libreoffice to load. The gcc toolchain has this optimization, but clang one does not. So I added a simple caching mechanism a few years ago that make this bearable. See revision 1.8 of AddressSpace.hpp. Of course it moved in the meantime and CVS lost all its history. https://cvsweb.openbsd.org/src/lib/libunwind/src/Attic/AddressSpace.hpp
