Re: refactor the i915 GVT support

["Wang, Zhi A" <zhi.a.wang@intel.com>]

On 9/29/21 6:55 PM, Jason Gunthorpe wrote:
> On Wed, Sep 29, 2021 at 06:27:16PM +0000, Wang, Zhi A wrote:
>> On 9/28/21 3:05 PM, Jason Gunthorpe wrote:
>>> On Tue, Sep 28, 2021 at 02:35:06PM +0000, Wang, Zhi A wrote:
>>>
>>>> Yes. I was thinking of the possibility of putting off some work later so
>>>> that we don't need to make a lot of changes. GVT-g needs to take a
>>>> snapshot of GPU registers as the initial virtual states for other vGPUs,
>>>> which requires the initialization happens at a certain early time of
>>>> initialization of i915. I was thinking maybe we can take other patches
>>>> from Christoph like "de-virtualize*" except this one because currently
>>>> we have to maintain a TEST-ONLY patch on our tree to prevent i915 built
>>>> as kernel module.
>>> How about just capture these registers in the main module/device and
>>> not try so hard to isolate it to the gvt stuff?
>> Hi Jason:
>>
>> Thanks for the idea. I am not sure i915 guys would take this idea since
>> that it's only for GVT-g, i915 doesn't use this at all. We need to take
>> a snapshot of both PCI configuration space and MMIO registers before
>> i915 driver starts to touch the HW.
> Given the code is already linked into i915 I don't see there is much
> to object to here. It can remain conditional on the kernel parameter
> as today.
>
> As a general philosophy this would all be much less strange if the
> mdev .ko is truely optional. It should be cleanly seperate from its
> base device and never request_module'd..
>
> In this case auxiliary device might be a good option, have i915 create
> one and the mdev module be loaded against it.
>
> In the mean time is there some shortcut to get this series to move
> ahead? Is patch 4 essential to the rest of the series?
>
> A really awful hack would be to push the pci_driver_register into a
> WQ so that the request_module is guarenteed to not be part of the
> module_init callchain.

Hi Jason and folks:

Thanks so much for the ideas. That sounds great and I was keeping 
thinking how to make progress on this. How about we do like this: We 
don't do request_module("kvmgt") in i915.ko, which resolves the circular 
module dependency. We keep the code of doing snapshot of registers in 
intel_gvt.c. When i915.enable_gvt=1, we do the snapshot. Then we export 
functions for kvmgt.ko in intel_gvt.c to check if gvt in i915 is enabled 
or not and get the snapshots.

How does that sounds? I just need to write another patch and put it on 
top of Christoph's series.

Thanks,

Zhi.

>> Also I was thinking if moving gvt into kvmgt.ko is the right direction.
>> It seems the module loading system in kernel is not designed for "module
>> A loading module B, which needs symbols from module A, in the
>> initialization path of module A".
> Of course not, that is a circular module dependency, it should not be
> that way. The SW layers need to be clean and orderly - meaning the
> i915 module needs to have the minimal amount of code to support the
> mdev module.
>
> Jason