Re: [PATCH i-g-t 20/39] tests/vkms_configfs: Test enablement without pipeline items

["José Expósito" <jose.exposito89@gmail.com>]

Hi everyone,

On Thu, Feb 27, 2025 at 05:47:59PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Feb 27, 2025 at 02:06:23PM +0100, Louis Chauvet wrote:
> > 
> > 
> > Le 18/02/2025 à 17:49, José Expósito a écrit :
> > > It shouldn't be possible to enable a device without creating and
> > > attaching its pipeline items.
> > > 
> > > Test that the device is not enabled and also that no actual device was
> > > created using libdrm.
> > > 
> > > Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> > > ---
> > >   tests/vkms/vkms_configfs.c | 60 ++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 60 insertions(+)
> > > 
> > > diff --git a/tests/vkms/vkms_configfs.c b/tests/vkms/vkms_configfs.c
> > > index dc3dbe80d..87d660fff 100644
> > > --- a/tests/vkms/vkms_configfs.c
> > > +++ b/tests/vkms/vkms_configfs.c
> > > @@ -108,6 +108,42 @@ static bool attach(const char *src_path, const char *dst_path,
> > >   	return ret == 0;
> > >   }
> > > +static drmDevicePtr find_device(const char *name, drmDevicePtr *devices,
> > > +				int n_devices)
> > > +{
> > > +	drmDevicePtr dev;
> > > +	const char *dev_name;
> > > +
> > > +	for (int i = 0; i < n_devices; i++) {
> > > +		dev = devices[i];
> > > +
> > > +		if (!(dev->available_nodes & BIT(DRM_NODE_PRIMARY)))
> > > +			continue;
> > > +
> > > +		if (dev->bustype != DRM_BUS_PLATFORM)
> > > +			continue;
> > 
> > Hummm, I think this may have issues with the change proposed by Greg in [1],
> > I don't know how the drmDevicePtr should look like with the new faux_device
> > struct.
> > 
> > The declaration in xf86drm.h is:
> > 
> > typedef struct _drmDevice {
> >     char **nodes; /* DRM_NODE_MAX sized array */
> >     int available_nodes; /* DRM_NODE_* bitmask */
> >     int bustype;
> >     union {
> >         drmPciBusInfoPtr pci;
> >         drmUsbBusInfoPtr usb;
> >         drmPlatformBusInfoPtr platform;
> >         drmHost1xBusInfoPtr host1x;
> >     } businfo;
> >     union {
> >         drmPciDeviceInfoPtr pci;
> >         drmUsbDeviceInfoPtr usb;
> >         drmPlatformDeviceInfoPtr platform;
> >         drmHost1xDeviceInfoPtr host1x;
> >     } deviceinfo;
> > } drmDevice, *drmDevicePtr;
> 
> Why does a drm device care about what type of "bus" it lives on?  That's
> not the user/kernel api to the drm device, the drm api is the way the
> device is handled.
> 
> Why not just treat it like any other "new bus" that gets written?  What
> happens if a bus type shows up in the kernel that no one has seen yet in
> this userspace code, and exposes a drm device node?  Why would that not
> "just work"?
> 
> thanks,
> 
> greg k-h

I already commented in the kernel ML [1], but I'm dropping a comment
here as well in case someone is following this thread.

drmGetDevices() doesn't return the devices in the faux bus, and libdrm
would need to be updated to handle the new bus.

To avoid blocking this series, I'll try to get the VKMS device using
udev in v2, following the approach taken by Mutter.

Let me know if you known a better API to solve this issue, please.

Thanks,
Jose

[1] https://lore.kernel.org/all/Z8Gduj1cDdCYzaBY@fedora/T/#m3fa9b60d5f637ebb0352366c605b94822fc45788