[PATCH 09/12] xfree86: Remove leaky /dev/mem checks from xf86OSInitVidMem
Mark Kettenis
mark.kettenis at xs4all.nl
Tue Jul 29 14:40:14 PDT 2014
> From: Adam Jackson <ajax at redhat.com>
> Date: Tue, 29 Jul 2014 15:00:16 -0400
>
> This is mostly a no-op, the checks didn't have much effect since
> pciaccess didn't end up using the fd we opened. Except for OpenBSD,
> where you have to pass that in from above, which is sort of a weird API
> decision. So this will break there until libpciaccess is made less
> dumb.
Sorry, but there are some fundamental problems with libpciaccess:
1. It isn't possible to properly log messages from inside libpciaccess.
2. For our privsep'ed X server the devices that give access to the raw
hardware need to be opened by the priviliged process, but ideally
for security reasons, we only want to initialize libpciaccess in
the unpriviliged process.
3. On OpenBSD/sparc64 and OpenBSD/macppc, memorty mapped io is handled
by the console driver. But the console device can only be opened
once by unpriviliged processes if nobody else has it open. So
libpciaccess can't do this since the server already opens the
device.
One way or another some way to pass a filedescriptor to use for memory
mapped io is needed, and the X server will need to do some initial setup.
We can certainly do some redesign of the interfaces here, but frankly
just keeping xf8OSInitVidMem() around for *BSD would not be such a bad
idea. I believe os-support/bus/bsd_pci.c:osPciInit() could call
xf86OsInitVidMem() directly without problems.
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
> hw/xfree86/os-support/bsd/alpha_video.c | 122 --------------------------------
> hw/xfree86/os-support/bsd/arm_video.c | 52 --------------
> hw/xfree86/os-support/bsd/i386_video.c | 104 ---------------------------
> hw/xfree86/os-support/hurd/hurd_video.c | 4 --
> 4 files changed, 282 deletions(-)
>
> diff --git a/hw/xfree86/os-support/bsd/alpha_video.c b/hw/xfree86/os-support/bsd/alpha_video.c
> index 236def6..fd8f823 100644
> --- a/hw/xfree86/os-support/bsd/alpha_video.c
> +++ b/hw/xfree86/os-support/bsd/alpha_video.c
> @@ -45,131 +45,9 @@
> #define MAP_FLAGS (MAP_FILE | MAP_SHARED)
> #endif
>
> -#ifndef __NetBSD__
> -extern unsigned long dense_base(void);
> -#else /* __NetBSD__ */
> -static struct alpha_bus_window *abw;
> -static int abw_count = -1;
> -
> -static void
> -init_abw(void)
> -{
> - if (abw_count < 0) {
> - abw_count = alpha_bus_getwindows(ALPHA_BUS_TYPE_PCI_MEM, &abw);
> - if (abw_count <= 0)
> - FatalError("init_abw: alpha_bus_getwindows failed\n");
> - }
> -}
> -
> -static unsigned long
> -dense_base(void)
> -{
> - if (abw_count < 0)
> - init_abw();
> -
> - /* XXX check abst_flags for ABST_DENSE just to be safe? */
> - xf86Msg(X_INFO, "dense base = %#lx\n", abw[0].abw_abst.abst_sys_start); /* XXXX */
> - return abw[0].abw_abst.abst_sys_start;
> -}
> -
> -#endif /* __NetBSD__ */
> -
> -#define BUS_BASE dense_base()
> -
> -/***************************************************************************/
> -/* Video Memory Mapping section */
> -/***************************************************************************/
> -
> -#ifdef __OpenBSD__
> -#define SYSCTL_MSG "\tCheck that you have set 'machdep.allowaperture=1'\n"\
> - "\tin /etc/sysctl.conf and reboot your machine\n" \
> - "\trefer to xf86(4) for details"
> -#endif
> -
> -static int devMemFd = -1;
> -
> -#ifdef HAS_APERTURE_DRV
> -#define DEV_APERTURE "/dev/xf86"
> -#endif
> -
> -/*
> - * Check if /dev/mem can be mmap'd. If it can't print a warning when
> - * "warn" is TRUE.
> - */
> -static void
> -checkDevMem(Bool warn)
> -{
> - static Bool devMemChecked = FALSE;
> - int fd;
> - void *base;
> -
> - if (devMemChecked)
> - return;
> - devMemChecked = TRUE;
> -
> -#ifdef HAS_APERTURE_DRV
> - /* Try the aperture driver first */
> - if ((fd = open(DEV_APERTURE, O_RDWR)) >= 0) {
> - /* Try to map a page at the VGA address */
> - base = mmap((caddr_t) 0, 4096, PROT_READ | PROT_WRITE,
> - MAP_FLAGS, fd, (off_t) 0xA0000 + BUS_BASE);
> -
> - if (base != MAP_FAILED) {
> - munmap((caddr_t) base, 4096);
> - devMemFd = fd;
> - xf86Msg(X_INFO, "checkDevMem: using aperture driver %s\n",
> - DEV_APERTURE);
> - return;
> - }
> - else {
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to mmap %s (%s)\n",
> - DEV_APERTURE, strerror(errno));
> - }
> - }
> - }
> -#endif
> - if ((fd = open(DEV_MEM, O_RDWR)) >= 0) {
> - /* Try to map a page at the VGA address */
> - base = mmap((caddr_t) 0, 4096, PROT_READ | PROT_WRITE,
> - MAP_FLAGS, fd, (off_t) 0xA0000 + BUS_BASE);
> -
> - if (base != MAP_FAILED) {
> - munmap((caddr_t) base, 4096);
> - devMemFd = fd;
> - return;
> - }
> - else {
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to mmap %s (%s)\n",
> - DEV_MEM, strerror(errno));
> - }
> - }
> - }
> - if (warn) {
> -#ifndef HAS_APERTURE_DRV
> - xf86Msg(X_WARNING, "checkDevMem: failed to open/mmap %s (%s)\n",
> - DEV_MEM, strerror(errno));
> -#else
> -#ifndef __OpenBSD__
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s and %s\n"
> - "\t(%s)\n", DEV_APERTURE, DEV_MEM, strerror(errno));
> -#else /* __OpenBSD__ */
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s and %s\n"
> - "\t(%s)\n%s", DEV_APERTURE, DEV_MEM, strerror(errno),
> - SYSCTL_MSG);
> -#endif /* __OpenBSD__ */
> -#endif
> - xf86ErrorF("\tlinear framebuffer access unavailable\n");
> - }
> - return;
> -}
> -
> void
> xf86OSInitVidMem(VidMemInfoPtr pVidMem)
> {
> - checkDevMem(TRUE);
> -
> pVidMem->initialised = TRUE;
> }
>
> diff --git a/hw/xfree86/os-support/bsd/arm_video.c b/hw/xfree86/os-support/bsd/arm_video.c
> index 3a639b8..fea8528 100644
> --- a/hw/xfree86/os-support/bsd/arm_video.c
> +++ b/hw/xfree86/os-support/bsd/arm_video.c
> @@ -72,61 +72,9 @@
> #define MAP_FLAGS (MAP_FILE | MAP_SHARED)
> #endif
>
> -#define BUS_BASE 0L
> -#define BUS_BASE_BWX 0L
> -
> -/***************************************************************************/
> -/* Video Memory Mapping section */
> -/***************************************************************************/
> -
> -static int devMemFd = -1;
> -
> -/*
> - * Check if /dev/mem can be mmap'd. If it can't print a warning when
> - * "warn" is TRUE.
> - */
> -static void
> -checkDevMem(Bool warn)
> -{
> - static Bool devMemChecked = FALSE;
> - int fd;
> - void *base;
> -
> - if (devMemChecked)
> - return;
> - devMemChecked = TRUE;
> -
> - if ((fd = open(DEV_MEM, O_RDWR)) >= 0) {
> - /* Try to map a page at the VGA address */
> - base = mmap((caddr_t) 0, 4096, PROT_READ | PROT_WRITE,
> - MAP_FLAGS, fd, (off_t) 0xA0000 + BUS_BASE);
> -
> - if (base != MAP_FAILED) {
> - munmap((caddr_t) base, 4096);
> - devMemFd = fd;
> - return;
> - }
> - else {
> - /* This should not happen */
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to mmap %s (%s)\n",
> - DEV_MEM, strerror(errno));
> - }
> - return;
> - }
> - }
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s (%s)\n",
> - DEV_MEM, strerror(errno));
> - }
> - return;
> -}
> -
> void
> xf86OSInitVidMem(VidMemInfoPtr pVidMem)
> {
> - checkDevMem(TRUE);
> -
> pVidMem->initialised = TRUE;
> }
>
> diff --git a/hw/xfree86/os-support/bsd/i386_video.c b/hw/xfree86/os-support/bsd/i386_video.c
> index 6c3bbcb..ffe97aa 100644
> --- a/hw/xfree86/os-support/bsd/i386_video.c
> +++ b/hw/xfree86/os-support/bsd/i386_video.c
> @@ -47,115 +47,11 @@
> #define SYSCTL_MSG "\tCheck that you have set 'machdep.allowaperture=1'\n"\
> "\tin /etc/sysctl.conf and reboot your machine\n" \
> "\trefer to xf86(4) for details"
> -#define SYSCTL_MSG2 \
> - "Check that you have set 'machdep.allowaperture=2'\n" \
> - "\tin /etc/sysctl.conf and reboot your machine\n" \
> - "\trefer to xf86(4) for details"
> #endif
>
> -/***************************************************************************/
> -/* Video Memory Mapping section */
> -/***************************************************************************/
> -
> -static Bool useDevMem = FALSE;
> -static int devMemFd = -1;
> -
> -#ifdef HAS_APERTURE_DRV
> -#define DEV_APERTURE "/dev/xf86"
> -#endif
> -
> -/*
> - * Check if /dev/mem can be mmap'd. If it can't print a warning when
> - * "warn" is TRUE.
> - */
> -static void
> -checkDevMem(Bool warn)
> -{
> - static Bool devMemChecked = FALSE;
> - int fd;
> - void *base;
> -
> - if (devMemChecked)
> - return;
> - devMemChecked = TRUE;
> -
> - if ((fd = open(DEV_MEM, O_RDWR)) >= 0) {
> - /* Try to map a page at the VGA address */
> - base = mmap((caddr_t) 0, 4096, PROT_READ | PROT_WRITE,
> - MAP_FLAGS, fd, (off_t) 0xA0000);
> -
> - if (base != MAP_FAILED) {
> - munmap((caddr_t) base, 4096);
> - devMemFd = fd;
> - useDevMem = TRUE;
> - return;
> - }
> - else {
> - /* This should not happen */
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to mmap %s (%s)\n",
> - DEV_MEM, strerror(errno));
> - }
> - useDevMem = FALSE;
> - return;
> - }
> - }
> -#ifndef HAS_APERTURE_DRV
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s (%s)\n",
> - DEV_MEM, strerror(errno));
> - }
> - useDevMem = FALSE;
> - return;
> -#else
> - /* Failed to open /dev/mem, try the aperture driver */
> - if ((fd = open(DEV_APERTURE, O_RDWR)) >= 0) {
> - /* Try to map a page at the VGA address */
> - base = mmap((caddr_t) 0, 4096, PROT_READ | PROT_WRITE,
> - MAP_FLAGS, fd, (off_t) 0xA0000);
> -
> - if (base != MAP_FAILED) {
> - munmap((caddr_t) base, 4096);
> - devMemFd = fd;
> - useDevMem = TRUE;
> - xf86Msg(X_INFO, "checkDevMem: using aperture driver %s\n",
> - DEV_APERTURE);
> - return;
> - }
> - else {
> -
> - if (warn) {
> - xf86Msg(X_WARNING, "checkDevMem: failed to mmap %s (%s)\n",
> - DEV_APERTURE, strerror(errno));
> - }
> - }
> - }
> - else {
> - if (warn) {
> -#ifndef __OpenBSD__
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s and %s\n"
> - "\t(%s)\n", DEV_MEM, DEV_APERTURE, strerror(errno));
> -#else /* __OpenBSD__ */
> - xf86Msg(X_WARNING, "checkDevMem: failed to open %s and %s\n"
> - "\t(%s)\n%s", DEV_MEM, DEV_APERTURE, strerror(errno),
> - SYSCTL_MSG);
> -#endif /* __OpenBSD__ */
> - }
> - }
> -
> - useDevMem = FALSE;
> - return;
> -
> -#endif
> -}
> -
> void
> xf86OSInitVidMem(VidMemInfoPtr pVidMem)
> {
> - checkDevMem(TRUE);
> -
> - pci_system_init_dev_mem(devMemFd);
> -
> pVidMem->initialised = TRUE;
> }
>
> diff --git a/hw/xfree86/os-support/hurd/hurd_video.c b/hw/xfree86/os-support/hurd/hurd_video.c
> index 2a96393..1e7d604 100644
> --- a/hw/xfree86/os-support/hurd/hurd_video.c
> +++ b/hw/xfree86/os-support/hurd/hurd_video.c
> @@ -40,10 +40,6 @@
> #include "xf86OSpriv.h"
>
> /**************************************************************************
> - * Video Memory Mapping section
> - ***************************************************************************/
> -
> -/**************************************************************************
> * I/O Permissions section
> ***************************************************************************/
>
> --
> 1.9.3
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list