Re: [Patch]: Integrate VA-API into xenocara

2020-01-23 Thread Jonathan Gray
On Fri, Jan 24, 2020 at 04:45:07PM +1100, Jonathan Gray wrote:
> On Thu, Jan 23, 2020 at 12:39:29PM +1100, Jonathan Gray wrote:
> > On Wed, Dec 18, 2019 at 03:28:48PM -0600, Brad DeMorrow wrote:
> > > This is a rather large patch that moves my previously submitted
> > > VA-API ports into xenocara. For your convenience, I've inlined
> > > a diff that shows you all of the changes I made to existing files
> > > that you can easily read in your MUA.  The attached patch also
> > > contains these changes and should be the only xenocara patch
> > > you need to apply.
> > > 
> > > Summary of Changes:
> > >  - libva added to xenocara/lib/libva
> > >  - vainfo added to xenocara/app/vainfo
> > >  - intel-vaapi-driver added to xenocara/driver/intel-vaapi-driver
> > >  - Mesa Makefile.bsd-wrapper updated to build with --enable-va flag
> > >  - 3RDPARTY file updated to include libva, libva-utils, and 
> > > intel-vaapi-driver
> > >  - BSD.x11.dist updated to include /usr/X11R6/include/va/ (separate patch)
> > 
> > It is difficult to see where you have made changes, can you send patches
> > against source from particular tarballs?
> 
> I built libva-2.6.1 but it does not work with non-intel drivers at
> runtime due to a Mesa bug:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=109929
> 
> libva info: VA-API version 1.6.0
> libva info: Trying to open /usr/X11R6/lib/dri/radeonsi_drv_video.so
> vainfo:/usr/X11R6/lib/dri/radeonsi_drv_video.so: undefined symbol 
> 'gl_nir_lower_samplers_as_deref'
> vainfo:/usr/X11R6/lib/dri/radeonsi_drv_video.so: undefined symbol 
> 'gl_nir_lower_samplers'
> libva error: dlopen of /usr/X11R6/lib/dri/radeonsi_drv_video.so failed: 
> Cannot load specified object
> libva info: va_openDriver() returns -1
> vaInitialize failed with error code -1 (unknown libva error),exit
> 
> Here is the diff for libva-2.6.1 from memory the horrible ifdef part
> came from FreeBSD via your port.

patch below for va-utils it installs quite a few binaries

/usr/X11R6/bin/avcenc
/usr/X11R6/bin/avcstreamoutdemo
/usr/X11R6/bin/h264encode
/usr/X11R6/bin/hevcencode
/usr/X11R6/bin/jpegenc
/usr/X11R6/bin/loadjpeg
/usr/X11R6/bin/mpeg2vaenc
/usr/X11R6/bin/mpeg2vldemo
/usr/X11R6/bin/putsurface
/usr/X11R6/bin/sfcsample
/usr/X11R6/bin/vainfo
/usr/X11R6/bin/vavpp
/usr/X11R6/bin/vp8enc
/usr/X11R6/bin/vp9enc
/usr/X11R6/bin/vppblending
/usr/X11R6/bin/vppchromasitting
/usr/X11R6/bin/vppdenoise
/usr/X11R6/bin/vppscaling_csc
/usr/X11R6/bin/vppscaling_n_

--- /dev/null   Fri Jan 24 16:43:41 2020
+++ libva-utils-2.6.0/Makefile.bsd-wrapper  Fri Jan 24 14:02:34 2020
@@ -0,0 +1,3 @@
+# $OpenBSD$
+
+.include 
diff -upr -x *.m4 -x Makefile.in libva-utils-2.6.0.orig/encode/jpegenc_utils.h 
libva-utils-2.6.0/encode/jpegenc_utils.h
--- libva-utils-2.6.0.orig/encode/jpegenc_utils.h   Wed Dec 26 21:29:26 2018
+++ libva-utils-2.6.0/encode/jpegenc_utils.hFri Jan 24 14:06:25 2020
@@ -50,7 +50,7 @@ struct __bitstream {
 typedef struct __bitstream bitstream;
 
 static unsigned int 
-swap32(unsigned int val)
+va_swap32(unsigned int val)
 {
 unsigned char *pval = (unsigned char *)
 
@@ -77,7 +77,7 @@ bitstream_end(bitstream *bs)
 int bit_left = 32 - bit_offset;
 
 if (bit_offset) {
-bs->buffer[pos] = swap32((bs->buffer[pos] << bit_left));
+bs->buffer[pos] = va_swap32((bs->buffer[pos] << bit_left));
 }
 }
  
@@ -101,7 +101,7 @@ bitstream_put_ui(bitstream *bs, unsigned int val, int 
 } else {
 size_in_bits -= bit_left;
 bs->buffer[pos] = (bs->buffer[pos] << bit_left) | (val >> 
size_in_bits);
-bs->buffer[pos] = swap32(bs->buffer[pos]);
+bs->buffer[pos] = va_swap32(bs->buffer[pos]);
 
 if (pos + 1 == bs->max_size_in_dword) {
 bs->max_size_in_dword += BITSTREAM_ALLOCATE_STEPPING;
diff -upr -x *.m4 -x Makefile.in libva-utils-2.6.0.orig/encode/mpeg2vaenc.c 
libva-utils-2.6.0/encode/mpeg2vaenc.c
--- libva-utils-2.6.0.orig/encode/mpeg2vaenc.c  Wed Dec 26 21:29:26 2018
+++ libva-utils-2.6.0/encode/mpeg2vaenc.c   Fri Jan 24 14:05:48 2020
@@ -163,7 +163,7 @@ struct __bitstream {
 typedef struct __bitstream bitstream;
 
 static unsigned int 
-swap32(unsigned int val)
+va_swap32(unsigned int val)
 {
 unsigned char *pval = (unsigned char *)
 
@@ -190,7 +190,7 @@ bitstream_end(bitstream *bs)
 int bit_left = 32 - bit_offset;
 
 if (bit_offset) {
-bs->buffer[pos] = swap32((bs->buffer[pos] << bit_left));
+bs->buffer[pos] = va_swap32((bs->buffer[pos] << bit_left));
 }
 }
  
@@ -214,7 +214,7 @@ bitstream_put_ui(bitstream *bs, unsigned int val, int 
 } else {
 size_in_bits -= bit_left;
 bs->buffer[pos] = (bs->buffer[pos] << bit_left) | (val >> 
size_in_bits);
-bs->buffer[pos] = swap32(bs->buffer[pos]);
+bs->buffer[pos] = va_swap32(bs->buffer[pos]);
 
 if (pos + 1 == bs->max_size_in_dword) {
 bs->max_size_in_dword += BITSTREAM_ALLOCATE_STEPPING;
diff -upr -x *.m4 -x Makefile.in 

Re: [Patch]: Integrate VA-API into xenocara

2020-01-23 Thread Jonathan Gray
On Thu, Jan 23, 2020 at 12:39:29PM +1100, Jonathan Gray wrote:
> On Wed, Dec 18, 2019 at 03:28:48PM -0600, Brad DeMorrow wrote:
> > This is a rather large patch that moves my previously submitted
> > VA-API ports into xenocara. For your convenience, I've inlined
> > a diff that shows you all of the changes I made to existing files
> > that you can easily read in your MUA.  The attached patch also
> > contains these changes and should be the only xenocara patch
> > you need to apply.
> > 
> > Summary of Changes:
> >  - libva added to xenocara/lib/libva
> >  - vainfo added to xenocara/app/vainfo
> >  - intel-vaapi-driver added to xenocara/driver/intel-vaapi-driver
> >  - Mesa Makefile.bsd-wrapper updated to build with --enable-va flag
> >  - 3RDPARTY file updated to include libva, libva-utils, and 
> > intel-vaapi-driver
> >  - BSD.x11.dist updated to include /usr/X11R6/include/va/ (separate patch)
> 
> It is difficult to see where you have made changes, can you send patches
> against source from particular tarballs?

I built libva-2.6.1 but it does not work with non-intel drivers at
runtime due to a Mesa bug:

https://bugs.freedesktop.org/show_bug.cgi?id=109929

libva info: VA-API version 1.6.0
libva info: Trying to open /usr/X11R6/lib/dri/radeonsi_drv_video.so
vainfo:/usr/X11R6/lib/dri/radeonsi_drv_video.so: undefined symbol 
'gl_nir_lower_samplers_as_deref'
vainfo:/usr/X11R6/lib/dri/radeonsi_drv_video.so: undefined symbol 
'gl_nir_lower_samplers'
libva error: dlopen of /usr/X11R6/lib/dri/radeonsi_drv_video.so failed: Cannot 
load specified object
libva info: va_openDriver() returns -1
vaInitialize failed with error code -1 (unknown libva error),exit

Here is the diff for libva-2.6.1 from memory the horrible ifdef part
came from FreeBSD via your port.

--- /dev/null   Fri Jan 24 16:36:40 2020
+++ libva-2.6.1/Makefile.bsd-wrapperFri Jan 24 13:56:21 2020
@@ -0,0 +1,5 @@
+# $OpenBSD$
+
+SHARED_LIBS=   va 0.0 va_drm 0.0 va_glx 0.0 va_x11 0.0
+
+.include 
diff -upr -x *.m4 -x Makefile.in libva-2.6.1.orig/va/Makefile.am 
libva-2.6.1/va/Makefile.am
--- libva-2.6.1.orig/va/Makefile.am Wed Dec 18 00:46:07 2019
+++ libva-2.6.1/va/Makefile.am  Fri Jan 24 13:55:21 2020
@@ -91,7 +91,7 @@ libva_la_SOURCES  = $(libva_source_c)
 libva_la_CFLAGS= $(libva_cflags)
 libva_la_LDFLAGS   = $(libva_ldflags)
 libva_la_DEPENDENCIES  = libva.syms
-libva_la_LIBADD= $(LIBVA_LIBS) -ldl
+libva_la_LIBADD= $(LIBVA_LIBS) $(DLOPEN_LIBS)
 
 if USE_DRM
 SUBDIRS+= drm
@@ -101,7 +101,7 @@ libva_drm_la_CFLAGS = $(libva_cflags)
 libva_drm_la_LDFLAGS   = $(LDADD)
 libva_drm_la_DEPENDENCIES  = libva.la drm/libva_drm.la
 libva_drm_la_LIBADD= libva.la drm/libva_drm.la \
-   $(LIBVA_LIBS) $(DRM_LIBS) -ldl
+   $(LIBVA_LIBS) $(DRM_LIBS) $(DLOPEN_LIBS)
 endif
 
 if USE_X11
@@ -113,7 +113,7 @@ libva_x11_la_CFLAGS = $(libva_cflags)
 libva_x11_la_LDFLAGS   = $(LDADD)
 libva_x11_la_DEPENDENCIES  = libva.la x11/libva_x11.la
 libva_x11_la_LIBADD= libva.la x11/libva_x11.la \
-   $(LIBVA_LIBS) $(X11_LIBS) $(XEXT_LIBS) $(XFIXES_LIBS) $(DRM_LIBS) -ldl
+   $(LIBVA_LIBS) $(X11_LIBS) $(XEXT_LIBS) $(XFIXES_LIBS) $(DRM_LIBS) 
$(DLOPEN_LIBS)
 endif
 
 if USE_GLX
@@ -124,7 +124,7 @@ libva_glx_la_CFLAGS = $(libva_cflags)
 libva_glx_la_LDFLAGS   = $(LDADD)
 libva_glx_la_DEPENDENCIES  = libva.la glx/libva_glx.la libva-x11.la
 libva_glx_la_LIBADD= libva.la glx/libva_glx.la libva-x11.la \
-   $(GLX_LIBS) -ldl
+   $(GLX_LIBS) $(DLOPEN_LIBS)
 endif
 
 if USE_WAYLAND
@@ -135,7 +135,7 @@ libva_wayland_la_CFLAGS = $(libva_cflags)
 libva_wayland_la_LDFLAGS   = $(LDADD)
 libva_wayland_la_DEPENDENCIES  = libva.la wayland/libva_wayland.la
 libva_wayland_la_LIBADD= libva.la wayland/libva_wayland.la \
-   $(WAYLAND_LIBS) $(DRM_LIBS) -ldl
+   $(WAYLAND_LIBS) $(DRM_LIBS) $(DLOPEN_LIBS)
 endif
 
 DIST_SUBDIRS = x11 glx drm wayland
Only in libva-2.6.1/va: Makefile.am.orig
diff -upr -x *.m4 -x Makefile.in libva-2.6.1.orig/va/va.c libva-2.6.1/va/va.c
--- libva-2.6.1.orig/va/va.cFri Jan 17 22:11:32 2020
+++ libva-2.6.1/va/va.c Fri Jan 24 13:55:21 2020
@@ -451,7 +451,7 @@ static VAStatus va_openDriver(VADisplay dpy, char *dri
 }
 
 va_infoMessage(dpy, "Trying to open %s\n", driver_path);
-#ifndef ANDROID
+#if defined(RTLD_NODELETE)
 handle = dlopen( driver_path, RTLD_NOW | RTLD_GLOBAL | RTLD_NODELETE );
 #else
 handle = dlopen( driver_path, RTLD_NOW| RTLD_GLOBAL);
Only in libva-2.6.1/va: va.c.orig
diff -upr -x *.m4 -x Makefile.in libva-2.6.1.orig/va/va_trace.c 
libva-2.6.1/va/va_trace.c
--- libva-2.6.1.orig/va/va_trace.c  Wed Dec 18 00:46:07 2019
+++ libva-2.6.1/va/va_trace.c   Fri Jan 24 13:55:21 2020
@@ -48,12 +48,42 @@
 #include 
 #include 
 #include 

remove spurious checksumming attempts

2020-01-23 Thread richard . n . procter
Hi, 

dlg@ pointed out what looks to be a spurious call in the bridge to 
in_proto_cksum_out(). I've checked the stack and found eight such calls 
I think can be safely removed.

in_proto_cksum_out() computes, for packets flagged as needing it, the 
transport checksum. It skips if the output interface is known and supports 
checksum offload.

The removed calls are either leftovers from when PF was opaque to 
checksums, or look to have been copied from these. (PF would modify 
packets leaving the invalidated the checksums to be recomputed later on, 
which induced recomputations on the input or forwarding paths.)

The correctness obligation: show for every call that it is not last on the 
only path that may emit un-checksummed packets, viz. the IP output path 
(i.e. non-forwarding, non-input).

if_bridge.c: the bridge operates at L2, so not on the IP output path.
ip_input.c: not in the IP output path
ip6_forward.c: not in the IP output path
ip6_output.c: ip6_output_ipsec_send() is called only by ip6_forward(), 
  so lies output the IP output path, too.

Testing: error symptoms would be bad transport checksums from 
non-offloading interfaces or in encapsulated packets.

Looking for OKs, or test reports, particularly for:
   - PF filtering the bridge
   - IP6 forwarding 

cheers, 
Richard. 

Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.338
diff -u -p -U12 -p -r1.338 if_bridge.c
--- net/if_bridge.c 6 Nov 2019 03:51:26 -   1.338
+++ net/if_bridge.c 24 Jan 2020 04:37:01 -
@@ -1561,32 +1561,25 @@ bridge_ipsec(struct ifnet *ifp, struct e
 * We don't need to do loop detection, the
 * bridge will do that for us.
 */
 #if NPF > 0
if ((encif = enc_getif(tdb->tdb_rdomain,
tdb->tdb_tap)) == NULL ||
pf_test(af, dir, encif, ) != PF_PASS) {
m_freem(m);
return (1);
}
if (m == NULL)
return (1);
-   else if (af == AF_INET)
-   in_proto_cksum_out(m, encif);
-#ifdef INET6
-   else if (af == AF_INET6)
-   in6_proto_cksum_out(m, encif);
-#endif /* INET6 */
 #endif /* NPF */
-
ip = mtod(m, struct ip *);
if ((af == AF_INET) &&
ip_mtudisc && (ip->ip_off & htons(IP_DF)) &&
tdb->tdb_mtu && ntohs(ip->ip_len) > tdb->tdb_mtu &&
tdb->tdb_mtutimeout > time_second)
bridge_send_icmp_err(ifp, eh, m,
hassnap, llc, tdb->tdb_mtu,
ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG);
else
error = ipsp_process_packet(m, tdb, af, 0);
return (1);
} else
@@ -1715,25 +1708,24 @@ bridge_ip(struct ifnet *brifp, int dir, 
/* Finally, we get to filter the packet! */
if (pf_test(AF_INET, dir, ifp, ) != PF_PASS)
goto dropit;
if (m == NULL)
goto dropit;
 #endif /* NPF > 0 */
 
/* Rebuild the IP header */
if (m->m_len < hlen && ((m = m_pullup(m, hlen)) == NULL))
return (NULL);
if (m->m_len < sizeof(struct ip))
goto dropit;
-   in_proto_cksum_out(m, ifp);
ip = mtod(m, struct ip *);
ip->ip_sum = 0;
if (0 && (ifp->if_capabilities & IFCAP_CSUM_IPv4))
m->m_pkthdr.csum_flags |= M_IPV4_CSUM_OUT;
else {
ipstat_inc(ips_outswcsum);
ip->ip_sum = in_cksum(m, hlen);
}
 
break;
 
 #ifdef INET6
@@ -1761,26 +1753,24 @@ bridge_ip(struct ifnet *brifp, int dir, 
if ((brifp->if_flags & IFF_LINK2) == IFF_LINK2 &&
bridge_ipsec(ifp, eh, hassnap, , dir, AF_INET6, hlen,
m))
return (NULL);
 #endif /* IPSEC */
 
 #if NPF > 0
if (pf_test(AF_INET6, dir, ifp, ) != PF_PASS)
goto dropit;
if (m == NULL)
return (NULL);
 #endif /* NPF > 0 */
-   in6_proto_cksum_out(m, ifp);
-
break;
}
 #endif /* INET6 */
 
default:
goto dropit;
break;
}
 
/* Reattach SNAP header */
if (hassnap) {
M_PREPEND(m, LLC_SNAPFRAMELEN, M_DONTWAIT);

Re: sunkbd: timeout_add(9) -> timeout_add_msec(9)

2020-01-23 Thread Scott Cheloha
On Sun, Dec 22, 2019 at 01:34:54PM -0600, Scott Cheloha wrote:
> As per dev/wscons/wsconsio.h, wskbd_bell_data.period is a count of
> milliseconds.
> 
> ok?

1 month bump.

> Index: sunkbd.c
> ===
> RCS file: /cvs/src/sys/dev/sun/sunkbd.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 sunkbd.c
> --- sunkbd.c  19 Mar 2016 11:41:56 -  1.27
> +++ sunkbd.c  22 Dec 2019 19:28:46 -
> @@ -84,7 +84,7 @@ sunkbd_attach(struct sunkbd_softc *sc, s
>  void
>  sunkbd_bell(struct sunkbd_softc *sc, u_int period, u_int pitch, u_int volume)
>  {
> - int nticks, s;
> + int s;
>   u_int8_t c = SKBD_CMD_BELLON;
>  
>  #if NTCTRL > 0
> @@ -103,14 +103,10 @@ sunkbd_bell(struct sunkbd_softc *sc, u_i
>   return;
>   }
>   if (sc->sc_bellactive == 0) {
> - nticks = (period * hz) / 1000;
> - if (nticks <= 0)
> - nticks = 1;
> -
>   sc->sc_bellactive = 1;
>   sc->sc_belltimeout = 1;
>   (*sc->sc_sendcmd)(sc, , 1);
> - timeout_add(>sc_bellto, nticks);
> + timeout_add_msec(>sc_bellto, period);
>   }
>   splx(s);
>  }



Re: dt(4) and allowkmem

2020-01-23 Thread Theo de Raadt
Sure.

There may be some man page locations missing, from a grep:

man2/sysctl.2:.It Dv KERN_ALLOWKMEM Pq Va kern.allowkmem
man3/sysctl.3:.It Dv KERN_ALLOWKMEM Pq Va kern.allowkmem
man7/securelevel.7:.Va kern.allowkmem ,


Martin Pieuchot  wrote:

> On 22/01/20(Wed) 14:56, Theo de Raadt wrote:
> > Todd C. Miller  wrote:
> > 
> > > On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote:
> > > 
> > > > dt(4) is a debugging interface that allows userland to read kernel
> > > > addresses.  So its access should be restricted by default, just like
> > > > mem(4).
> > > >
> > > > Diff prevent opening the pseudo-device unless `allowkmem' is set.
> > > 
> > > Does it really make sense to reuse `allowkmem' for this?  This will
> > > mean that in order to use dt(4) you also have to open up mem(4).
> > > I don't think that is desirable.
> > 
> > The things you can learn via dt are a stong inspection window into
> > kmem.  I think it's stronger than immediately obvious.
> > 
> > > If you want to disable dt(4) by default I think you are better off
> > > using a new sysctl knob.
> > 
> > I'm on the fence about it.  But it is small, so I think allowdt is
> > better.
> 
> Sure!  Diff below does that, ok?
> 
> Index: dev/dt/dt_dev.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 dt_dev.c
> --- dev/dt/dt_dev.c   21 Jan 2020 16:16:23 -  1.1
> +++ dev/dt/dt_dev.c   23 Jan 2020 08:56:00 -
> @@ -132,6 +132,10 @@ dtopen(dev_t dev, int flags, int mode, s
>  {
>   struct dt_softc *sc;
>   int unit = minor(dev);
> + extern int allowdt;
> +
> + if (!allowdt)
> + return EPERM;
>  
>   KASSERT(dtlookup(unit) == NULL);
>  
> Index: kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.369
> diff -u -p -r1.369 kern_sysctl.c
> --- kern/kern_sysctl.c2 Jan 2020 08:52:53 -   1.369
> +++ kern/kern_sysctl.c23 Jan 2020 08:54:12 -
> @@ -129,6 +129,7 @@ extern int audio_record_enable;
>  #endif
>  
>  int allowkmem;
> +int allowdt;
>  
>  int sysctl_diskinit(int, struct proc *);
>  int sysctl_proc_args(int *, u_int, void *, size_t *, struct proc *);
> @@ -358,12 +359,14 @@ kern_sysctl(int *name, u_int namelen, vo
>   return (EPERM);
>   securelevel = level;
>   return (0);
> + case KERN_ALLOWDT:
> + if (securelevel > 0)
> + return (sysctl_rdint(oldp, oldlenp, newp, allowdt));
> + return (sysctl_int(oldp, oldlenp, newp, newlen,  ));
>   case KERN_ALLOWKMEM:
>   if (securelevel > 0)
> - return (sysctl_rdint(oldp, oldlenp, newp,
> - allowkmem));
> - return (sysctl_int(oldp, oldlenp, newp, newlen,
> - ));
> + return (sysctl_rdint(oldp, oldlenp, newp, allowkmem));
> + return (sysctl_int(oldp, oldlenp, newp, newlen, ));
>   case KERN_HOSTNAME:
>   error = sysctl_tstring(oldp, oldlenp, newp, newlen,
>   hostname, sizeof(hostname));
> Index: sys/sysctl.h
> ===
> RCS file: /cvs/src/sys/sys/sysctl.h,v
> retrieving revision 1.199
> diff -u -p -r1.199 sysctl.h
> --- sys/sysctl.h  24 Dec 2019 13:13:54 -  1.199
> +++ sys/sysctl.h  23 Jan 2020 08:55:26 -
> @@ -165,7 +165,7 @@ struct ctlname {
>  #define  KERN_SHMINFO62  /* struct: SysV struct shminfo 
> */
>  #define KERN_INTRCNT 63  /* node: interrupt counters */
>  #define  KERN_WATCHDOG   64  /* node: watchdog */
> -/* was KERN_EMUL 65  */
> +#define KERN_ALLOWDT 65  /* int: allowdt */
>  #define  KERN_PROC   66  /* struct: process entries */
>  #define  KERN_MAXCLUSTERS67  /* number of mclusters */
>  #define KERN_EVCOUNT 68  /* node: event counters */
> @@ -257,7 +257,7 @@ struct ctlname {
>   { "shminfo", CTLTYPE_STRUCT }, \
>   { "intrcnt", CTLTYPE_NODE }, \
>   { "watchdog", CTLTYPE_NODE }, \
> - { "gap", 0 }, \
> + { "allowdt", CTLTYPE_INT }, \
>   { "proc", CTLTYPE_STRUCT }, \
>   { "maxclusters", CTLTYPE_INT }, \
>   { "evcount", CTLTYPE_NODE }, \



Re: EFI frame buffer > 4GB

2020-01-23 Thread YASUOKA Masahiko
Yes, the diff fixed the problem of my vaio.

Thanks,

On Fri, 24 Jan 2020 01:52:56 +0100
Mark Kettenis  wrote:
> Mike Larkin and I came up with the folowing diff that keeps mapping
> the framebuffer early. We tested this on a small number of machines
> here that have the framebuffer < 4GB.
> It'd be great if we can confirm this also works on machine where it is
>> 4GB.
> 
> Thanks,
> 
> Mark



Re: MSI-X & Interrupting CPU > 0

2020-01-23 Thread Jonathan Matthew
On Thu, Jan 23, 2020 at 11:28:50AM +0100, Martin Pieuchot wrote:
> I'd like to make progress towards interrupting multiple CPUs in order to
> one day make use of multiple queues in some network drivers.  The road
> towards that goal is consequent and I'd like to proceed in steps to make
> it easier to squash bugs.  I'm currently thinking of the following steps:
> 
>  1. Is my interrupt handler safe to be executed on CPU != CPU0?
> 
>  2. Is it safe to execute this handler on two or more CPUs at the same
> time?
> 
>  3. How does interrupting multiple CPUs influence packet processing in
> the softnet thread?  Is any knowledge required (CPU affinity?) to
> have an optimum processing when multiple softnet threads are used?
> 
>  4. How to split traffic in one incoming NIC between multiple processing
> units?
> 
> This new journey comes with the requirement of being able to interrupt
> an arbitrary CPU.  For that we need a new API.  Patrick gave me the
> diff below during u2k20 and I'd like to use it to start a discussion.
> 
> We currently have 6 drivers using pci_intr_map_msix().  Since we want to
> be able to specify a CPU should we introduce a new function like in the
> diff below or do we prefer to add a new argument (cpuid_t?) to this one?
> This change in itself should already allow us to proceed with the first
> item of the above list.
> 
> Then we need a way to read the MSI-X control table size using the define
> PCI_MSIX_CTL_TBLSIZE() below.  This can be done in MI, we might also
> want to print that information in dmesg, some maybe cache it in pci(4)?
> 
> Does somebody has a better/stronger/magic way to achieve this goal?

I worked on this in mcx(4) a while ago, but I didn't manage to get the nic's
RSS hashing working at the time so all I really achieved was moving the one
rx interrupt to some other cpu.  I think I got tx interrupts across multiple
cpus without any problems, but that's not the interesting bit.

The approach I took (diff below) was to, for msi-x vectors marked MPSAFE,
try to put vector n on the nth online cpu.  This means the driver doesn't
have to think the details, it just asks for a bunch of interrupts and they
end up spread across the system.  I'd probably just want a function that
tells me the most vectors I can usefully use for a given device, which would
combine the table size for the device, the number of cpus online, and for nic
drivers, the number of softnet threads.

The driver would then either set up rings to match the number of vectors, or
map vectors to however many rings it has accordingly.  For the drivers I've
worked on (mcx, ixl, bnxt, iavf), there's no coordination required between the
vectors, so running them concurrently on multiple cpus is safe.  All we need
is to set up multiple rings and RSS hashing.


Index: 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
--- amd64/acpi_machdep.c20 Dec 2019 07:49:31 -  1.89
+++ amd64/acpi_machdep.c23 Jan 2020 22:45:17 -
@@ -194,7 +194,7 @@ acpi_intr_establish(int irq, int flags, 
 
type = (flags & LR_EXTIRQ_MODE) ? IST_EDGE : IST_LEVEL;
return (intr_establish(-1, (struct pic *)apic, map->ioapic_pin,
-   type, level, handler, arg, what));
+   type, level, handler, arg, what, 0));
 #else
return NULL;
 #endif
Index: amd64/intr.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/intr.c,v
retrieving revision 1.52
diff -u -p -r1.52 intr.c
--- amd64/intr.c25 Mar 2019 18:45:27 -  1.52
+++ amd64/intr.c23 Jan 2020 22:45:17 -
@@ -220,7 +220,7 @@ intr_allocate_slot_cpu(struct cpu_info *
  */
 int
 intr_allocate_slot(struct pic *pic, int legacy_irq, int pin, int level,
-struct cpu_info **cip, int *index, int *idt_slot)
+struct cpu_info **cip, int *index, int *idt_slot, int cpuhint)
 {
CPU_INFO_ITERATOR cii;
struct cpu_info *ci;
@@ -282,24 +282,33 @@ duplicate:
} else {
 other:
/*
-* Otherwise, look for a free slot elsewhere. Do the primary
-* CPU first.
-*/
-   ci = _info_primary;
-   error = intr_allocate_slot_cpu(ci, pic, pin, );
-   if (error == 0)
-   goto found;
-
-   /*
-* ..now try the others.
+* Find an online cpu to allocate a slot on.
+* Skip 'cpuhint' candidates to spread interrupts across cpus,
+* but keep checking following candidates after that if we 
didn't
+* find a slot.
 */
+   cpuhint %= sysctl_hwncpuonline();
CPU_INFO_FOREACH(cii, ci) {
-   if (CPU_IS_PRIMARY(ci))
+   if 

Re: EFI frame buffer > 4GB

2020-01-23 Thread Mark Kettenis
Mike Larkin and I came up with the folowing diff that keeps mapping the 
framebuffer early. We tested this on a small number of machines here 
that have the framebuffer < 4GB.
It'd be great if we can confirm this also works on machine where it is > 
4GB.


Thanks,

Mark? arch/amd64/compile/GENERIC.MP/obj
Index: arch/amd64/amd64/efifb.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
retrieving revision 1.26
diff -u -p -r1.26 efifb.c
--- arch/amd64/amd64/efifb.c	26 Nov 2019 02:20:50 -	1.26
+++ arch/amd64/amd64/efifb.c	24 Jan 2020 00:42:15 -
@@ -105,6 +105,8 @@ int	 efifb_load_font(void *, void *, str
 void	 efifb_scrollback(void *, void *, int lines);
 void	 efifb_efiinfo_init(struct efifb *);
 void	 efifb_cnattach_common(void);
+vaddr_t	 efifb_early_map(paddr_t);
+void	 efifb_early_cleanup(void);
 
 struct cb_framebuffer *cb_find_fb(paddr_t);
 
@@ -430,7 +432,7 @@ efifb_cnattach_common(void)
 	struct rasops_info	*ri = >rinfo;
 	long			 defattr = 0;
 
-	ri->ri_bits = (u_char *)PMAP_DIRECT_MAP(fb->paddr);
+	ri->ri_bits = (u_char *)efifb_early_map(fb->paddr);
 
 	efifb_rasops_preinit(fb);
 
@@ -459,14 +461,17 @@ efifb_cnremap(void)
 		return;
 
 	if (_bus_space_map(iot, fb->paddr, fb->psize,
-	BUS_SPACE_MAP_PREFETCHABLE | BUS_SPACE_MAP_LINEAR, ) == 0)
-		ri->ri_origbits = bus_space_vaddr(iot, ioh);
+	BUS_SPACE_MAP_PREFETCHABLE | BUS_SPACE_MAP_LINEAR, ))
+		panic("can't remap framebuffer");
+	ri->ri_origbits = bus_space_vaddr(iot, ioh);
 
 	efifb_rasops_preinit(fb);
 	ri->ri_flg &= ~RI_CLEAR;
 	ri->ri_flg |= RI_CENTER | RI_WRONLY;
 
 	rasops_init(ri, efifb_std_descr.nrows, efifb_std_descr.ncols);
+
+	efifb_early_cleanup();
 }
 
 int
@@ -636,4 +641,16 @@ efifb_stolen(void)
 {
 	struct efifb *fb = _console;
 	return fb->psize;
+}
+
+vaddr_t
+efifb_early_map(paddr_t pa)
+{
+	return pmap_set_pml4_early(pa);
+}
+
+void
+efifb_early_cleanup(void)
+{
+	pmap_clear_pml4_early();
 }
Index: arch/amd64/amd64/locore0.S
===
RCS file: /cvs/src/sys/arch/amd64/amd64/locore0.S,v
retrieving revision 1.17
diff -u -p -r1.17 locore0.S
--- arch/amd64/amd64/locore0.S	18 Feb 2019 08:26:20 -	1.17
+++ arch/amd64/amd64/locore0.S	24 Jan 2020 00:42:15 -
@@ -321,7 +321,7 @@ cont:
 #define PROC0_DMP2_OFF	(PROC0_DMP3_OFF + NDML3_ENTRIES * NBPG)
 #define TABLESIZE \
 ((NKL4_KIMG_ENTRIES + TABLE_L3_ENTRIES + TABLE_L2_ENTRIES + 1 + UPAGES + \
-	NDML3_ENTRIES + NDML2_ENTRIES) * NBPG)
+	NDML3_ENTRIES + NDML2_ENTRIES + 3) * NBPG)
 
 #define fillkpt \
 1:	movl	%eax,(%ebx)	;	/* store phys addr */ \
@@ -669,10 +669,11 @@ longmode_hi:
 	movw	%ax,%gs
 	movw	%ax,%fs
 
-	/* XXX merge these */
 	leaq	TABLESIZE(%rsi),%rdi
-	call	_C_LABEL(init_x86_64)
+	subq	$(NBPG*3), %rdi
 
+	/* XXX merge these */
+	call	_C_LABEL(init_x86_64)
 	call	_C_LABEL(main)
 
 	.section .codepatch,"a"
Index: arch/amd64/amd64/machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.260
diff -u -p -r1.260 machdep.c
--- arch/amd64/amd64/machdep.c	20 Dec 2019 07:49:31 -	1.260
+++ arch/amd64/amd64/machdep.c	24 Jan 2020 00:42:15 -
@@ -1365,6 +1365,8 @@ map_tramps(void)
 typedef void (vector)(void);
 extern vector *IDTVEC(exceptions)[];
 
+paddr_t early_pte_pages;
+
 void
 init_x86_64(paddr_t first_avail)
 {
@@ -1372,6 +1374,15 @@ init_x86_64(paddr_t first_avail)
 	bios_memmap_t *bmp;
 	int x, ist;
 	uint64_t max_dm_size = ((uint64_t)512 * NUM_L4_SLOT_DIRECT) << 30;
+
+	/*
+	 * locore0 mapped 3 pages for use before the pmap is initialized
+	 * starting at first_avail. These pages are currently used by
+	 * efifb to create early-use VAs for the framebuffer before efifb
+	 * is attached.
+	 */
+	early_pte_pages = first_avail;
+	first_avail += 3 * NBPG;
 
 	cpu_init_msrs(_info_primary);
 
Index: arch/amd64/amd64/pmap.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/pmap.c,v
retrieving revision 1.137
diff -u -p -r1.137 pmap.c
--- arch/amd64/amd64/pmap.c	19 Dec 2019 17:42:17 -	1.137
+++ arch/amd64/amd64/pmap.c	24 Jan 2020 00:42:15 -
@@ -544,6 +544,78 @@ pmap_kremove(vaddr_t sva, vsize_t len)
 }
 
 /*
+ * pmap_set_pml4_early
+ *
+ * Utility function to map 2GB of 2MB pages to 'pa'. The VA that is assigned
+ * is the pml4 entry for 'early mappings' (see pmap.h). This function is used
+ * by display drivers that need to map their framebuffers early, before the
+ * pmap is fully initialized (eg, to show panic messages).
+ *
+ * Users of this function must call pmap_clear_pml4_early to remove the
+ * mapping when finished.
+ *
+ * Parameters:
+ *  pa: phys addr to map
+ *
+ * Return value:
+ *  VA mapping to 'pa'. This mapping is 2GB in size and starts at the base
+ *   of the 2MB region containing 'va'.
+ */
+vaddr_t
+pmap_set_pml4_early(paddr_t pa)
+{
+	extern paddr_t 

Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Patrick Wildt
On Thu, Jan 23, 2020 at 11:25:51PM +0100, Mark Kettenis wrote:
> Since acpi_set_brightness() can be called from anywhere, it should really
> use the acpi task queue to do its thing instead of calling AML directly.

I didn't know there's an acpi task queue, so that's awesome!  I just saw
that acpithinkpad(4) also uses that.  Changing this diff to do it that
way was also quite easy, so this should read much better.  Feel free to
give it a go.

ok?

Patrick

diff --git a/sys/dev/acpi/acpivout.c b/sys/dev/acpi/acpivout.c
index 58e8e3d431c..89922f4723b 100644
--- a/sys/dev/acpi/acpivout.c
+++ b/sys/dev/acpi/acpivout.c
@@ -60,14 +60,17 @@ struct acpivout_softc {
 
int *sc_bcl;
size_t  sc_bcl_len;
+
+   int sc_brightness;
 };
 
 void   acpivout_brightness_cycle(struct acpivout_softc *);
 void   acpivout_brightness_step(struct acpivout_softc *, int);
 void   acpivout_brightness_zero(struct acpivout_softc *);
 intacpivout_get_brightness(struct acpivout_softc *);
+intacpivout_select_brightness(struct acpivout_softc *, int);
 intacpivout_find_brightness(struct acpivout_softc *, int);
-void   acpivout_set_brightness(struct acpivout_softc *, int);
+void   acpivout_set_brightness(void *, int);
 void   acpivout_get_bcl(struct acpivout_softc *);
 
 /* wconsole hook functions */
@@ -117,6 +120,8 @@ acpivout_attach(struct device *parent, struct device *self, 
void *aux)
ws_set_param = acpivout_set_param;
 
acpivout_get_bcl(sc);
+
+   sc->sc_brightness = acpivout_get_brightness(sc);
 }
 
 int
@@ -124,6 +129,9 @@ acpivout_notify(struct aml_node *node, int notify, void 
*arg)
 {
struct acpivout_softc *sc = arg;
 
+   if (ws_get_param == NULL || ws_set_param == NULL)
+   return (0);
+
switch (notify) {
case NOTIFY_BRIGHTNESS_CYCLE:
acpivout_brightness_cycle(sc);
@@ -151,12 +159,13 @@ acpivout_notify(struct aml_node *node, int notify, void 
*arg)
 void
 acpivout_brightness_cycle(struct acpivout_softc *sc)
 {
-   int cur_level;
+   struct wsdisplay_param dp;
 
-   if (sc->sc_bcl_len == 0)
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;
-   cur_level = acpivout_get_brightness(sc);
-   if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
+
+   if (dp.curval == dp.max)
acpivout_brightness_zero(sc);
else
acpivout_brightness_step(sc, 1);
@@ -165,33 +174,45 @@ acpivout_brightness_cycle(struct acpivout_softc *sc)
 void
 acpivout_brightness_step(struct acpivout_softc *sc, int dir)
 {
-   int level, nindex;
+   struct wsdisplay_param dp;
+   int delta, new;
 
-   if (sc->sc_bcl_len == 0)
-   return;
-   level = acpivout_get_brightness(sc);
-   if (level == -1)
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;
 
-   nindex = acpivout_find_brightness(sc, level + (dir * BRIGHTNESS_STEP));
-   if (sc->sc_bcl[nindex] == level) {
-   if (dir == 1 && (nindex + 1 < sc->sc_bcl_len))
-   nindex++;
-   else if (dir == -1 && (nindex - 1 >= 0))
-   nindex--;
+   new = dp.curval;
+   delta = ((dp.max - dp.min) * BRIGHTNESS_STEP) / 100;
+   if (dir > 0) {
+   if (delta > dp.max - dp.curval)
+   new = dp.max;
+   else
+   new += delta;
+   } else if (dir < 0) {
+   if (delta > dp.curval - dp.min)
+   new = dp.min;
+   else
+   new -= delta;
}
-   if (sc->sc_bcl[nindex] == level)
+
+   if (dp.curval == new)
return;
 
-   acpivout_set_brightness(sc, sc->sc_bcl[nindex]);
+   dp.curval = new;
+   ws_set_param();
 }
 
 void
 acpivout_brightness_zero(struct acpivout_softc *sc)
 {
-   if (sc->sc_bcl_len == 0)
+   struct wsdisplay_param dp;
+
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;
-   acpivout_set_brightness(sc, sc->sc_bcl[0]);
+
+   dp.curval = dp.min;
+   ws_set_param();
 }
 
 int
@@ -211,6 +232,26 @@ acpivout_get_brightness(struct acpivout_softc *sc)
return (level);
 }
 
+int
+acpivout_select_brightness(struct acpivout_softc *sc, int nlevel)
+{
+   int nindex, level;
+
+   level = sc->sc_brightness;
+   if (level == -1)
+   return level;
+
+   nindex = acpivout_find_brightness(sc, nlevel);
+   if (sc->sc_bcl[nindex] == level) {
+   if (nlevel > level && (nindex + 1 < sc->sc_bcl_len))
+   nindex++;
+   else if (nlevel < level && (nindex - 1 >= 0))
+   nindex--;
+   }
+
+   return nindex;
+}
+
 int
 acpivout_find_brightness(struct acpivout_softc *sc, int level)
 {
@@ -230,15 +271,16 @@ 

Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Mark Kettenis

Patrick Wildt schreef op 2020-01-23 08:26:

Hi,

this is the second attempt of a diff that prepares acpivout(4) to work
with the X395.  The previous one broke due to recursive ACPI locking.

So apparently we cannot change the brightness using the acpivout(4) 
ACPI

methods.  Instead we need to call amdgpu(4) to change the brightness.
But the brightness key events are still reported by acpivout(4).  That
means that we need to seperate the keystroke events from the actual
brightness change.

This diff changes the code to always use ws_[gs]et_param instead of 
just

calling the ACPI methods.  This means that the function pointers can
point to acpivout(4) or amdgpu(4), it shouldn't matter.

Unfortunately there's an issue with acpivout(4) calling itself.  The
keystroke event handler runs with the ACPI lock, so if we call our own
function, we try to grab the ACPI lock again and panic.  So, in this
diff I check whether we already have it and skip taking it again.

I'm sure this looks ugly, and I'm wondering if it's too ugly.  If it is
indeed too ugly, I hope you'll also provide another idea how we can 
work

around it.


Since acpi_set_brightness() can be called from anywhere, it should 
really

use the acpi task queue to do its thing instead of calling AML directly.



More testing would be appreciated.

Thanks,
Patrick

diff --git a/sys/dev/acpi/acpivout.c b/sys/dev/acpi/acpivout.c
index 58e8e3d431c..f3de0c582fb 100644
--- a/sys/dev/acpi/acpivout.c
+++ b/sys/dev/acpi/acpivout.c
@@ -66,6 +66,7 @@ void	acpivout_brightness_cycle(struct acpivout_softc 
*);

 void   acpivout_brightness_step(struct acpivout_softc *, int);
 void   acpivout_brightness_zero(struct acpivout_softc *);
 intacpivout_get_brightness(struct acpivout_softc *);
+intacpivout_select_brightness(struct acpivout_softc *, int);
 intacpivout_find_brightness(struct acpivout_softc *, int);
 void   acpivout_set_brightness(struct acpivout_softc *, int);
 void   acpivout_get_bcl(struct acpivout_softc *);
@@ -124,6 +125,9 @@ acpivout_notify(struct aml_node *node, int notify,
void *arg)
 {
struct acpivout_softc *sc = arg;

+   if (ws_get_param == NULL || ws_set_param == NULL)
+   return (0);
+
switch (notify) {
case NOTIFY_BRIGHTNESS_CYCLE:
acpivout_brightness_cycle(sc);
@@ -151,12 +155,13 @@ acpivout_notify(struct aml_node *node, int
notify, void *arg)
 void
 acpivout_brightness_cycle(struct acpivout_softc *sc)
 {
-   int cur_level;
+   struct wsdisplay_param dp;

-   if (sc->sc_bcl_len == 0)
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;
-   cur_level = acpivout_get_brightness(sc);
-   if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
+
+   if (dp.curval == dp.max)
acpivout_brightness_zero(sc);
else
acpivout_brightness_step(sc, 1);
@@ -165,33 +170,45 @@ acpivout_brightness_cycle(struct acpivout_softc 
*sc)

 void
 acpivout_brightness_step(struct acpivout_softc *sc, int dir)
 {
-   int level, nindex;
+   struct wsdisplay_param dp;
+   int delta, new;

-   if (sc->sc_bcl_len == 0)
-   return;
-   level = acpivout_get_brightness(sc);
-   if (level == -1)
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;

-	nindex = acpivout_find_brightness(sc, level + (dir * 
BRIGHTNESS_STEP));

-   if (sc->sc_bcl[nindex] == level) {
-   if (dir == 1 && (nindex + 1 < sc->sc_bcl_len))
-   nindex++;
-   else if (dir == -1 && (nindex - 1 >= 0))
-   nindex--;
+   new = dp.curval;
+   delta = ((dp.max - dp.min) * BRIGHTNESS_STEP) / 100;
+   if (dir > 0) {
+   if (delta > dp.max - dp.curval)
+   new = dp.max;
+   else
+   new += delta;
+   } else if (dir < 0) {
+   if (delta > dp.curval - dp.min)
+   new = dp.min;
+   else
+   new -= delta;
}
-   if (sc->sc_bcl[nindex] == level)
+
+   if (dp.curval == new)
return;

-   acpivout_set_brightness(sc, sc->sc_bcl[nindex]);
+   dp.curval = new;
+   ws_set_param();
 }

 void
 acpivout_brightness_zero(struct acpivout_softc *sc)
 {
-   if (sc->sc_bcl_len == 0)
+   struct wsdisplay_param dp;
+
+   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
+   if (ws_get_param())
return;
-   acpivout_set_brightness(sc, sc->sc_bcl[0]);
+
+   dp.curval = dp.min;
+   ws_set_param();
 }

 int
@@ -211,6 +228,26 @@ acpivout_get_brightness(struct acpivout_softc *sc)
return (level);
 }

+int
+acpivout_select_brightness(struct acpivout_softc *sc, int nlevel)
+{
+   int nindex, level;
+
+   level = acpivout_get_brightness(sc);
+   if (level == -1)
+   return level;
+
+  

Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Patrick Wildt
On Thu, Jan 23, 2020 at 05:03:01PM -0500, Ted Unangst wrote:
> Patrick Wildt wrote:
> > acpivout_[gs]et_param don't know if they are being called by itself
> > or by someone else.  I don't need to grab it again, I just need to
> > have it before calling an ACPI method.  But when acpivout(4) calls
> > itself, it already has it, so taking it again would break it, as it's
> > non recursive.
> > 
> > Since it's a global function pointer that hides the implementation, the
> > caller cannot just take the ACPI lock before calling ws_[gs]et_param.
> > The caller doesn't know that the implementation needs ACPI.  On my X395
> > the function set in ws_[gs]et_param have nothing to do with ACPI at all.
> 
> Can you release the lock before calling ws_set_param?

That's a good point, that would make it a lot easier.  I'm not sure,
maybe kettenis@ can shed a light on this?



Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Ted Unangst
Patrick Wildt wrote:
> acpivout_[gs]et_param don't know if they are being called by itself
> or by someone else.  I don't need to grab it again, I just need to
> have it before calling an ACPI method.  But when acpivout(4) calls
> itself, it already has it, so taking it again would break it, as it's
> non recursive.
> 
> Since it's a global function pointer that hides the implementation, the
> caller cannot just take the ACPI lock before calling ws_[gs]et_param.
> The caller doesn't know that the implementation needs ACPI.  On my X395
> the function set in ws_[gs]et_param have nothing to do with ACPI at all.

Can you release the lock before calling ws_set_param?



Re: ospf6d: simplify lsa_snap()

2020-01-23 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2020.01.22 06:49:53 +0100:
> On Wed, Jan 22, 2020 at 12:56:00AM +0100, Claudio Jeker wrote:
> > On Tue, Jan 21, 2020 at 03:58:58PM +0100, Remi Locherer wrote:
> > > On Tue, Jan 21, 2020 at 01:09:30PM +0100, Denis Fondras wrote:
> > > > On Tue, Jan 21, 2020 at 09:35:06AM +0100, Remi Locherer wrote:
> > > > > > @@ -235,6 +233,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
> > > > > > case LSA_TYPE_NETWORK:
> > > > > > if ((len % sizeof(u_int32_t)) ||
> > > > > > len < sizeof(lsa->hdr) + sizeof(u_int32_t)) {
> > > > > > +   log_warnx("lsa_check: bad LSA network packet");
> > > > > 
> > > > > please use __func__
> > > > > 
> > > > 
> > > > None use __func__ currently.
> > > > 
> > > 
> > > Right, it's not often used in ospf6d.
> > > 
> > > I think we should use it more in such cases.
> > > 
> > > But you have my OK with or without that.
> > > 
> > 
> > I think the log_warnx should use __func__ less and instead use better
> > messages that an operator can understand without having to check the code.
> > As a developer 'lsa_check: bad LSA network packet' sounds great since I
> > can find the code but as an operator 'dropped LSA network packet with bad
> > size from neighbor XY' would be more effective. I'm probably the source of
> > most of those messages that's why I think they could be better :)
> > But changing those can happen some other time.
> 
> I agree with that point. But when the function name is used in a message
> I prefer if __func__ is used.

Another consideration: if you have the same message in multiple functions,
please use it so we know which one it was.



Re: cwm: remove menu-ssh

2020-01-23 Thread prx
Using x11/dmeny with à little script like this (to improve) :

https://dev.ybad.name/Scripts/mybin/dsshmenu



Le 23 janvier 2020 16:01:54 GMT+01:00, Mikhail  a écrit :
>Can you elaborate on tools for menu-ssh replacement?
>
>On Wed, Jan 22, 2020 at 11:16 PM Okan Demirmen 
>wrote:
>>
>> Hi,
>>
>> I think we've (or at least I have) mused about this for a while; a
>> recent mail reminded me that this feature should go - a window
>manager
>> doesn't need to parse the ssh known_hosts file for a menu; there are
>> better tools for this.
>>
>> Remove menu-ssh.
>>
>> okay?
>>
>> Index: calmwm.h
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
>> retrieving revision 1.372
>> diff -u -p -r1.372 calmwm.h
>> --- calmwm.h22 Jan 2020 19:58:35 -  1.372
>> +++ calmwm.h22 Jan 2020 20:09:13 -
>> @@ -304,7 +304,6 @@ struct conf {
>> int  xrandr;
>> int  xrandr_event_base;
>> char*conf_file;
>> -   char*known_hosts;
>> char*wm_argv;
>> int  debug;
>>  };
>> @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void
>*, struct c
>>  voidkbfunc_menu_group(void *, struct cargs *);
>>  voidkbfunc_menu_wm(void *, struct cargs *);
>>  voidkbfunc_menu_exec(void *, struct cargs *);
>> -voidkbfunc_menu_ssh(void *, struct cargs *);
>>  voidkbfunc_client_menu_label(void *, struct
>cargs *);
>>  voidkbfunc_exec_cmd(void *, struct cargs *);
>>  voidkbfunc_exec_lock(void *, struct cargs *);
>> Index: conf.c
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
>> retrieving revision 1.249
>> diff -u -p -r1.249 conf.c
>> --- conf.c  7 Mar 2019 12:54:21 -   1.249
>> +++ conf.c  22 Jan 2020 20:09:24 -
>> @@ -179,7 +179,6 @@ static const struct {
>>
>> { FUNC_SC(menu-cmd, menu_cmd, 0) },
>> { FUNC_SC(menu-group, menu_group, 0) },
>> -   { FUNC_SC(menu-ssh, menu_ssh, 0) },
>> { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
>> { FUNC_SC(menu-window-hidden, menu_client,
>CWM_MENU_WINDOW_HIDDEN) },
>> { FUNC_SC(menu-exec, menu_exec, 0) },
>> @@ -210,7 +209,6 @@ static const struct {
>> { "CM-Delete",  "lock" },
>> { "M-question", "menu-exec" },
>> { "CM-w",   "menu-exec-wm" },
>> -   { "M-period",   "menu-ssh" },
>> { "M-Return",   "window-hide" },
>> { "M-Down", "window-lower" },
>> { "M-Up",   "window-raise" },
>> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
>> home = "/";
>> }
>> xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
>> -   xasprintf(>known_hosts, "%s/%s", home,
>".ssh/known_hosts");
>>  }
>>
>>  void
>> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
>> free(c->color[i]);
>>
>> free(c->conf_file);
>> -   free(c->known_hosts);
>> free(c->font);
>> free(c->wmname);
>>  }
>> Index: cwm.1
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
>> retrieving revision 1.65
>> diff -u -p -r1.65 cwm.1
>> --- cwm.1   9 Jul 2019 21:38:44 -   1.65
>> +++ cwm.1   22 Jan 2020 20:08:19 -
>> @@ -140,15 +140,6 @@ Resize window by a large amount; see
>>  Spawn
>>  .Dq exec program
>>  dialog.
>> -.It Ic M-period
>> -Spawn
>> -.Dq ssh to
>> -dialog.
>> -This parses
>> -.Pa $HOME/.ssh/known_hosts
>> -to provide host auto-completion.
>> -.Xr ssh 1
>> -will be executed via the configured terminal emulator.
>>  .It Ic CM-w
>>  Spawn
>>  .Dq exec WindowManager
>> Index: cwmrc.5
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
>> retrieving revision 1.73
>> diff -u -p -r1.73 cwmrc.5
>> --- cwmrc.5 2 Jul 2019 23:37:47 -   1.73
>> +++ cwmrc.5 22 Jan 2020 20:07:57 -
>> @@ -280,10 +280,6 @@ menu.
>>  Launch
>>  .Dq exec WindowManager
>>  menu.
>> -.It menu-ssh
>> -Launch
>> -.Dq ssh
>> -menu.
>>  .It group-toggle-[n]
>>  Toggle visibility of group n, where n is 1-9.
>>  .It group-only-[n]
>> Index: kbfunc.c
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
>> retrieving revision 1.167
>> diff -u -p -r1.167 kbfunc.c
>> --- kbfunc.c21 Jan 2020 15:50:03 -  1.167
>> +++ kbfunc.c22 Jan 2020 20:09:03 -
>> @@ -647,72 +647,6 @@ out:
>>  }
>>
>>  void
>> -kbfunc_menu_ssh(void *ctx, struct cargs *cargs)
>> -{
>> -   struct screen_ctx   *sc = ctx;
>> -   struct cmd_ctx  *cmd;

Re: cwm: remove menu-ssh

2020-01-23 Thread su.root
yeah am not sure I am following the logic here...why install rofi when
terminal will suffice

On 23/01   18:42, Uwe Werler wrote:
> rofi. It's in the ports.
>
> Am 23. Januar 2020 15:01:54 GMT+00:00 schrieb Mikhail :
> >Can you elaborate on tools for menu-ssh replacement?
> >
> >On Wed, Jan 22, 2020 at 11:16 PM Okan Demirmen 
> >wrote:
> >>
> >> Hi,
> >>
> >> I think we've (or at least I have) mused about this for a while; a
> >> recent mail reminded me that this feature should go - a window
> >manager
> >> doesn't need to parse the ssh known_hosts file for a menu; there are
> >> better tools for this.
> >>
> >> Remove menu-ssh.
> >>
> >> okay?
> >>
> >> Index: calmwm.h
> >> ===
> >> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
> >> retrieving revision 1.372
> >> diff -u -p -r1.372 calmwm.h
> >> --- calmwm.h22 Jan 2020 19:58:35 -  1.372
> >> +++ calmwm.h22 Jan 2020 20:09:13 -
> >> @@ -304,7 +304,6 @@ struct conf {
> >> int  xrandr;
> >> int  xrandr_event_base;
> >> char*conf_file;
> >> -   char*known_hosts;
> >> char*wm_argv;
> >> int  debug;
> >>  };
> >> @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void
> >*, struct c
> >>  voidkbfunc_menu_group(void *, struct cargs *);
> >>  voidkbfunc_menu_wm(void *, struct cargs *);
> >>  voidkbfunc_menu_exec(void *, struct cargs *);
> >> -voidkbfunc_menu_ssh(void *, struct cargs *);
> >>  voidkbfunc_client_menu_label(void *, struct
> >cargs *);
> >>  voidkbfunc_exec_cmd(void *, struct cargs *);
> >>  voidkbfunc_exec_lock(void *, struct cargs *);
> >> Index: conf.c
> >> ===
> >> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
> >> retrieving revision 1.249
> >> diff -u -p -r1.249 conf.c
> >> --- conf.c  7 Mar 2019 12:54:21 -   1.249
> >> +++ conf.c  22 Jan 2020 20:09:24 -
> >> @@ -179,7 +179,6 @@ static const struct {
> >>
> >> { FUNC_SC(menu-cmd, menu_cmd, 0) },
> >> { FUNC_SC(menu-group, menu_group, 0) },
> >> -   { FUNC_SC(menu-ssh, menu_ssh, 0) },
> >> { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
> >> { FUNC_SC(menu-window-hidden, menu_client,
> >CWM_MENU_WINDOW_HIDDEN) },
> >> { FUNC_SC(menu-exec, menu_exec, 0) },
> >> @@ -210,7 +209,6 @@ static const struct {
> >> { "CM-Delete",  "lock" },
> >> { "M-question", "menu-exec" },
> >> { "CM-w",   "menu-exec-wm" },
> >> -   { "M-period",   "menu-ssh" },
> >> { "M-Return",   "window-hide" },
> >> { "M-Down", "window-lower" },
> >> { "M-Up",   "window-raise" },
> >> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
> >> home = "/";
> >> }
> >> xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
> >> -   xasprintf(>known_hosts, "%s/%s", home,
> >".ssh/known_hosts");
> >>  }
> >>
> >>  void
> >> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
> >> free(c->color[i]);
> >>
> >> free(c->conf_file);
> >> -   free(c->known_hosts);
> >> free(c->font);
> >> free(c->wmname);
> >>  }
> >> Index: cwm.1
> >> ===
> >> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
> >> retrieving revision 1.65
> >> diff -u -p -r1.65 cwm.1
> >> --- cwm.1   9 Jul 2019 21:38:44 -   1.65
> >> +++ cwm.1   22 Jan 2020 20:08:19 -
> >> @@ -140,15 +140,6 @@ Resize window by a large amount; see
> >>  Spawn
> >>  .Dq exec program
> >>  dialog.
> >> -.It Ic M-period
> >> -Spawn
> >> -.Dq ssh to
> >> -dialog.
> >> -This parses
> >> -.Pa $HOME/.ssh/known_hosts
> >> -to provide host auto-completion.
> >> -.Xr ssh 1
> >> -will be executed via the configured terminal emulator.
> >>  .It Ic CM-w
> >>  Spawn
> >>  .Dq exec WindowManager
> >> Index: cwmrc.5
> >> ===
> >> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
> >> retrieving revision 1.73
> >> diff -u -p -r1.73 cwmrc.5
> >> --- cwmrc.5 2 Jul 2019 23:37:47 -   1.73
> >> +++ cwmrc.5 22 Jan 2020 20:07:57 -
> >> @@ -280,10 +280,6 @@ menu.
> >>  Launch
> >>  .Dq exec WindowManager
> >>  menu.
> >> -.It menu-ssh
> >> -Launch
> >> -.Dq ssh
> >> -menu.
> >>  .It group-toggle-[n]
> >>  Toggle visibility of group n, where n is 1-9.
> >>  .It group-only-[n]
> >> Index: kbfunc.c
> >> ===
> >> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> >> retrieving revision 1.167
> >> diff -u -p 

Re: cwm: remove menu-ssh

2020-01-23 Thread Uwe Werler
rofi. It's in the ports.

Am 23. Januar 2020 15:01:54 GMT+00:00 schrieb Mikhail :
>Can you elaborate on tools for menu-ssh replacement?
>
>On Wed, Jan 22, 2020 at 11:16 PM Okan Demirmen 
>wrote:
>>
>> Hi,
>>
>> I think we've (or at least I have) mused about this for a while; a
>> recent mail reminded me that this feature should go - a window
>manager
>> doesn't need to parse the ssh known_hosts file for a menu; there are
>> better tools for this.
>>
>> Remove menu-ssh.
>>
>> okay?
>>
>> Index: calmwm.h
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
>> retrieving revision 1.372
>> diff -u -p -r1.372 calmwm.h
>> --- calmwm.h22 Jan 2020 19:58:35 -  1.372
>> +++ calmwm.h22 Jan 2020 20:09:13 -
>> @@ -304,7 +304,6 @@ struct conf {
>> int  xrandr;
>> int  xrandr_event_base;
>> char*conf_file;
>> -   char*known_hosts;
>> char*wm_argv;
>> int  debug;
>>  };
>> @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void
>*, struct c
>>  voidkbfunc_menu_group(void *, struct cargs *);
>>  voidkbfunc_menu_wm(void *, struct cargs *);
>>  voidkbfunc_menu_exec(void *, struct cargs *);
>> -voidkbfunc_menu_ssh(void *, struct cargs *);
>>  voidkbfunc_client_menu_label(void *, struct
>cargs *);
>>  voidkbfunc_exec_cmd(void *, struct cargs *);
>>  voidkbfunc_exec_lock(void *, struct cargs *);
>> Index: conf.c
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
>> retrieving revision 1.249
>> diff -u -p -r1.249 conf.c
>> --- conf.c  7 Mar 2019 12:54:21 -   1.249
>> +++ conf.c  22 Jan 2020 20:09:24 -
>> @@ -179,7 +179,6 @@ static const struct {
>>
>> { FUNC_SC(menu-cmd, menu_cmd, 0) },
>> { FUNC_SC(menu-group, menu_group, 0) },
>> -   { FUNC_SC(menu-ssh, menu_ssh, 0) },
>> { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
>> { FUNC_SC(menu-window-hidden, menu_client,
>CWM_MENU_WINDOW_HIDDEN) },
>> { FUNC_SC(menu-exec, menu_exec, 0) },
>> @@ -210,7 +209,6 @@ static const struct {
>> { "CM-Delete",  "lock" },
>> { "M-question", "menu-exec" },
>> { "CM-w",   "menu-exec-wm" },
>> -   { "M-period",   "menu-ssh" },
>> { "M-Return",   "window-hide" },
>> { "M-Down", "window-lower" },
>> { "M-Up",   "window-raise" },
>> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
>> home = "/";
>> }
>> xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
>> -   xasprintf(>known_hosts, "%s/%s", home,
>".ssh/known_hosts");
>>  }
>>
>>  void
>> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
>> free(c->color[i]);
>>
>> free(c->conf_file);
>> -   free(c->known_hosts);
>> free(c->font);
>> free(c->wmname);
>>  }
>> Index: cwm.1
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
>> retrieving revision 1.65
>> diff -u -p -r1.65 cwm.1
>> --- cwm.1   9 Jul 2019 21:38:44 -   1.65
>> +++ cwm.1   22 Jan 2020 20:08:19 -
>> @@ -140,15 +140,6 @@ Resize window by a large amount; see
>>  Spawn
>>  .Dq exec program
>>  dialog.
>> -.It Ic M-period
>> -Spawn
>> -.Dq ssh to
>> -dialog.
>> -This parses
>> -.Pa $HOME/.ssh/known_hosts
>> -to provide host auto-completion.
>> -.Xr ssh 1
>> -will be executed via the configured terminal emulator.
>>  .It Ic CM-w
>>  Spawn
>>  .Dq exec WindowManager
>> Index: cwmrc.5
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
>> retrieving revision 1.73
>> diff -u -p -r1.73 cwmrc.5
>> --- cwmrc.5 2 Jul 2019 23:37:47 -   1.73
>> +++ cwmrc.5 22 Jan 2020 20:07:57 -
>> @@ -280,10 +280,6 @@ menu.
>>  Launch
>>  .Dq exec WindowManager
>>  menu.
>> -.It menu-ssh
>> -Launch
>> -.Dq ssh
>> -menu.
>>  .It group-toggle-[n]
>>  Toggle visibility of group n, where n is 1-9.
>>  .It group-only-[n]
>> Index: kbfunc.c
>> ===
>> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
>> retrieving revision 1.167
>> diff -u -p -r1.167 kbfunc.c
>> --- kbfunc.c21 Jan 2020 15:50:03 -  1.167
>> +++ kbfunc.c22 Jan 2020 20:09:03 -
>> @@ -647,72 +647,6 @@ out:
>>  }
>>
>>  void
>> -kbfunc_menu_ssh(void *ctx, struct cargs *cargs)
>> -{
>> -   struct screen_ctx   *sc = ctx;
>> -   struct cmd_ctx  *cmd;
>> -   struct menu *mi;
>> -   struct menu_qmenuq;
>> 

Re: MSI-X & Interrupting CPU > 0

2020-01-23 Thread Alexandr Nedvedicky
Hello,


> 
>  3. How does interrupting multiple CPUs influence packet processing in
> the softnet thread?  Is any knowledge required (CPU affinity?) to
> have an optimum processing when multiple softnet threads are used?
> 

I think it's hard to tell in advance. IMO we should try to make RX
running in parallel to create some pressure on code we already have.
We can get back to CPU affinity once we will have an experience
with code on multiple cores. Perhaps we should also slowly move from
thinking in terms of RX/forward/local/TX to more general pattern
I/O scheduling. I think we are not there yet.

I think we need experiments to gain the knowledge.

regards
sashan



Re: MSI-X & Interrupting CPU > 0

2020-01-23 Thread Martin Pieuchot
On 23/01/20(Thu) 13:38, Mark Kettenis wrote:
> Martin Pieuchot schreef op 2020-01-23 11:28:
> > [...] 
> > We currently have 6 drivers using pci_intr_map_msix().  Since we want to
> > be able to specify a CPU should we introduce a new function like in the
> > diff below or do we prefer to add a new argument (cpuid_t?) to this one?
> > This change in itself should already allow us to proceed with the first
> > item of the above list.
> 
> I'm not sure you want to have the driver pick the CPU to which to assign the
> interrupt.  In fact I think that doesn't make sense at all.  The CPU
> should be picked by more generic code instead.  But perhaps we do need to
> pass a hint from the driver to that code.

We are heading towards running multiple softnet threads.  What should
be the relation, if any, between a RX interrupt handler, the CPU
executing it and the softnet thread being scheduled to process the
packets?  If such relation makes sense, what kind of hint do we need
to represent it?



ieee80211_node diff

2020-01-23 Thread Mikhail
There is no way ic_bgscan_fail can be less then zero, since it's unsigned[1].

Sorry for the attachment - web browser mail in act.

[1] - http://bxr.su/OpenBSD/sys/net80211/ieee80211_var.h#251


ieee80211_node.patch
Description: Binary data


Re: cwm: remove menu-ssh

2020-01-23 Thread Mikhail
Can you elaborate on tools for menu-ssh replacement?

On Wed, Jan 22, 2020 at 11:16 PM Okan Demirmen  wrote:
>
> Hi,
>
> I think we've (or at least I have) mused about this for a while; a
> recent mail reminded me that this feature should go - a window manager
> doesn't need to parse the ssh known_hosts file for a menu; there are
> better tools for this.
>
> Remove menu-ssh.
>
> okay?
>
> Index: calmwm.h
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
> retrieving revision 1.372
> diff -u -p -r1.372 calmwm.h
> --- calmwm.h22 Jan 2020 19:58:35 -  1.372
> +++ calmwm.h22 Jan 2020 20:09:13 -
> @@ -304,7 +304,6 @@ struct conf {
> int  xrandr;
> int  xrandr_event_base;
> char*conf_file;
> -   char*known_hosts;
> char*wm_argv;
> int  debug;
>  };
> @@ -517,7 +516,6 @@ void kbfunc_menu_cmd(void *, 
> struct c
>  voidkbfunc_menu_group(void *, struct cargs *);
>  voidkbfunc_menu_wm(void *, struct cargs *);
>  voidkbfunc_menu_exec(void *, struct cargs *);
> -voidkbfunc_menu_ssh(void *, struct cargs *);
>  voidkbfunc_client_menu_label(void *, struct cargs *);
>  voidkbfunc_exec_cmd(void *, struct cargs *);
>  voidkbfunc_exec_lock(void *, struct cargs *);
> Index: conf.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
> retrieving revision 1.249
> diff -u -p -r1.249 conf.c
> --- conf.c  7 Mar 2019 12:54:21 -   1.249
> +++ conf.c  22 Jan 2020 20:09:24 -
> @@ -179,7 +179,6 @@ static const struct {
>
> { FUNC_SC(menu-cmd, menu_cmd, 0) },
> { FUNC_SC(menu-group, menu_group, 0) },
> -   { FUNC_SC(menu-ssh, menu_ssh, 0) },
> { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
> { FUNC_SC(menu-window-hidden, menu_client, CWM_MENU_WINDOW_HIDDEN) },
> { FUNC_SC(menu-exec, menu_exec, 0) },
> @@ -210,7 +209,6 @@ static const struct {
> { "CM-Delete",  "lock" },
> { "M-question", "menu-exec" },
> { "CM-w",   "menu-exec-wm" },
> -   { "M-period",   "menu-ssh" },
> { "M-Return",   "window-hide" },
> { "M-Down", "window-lower" },
> { "M-Up",   "window-raise" },
> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
> home = "/";
> }
> xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
> -   xasprintf(>known_hosts, "%s/%s", home, ".ssh/known_hosts");
>  }
>
>  void
> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
> free(c->color[i]);
>
> free(c->conf_file);
> -   free(c->known_hosts);
> free(c->font);
> free(c->wmname);
>  }
> Index: cwm.1
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
> retrieving revision 1.65
> diff -u -p -r1.65 cwm.1
> --- cwm.1   9 Jul 2019 21:38:44 -   1.65
> +++ cwm.1   22 Jan 2020 20:08:19 -
> @@ -140,15 +140,6 @@ Resize window by a large amount; see
>  Spawn
>  .Dq exec program
>  dialog.
> -.It Ic M-period
> -Spawn
> -.Dq ssh to
> -dialog.
> -This parses
> -.Pa $HOME/.ssh/known_hosts
> -to provide host auto-completion.
> -.Xr ssh 1
> -will be executed via the configured terminal emulator.
>  .It Ic CM-w
>  Spawn
>  .Dq exec WindowManager
> Index: cwmrc.5
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
> retrieving revision 1.73
> diff -u -p -r1.73 cwmrc.5
> --- cwmrc.5 2 Jul 2019 23:37:47 -   1.73
> +++ cwmrc.5 22 Jan 2020 20:07:57 -
> @@ -280,10 +280,6 @@ menu.
>  Launch
>  .Dq exec WindowManager
>  menu.
> -.It menu-ssh
> -Launch
> -.Dq ssh
> -menu.
>  .It group-toggle-[n]
>  Toggle visibility of group n, where n is 1-9.
>  .It group-only-[n]
> Index: kbfunc.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> retrieving revision 1.167
> diff -u -p -r1.167 kbfunc.c
> --- kbfunc.c21 Jan 2020 15:50:03 -  1.167
> +++ kbfunc.c22 Jan 2020 20:09:03 -
> @@ -647,72 +647,6 @@ out:
>  }
>
>  void
> -kbfunc_menu_ssh(void *ctx, struct cargs *cargs)
> -{
> -   struct screen_ctx   *sc = ctx;
> -   struct cmd_ctx  *cmd;
> -   struct menu *mi;
> -   struct menu_qmenuq;
> -   FILE*fp;
> -   char*buf, *lbuf, *p;
> -   char hostbuf[HOST_NAME_MAX+1];
> -   char path[PATH_MAX];
> -   int 

Re: MSI-X & Interrupting CPU > 0

2020-01-23 Thread Mark Kettenis

Martin Pieuchot schreef op 2020-01-23 11:28:
I'd like to make progress towards interrupting multiple CPUs in order 
to

one day make use of multiple queues in some network drivers.  The road
towards that goal is consequent and I'd like to proceed in steps to 
make
it easier to squash bugs.  I'm currently thinking of the following 
steps:


 1. Is my interrupt handler safe to be executed on CPU != CPU0?


Except for things that are inherently tied to a specific CPU (clock 
interrupts,
performance counters, etc) I think the answer here should always be 
"yes".
It probably only makes sense for mpsafe handlers to run on secondary 
CPUs though.



 2. Is it safe to execute this handler on two or more CPUs at the same
time?


I think that is never safe.  Unless you run execute the handler on 
different "data".

Running multiple rx interrupt handlers on different CPUs should be fine.


 3. How does interrupting multiple CPUs influence packet processing in
the softnet thread?  Is any knowledge required (CPU affinity?) to
have an optimum processing when multiple softnet threads are used?

 4. How to split traffic in one incoming NIC between multiple 
processing

units?


You'll need to have some sort of hardware filter that uses a hash of the
packet header to assign an rx queue such that all packets from a single 
"flow"
end up on the same queue and therefore will be processed by the same 
interrupt

handler.


This new journey comes with the requirement of being able to interrupt
an arbitrary CPU.  For that we need a new API.  Patrick gave me the
diff below during u2k20 and I'd like to use it to start a discussion.

We currently have 6 drivers using pci_intr_map_msix().  Since we want 
to

be able to specify a CPU should we introduce a new function like in the
diff below or do we prefer to add a new argument (cpuid_t?) to this 
one?

This change in itself should already allow us to proceed with the first
item of the above list.


I'm not sure you want to have the driver pick the CPU to which to assign 
the

interrupt.  In fact I think that doesn't make sense at all.  The CPU
should be picked by more generic code instead.  But perhaps we do need 
to

pass a hint from the driver to that code.

Then we need a way to read the MSI-X control table size using the 
define

PCI_MSIX_CTL_TBLSIZE() below.  This can be done in MI, we might also
want to print that information in dmesg, some maybe cache it in pci(4)?


There are already defines for MSIX in pcireg.h, some of which are 
duplicated
by the defines in this diff.  Don't think caching makes all that much 
sense.

Don't think we need to print the table size in dmesg; pcidump(8) already
prints it.  Might make sense to print the vector number though.


Does somebody has a better/stronger/magic way to achieve this goal?


I playes a little bit with assigning interrupts to different CPUs in the
past, but at that point this didn't really result in a performance 
boost.
That was quite a while ago though.  I don't think there are fundamental 
problems

in getting this going.


What do you think?

Index: arch/alpha/pci/pci_machdep.h
===
RCS file: /cvs/src/sys/arch/alpha/pci/pci_machdep.h,v
retrieving revision 1.30
diff -u -p -r1.30 pci_machdep.h
--- arch/alpha/pci/pci_machdep.h4 May 2016 14:30:00 -   1.30
+++ arch/alpha/pci/pci_machdep.h23 Jan 2020 09:54:50 -
@@ -105,6 +105,8 @@ int alpha_sysctl_chipset(int *, u_int, c
 (*(c)->pc_conf_write)((c)->pc_conf_v, (t), (r), (v))
 #definepci_intr_map_msi(pa, ihp)   (-1)
 #definepci_intr_map_msix(pa, vec, ihp) (-1)
+#definepci_intr_map_msix_cpuid(pa, vec, ihp, cpu) (-1)
+#definepci_intr_msix_count(c, t)   (0)
 #definepci_intr_string(c, ih)  
\
 (*(c)->pc_intr_string)((c)->pc_intr_v, (ih))
 #definepci_intr_line(c, ih)
\
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 23 Jan 2020 09:54:50 -
@@ -194,7 +194,7 @@ acpi_intr_establish(int irq, int flags,

type = (flags & LR_EXTIRQ_MODE) ? IST_EDGE : IST_LEVEL;
return (intr_establish(-1, (struct pic *)apic, map->ioapic_pin,
-   type, level, handler, arg, what));
+   type, level, NULL, handler, arg, what));
 #else
return NULL;
 #endif
Index: arch/amd64/amd64/intr.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/intr.c,v
retrieving revision 1.52
diff -u -p -r1.52 intr.c
--- arch/amd64/amd64/intr.c 25 Mar 2019 18:45:27 -  1.52
+++ 

Re: dt(4) and allowkmem

2020-01-23 Thread Todd C . Miller
On Thu, 23 Jan 2020 10:03:08 +0100, Martin Pieuchot wrote:

> Sure!  Diff below does that, ok?

Looks good.  OK millert@

 - todd



Re: cwm: remove menu-ssh

2020-01-23 Thread su.root
ok

On 22/01   15:15, Okan Demirmen wrote:
> Hi,
> 
> I think we've (or at least I have) mused about this for a while; a
> recent mail reminded me that this feature should go - a window manager
> doesn't need to parse the ssh known_hosts file for a menu; there are
> better tools for this.
> 
> Remove menu-ssh.
> 
> okay?
> 
> Index: calmwm.h
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
> retrieving revision 1.372
> diff -u -p -r1.372 calmwm.h
> --- calmwm.h  22 Jan 2020 19:58:35 -  1.372
> +++ calmwm.h  22 Jan 2020 20:09:13 -
> @@ -304,7 +304,6 @@ struct conf {
>   int  xrandr;
>   int  xrandr_event_base;
>   char*conf_file;
> - char*known_hosts;
>   char*wm_argv;
>   int  debug;
>  };
> @@ -517,7 +516,6 @@ void   kbfunc_menu_cmd(void *, struct 
> c
>  void  kbfunc_menu_group(void *, struct cargs *);
>  void  kbfunc_menu_wm(void *, struct cargs *);
>  void  kbfunc_menu_exec(void *, struct cargs *);
> -void  kbfunc_menu_ssh(void *, struct cargs *);
>  void  kbfunc_client_menu_label(void *, struct cargs *);
>  void  kbfunc_exec_cmd(void *, struct cargs *);
>  void  kbfunc_exec_lock(void *, struct cargs *);
> Index: conf.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
> retrieving revision 1.249
> diff -u -p -r1.249 conf.c
> --- conf.c7 Mar 2019 12:54:21 -   1.249
> +++ conf.c22 Jan 2020 20:09:24 -
> @@ -179,7 +179,6 @@ static const struct {
>  
>   { FUNC_SC(menu-cmd, menu_cmd, 0) },
>   { FUNC_SC(menu-group, menu_group, 0) },
> - { FUNC_SC(menu-ssh, menu_ssh, 0) },
>   { FUNC_SC(menu-window, menu_client, CWM_MENU_WINDOW_ALL) },
>   { FUNC_SC(menu-window-hidden, menu_client, CWM_MENU_WINDOW_HIDDEN) },
>   { FUNC_SC(menu-exec, menu_exec, 0) },
> @@ -210,7 +209,6 @@ static const struct {
>   { "CM-Delete",  "lock" },
>   { "M-question", "menu-exec" },
>   { "CM-w",   "menu-exec-wm" },
> - { "M-period",   "menu-ssh" },
>   { "M-Return",   "window-hide" },
>   { "M-Down", "window-lower" },
>   { "M-Up",   "window-raise" },
> @@ -316,7 +314,6 @@ conf_init(struct conf *c)
>   home = "/";
>   }
>   xasprintf(>conf_file, "%s/%s", home, ".cwmrc");
> - xasprintf(>known_hosts, "%s/%s", home, ".ssh/known_hosts");
>  }
>  
>  void
> @@ -363,7 +360,6 @@ conf_clear(struct conf *c)
>   free(c->color[i]);
>  
>   free(c->conf_file);
> - free(c->known_hosts);
>   free(c->font);
>   free(c->wmname);
>  }
> Index: cwm.1
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwm.1,v
> retrieving revision 1.65
> diff -u -p -r1.65 cwm.1
> --- cwm.1 9 Jul 2019 21:38:44 -   1.65
> +++ cwm.1 22 Jan 2020 20:08:19 -
> @@ -140,15 +140,6 @@ Resize window by a large amount; see
>  Spawn
>  .Dq exec program
>  dialog.
> -.It Ic M-period
> -Spawn
> -.Dq ssh to
> -dialog.
> -This parses
> -.Pa $HOME/.ssh/known_hosts
> -to provide host auto-completion.
> -.Xr ssh 1
> -will be executed via the configured terminal emulator.
>  .It Ic CM-w
>  Spawn
>  .Dq exec WindowManager
> Index: cwmrc.5
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
> retrieving revision 1.73
> diff -u -p -r1.73 cwmrc.5
> --- cwmrc.5   2 Jul 2019 23:37:47 -   1.73
> +++ cwmrc.5   22 Jan 2020 20:07:57 -
> @@ -280,10 +280,6 @@ menu.
>  Launch
>  .Dq exec WindowManager
>  menu.
> -.It menu-ssh
> -Launch
> -.Dq ssh
> -menu.
>  .It group-toggle-[n]
>  Toggle visibility of group n, where n is 1-9.
>  .It group-only-[n]
> Index: kbfunc.c
> ===
> RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
> retrieving revision 1.167
> diff -u -p -r1.167 kbfunc.c
> --- kbfunc.c  21 Jan 2020 15:50:03 -  1.167
> +++ kbfunc.c  22 Jan 2020 20:09:03 -
> @@ -647,72 +647,6 @@ out:
>  }
>  
>  void
> -kbfunc_menu_ssh(void *ctx, struct cargs *cargs)
> -{
> - struct screen_ctx   *sc = ctx;
> - struct cmd_ctx  *cmd;
> - struct menu *mi;
> - struct menu_qmenuq;
> - FILE*fp;
> - char*buf, *lbuf, *p;
> - char hostbuf[HOST_NAME_MAX+1];
> - char path[PATH_MAX];
> - int  l;
> - size_t   len;
> - ssize_t  slen;
> - int  mflags = (CWM_MENU_DUMMY);
> -
> - 

MSI-X & Interrupting CPU > 0

2020-01-23 Thread Martin Pieuchot
I'd like to make progress towards interrupting multiple CPUs in order to
one day make use of multiple queues in some network drivers.  The road
towards that goal is consequent and I'd like to proceed in steps to make
it easier to squash bugs.  I'm currently thinking of the following steps:

 1. Is my interrupt handler safe to be executed on CPU != CPU0?

 2. Is it safe to execute this handler on two or more CPUs at the same
time?

 3. How does interrupting multiple CPUs influence packet processing in
the softnet thread?  Is any knowledge required (CPU affinity?) to
have an optimum processing when multiple softnet threads are used?

 4. How to split traffic in one incoming NIC between multiple processing
units?

This new journey comes with the requirement of being able to interrupt
an arbitrary CPU.  For that we need a new API.  Patrick gave me the
diff below during u2k20 and I'd like to use it to start a discussion.

We currently have 6 drivers using pci_intr_map_msix().  Since we want to
be able to specify a CPU should we introduce a new function like in the
diff below or do we prefer to add a new argument (cpuid_t?) to this one?
This change in itself should already allow us to proceed with the first
item of the above list.

Then we need a way to read the MSI-X control table size using the define
PCI_MSIX_CTL_TBLSIZE() below.  This can be done in MI, we might also
want to print that information in dmesg, some maybe cache it in pci(4)?

Does somebody has a better/stronger/magic way to achieve this goal?

What do you think?

Index: arch/alpha/pci/pci_machdep.h
===
RCS file: /cvs/src/sys/arch/alpha/pci/pci_machdep.h,v
retrieving revision 1.30
diff -u -p -r1.30 pci_machdep.h
--- arch/alpha/pci/pci_machdep.h4 May 2016 14:30:00 -   1.30
+++ arch/alpha/pci/pci_machdep.h23 Jan 2020 09:54:50 -
@@ -105,6 +105,8 @@ int alpha_sysctl_chipset(int *, u_int, c
 (*(c)->pc_conf_write)((c)->pc_conf_v, (t), (r), (v))
 #definepci_intr_map_msi(pa, ihp)   (-1)
 #definepci_intr_map_msix(pa, vec, ihp) (-1)
+#definepci_intr_map_msix_cpuid(pa, vec, ihp, cpu) (-1)
+#definepci_intr_msix_count(c, t)   (0)
 #definepci_intr_string(c, ih)  
\
 (*(c)->pc_intr_string)((c)->pc_intr_v, (ih))
 #definepci_intr_line(c, ih)
\
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 23 Jan 2020 09:54:50 -
@@ -194,7 +194,7 @@ acpi_intr_establish(int irq, int flags, 
 
type = (flags & LR_EXTIRQ_MODE) ? IST_EDGE : IST_LEVEL;
return (intr_establish(-1, (struct pic *)apic, map->ioapic_pin,
-   type, level, handler, arg, what));
+   type, level, NULL, handler, arg, what));
 #else
return NULL;
 #endif
Index: arch/amd64/amd64/intr.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/intr.c,v
retrieving revision 1.52
diff -u -p -r1.52 intr.c
--- arch/amd64/amd64/intr.c 25 Mar 2019 18:45:27 -  1.52
+++ arch/amd64/amd64/intr.c 23 Jan 2020 09:54:50 -
@@ -282,13 +282,20 @@ duplicate:
} else {
 other:
/*
-* Otherwise, look for a free slot elsewhere. Do the primary
-* CPU first.
+* Otherwise, look for a free slot elsewhere. If cip is null, it
+* means try primary cpu but accept secondary, otherwise we need
+* a slot on the requested cpu.
 */
-   ci = _info_primary;
+   if (*cip == NULL)
+   ci = _info_primary;
+   else
+   ci = *cip;
error = intr_allocate_slot_cpu(ci, pic, pin, );
if (error == 0)
goto found;
+   /* Can't alloc on the requested cpu, fail. */
+   if (*cip != NULL)
+   return EBUSY;
 
/*
 * ..now try the others.
@@ -323,10 +330,9 @@ intintr_shared_edge;
 
 void *
 intr_establish(int legacy_irq, struct pic *pic, int pin, int type, int level,
-int (*handler)(void *), void *arg, const char *what)
+struct cpu_info *ci, int (*handler)(void *), void *arg, const char *what)
 {
struct intrhand **p, *q, *ih;
-   struct cpu_info *ci;
int slot, error, idt_vec;
struct intrsource *source;
struct intrstub *stubp;
Index: arch/amd64/include/intr.h
===
RCS file: 

Re: sshd proctitle [Re: CVS: cvs.openbsd.org: src]

2020-01-23 Thread Damien Miller
On Thu, 23 Jan 2020, Damien Miller wrote:

> On Thu, 23 Jan 2020, Damien Miller wrote:
> 
> > What information would you like there? We could put the first N listen
> > addrs in the proctitle if that would help.
> 
> Maybe like this:
> 
> 63817 ??  S0:00.05 sshd: [listen] on [0.0.0.0]:22, [::]:22, 0 of 
> 10-100

antoine@ said this was not sufficient, so please try the following:

63817 ??  I0:00.09 sshd: /usr/sbin/sshd [listener] 0 of 10-100 startups


diff --git a/sshd.c b/sshd.c
index f6139fe..b7ed0f3 100644
--- a/sshd.c
+++ b/sshd.c
@@ -240,6 +240,8 @@ void destroy_sensitive_data(void);
 void demote_sensitive_data(void);
 static void do_ssh2_kex(struct ssh *);
 
+static char *listener_proctitle;
+
 /*
  * Close all listening sockets
  */
@@ -1032,9 +1034,9 @@ server_accept_loop(int *sock_in, int *sock_out, int 
*newsock, int *config_s)
 */
for (;;) {
if (ostartups != startups) {
-   setproctitle("[listener] %d of %d-%d startups",
-   startups, options.max_startups_begin,
-   options.max_startups);
+   setproctitle("%s [listener] %d of %d-%d startups",
+   listener_proctitle, startups,
+   options.max_startups_begin, options.max_startups);
ostartups = startups;
}
if (received_sighup) {
@@ -1347,6 +1349,41 @@ accumulate_host_timing_secret(struct sshbuf *server_cfg,
sshbuf_free(buf);
 }
 
+static void
+xextendf(char **s, const char *sep, const char *fmt, ...)
+__attribute__((__format__ (printf, 3, 4))) __attribute__((__nonnull__ 
(3)));
+static void
+xextendf(char **sp, const char *sep, const char *fmt, ...)
+{
+   va_list ap;
+   char *tmp1, *tmp2;
+
+   va_start(ap, fmt);
+   xvasprintf(, fmt, ap);
+   va_end(ap);
+
+   if (*sp == NULL || **sp == '\0') {
+   free(*sp);
+   *sp = tmp1;
+   return;
+   }
+   xasprintf(, "%s%s%s", *sp, sep, tmp1);
+   free(tmp1);
+   free(*sp);
+   *sp = tmp2;
+}
+
+static char *
+prepare_proctitle(int ac, char **av)
+{
+   char *ret = NULL;
+   int i;
+
+   for (i = 0; i < ac; i++)
+   xextendf(, " ", "%s", av[i]);
+   return ret;
+}
+
 /*
  * Main program for the daemon.
  */
@@ -1774,6 +1811,7 @@ main(int ac, char **av)
rexec_argv[rexec_argc] = "-R";
rexec_argv[rexec_argc + 1] = NULL;
}
+   listener_proctitle = prepare_proctitle(ac, av);
 
/* Ensure that umask disallows at least group and world write */
new_umask = umask(0077) | 0022;



Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Patrick Wildt
On Thu, Jan 23, 2020 at 09:51:11AM +0100, Martin Pieuchot wrote:
> On 23/01/20(Thu) 08:26, Patrick Wildt wrote:
> > Hi,
> > 
> > this is the second attempt of a diff that prepares acpivout(4) to work
> > with the X395.  The previous one broke due to recursive ACPI locking.
> > 
> > So apparently we cannot change the brightness using the acpivout(4) ACPI
> > methods.  Instead we need to call amdgpu(4) to change the brightness.
> > But the brightness key events are still reported by acpivout(4).  That
> > means that we need to seperate the keystroke events from the actual
> > brightness change.
> > 
> > This diff changes the code to always use ws_[gs]et_param instead of just
> > calling the ACPI methods.  This means that the function pointers can
> > point to acpivout(4) or amdgpu(4), it shouldn't matter.
> > 
> > Unfortunately there's an issue with acpivout(4) calling itself.  The
> > keystroke event handler runs with the ACPI lock, so if we call our own
> > function, we try to grab the ACPI lock again and panic.  So, in this
> > diff I check whether we already have it and skip taking it again.
> 
> Why do you need to grab the ACPI lock again?  Can't you assert the lock
> is always taken and use a wrapper in the code path where it isn't?

acpivout_[gs]et_param don't know if they are being called by itself
or by someone else.  I don't need to grab it again, I just need to
have it before calling an ACPI method.  But when acpivout(4) calls
itself, it already has it, so taking it again would break it, as it's
non recursive.

Since it's a global function pointer that hides the implementation, the
caller cannot just take the ACPI lock before calling ws_[gs]et_param.
The caller doesn't know that the implementation needs ACPI.  On my X395
the function set in ws_[gs]et_param have nothing to do with ACPI at all.

> > I'm sure this looks ugly, and I'm wondering if it's too ugly.  If it is
> > indeed too ugly, I hope you'll also provide another idea how we can work
> > around it.
> > More testing would be appreciated.
> > 
> > Thanks,
> > Patrick
> > 
> > diff --git a/sys/dev/acpi/acpivout.c b/sys/dev/acpi/acpivout.c
> > index 58e8e3d431c..f3de0c582fb 100644
> > --- a/sys/dev/acpi/acpivout.c
> > +++ b/sys/dev/acpi/acpivout.c
> > @@ -66,6 +66,7 @@ void  acpivout_brightness_cycle(struct acpivout_softc 
> > *);
> >  void   acpivout_brightness_step(struct acpivout_softc *, int);
> >  void   acpivout_brightness_zero(struct acpivout_softc *);
> >  intacpivout_get_brightness(struct acpivout_softc *);
> > +intacpivout_select_brightness(struct acpivout_softc *, int);
> >  intacpivout_find_brightness(struct acpivout_softc *, int);
> >  void   acpivout_set_brightness(struct acpivout_softc *, int);
> >  void   acpivout_get_bcl(struct acpivout_softc *);
> > @@ -124,6 +125,9 @@ acpivout_notify(struct aml_node *node, int notify, void 
> > *arg)
> >  {
> > struct acpivout_softc *sc = arg;
> >  
> > +   if (ws_get_param == NULL || ws_set_param == NULL)
> > +   return (0);
> > +
> > switch (notify) {
> > case NOTIFY_BRIGHTNESS_CYCLE:
> > acpivout_brightness_cycle(sc);
> > @@ -151,12 +155,13 @@ acpivout_notify(struct aml_node *node, int notify, 
> > void *arg)
> >  void
> >  acpivout_brightness_cycle(struct acpivout_softc *sc)
> >  {
> > -   int cur_level;
> > +   struct wsdisplay_param dp;
> >  
> > -   if (sc->sc_bcl_len == 0)
> > +   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> > +   if (ws_get_param())
> > return;
> > -   cur_level = acpivout_get_brightness(sc);
> > -   if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
> > +
> > +   if (dp.curval == dp.max)
> > acpivout_brightness_zero(sc);
> > else
> > acpivout_brightness_step(sc, 1);
> > @@ -165,33 +170,45 @@ acpivout_brightness_cycle(struct acpivout_softc *sc)
> >  void
> >  acpivout_brightness_step(struct acpivout_softc *sc, int dir)
> >  {
> > -   int level, nindex;
> > +   struct wsdisplay_param dp;
> > +   int delta, new;
> >  
> > -   if (sc->sc_bcl_len == 0)
> > -   return;
> > -   level = acpivout_get_brightness(sc);
> > -   if (level == -1)
> > +   dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> > +   if (ws_get_param())
> > return;
> >  
> > -   nindex = acpivout_find_brightness(sc, level + (dir * BRIGHTNESS_STEP));
> > -   if (sc->sc_bcl[nindex] == level) {
> > -   if (dir == 1 && (nindex + 1 < sc->sc_bcl_len))
> > -   nindex++;
> > -   else if (dir == -1 && (nindex - 1 >= 0))
> > -   nindex--;
> > +   new = dp.curval;
> > +   delta = ((dp.max - dp.min) * BRIGHTNESS_STEP) / 100;
> > +   if (dir > 0) {
> > +   if (delta > dp.max - dp.curval)
> > +   new = dp.max;
> > +   else
> > +   new += delta;
> > +   } else if (dir < 0) {
> > +   if (delta > dp.curval - dp.min)
> > +   new = dp.min;
> > +   else
> > +   

Re: acpivout(4): directly call ws_[gs]et_param

2020-01-23 Thread Martin Pieuchot
On 23/01/20(Thu) 08:26, Patrick Wildt wrote:
> Hi,
> 
> this is the second attempt of a diff that prepares acpivout(4) to work
> with the X395.  The previous one broke due to recursive ACPI locking.
> 
> So apparently we cannot change the brightness using the acpivout(4) ACPI
> methods.  Instead we need to call amdgpu(4) to change the brightness.
> But the brightness key events are still reported by acpivout(4).  That
> means that we need to seperate the keystroke events from the actual
> brightness change.
> 
> This diff changes the code to always use ws_[gs]et_param instead of just
> calling the ACPI methods.  This means that the function pointers can
> point to acpivout(4) or amdgpu(4), it shouldn't matter.
> 
> Unfortunately there's an issue with acpivout(4) calling itself.  The
> keystroke event handler runs with the ACPI lock, so if we call our own
> function, we try to grab the ACPI lock again and panic.  So, in this
> diff I check whether we already have it and skip taking it again.

Why do you need to grab the ACPI lock again?  Can't you assert the lock
is always taken and use a wrapper in the code path where it isn't?

> I'm sure this looks ugly, and I'm wondering if it's too ugly.  If it is
> indeed too ugly, I hope you'll also provide another idea how we can work
> around it.
> More testing would be appreciated.
> 
> Thanks,
> Patrick
> 
> diff --git a/sys/dev/acpi/acpivout.c b/sys/dev/acpi/acpivout.c
> index 58e8e3d431c..f3de0c582fb 100644
> --- a/sys/dev/acpi/acpivout.c
> +++ b/sys/dev/acpi/acpivout.c
> @@ -66,6 +66,7 @@ voidacpivout_brightness_cycle(struct acpivout_softc 
> *);
>  void acpivout_brightness_step(struct acpivout_softc *, int);
>  void acpivout_brightness_zero(struct acpivout_softc *);
>  int  acpivout_get_brightness(struct acpivout_softc *);
> +int  acpivout_select_brightness(struct acpivout_softc *, int);
>  int  acpivout_find_brightness(struct acpivout_softc *, int);
>  void acpivout_set_brightness(struct acpivout_softc *, int);
>  void acpivout_get_bcl(struct acpivout_softc *);
> @@ -124,6 +125,9 @@ acpivout_notify(struct aml_node *node, int notify, void 
> *arg)
>  {
>   struct acpivout_softc *sc = arg;
>  
> + if (ws_get_param == NULL || ws_set_param == NULL)
> + return (0);
> +
>   switch (notify) {
>   case NOTIFY_BRIGHTNESS_CYCLE:
>   acpivout_brightness_cycle(sc);
> @@ -151,12 +155,13 @@ acpivout_notify(struct aml_node *node, int notify, void 
> *arg)
>  void
>  acpivout_brightness_cycle(struct acpivout_softc *sc)
>  {
> - int cur_level;
> + struct wsdisplay_param dp;
>  
> - if (sc->sc_bcl_len == 0)
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param())
>   return;
> - cur_level = acpivout_get_brightness(sc);
> - if (cur_level == sc->sc_bcl[sc->sc_bcl_len - 1])
> +
> + if (dp.curval == dp.max)
>   acpivout_brightness_zero(sc);
>   else
>   acpivout_brightness_step(sc, 1);
> @@ -165,33 +170,45 @@ acpivout_brightness_cycle(struct acpivout_softc *sc)
>  void
>  acpivout_brightness_step(struct acpivout_softc *sc, int dir)
>  {
> - int level, nindex;
> + struct wsdisplay_param dp;
> + int delta, new;
>  
> - if (sc->sc_bcl_len == 0)
> - return;
> - level = acpivout_get_brightness(sc);
> - if (level == -1)
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param())
>   return;
>  
> - nindex = acpivout_find_brightness(sc, level + (dir * BRIGHTNESS_STEP));
> - if (sc->sc_bcl[nindex] == level) {
> - if (dir == 1 && (nindex + 1 < sc->sc_bcl_len))
> - nindex++;
> - else if (dir == -1 && (nindex - 1 >= 0))
> - nindex--;
> + new = dp.curval;
> + delta = ((dp.max - dp.min) * BRIGHTNESS_STEP) / 100;
> + if (dir > 0) {
> + if (delta > dp.max - dp.curval)
> + new = dp.max;
> + else
> + new += delta;
> + } else if (dir < 0) {
> + if (delta > dp.curval - dp.min)
> + new = dp.min;
> + else
> + new -= delta;
>   }
> - if (sc->sc_bcl[nindex] == level)
> +
> + if (dp.curval == new)
>   return;
>  
> - acpivout_set_brightness(sc, sc->sc_bcl[nindex]);
> + dp.curval = new;
> + ws_set_param();
>  }
>  
>  void
>  acpivout_brightness_zero(struct acpivout_softc *sc)
>  {
> - if (sc->sc_bcl_len == 0)
> + struct wsdisplay_param dp;
> +
> + dp.param = WSDISPLAYIO_PARAM_BRIGHTNESS;
> + if (ws_get_param())
>   return;
> - acpivout_set_brightness(sc, sc->sc_bcl[0]);
> +
> + dp.curval = dp.min;
> + ws_set_param();
>  }
>  
>  int
> @@ -211,6 +228,26 @@ acpivout_get_brightness(struct acpivout_softc *sc)
>   return (level);
>  }
>  
> +int
> +acpivout_select_brightness(struct acpivout_softc *sc, int nlevel)
> +{
> 

Re: dt(4) and allowkmem

2020-01-23 Thread Martin Pieuchot
On 22/01/20(Wed) 14:56, Theo de Raadt wrote:
> Todd C. Miller  wrote:
> 
> > On Wed, 22 Jan 2020 15:12:25 +0100, Martin Pieuchot wrote:
> > 
> > > dt(4) is a debugging interface that allows userland to read kernel
> > > addresses.  So its access should be restricted by default, just like
> > > mem(4).
> > >
> > > Diff prevent opening the pseudo-device unless `allowkmem' is set.
> > 
> > Does it really make sense to reuse `allowkmem' for this?  This will
> > mean that in order to use dt(4) you also have to open up mem(4).
> > I don't think that is desirable.
> 
> The things you can learn via dt are a stong inspection window into
> kmem.  I think it's stronger than immediately obvious.
> 
> > If you want to disable dt(4) by default I think you are better off
> > using a new sysctl knob.
> 
> I'm on the fence about it.  But it is small, so I think allowdt is
> better.

Sure!  Diff below does that, ok?

Index: dev/dt/dt_dev.c
===
RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
retrieving revision 1.1
diff -u -p -r1.1 dt_dev.c
--- dev/dt/dt_dev.c 21 Jan 2020 16:16:23 -  1.1
+++ dev/dt/dt_dev.c 23 Jan 2020 08:56:00 -
@@ -132,6 +132,10 @@ dtopen(dev_t dev, int flags, int mode, s
 {
struct dt_softc *sc;
int unit = minor(dev);
+   extern int allowdt;
+
+   if (!allowdt)
+   return EPERM;
 
KASSERT(dtlookup(unit) == NULL);
 
Index: kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.369
diff -u -p -r1.369 kern_sysctl.c
--- kern/kern_sysctl.c  2 Jan 2020 08:52:53 -   1.369
+++ kern/kern_sysctl.c  23 Jan 2020 08:54:12 -
@@ -129,6 +129,7 @@ extern int audio_record_enable;
 #endif
 
 int allowkmem;
+int allowdt;
 
 int sysctl_diskinit(int, struct proc *);
 int sysctl_proc_args(int *, u_int, void *, size_t *, struct proc *);
@@ -358,12 +359,14 @@ kern_sysctl(int *name, u_int namelen, vo
return (EPERM);
securelevel = level;
return (0);
+   case KERN_ALLOWDT:
+   if (securelevel > 0)
+   return (sysctl_rdint(oldp, oldlenp, newp, allowdt));
+   return (sysctl_int(oldp, oldlenp, newp, newlen,  ));
case KERN_ALLOWKMEM:
if (securelevel > 0)
-   return (sysctl_rdint(oldp, oldlenp, newp,
-   allowkmem));
-   return (sysctl_int(oldp, oldlenp, newp, newlen,
-   ));
+   return (sysctl_rdint(oldp, oldlenp, newp, allowkmem));
+   return (sysctl_int(oldp, oldlenp, newp, newlen, ));
case KERN_HOSTNAME:
error = sysctl_tstring(oldp, oldlenp, newp, newlen,
hostname, sizeof(hostname));
Index: sys/sysctl.h
===
RCS file: /cvs/src/sys/sys/sysctl.h,v
retrieving revision 1.199
diff -u -p -r1.199 sysctl.h
--- sys/sysctl.h24 Dec 2019 13:13:54 -  1.199
+++ sys/sysctl.h23 Jan 2020 08:55:26 -
@@ -165,7 +165,7 @@ struct ctlname {
 #defineKERN_SHMINFO62  /* struct: SysV struct shminfo 
*/
 #define KERN_INTRCNT   63  /* node: interrupt counters */
 #defineKERN_WATCHDOG   64  /* node: watchdog */
-/* was KERN_EMUL   65  */
+#define KERN_ALLOWDT   65  /* int: allowdt */
 #defineKERN_PROC   66  /* struct: process entries */
 #defineKERN_MAXCLUSTERS67  /* number of mclusters */
 #define KERN_EVCOUNT   68  /* node: event counters */
@@ -257,7 +257,7 @@ struct ctlname {
{ "shminfo", CTLTYPE_STRUCT }, \
{ "intrcnt", CTLTYPE_NODE }, \
{ "watchdog", CTLTYPE_NODE }, \
-   { "gap", 0 }, \
+   { "allowdt", CTLTYPE_INT }, \
{ "proc", CTLTYPE_STRUCT }, \
{ "maxclusters", CTLTYPE_INT }, \
{ "evcount", CTLTYPE_NODE }, \