Re: Remove some customization from our perl build

2020-04-10 Thread Theo de Raadt
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

2020-04-10 Thread Todd C . Miller
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

2020-04-10 Thread Andrew Hewus Fresh
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

2020-04-10 Thread enh
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

2020-04-10 Thread Mark Kettenis
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)

2020-04-10 Thread Mark Kettenis
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

2020-04-10 Thread Vitaliy Makkoveev
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

2020-04-10 Thread Martin Pieuchot
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()

2020-04-10 Thread Martin Pieuchot
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

2020-04-10 Thread Martin Pieuchot
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

2020-04-10 Thread Martin Pieuchot
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

2020-04-10 Thread Kinichiro Inoguchi
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 = "-";