Crash due to drm_device_keep_trying.patch
Hello,
I pointed this out to Maarten half a year ago, and he seemed concerned
but didn't actually do anything. The "drm_device_keep_trying.patch" X
server patch in Ubuntu crashes the X server with Dave Airlie's virtio
GPU device and driver, and probably with any driver on a non-USB bus
attached to PCI. The updated patch attached at least fixes the crash,
and in the case of virtio it is probably actually correct, since as far
as I can see the devices include the bus, so the PCI id applies to the
device too. Perhaps just converting the USB bit to apply to all unknown
buses would be the right thing? By the way, note the changed format
string too.
I am not subscribed to this list, keeping me on CC would be nice.
Regards,
Michael
--
ORACLE Deutschland B.V. & Co. KG Michael Thayer
Werkstrasse 24 VirtualBox engineering
71384 Weinstadt, Germany mailto:michael.thayer@oracle.com
ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstraße 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Subject: [PATCH] do not use drmGetBusid to grab the pci-id name
The kernel returns EACCES or EAGAIN on drm open when the drm device is
currently unavailable, such as if it is in use by another process
(e.g. plymouth), or hasn't finished initializing (e.g. on a really fast
SSD). Because the probing is done before a vt switch is completed,
we have no way to ensure that we can own DRM master. This results
in failing to boot.
Also attrib->unowned is not initialized, always initialize it to fix
a valgrind warning, and to prevent adding the same device a second time
after a vt switch.
Fixes: https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/982889
Signed-off-by: Bryce Harrington <bryce@canonical.com>
---
hw/xfree86/os-support/linux/lnx_platform.c | 29 +++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
--- a/config/udev.c
+++ b/config/udev.c
@@ -116,7 +116,7 @@
if (xf86_find_platform_device_by_devnum(major(devnum), minor(devnum)))
return;
- LogMessage(X_INFO, "config/udev: Adding drm device (%s)\n", path);
+ LogMessage(X_INFO, "config/udev: Adding drm device (%s) %s %s\n", path, sysname, syspath);
config_udev_odev_setup_attribs(path, syspath, major(devnum),
minor(devnum), NewGPUDeviceRequest);
@@ -456,18 +456,54 @@
#ifdef CONFIG_UDEV_KMS
+static Bool
+get_pci_busid(const char *in, char *pci_str)
+{
+ int ret, domain, bus, dev, func;
+ ret = sscanf(in, "/%04x:%02x:%02x.%d", &domain, &bus, &dev, &func);
+ if (ret != 4)
+ return FALSE;
+ sprintf(pci_str, "pci:%04x:%02x:%02x.%d", domain, bus, dev, func);
+ return TRUE;
+}
+
static void
config_udev_odev_setup_attribs(const char *path, const char *syspath,
int major, int minor,
config_odev_probe_proc_ptr probe_callback)
{
struct OdevAttributes *attribs = config_odev_allocate_attributes();
+ const char *platform;
attribs->path = XNFstrdup(path);
attribs->syspath = XNFstrdup(syspath);
attribs->major = major;
attribs->minor = minor;
+ if (strstr(syspath, "/devices/pci")) {
+ char pci_str[17];
+ const char *end = strstr(syspath, "/drm/card");
+ if (strstr(syspath, "/usb"))
+ attribs->busid = strdup("");
+ else if (get_pci_busid(strstr(syspath, "/devices/pci") + 19, pci_str))
+ attribs->busid = strdup(pci_str);
+ } else if ((platform = strstr(syspath, "/devices/platform/"))) {
+ /* OMAP relies on this, modesetting doesn't use it */
+ const char *end;
+ platform += 18;
+ end = strchr(platform, '.');
+ if (end) {
+ char *busid;
+ int ret;
+
+ ret = asprintf(&busid, "platform:%.*s:%02li",
+ (int)(end - platform), platform, strtol(end + 1, NULL, 10));
+ if (ret >= 0) {
+ attribs->busid = busid;
+ }
+ }
+ }
+
/* ownership of attribs is passed to probe layer */
probe_callback(attribs);
}
--- a/hw/xfree86/os-support/linux/lnx_platform.c
+++ b/hw/xfree86/os-support/linux/lnx_platform.c
@@ -23,11 +23,8 @@
static Bool
get_drm_info(struct OdevAttributes *attribs, char *path, int delayed_index)
{
- drmSetVersion sv;
drmVersionPtr v;
- char *buf;
int major, minor, fd;
- int err = 0;
Bool paused, server_fd = FALSE;
major = attribs->major;
@@ -51,18 +48,6 @@
if (fd == -1)
return FALSE;
- sv.drm_di_major = 1;
- sv.drm_di_minor = 4;
- sv.drm_dd_major = -1; /* Don't care */
- sv.drm_dd_minor = -1; /* Don't care */
-
- err = drmSetInterfaceVersion(fd, &sv);
- if (err) {
- xf86Msg(X_ERROR, "%s: failed to set DRM interface version 1.4: %s\n",
- path, strerror(-err));
- goto out;
- }
-
/* for a delayed probe we've already added the device */
if (delayed_index == -1) {
xf86_add_platform_device(attribs, FALSE);
@@ -72,10 +57,6 @@
if (server_fd)
xf86_platform_devices[delayed_index].flags |= XF86_PDEV_SERVER_FD;
- buf = drmGetBusid(fd);
- xf86_platform_odev_attributes(delayed_index)->busid = XNFstrdup(buf);
- drmFreeBusid(buf);
-
v = drmGetVersion(fd);
if (!v) {
xf86Msg(X_ERROR, "%s: failed to query DRM version\n", path);
@@ -88,7 +69,7 @@
out:
if (!server_fd)
close(fd);
- return (err == 0);
+ return TRUE;
}
Bool
@@ -158,8 +139,11 @@
break;
}
- if (i != xf86_num_platform_devices)
+ if (i != xf86_num_platform_devices) {
+ LogMessage(X_INFO, "config/udev: Ignoring already known drm device (%s)\n",
+ path);
goto out_free;
+ }
LogMessage(X_INFO, "xfree86: Adding drm device (%s)\n", path);
Reply to: