[Mesa-dev] [PATCH 02/16] loader: slim down loader_get_pci_id_for_fd implementation(s)

[Jonathan Gray]

On Sat, Oct 15, 2016 at 01:32:02PM +0100, Emil Velikov wrote:
> On Saturday, 15 October 2016, Jonathan Gray <jsg at jsg.id.au> wrote:
> 
> > On Tue, Oct 11, 2016 at 07:31:46PM +0100, Emil Velikov wrote:
> > > From: Emil Velikov <emil.velikov at collabora.com <javascript:;>>
> > >
> > > Currently mesa has three code paths in the loader - libudev, manual
> > > sysfs and drm ioctl one.
> > >
> > > Considering the issues we had with libudev - strip those down in favour
> > > of the libdrm drm device API. The latter can be implemented in any way
> > > depending on the platform and can be reused by others.
> > >
> > > Cc: Jonathan Gray <jsg at jsg.id.au <javascript:;>>
> > > Cc: Jean-S??bastien P??dron <dumbbell at FreeBSD.org>
> > > Signed-off-by: Emil Velikov <emil.velikov at collabora.com <javascript:;>>
> > > ---
> > > Jonathan, Jean-S??bastien I believe I've prodded you guys for a *BSD
> > > implementation of the drm*Device API a while back. With this commit
> > > you'll be 'forced' to prep some ;-)
> >
> > It has been a while since I looked into that.  The design seemed to
> > assume that the user running code that called into libdrm had the
> > ability to enumerate pci busses.
> >
> > On OpenBSD /dev/pci* is only read/writable by root.  /dev/drm* is
> > chowned after a user logs into a console.
> >
> > Yes that's correct. The principle is that the platform/kernel has a method
> of enumerating and providing basic information for the available devices.
> Note that there are multiple applications explicitly dropping OpenBSD
> support from their TODO because it lacks the ability [from unprivileged
> context].
> 
> We don't use filesystems for communicating with the kernel like linux
> > does so ioctls are really the best fit.  The loader parts used at the
> > moment use drm driver specific ioctls, hopefully a generic version of
> > those that can return the vid/pid, subids and function id numbers would
> > cover most of it.
> 
> Afaict retrieving the vendor/device ID et al is not a security concern
> (admittedly I'm not a security person) nor drm specific.
> 
> As-is one will end up with multiple implementations - one per subsystem
> leading to extra code and maintenance burden on both OpenBSD end and for
> apps who want to support the platform.
> 
> Hope that makes things clear and doesn't sound too ranty ;-)
> Emil

The drm functions in question ask for pci information by passing in
major and minor numbers of drm devices or in the case of
drmParsePciDeviceInfo bizzarely just "drm0", not sure why the
inconsistency there.  They aren't enumerating buses they are responding
based on information from drm sources.  So it's going to be drm not pci
ioctls.

The real problem is going to be in adding drm ioctls I'd have to convince
the linux kernel people to reserve numbers, people involved in libdrm
and people involved in OpenBSD.  Though the functions would then be
useable for systems beyond just those with sysfs if they implemented the
ioctls.

I had diffs from a few months ago to do drmGetMinorNameForFD based on
loader drm_get_device_name_for_fd, drmParsePciDeviceInfo based on 
drm_get_pci_id_for_fd and defaulting to DRM_BUS_PCI rather than an error
in drmParseSubsystemType.  I don't think there is a way to retrieve the
drmParsePciBusInfo data with the existing ioctls though.