[igt-dev] [PATCH i-g-t] [RFC] Add support to force specific module load

[Rodrigo Siqueira]

Hi,

Thanks a lot for your feedback.

I believe that I understood all of your comments. I will start the first
version of the patch.

Best Regards
Rodrigo Siqueira

On 06/18, Petri Latvala wrote:
> On Sat, Jun 16, 2018 at 09:26:31PM -0300, Rodrigo Siqueira wrote:
> > Hi,
> > 
> > First of all, thanks for your feedback :)
> > 
> > On 06/13, Petri Latvala wrote:
> > > On Tue, May 29, 2018 at 09:45:20PM -0300, Rodrigo Siqueira wrote:
> > > > This patch adds a new option to force the use of a module specified from
> > > > the command line.  The force command expects a module name which will be
> > > > used on the target test (changing the standard behavior). This feature
> > > > can be useful for developers that have to create a new module since it
> > > > is possible to use some of the tests already provided by IGT (e.g.,
> > > > kms_color, core, etc.) as a start point. Additionally, it can
> > > > also be useful for someone that wants to implement a new set of tests
> > > > for a specific driver because the developer can first check the behavior
> > > > of any test in the target module. It is important to highlight, that a
> > > > force option can produce unexpected results and developers should be
> > > > aware of that.
> > > 
> > > 
> > > Is this meant for testing replacement drivers for hardware with
> > > already existing drivers? If not, I'm not sure what the goal is here.
> > 
> > Just for provide some additional information, follows my context and
> > motivation for this patch:
> > 
> > We are working on a new module named VKMS [1,2]. In some way, our
> > development approach is similar to Test-Driven Development (TDD), since
> > we use IGT tests to support our development process. For example, at the
> > beginning of the VKMS development we focused on making core_* tests
> > pass, and now we are using kms_flip and others to help us to implement
> > new features. For doing it, we changed IGT code to load our
> > module. After thinking about the changes we made in the IGT, I realize
> > that force a particular module via command line could be useful for
> > other users because they can utilize the available tests to help them to
> > create their modules (as we are doing). Additionally, I think this
> > feature could be used for test some basic features on modules that
> > currently aren't part of the IGT.
> > 
> > So, what do you think about that? Do you think that makes sense to have
> > this feature in the IGT?
> 
> Yes! The devil is in the details though.
> 
> Forcing the use of a specific driver, that's what we're going to need
> in IGT in some form.
> 
> After discussing this with you on IRC, I learned that you're running a
> VM without any other DRM drivers (loaded). That's why you have this
> system working at this time.
> 
> When the tests want an fd to a DRM device, drmtest.c tries to open the
> first /dev/dri/card<num> that matches what is requested. If everything
> fails, it loads all modules it knows about and tries another time,
> again opening the first /dev/dri/card<num> that matches the request.
> 
> If you had, say, i915 loaded or builtin, you'd have /dev/dri/card0
> already, and it would be used for DRIVER_ANY opens regardless of
> whether you modprobe vkms. If vkms was builtin, modprobe would change
> nothing.
> 
> One force option that we absolutely need is selecting what DRIVER_ANY
> means, in a device that has multiple DRM drivers available. Another is
> forcing which /dev/dri/card<num> to use, with or without overriding
> what DRIVER_ANY means. Forcing a modprobe on a particular driver isn't
> strictly speaking a third option, but tied to the two.
> 
> (One could argue that modprobing a driver can also be done from one's
> testing scripts, but drivers can be left unloaded by e.g. a test that
> checks module unloading.)
> 
> An approach I came up with is setting a string and in drmtest.c,
> __open_device(),
> 
> if (force_string && chipset == DRIVER_ANY && __is_device(fd,
> force_string))
> 	return fd;
> 
> That would force vkms to be used by setting force_string to "vkms",
> assuming DRM_IOCTL_VERSION gives that as its name. That allows vkms to
> be builtin, and other drivers to be loaded.
> 
> In addition to that, setting another force string could be used to
> modprobe a specific module. In addition to what is already loaded by
> the modprobe loop, forcing the device name in the above code would
> mean other drivers won't be used.
> 
> 
> > 
> > > Setting a forced module target in this patch changes which module is
> > > loaded by the kernel, but the driver that's opened by IGT is
> > > unchanged. Force-loading my-fancy-driver.ko still makes
> > > drm_open_driver(DRIVER_INTEL) open the one driven by i915.ko, and
> > > drm_open_driver(DRIVER_ANY) still opens the first one that is
> > > recognized.
> > 
> > I think it is better to force open and load, right?
> >  
> > > If this is for testing new drivers for not-already-supported devices,
> > > you need to instead force what drm_open_driver(DRIVER_ANY) will open,
> > > and not reject unknown devices.
> 
> 
> For those following along at home, correction to this: drm_open_driver
> does not reject unknown devices, I misread the code.
> 
> 
> > I did not completely understand the idea, what do you mean by force
> > option through the environment? Make the force command set a flag and
> > change the code to check it?
> 
> As you might have learned now, the command line piglit (or the new
> runner currently in review phase) uses to launch IGT tests is
> hardcoded. What I meant was setting force options through the
> environment, and then launching piglit or the tests themselves.
> 
> 
> -- 
> Petri Latvala