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