Re: xf86EnableIO & ExtendedEnabled vs. the input thread
On Fri, 2019-02-15 at 17:00 -0800, Alan Coopersmith wrote: > but I'm guessing instead we should have some sort of callback or ddx-specific > initialization called from inputthread, that happens to be an empty stub > everywhere but the xf86 ddx for Solaris. Yeah, that sounds better. Bool ddxInputThreadInit(void) perhaps. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: xf86EnableIO & ExtendedEnabled vs. the input thread
On 01/31/19 07:23 AM, Adam Jackson wrote:
On Wed, 2019-01-23 at 14:22 -0800, Alan Coopersmith wrote:
Is there any reason we just don't drop ExtendedEnabled altogether and
just always pass the calls through to the kernel? That'd leave us
still needing to call xf86EnableIO() in the input thread on Solaris,
unlike Linux, but that's more sensible than forcing a DisableIO()
first just to reset the ExtendedEnabled state.
I can't think of a good reason to track ExtendedEnabled, no. All of the
implementations look like they're idempotent (though I had to read the
weird Linux ppc thing twice to be sure). The ExtendedEnabled logic
itself seems to predate XFree86 4.0, which is back far enough in the
dark ages that I don't think we'd ever find a useful changelog for its
motivation. I say nuke it.
Okay, I'll do that - but that then leaves the problem of how to enable it in
the input thread, since the input thread is in the core/shared X server code,
but needs to call a routine in the XFree86 DDX.
My current unclean solution is:
index 97e59d21f..8c8bf090a 100644
--- a/os/inputthread.c
+++ b/os/inputthread.c
@@ -39,6 +39,10 @@
#include "opaque.h"
#include "osdep.h"
+#ifdef __sun
+#include
+#endif
+
#if INPUTTHREAD
Bool InputThreadEnable = TRUE;
@@ -326,6 +330,16 @@ InputThreadDoWork(void *arg)
pthread_setname_np ("InputThread");
#endif
+#ifdef __sun
+{
+Bool (*enableIO)(void) = (Bool (*)(void))
+dlsym(RTLD_PROBE, "xf86EnableIO");
+
+if (enableIO != NULL)
+(*enableIO)();
+}
+#endif
+
ospoll_add(inputThreadInfo->fds, hotplugPipeRead,
ospoll_trigger_level,
InputThreadPipeNotify,
but I'm guessing instead we should have some sort of callback or ddx-specific
initialization called from inputthread, that happens to be an empty stub
everywhere but the xf86 ddx for Solaris.
Thoughts on that?
Thanks,
-alan-
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: xf86EnableIO & ExtendedEnabled vs. the input thread
On Wed, 2019-01-23 at 14:22 -0800, Alan Coopersmith wrote: > I've confirmed with our kernel folks that the syscall we call from > xf86EnableIO() on Solaris has always only set the IOPL for the calling > thread and not other threads. They believe the primary difference between > Linux & Solaris is that when a new thread is spawned, it inherits the IOPL > on Linux, but on Solaris we reset the IOPL for the new thread instead of > inheriting it. Makes sense to me. > While we could update this in the kernel, that won't solve the problem for > people on older kernels, or one of the OpenSolaris off-shoots (and wearing > my security hat, I actually prefer the least-privilege method of not having > all new threads inherit the iopl from the main thread). Agreed. > But I have to ask if anyone remembers why we keep track of this flag in > userspace at all - it's not just Solaris, but it seems to have been > cargo-culted to most platforms: > > hw/xfree86/os-support/bsd/arm_video.c > hw/xfree86/os-support/bsd/i386_video.c > hw/xfree86/os-support/linux/lnx_video.c > hw/xfree86/os-support/solaris/sun_vid.c > > Is there any reason we just don't drop ExtendedEnabled altogether and > just always pass the calls through to the kernel? That'd leave us > still needing to call xf86EnableIO() in the input thread on Solaris, > unlike Linux, but that's more sensible than forcing a DisableIO() > first just to reset the ExtendedEnabled state. I can't think of a good reason to track ExtendedEnabled, no. All of the implementations look like they're idempotent (though I had to read the weird Linux ppc thing twice to be sure). The ExtendedEnabled logic itself seems to predate XFree86 4.0, which is back far enough in the dark ages that I don't think we'd ever find a useful changelog for its motivation. I say nuke it. - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
xf86EnableIO & ExtendedEnabled vs. the input thread
Our friends at VMWare have recently tracked down issues on Solaris using the xf86-input-vmmouse driver to a subtle difference in how IOPL is set between OS'es. They found: It's indeed outl() that segfaults when trying to program the hardware cursor. Now prepending vmwareWriteReg with xf86EnableIO() doesn't help, so the X server thinks IO is enabled. But it seems like prepending vmwareWriteReg with xf86DisableIO() xf86EnableIO() makes things work again So a pretty qualified guess at what's happening here is that when the Xserver became multithreaded not too long ago, with a separate thread for mouse movement, setting xf86EnableIO() on Solaris doesn't apply to all threads, or if the threads are created *after* setting xf86EnableIO, the IOPL status doesn't propagate to the new threads. Note that Solaris xf86EnableIO() keeps a static variable to determine whether IO support was already enabled, so calling it from multiple threads only enables IO for the first thread. I've confirmed with our kernel folks that the syscall we call from xf86EnableIO() on Solaris has always only set the IOPL for the calling thread and not other threads. They believe the primary difference between Linux & Solaris is that when a new thread is spawned, it inherits the IOPL on Linux, but on Solaris we reset the IOPL for the new thread instead of inheriting it. While we could update this in the kernel, that won't solve the problem for people on older kernels, or one of the OpenSolaris off-shoots (and wearing my security hat, I actually prefer the least-privilege method of not having all new threads inherit the iopl from the main thread). The one-line solution in the Xorg sources would be to make the ExtendedEnabled boolean that tracks if the IOPL is set be a thread-specific variable. But I have to ask if anyone remembers why we keep track of this flag in userspace at all - it's not just Solaris, but it seems to have been cargo-culted to most platforms: hw/xfree86/os-support/bsd/arm_video.c hw/xfree86/os-support/bsd/i386_video.c hw/xfree86/os-support/linux/lnx_video.c hw/xfree86/os-support/solaris/sun_vid.c Is there any reason we just don't drop ExtendedEnabled altogether and just always pass the calls through to the kernel? That'd leave us still needing to call xf86EnableIO() in the input thread on Solaris, unlike Linux, but that's more sensible than forcing a DisableIO() first just to reset the ExtendedEnabled state. -- -Alan Coopersmith- alan.coopersm...@oracle.com Oracle Solaris Engineering - https://blogs.oracle.com/alanc ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
