Re: Remove some customization from our perl build
Todd C. Miller wrote: > On Fri, 10 Apr 2020 18:17:33 -0700, Andrew Hewus Fresh wrote: > > > Recently it was pointed out that we don't link /usr/lib/libperl.so.* to > > libm the way is expected for code that also links to libperl. That led > > me to go digging again into the customization we have around the perl > > build and getting terribly confused. That did somewhat clear up after > > reading more about bsd.*.mk, but still feel like some of this mess was > > to make the vax work, but I couldn't actually figure it out from the cvs > > logs why it exists. > > The way OpenBSD builds is that the libraries are built and installed > first, then the binaries that link against those binaries get built > and linked against the new libs. This guarantees that we don't > link new binaries against the old libs. > > In the past, vi was linked against libperl. However, I don't think > there is anything in base linking against libperl now so we can > probably let perl build and install its own libraries. Sounds right to me.
Re: Remove some customization from our perl build
On Fri, 10 Apr 2020 18:17:33 -0700, Andrew Hewus Fresh wrote: > Recently it was pointed out that we don't link /usr/lib/libperl.so.* to > libm the way is expected for code that also links to libperl. That led > me to go digging again into the customization we have around the perl > build and getting terribly confused. That did somewhat clear up after > reading more about bsd.*.mk, but still feel like some of this mess was > to make the vax work, but I couldn't actually figure it out from the cvs > logs why it exists. The way OpenBSD builds is that the libraries are built and installed first, then the binaries that link against those binaries get built and linked against the new libs. This guarantees that we don't link new binaries against the old libs. In the past, vi was linked against libperl. However, I don't think there is anything in base linking against libperl now so we can probably let perl build and install its own libraries. - todd
Remove some customization from our perl build
Recently it was pointed out that we don't link /usr/lib/libperl.so.* to libm the way is expected for code that also links to libperl. That led me to go digging again into the customization we have around the perl build and getting terribly confused. That did somewhat clear up after reading more about bsd.*.mk, but still feel like some of this mess was to make the vax work, but I couldn't actually figure it out from the cvs logs why it exists. In any case, this patch does a few things, some of which I can split up and put in separately if it comes to that. * Puts back some of the upstream Makefile.SH that we removed * and a little Dynaloader too * Changes Configure flags to -de instead of -dsE * So Configure does the work previously handled by depend.done * Adjusts the installperl script to put libperl where we want it * Moves some build flag discovery into hint/openbsd.sh (Which I can then push upstream) * Figuring out the correct PICFLAG, which means perl will now use the same one, not -fpic for things built by Makefile.bsdwapper on archs that want it and -fPIC for everything else. * Using no-tree-ter on alpha, due to a compiler bug. * Lets the perl infrastructure build libperl again * Notably, this stops creating libperl.a, but I have a patch around that puts it back, just not sure if we need it. * Which means "we" don't actually build anything anymore, we leave all that to the perl upstream Makefile so all the "stuff" to do with that can go away. * Which means ldd now mentions libm as it should * Some tidying of the rest of Makefile.bsd-wrapper* It seems to build fine on my alpha, amd64, arm64, armv7, i386, macppc, octeon, and sparc64. The individual changes are committed, in a clean-up-build branch, to the GitHub repo where I keep track of them, along with build logs from my test machines both with and without this patch: https://github.com/afresh1/OpenBSD-perl/tree/clean-up-build Does anyone know if we need any this and if so, why? (especially libperl.a) Is what I did in the hints file a reasonable way to find the PICFLAG? This script should only ever run on OpenBSD, so should I just assume bsd.own.mk exists? There is probably further cleanup that can be done. Thanks for your feedback. Index: distrib/sets/lists/comp/mi === RCS file: /cvs/src/distrib/sets/lists/comp/mi,v retrieving revision 1.1495 diff -u -p -r1.1495 mi --- distrib/sets/lists/comp/mi 2 Mar 2020 20:59:38 - 1.1495 +++ distrib/sets/lists/comp/mi 9 Apr 2020 02:29:59 - @@ -1531,7 +1531,6 @@ ./usr/lib/libpanelw_p.a ./usr/lib/libpcap.a ./usr/lib/libpcap_p.a -./usr/lib/libperl.a ./usr/lib/libpthread.a ./usr/lib/libpthread_p.a ./usr/lib/libradius.a Index: gnu/usr.bin/perl/installperl === RCS file: /cvs/src/gnu/usr.bin/perl/installperl,v retrieving revision 1.49 diff -u -p -r1.49 installperl --- gnu/usr.bin/perl/installperl30 Dec 2019 02:15:16 - 1.49 +++ gnu/usr.bin/perl/installperl9 Apr 2020 02:30:06 - @@ -383,7 +383,7 @@ elsif ($Is_Cygwin) { # On Cygwin symlink # [als] hard-coded 'libperl' name... not good! #@corefiles = <*.h libperl*.* perl*$Config{lib_ext}>; @corefiles = <*.h *.inc perl*$Config{lib_ext}>; -push(@corefiles,) unless defined($ENV{"NOLIBINSTALL"}); +install($libperl, "$opts{destdir}$Config{glibpth}/$libperl", "0444"); # AIX needs perl.exp installed as well. push(@corefiles,'perl.exp') if $^O eq 'aix'; Index: gnu/usr.bin/perl/hints/openbsd.sh === RCS file: /cvs/src/gnu/usr.bin/perl/hints/openbsd.sh,v retrieving revision 1.71 diff -u -p -r1.71 openbsd.sh --- gnu/usr.bin/perl/hints/openbsd.sh 30 Dec 2019 02:15:18 - 1.71 +++ gnu/usr.bin/perl/hints/openbsd.sh 9 Apr 2020 02:30:37 - @@ -47,7 +47,11 @@ alpha-2.[0-8]|mips-2.[0-8]|powerpc-2.[0- test -z "$usedl" && usedl=$define # We use -fPIC here because -fpic is *NOT* enough for some of the # extensions like Tk on some OpenBSD platforms (ie: sparc) - cccdlflags="-DPIC -fPIC $cccdlflags" + PICFLAG=-fPIC + if [ -e /usr/share/mk/bsd.own.mk ]; then + PICFLAG=`make -f /usr/share/mk/bsd.own.mk -V PICFLAG` + fi + cccdlflags="-DPIC ${PICFLAG} $cccdlflags" case "$osvers" in [01].*|2.[0-7]|2.[0-7].*) lddlflags="-Bshareable $lddlflags" @@ -58,7 +62,7 @@ alpha-2.[0-8]|mips-2.[0-8]|powerpc-2.[0- ;; *) # from 3.1 onwards ld=${cc:-cc} - lddlflags="-shared -fPIC $lddlflags" + lddlflags="-shared ${PICFLAG} $lddlflags" libswanted=`echo $libswanted | sed 's/ dl / /'` ;; esac @@ -84,6 +88,7 @@ esac # around for old NetBSD binaries. libswanted=`echo $libs
__hldtoa broken for ld128
this was found by fuzzing the LLVM __cxa_demangle on an ld128 Android system using hwasan, but it turns out no to simply be a buffer overflow --- the results are just wrong. (which shows how much anyone uses ld128 in conjunction with %a!) this was the minimized test case: free(__cxa_demangle("1\006ILeee", 0, 0, 0)); which, going down a level to snprintf was working out to something like this gtest: char buf[BUFSIZ]; union { uint64_t a[2]; long double v; } u; u.a[0] = UINT64_C(0xcececececececece); u.a[1] = UINT64_C(0xcececececececece); EXPECT_EQ(41, snprintf(buf, sizeof(buf), "<%La>", u.v)); EXPECT_STREQ("<-0x1.cecececececececececececececep+3791>", buf); here's a fix, the diff being relative to the copy of the OpenBSD source that's used in Android (but the NetBSD source looks the same, with minor cosmetic differences, and this should fix both): --- a/libc/upstream-openbsd/lib/libc/gdtoa/hdtoa.c +++ b/libc/upstream-openbsd/lib/libc/gdtoa/hdtoa.c @@ -278,18 +278,18 @@ __hldtoa(long double e, const char *xdigs, int ndigits, int *decpt, int *sign, p->ext_fracl >>= 4; } #ifdef EXT_FRACHMBITS - for (; s > s0; s--) { + for (; s > s0 + sigfigs - ((EXT_FRACLBITS + EXT_FRACHMBITS) / 4) - 1; s--) { *s = p->ext_frachm & 0xf; p->ext_frachm >>= 4; } #endif #ifdef EXT_FRACLMBITS - for (; s > s0; s--) { + for (; s > s0 + sigfigs - ((EXT_FRACLBITS + EXT_FRACHMBITS + EXT_FRACLMBITS) / 4) - 1; s--) { *s = p->ext_fraclm & 0xf; p->ext_fraclm >>= 4; } #endif - for (; s > s0; s--) { + for (; s > s0 + sigfigs - ((EXT_FRACLBITS + EXT_FRACHMBITS + EXT_FRACLMBITS + EXT_FRACHBITS) / 4) - 1; s--) { *s = p->ext_frach & 0xf; p->ext_frach >>= 4; } @@ -300,7 +300,7 @@ __hldtoa(long double e, const char *xdigs, int ndigits, int *decpt, int *sign, * (partial) nibble, which is dealt with by the next * statement. We also tack on the implicit normalization bit. */ - *s = p->ext_frach | (1U << ((LDBL_MANT_DIG - 1) % 4)); + *s = (p->ext_frach | (1U << ((LDBL_MANT_DIG - 1) % 4))) & 0xf; /* If ndigits < 0, we are expected to auto-size the precision. */ if (ndigits < 0) { if you want to watch it going wrong live in a way that makes the bug quite plain, i recommend adding `fprintf(stderr, "%02x // %x\n", *s, p->ext_);` between each `*s =` line and the `>>= 4` line that follows. that was where i understood what was going wrong. thanks!
Cache incoherent ACPI support
Some semi-recent ACPI revision introduced the _CCA method. This method indicates whether DMA is cache-coherent for a device. This isn't really relevant on i386/amd64 since its busses are pretty much always fully cache-coherent. But on amr64 things are different. Server machines typically have coherent busses, but the smaller systems are incoherent. So far we have ignored this aspect, since our arm64 ACPI support is mostly there to support server systems. But now there is an EDK2-based UEFI firmware for the Raspberry Pi3 and Pi4. This firmware includes ACPI support and ACPI is the default. The diff below adds support for _CCA by making acpi(4) use two differt bus_dma tags: one for cache-coherent DMA and one for cache-incoherent DMA. On i386/amd64 both of these are set to the same value. But for am64 they are different. If the _CCA method is absent we assume DMA is cache-coherent. With this diff (and the xhci diff I just mailed out) USB 3.0 works on the rpi4 and the od1000 and ampere systems still work. I also tested this on amd64. ok? Index: arch/amd64/amd64/acpi_machdep.c === RCS file: /cvs/src/sys/arch/amd64/amd64/acpi_machdep.c,v retrieving revision 1.89 diff -u -p -r1.89 acpi_machdep.c --- arch/amd64/amd64/acpi_machdep.c 20 Dec 2019 07:49:31 - 1.89 +++ arch/amd64/amd64/acpi_machdep.c 10 Apr 2020 15:35:50 - @@ -96,7 +96,8 @@ acpi_attach(struct device *parent, struc sc->sc_iot = ba->ba_iot; sc->sc_memt = ba->ba_memt; - sc->sc_dmat = &pci_bus_dma_tag; + sc->sc_cc_dmat = &pci_bus_dma_tag; + sc->sc_ci_dmat = &pci_bus_dma_tag; acpi_attach_common(sc, ba->ba_acpipbase); } Index: arch/arm64/arm64/acpi_machdep.c === RCS file: /cvs/src/sys/arch/arm64/arm64/acpi_machdep.c,v retrieving revision 1.3 diff -u -p -r1.3 acpi_machdep.c --- arch/arm64/arm64/acpi_machdep.c 27 Aug 2019 22:39:53 - 1.3 +++ arch/arm64/arm64/acpi_machdep.c 10 Apr 2020 15:35:50 - @@ -56,13 +56,14 @@ acpi_fdt_attach(struct device *parent, s struct fdt_attach_args *faa = aux; bus_dma_tag_t dmat; + sc->sc_memt = faa->fa_iot; + sc->sc_ci_dmat = faa->fa_dmat; + /* Create coherent DMA tag. */ - dmat = malloc(sizeof(*sc->sc_dmat), M_DEVBUF, M_WAITOK | M_ZERO); + dmat = malloc(sizeof(*sc->sc_cc_dmat), M_DEVBUF, M_WAITOK | M_ZERO); memcpy(dmat, faa->fa_dmat, sizeof(*dmat)); dmat->_flags |= BUS_DMA_COHERENT; - - sc->sc_memt = faa->fa_iot; - sc->sc_dmat = dmat; + sc->sc_cc_dmat = dmat; acpi_attach_common(sc, faa->fa_reg[0].addr); } Index: arch/i386/i386/acpi_machdep.c === RCS file: /cvs/src/sys/arch/i386/i386/acpi_machdep.c,v retrieving revision 1.72 diff -u -p -r1.72 acpi_machdep.c --- arch/i386/i386/acpi_machdep.c 20 Dec 2019 07:55:30 - 1.72 +++ arch/i386/i386/acpi_machdep.c 10 Apr 2020 15:35:50 - @@ -106,7 +106,8 @@ acpi_attach(struct device *parent, struc sc->sc_iot = ba->ba_iot; sc->sc_memt = ba->ba_memt; - sc->sc_dmat = &pci_bus_dma_tag; + sc->sc_cc_dmat = &pci_bus_dma_tag; + sc->sc_ci_dmat = &pci_bus_dma_tag; acpi_attach_common(sc, ba->ba_acpipbase); } Index: dev/acpi/acpi.c === RCS file: /cvs/src/sys/dev/acpi/acpi.c,v retrieving revision 1.380 diff -u -p -r1.380 acpi.c --- dev/acpi/acpi.c 7 Apr 2020 13:27:51 - 1.380 +++ dev/acpi/acpi.c 10 Apr 2020 15:35:50 - @@ -3122,6 +3122,7 @@ acpi_foundhid(struct aml_node *node, voi char dev[32]; struct acpi_attach_args aaa; int64_t sta; + int64_t cca; #ifndef SMALL_KERNEL int i; #endif @@ -3133,12 +3134,15 @@ acpi_foundhid(struct aml_node *node, voi if ((sta & STA_PRESENT) == 0) return (0); + if (aml_evalinteger(sc, node->parent, "_CCA", 0, NULL, &cca)) + cca = 1; + acpi_attach_deps(sc, node->parent); memset(&aaa, 0, sizeof(aaa)); aaa.aaa_iot = sc->sc_iot; aaa.aaa_memt = sc->sc_memt; - aaa.aaa_dmat = sc->sc_dmat; + aaa.aaa_dmat = cca ? sc->sc_cc_dmat : sc->sc_ci_dmat; aaa.aaa_node = node->parent; aaa.aaa_dev = dev; aaa.aaa_cdev = cdev; Index: dev/acpi/acpivar.h === RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v retrieving revision 1.105 diff -u -p -r1.105 acpivar.h --- dev/acpi/acpivar.h 7 Sep 2019 13:46:20 - 1.105 +++ dev/acpi/acpivar.h 10 Apr 2020 15:35:50 - @@ -208,7 +208,8 @@ struct acpi_softc { bus_space_tag_t sc_iot; bus_space_tag_t
xhci(4) and acpi(4)
The Rapsberry Pi4 UEFI firmware (in ACPI mode) uses a QWord() resource descriptor for the address. ok? P.S. I still plan to move this parsing code into something a bit more generic at some point instead of replicating slightly different versions in each driver. Index: dev/acpi/xhci_acpi.c === RCS file: /cvs/src/sys/dev/acpi/xhci_acpi.c,v retrieving revision 1.1 diff -u -p -r1.1 xhci_acpi.c --- dev/acpi/xhci_acpi.c2 Jul 2018 11:23:19 - 1.1 +++ dev/acpi/xhci_acpi.c10 Apr 2020 15:32:52 - @@ -153,6 +153,13 @@ xhci_acpi_parse_resources(int crsidx, un sc->sc_size = crs->lr_m32fixed._len; } break; + case LR_QWORD: + /* XHCI registers are specified by the first resource. */ + if (sc->sc_size == 0) { + sc->sc_addr = crs->lr_qword._min; + sc->sc_size = crs->lr_qword._len; + } + break; case LR_EXTIRQ: sc->sc_irq = crs->lr_extirq.irq[0]; sc->sc_irq_flags = crs->lr_extirq.flags;
pppx(4): kill useless rwlocks
Subj. Index: net/if_pppx.c === RCS file: /cvs/src/sys/net/if_pppx.c,v retrieving revision 1.83 diff -u -p -r1.83 if_pppx.c --- net/if_pppx.c 10 Apr 2020 07:36:52 - 1.83 +++ net/if_pppx.c 10 Apr 2020 11:16:53 - @@ -132,12 +132,11 @@ struct pppx_dev { LIST_HEAD(,pppx_if) pxd_pxis; }; -struct rwlock pppx_devs_lk = RWLOCK_INITIALIZER("pppxdevs"); LIST_HEAD(, pppx_dev) pppx_devs = LIST_HEAD_INITIALIZER(pppx_devs); struct pool*pppx_if_pl; struct pppx_dev*pppx_dev_lookup(dev_t); -struct pppx_dev*pppx_dev2pxd(dev_t); +static inline struct pppx_dev *pppx_dev2pxd(dev_t); struct pppx_if_key { int pxik_session_id; @@ -165,7 +164,6 @@ pppx_if_cmp(const struct pppx_if *a, con return memcmp(&a->pxi_key, &b->pxi_key, sizeof(a->pxi_key)); } -struct rwlock pppx_ifs_lk = RWLOCK_INITIALIZER("pppxifs"); RBT_HEAD(pppx_ifs, pppx_if)pppx_ifs = RBT_INITIALIZER(&pppx_ifs); RBT_PROTOTYPE(pppx_ifs, pppx_if, pxi_entry, pppx_if_cmp); @@ -219,8 +217,6 @@ pppx_dev_lookup(dev_t dev) struct pppx_dev *pxd; int unit = minor(dev); - /* must hold pppx_devs_lk */ - LIST_FOREACH(pxd, &pppx_devs, pxd_entry) { if (pxd->pxd_unit == unit) return (pxd); @@ -229,16 +225,10 @@ pppx_dev_lookup(dev_t dev) return (NULL); } -struct pppx_dev * +static inline struct pppx_dev * pppx_dev2pxd(dev_t dev) { - struct pppx_dev *pxd; - - rw_enter_read(&pppx_devs_lk); - pxd = pppx_dev_lookup(dev); - rw_exit_read(&pppx_devs_lk); - - return (pxd); + return pppx_dev_lookup(dev); } void @@ -251,17 +241,10 @@ int pppxopen(dev_t dev, int flags, int mode, struct proc *p) { struct pppx_dev *pxd; - int rv = 0; - - rv = rw_enter(&pppx_devs_lk, RW_WRITE | RW_INTR); - if (rv != 0) - return (rv); pxd = pppx_dev_lookup(dev); - if (pxd != NULL) { - rv = EBUSY; - goto out; - } + if (pxd != NULL) + return EBUSY; if (LIST_EMPTY(&pppx_devs) && pppx_if_pl == NULL) { pppx_if_pl = malloc(sizeof(*pppx_if_pl), M_DEVBUF, M_WAITOK); @@ -279,9 +262,7 @@ pppxopen(dev_t dev, int flags, int mode, mq_init(&pxd->pxd_svcq, 128, IPL_NET); LIST_INSERT_HEAD(&pppx_devs, pxd, pxd_entry); -out: - rw_exit(&pppx_devs_lk); - return (rv); + return 0; } int @@ -588,8 +569,6 @@ pppxclose(dev_t dev, int flags, int mode struct pppx_dev *pxd; struct pppx_if *pxi; - rw_enter_write(&pppx_devs_lk); - pxd = pppx_dev_lookup(dev); /* XXX */ @@ -610,7 +589,6 @@ pppxclose(dev_t dev, int flags, int mode pppx_if_pl = NULL; } - rw_exit_write(&pppx_devs_lk); return (0); } @@ -620,8 +598,6 @@ pppx_if_next_unit(void) struct pppx_if *pxi; int unit = 0; - rw_assert_wrlock(&pppx_ifs_lk); - /* this is safe without splnet since we're not modifying it */ do { int found = 0; @@ -650,11 +626,9 @@ pppx_if_find(struct pppx_dev *pxd, int s key.pxik_session_id = session_id; key.pxik_protocol = protocol; - rw_enter_read(&pppx_ifs_lk); pxi = RBT_FIND(pppx_ifs, &pppx_ifs, (struct pppx_if *)&key); if (pxi && pxi->pxi_ready == 0) pxi = NULL; - rw_exit_read(&pppx_ifs_lk); return pxi; } @@ -828,12 +802,10 @@ pppx_add_session(struct pppx_dev *pxd, s #endif /* try to set the interface up */ - rw_enter_write(&pppx_ifs_lk); unit = pppx_if_next_unit(); if (unit < 0) { pool_put(pppx_if_pl, pxi); error = ENOMEM; - rw_exit_write(&pppx_ifs_lk); goto out; } @@ -846,14 +818,12 @@ pppx_add_session(struct pppx_dev *pxd, s if (RBT_FIND(pppx_ifs, &pppx_ifs, pxi) != NULL) { pool_put(pppx_if_pl, pxi); error = EADDRINUSE; - rw_exit_write(&pppx_ifs_lk); goto out; } if (RBT_INSERT(pppx_ifs, &pppx_ifs, pxi) != NULL) panic("%s: pppx_ifs modified while lock was held", __func__); LIST_INSERT_HEAD(&pxd->pxd_pxis, pxi, pxi_list); - rw_exit_write(&pppx_ifs_lk); snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit); ifp->if_mtu = req->pr_peer_mru; /* XXX */ @@ -935,9 +905,8 @@ pppx_add_session(struct pppx_dev *pxd, s } else { if_addrhooks_run(ifp); } - rw_enter_write(&pppx_ifs_lk); + pxi->pxi_ready = 1; - rw_exit_write(&pppx_ifs_lk); out: return (error); @@ -1038,11 +1007,9 @@ pppx_if_
kqueue_scan() refactoring
Diff below reduces kqueue_scan() to the collect of events on a given kqueue and let its caller, sys_kevent(), responsible for the copyout(9). Apart from the code simplification, this refactoring clearly separates kqueue_scan() from the syscall logic. That should allow us to re-use the function in a different context and to address its need for locking independently. Since the number of events that are ready to be collected can be bigger than the size of the array, the pair kqueue_scan()/copyout() may be called multiple times. In that case, successive calls should no longer block, this is performed by using a zero, but not NULL, timeout which correspond to a non-blocking scan. This is the next piece of the ongoing work to move select/poll/kqueue. I'd like to be sure it doesn't introduce any regression. Comments, tests and oks welcome :o) Index: kern/kern_event.c === RCS file: /cvs/src/sys/kern/kern_event.c,v retrieving revision 1.131 diff -u -p -r1.131 kern_event.c --- kern/kern_event.c 7 Apr 2020 13:27:51 - 1.131 +++ kern/kern_event.c 9 Apr 2020 17:09:58 - @@ -62,9 +62,8 @@ void KQREF(struct kqueue *); void KQRELE(struct kqueue *); intkqueue_sleep(struct kqueue *, struct timespec *); -intkqueue_scan(struct kqueue *kq, int maxevents, - struct kevent *ulistp, struct timespec *timeout, - struct proc *p, int *retval); +intkqueue_scan(struct kqueue *, struct kevent *, int, struct timespec *, + struct proc *, int *); intkqueue_read(struct file *, struct uio *, int); intkqueue_write(struct file *, struct uio *, int); @@ -544,6 +543,7 @@ sys_kevent(struct proc *p, void *v, regi struct timespec ts; struct timespec *tsp = NULL; int i, n, nerrors, error; + int ready, total; struct kevent kev[KQ_NEVENTS]; if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL) @@ -612,10 +612,32 @@ sys_kevent(struct proc *p, void *v, regi KQREF(kq); FRELE(fp, p); - error = kqueue_scan(kq, SCARG(uap, nevents), SCARG(uap, eventlist), - tsp, p, &n); + /* +* Collect as many events as we can. The timeout on successive +* loops is disabled (kqueue_scan() becomes non-blocking). +*/ + total = 0; + error = 0; + while ((n = SCARG(uap, nevents) - total) > 0) { + if (n > nitems(kev)) + n = nitems(kev); + ready = kqueue_scan(kq, kev, n, tsp, p, &error); + if (ready == 0) + break; + error = copyout(kev, SCARG(uap, eventlist) + total, + sizeof(struct kevent) * ready); + total += ready; + /* +* Stop if there was an error or if we had enough +* place to collect all events that were ready. +*/ + if (error || ready < n) + break; + tsp = &ts; /* successive loops non-blocking */ + timespecclear(tsp); + } KQRELE(kq); - *retval = n; + *retval = total; return (error); done: @@ -869,18 +891,18 @@ kqueue_sleep(struct kqueue *kq, struct t return (error); } +/* + * Scan the kqueue, blocking if necessary until the target time is reached. + * If tsp is NULL we block indefinitely. If tsp->ts_secs/nsecs are both + * 0 we do not block at all. + */ int -kqueue_scan(struct kqueue *kq, int maxevents, struct kevent *ulistp, -struct timespec *tsp, struct proc *p, int *retval) +kqueue_scan(struct kqueue *kq, struct kevent *kev, int count, +struct timespec *tsp, struct proc *p, int *errorp) { - struct kevent *kevp; struct knote mend, mstart, *kn; - int s, count, nkev = 0, error = 0; - struct kevent kev[KQ_NEVENTS]; - - count = maxevents; - if (count == 0) - goto done; + int s, nkev = 0, error = 0; + struct kevent *kevp = kev; memset(&mstart, 0, sizeof(mstart)); memset(&mend, 0, sizeof(mend)); @@ -891,7 +913,6 @@ retry: goto done; } - kevp = &kev[0]; s = splhigh(); if (kq->kq_count == 0) { if (tsp != NULL && !timespecisset(tsp)) { @@ -910,6 +931,9 @@ retry: goto done; } + /* +* Collect events +*/ mstart.kn_filter = EVFILT_MARKER; mstart.kn_status = KN_PROCESSING; TAILQ_INSERT_HEAD(&kq->kq_head, &mstart, kn_tqe); @@ -919,14 +943,8 @@ retry: while (count) { kn = TAILQ_NEXT(&mstart, kn_tqe); if (kn->kn_filter == EVFILT_MARKER) { - if (kn == &mend) { - TAILQ_REMOVE(&kq->kq_head, &mend, kn_tqe); - TAILQ_REMOVE(&kq->kq_head, &m
em(4) and FOREACH_QUEUE()
Unlike ix(4) the em(4) driver still needs some more code shuffling in order to be ready for multi-queue support. The diff below introduces the macro FOREACH_QUEUE() and use it where some per-queue setup is required. It only convert the trivial places and whitespace changes aren't included to ease the review. Other non trivial places as well as the remaining architectural issues will be dealt in later diffs. Attachment (em.diff) is the full unified diff. Tests and oks welcome :o) Index: dev/pci/if_em.c === RCS file: /cvs/src/sys/dev/pci/if_em.c,v retrieving revision 1.349 diff -u -p -b -r1.349 if_em.c --- dev/pci/if_em.c 24 Mar 2020 09:30:06 - 1.349 +++ dev/pci/if_em.c 9 Apr 2020 15:15:55 - @@ -1779,7 +1779,7 @@ em_free_pci_resources(struct em_softc *s sc->osdep.em_memsize); sc->osdep.em_membase = 0; - que = sc->queues; /* Use only first queue. */ + FOREACH_QUEUE(sc, que) { if (que->rx.sc_rx_desc_ring != NULL) { que->rx.sc_rx_desc_ring = NULL; em_dma_free(sc, &que->rx.sc_rx_dma); @@ -1794,6 +1794,7 @@ em_free_pci_resources(struct em_softc *s que->eims = 0; que->me = 0; que->sc = NULL; + } sc->legacy_irq = 0; sc->msix_linkvec = 0; sc->msix_queuesmask = 0; @@ -2152,8 +2153,9 @@ em_dma_free(struct em_softc *sc, struct int em_allocate_transmit_structures(struct em_softc *sc) { - struct em_queue *que = sc->queues; /* Use only first queue. */ + struct em_queue *que; + FOREACH_QUEUE(sc, que) { bus_dmamap_sync(sc->sc_dmat, que->tx.sc_tx_dma.dma_map, 0, que->tx.sc_tx_dma.dma_map->dm_mapsize, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); @@ -2165,6 +2167,7 @@ em_allocate_transmit_structures(struct e DEVNAME(sc)); return (ENOMEM); } + } return (0); } @@ -2177,13 +2180,14 @@ em_allocate_transmit_structures(struct e int em_setup_transmit_structures(struct em_softc *sc) { - struct em_queue *que = sc->queues; /* Use only first queue. */ + struct em_queue *que; struct em_packet *pkt; int error, i; if ((error = em_allocate_transmit_structures(sc)) != 0) goto fail; + FOREACH_QUEUE(sc, que) { bzero((void *) que->tx.sc_tx_desc_ring, (sizeof(struct em_tx_desc)) * sc->sc_tx_slots); @@ -2204,6 +2208,7 @@ em_setup_transmit_structures(struct em_s /* Set checksum context */ que->tx.active_checksum_context = OFFLOAD_NONE; + } return (0); @@ -2220,12 +2225,13 @@ fail: void em_initialize_transmit_unit(struct em_softc *sc) { - struct em_queue *que = sc->queues; /* Use only first queue. */ u_int32_t reg_tctl, reg_tipg = 0; u_int64_t bus_addr; + struct em_queue *que; INIT_DEBUGOUT("em_initialize_transmit_unit: begin"); + FOREACH_QUEUE(sc, que) { /* Setup the Base and Length of the Tx Descriptor Ring */ bus_addr = que->tx.sc_tx_dma.dma_map->dm_segs[0].ds_addr; E1000_WRITE_REG(&sc->hw, TDLEN(que->me), @@ -2282,6 +2288,7 @@ em_initialize_transmit_unit(struct em_so E1000_WRITE_REG(&sc->hw, TXDCTL(que->me), reg_tctl); } else if (sc->tx_int_delay > 0) que->tx.sc_txd_cmd |= E1000_TXD_CMD_IDE; + } /* Program the Transmit Control Register */ reg_tctl = E1000_TCTL_PSP | E1000_TCTL_EN | @@ -2320,12 +2327,13 @@ em_initialize_transmit_unit(struct em_so void em_free_transmit_structures(struct em_softc *sc) { - struct em_queue *que = sc->queues; /* Use only first queue. */ + struct em_queue *que; struct em_packet *pkt; int i; INIT_DEBUGOUT("free_transmit_structures: begin"); + FOREACH_QUEUE(sc, que) { if (que->tx.sc_tx_pkts_ring != NULL) { for (i = 0; i < sc->sc_tx_slots; i++) { pkt = &que->tx.sc_tx_pkts_ring[i]; @@ -2334,14 +2342,16 @@ em_free_transmit_structures(struct em_so bus_dmamap_sync(sc->sc_dmat, pkt->pkt_map, 0, pkt->pkt_map->dm_mapsize, BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(sc->sc_dmat, pkt->pkt_map); + bus_dmamap_unload(sc->sc_dmat, + pkt->pkt_map); m_freem(pkt->pkt_m); pkt->pkt_m = NULL; } if (pkt->pkt_map != NULL) { - bus_dmamap_destroy(sc->sc_dmat, pkt->pkt_map); + bus_dmamap_destroy(sc->sc_dmat, +
switch powerpc to MI mplock
In order to reduce the differences with other architecture and to be able to use WITNESS on powerpc I'm proposing the diff below that makes use of the MI mp (ticket) lock implementation for powerpc. This has been tested by Peter J. Philipp but I'd like to have more tests before proceeding. As explained previously the pmap code, which is using a recursive spinlock to protect the hash, still uses the old lock implementation with this diff. Please fire your MP macppc and report back :o) Index: arch/powerpc/include/mplock.h === RCS file: /cvs/src/sys/arch/powerpc/include/mplock.h,v retrieving revision 1.3 diff -u -p -r1.3 mplock.h --- arch/powerpc/include/mplock.h 4 Dec 2017 09:51:03 - 1.3 +++ arch/powerpc/include/mplock.h 9 Apr 2020 16:21:55 - @@ -27,25 +27,27 @@ #ifndef _POWERPC_MPLOCK_H_ #define _POWERPC_MPLOCK_H_ +#define __USE_MI_MPLOCK + /* * Really simple spinlock implementation with recursive capabilities. * Correctness is paramount, no fancyness allowed. */ -struct __mp_lock { +struct __ppc_lock { volatile struct cpu_info *mpl_cpu; volatile long mpl_count; }; #ifndef _LOCORE -void __mp_lock_init(struct __mp_lock *); -void __mp_lock(struct __mp_lock *); -void __mp_unlock(struct __mp_lock *); -int __mp_release_all(struct __mp_lock *); -int __mp_release_all_but_one(struct __mp_lock *); -void __mp_acquire_count(struct __mp_lock *, int); -int __mp_lock_held(struct __mp_lock *, struct cpu_info *); +void __ppc_lock_init(struct __ppc_lock *); +void __ppc_lock(struct __ppc_lock *); +void __ppc_unlock(struct __ppc_lock *); +int __ppc_release_all(struct __ppc_lock *); +int __ppc_release_all_but_one(struct __ppc_lock *); +void __ppc_acquire_count(struct __ppc_lock *, int); +int __ppc_lock_held(struct __ppc_lock *, struct cpu_info *); #endif Index: arch/powerpc/powerpc/lock_machdep.c === RCS file: /cvs/src/sys/arch/powerpc/powerpc/lock_machdep.c,v retrieving revision 1.8 diff -u -p -r1.8 lock_machdep.c --- arch/powerpc/powerpc/lock_machdep.c 5 Mar 2020 09:28:31 - 1.8 +++ arch/powerpc/powerpc/lock_machdep.c 9 Apr 2020 16:21:01 - @@ -27,7 +27,7 @@ #include void -__mp_lock_init(struct __mp_lock *lock) +__ppc_lock_init(struct __ppc_lock *lock) { lock->mpl_cpu = NULL; lock->mpl_count = 0; @@ -43,7 +43,7 @@ extern int __mp_lock_spinout; #endif static __inline void -__mp_lock_spin(struct __mp_lock *mpl) +__ppc_lock_spin(struct __ppc_lock *mpl) { #ifndef MP_LOCKDEBUG while (mpl->mpl_count != 0) @@ -55,14 +55,14 @@ __mp_lock_spin(struct __mp_lock *mpl) CPU_BUSY_CYCLE(); if (nticks == 0) { - db_printf("__mp_lock(%p): lock spun out\n", mpl); + db_printf("__ppc_lock(%p): lock spun out\n", mpl); db_enter(); } #endif } void -__mp_lock(struct __mp_lock *mpl) +__ppc_lock(struct __ppc_lock *mpl) { /* * Please notice that mpl_count gets incremented twice for the @@ -92,18 +92,18 @@ __mp_lock(struct __mp_lock *mpl) } ppc_intr_enable(s); - __mp_lock_spin(mpl); + __ppc_lock_spin(mpl); } } void -__mp_unlock(struct __mp_lock *mpl) +__ppc_unlock(struct __ppc_lock *mpl) { int s; #ifdef MP_LOCKDEBUG if (mpl->mpl_cpu != curcpu()) { - db_printf("__mp_unlock(%p): not held lock\n", mpl); + db_printf("__ppc_unlock(%p): not held lock\n", mpl); db_enter(); } #endif @@ -118,14 +118,14 @@ __mp_unlock(struct __mp_lock *mpl) } int -__mp_release_all(struct __mp_lock *mpl) +__ppc_release_all(struct __ppc_lock *mpl) { int rv = mpl->mpl_count - 1; int s; #ifdef MP_LOCKDEBUG if (mpl->mpl_cpu != curcpu()) { - db_printf("__mp_release_all(%p): not held lock\n", mpl); + db_printf("__ppc_release_all(%p): not held lock\n", mpl); db_enter(); } #endif @@ -140,13 +140,13 @@ __mp_release_all(struct __mp_lock *mpl) } int -__mp_release_all_but_one(struct __mp_lock *mpl) +__ppc_release_all_but_one(struct __ppc_lock *mpl) { int rv = mpl->mpl_count - 2; #ifdef MP_LOCKDEBUG if (mpl->mpl_cpu != curcpu()) { - db_printf("__mp_release_all_but_one(%p): not held lock\n", mpl); + db_printf("__ppc_release_all_but_one(%p): not held lock\n", mpl); db_enter(); } #endif @@ -157,14 +157,14 @@ __mp_release_all_but_one(struct __mp_loc } void -__mp_acquire_count(struct __mp_lock *mpl, int count) +__ppc_acquire_count(struct __ppc_lock *mpl, int count) { while (count--) - __mp_lock(mpl); + __ppc_lock(mpl); } int -__mp_lock_held(struct __mp_lock *mpl, struct cpu_info *ci) +__ppc_lock_held(
Re: powerpc: mplock & WITNESS
On 09/04/20(Thu) 22:58, George Koehler wrote: > On Thu, 9 Apr 2020 20:05:34 +0200 > Martin Pieuchot wrote: > [...] > In the trace, #0 and #1 are wrong, but the rest of the trace looks > good enough for WITNESS. I added an artificial lock order reversal to > ums(4) for WITNESS to catch. I got this trace, > > #0 0xe4d764 > #1 witness_checkorder+0x308 > #2 mtx_enter+0x50 > #3 ums_attach+0x68 > #4 config_attach+0x270 > ... > > "#0 0xe4d764" is stack garbage: a function saves its lr in its > caller's frame, but stacktrace_save_at() first reads the lr slot in > its own frame. > > "#1 witness_checkorder+0x308" is a dead value. In the disassembly > (objdump -dlr db_trace.o), clang optimized stacktrace_save_at() to > skip saving its lr on the stack, because it is a leaf function (that > never calls other functions). The lr from the stack isn't the return > address for stacktrace_save_at(), but might be a leftover return > address from some other function (that needed to save lr). > > #2 and after seem to be correct; "#3 ums_attach+0x68" points exactly > to where I am grabbing the second lock. This is enough for WITNESS, > so we might want to use your diff now, and fix #0 and #1 later. > > Also know that a compiler may optimize stacktrace_save_at() to have > no stack frame. Our clang 8.0.1 never does this (because it always > sets r31 = stack pointer r1, so it always needs a stack frame to save > the caller's r31), but gcc and newer clang might. I don't know how > __builtin_frame_address(0) works if the stack frame is gone. Thanks for the debugging and explanation George! I committed the function it was. I hope you or someone else can fix it ;)
Re: openssl(1) x509, change in serial number output between 6.5 and 6.6
On Thu, Apr 09, 2020 at 07:44:51PM +0100, Stuart Henderson wrote: > On 2020/04/09 20:13, Theo Buehler wrote: > > On Thu, Apr 09, 2020 at 05:56:55PM +0100, Stuart Henderson wrote: > > > Not new - this happened somewhere between 6.5 and 6.6 - but some > > > certificates are now showing up with bad serial numbers in "openssl x509". > > > Example below, a few other certs are affected. > > > > > > From the current /etc/ssl/cert.pem, these are the ones showing the same: > > > > > > -Serial Number: 11806822484801597146 (0xa3da427ea4b1aeda) > > > -Serial Number: 14541511773111788494 (0xc9cdd3e9d57d23ce) > > > -Serial Number: 18364802974209362175 (0xfedce3010fc948ff) > > > -Serial Number: 10572350602393338211 (0x92b888dbb08ac163) > > > -Serial Number: 14014712776195784473 (0xc27e43044e473f19) > > > -Serial Number: 13492815561806991280 (0xbb401c43f55e4fb0) > > > -Serial Number: 9548242946988625984 (0x84822c5f1c62d040) > > > -Serial Number: 15752444095811006489 (0xda9bec71f303b019) > > > > This is due to r1.34 of src/lib/libcrypto/asn1/a_int.c which changed > > ASN1_INTEGER_get() to avoid undefined behavior. It now returns -1 more > > often. Note that your examples are all larger than LONG_MAX. > > > > Minimal fix for this issue is to use the fallback to colon separated hex > > digits in X509_print_ex() in case ASN1_INTEGER_get() returns an error so > > that your example cert yields: > > > > Serial Number: > > da:9b:ec:71:f3:03:b0:19 > > Aha - this matches what OpenSSL does. I'm OK with this. > > > Index: asn1/t_x509.c > > === > > RCS file: /var/cvs/src/lib/libcrypto/asn1/t_x509.c,v > > retrieving revision 1.31 > > diff -u -p -r1.31 t_x509.c > > --- asn1/t_x509.c 18 May 2018 18:23:24 - 1.31 > > +++ asn1/t_x509.c 9 Apr 2020 17:53:51 - > > @@ -145,8 +145,10 @@ X509_print_ex(BIO *bp, X509 *x, unsigned > > goto err; > > > > bs = X509_get_serialNumber(x); > > - if (bs->length <= (int)sizeof(long)) { > > + l = -1; > > + if (bs->length <= (int)sizeof(long)) > > l = ASN1_INTEGER_get(bs); > > + if (l != -1) { > > if (bs->type == V_ASN1_NEG_INTEGER) { > > l = -l; > > neg = "-"; > > I'm fine with this fix, but 1 comment. How about doing below rather than assign -1 to 'l' for readability ? Index: t_x509.c === RCS file: /cvs/src/lib/libcrypto/asn1/t_x509.c,v retrieving revision 1.31 diff -u -p -u -p -r1.31 t_x509.c --- t_x509.c18 May 2018 18:23:24 - 1.31 +++ t_x509.c10 Apr 2020 07:08:43 - @@ -145,8 +145,8 @@ X509_print_ex(BIO *bp, X509 *x, unsigned goto err; bs = X509_get_serialNumber(x); - if (bs->length <= (int)sizeof(long)) { - l = ASN1_INTEGER_get(bs); + if (bs->length <= (int)sizeof(long) && + (l = ASN1_INTEGER_get(bs)) != -1) { if (bs->type == V_ASN1_NEG_INTEGER) { l = -l; neg = "-";