On 2021.08.20 16:17:24 +0200, Christoph Hellwig wrote:

On Thu, Aug 19, 2021 at 04:29:29PM +0800, Zhenyu Wang wrote:

...

I'm working on below patch to resolve this. But I met a weird issue in case when building i915 as module and also kvmgt module, it caused busy wait on request_module("kvmgt") when boot, it doesn't happen if building i915 into kernel. I'm not sure what could be the reason?

Luis, do you know if there is a problem with a request_module from a driver ->probe routine that is probably called by a module_init function itself?

In the meantime I'll try to reproduce it locally, but I always had a hard time getting useful results out of a modular i915, especially when combined with module paramters. (no blame on i915, just the problem with modules needed early on).

...

...

But the problem I see is that after moving gvt device model (gvt/*.c except kvmgt.c) into kvmgt module, we'll have issue with initial mmio state which current gvt relies on, that is in design supposed to get initial HW state before i915 driver has taken any operation. Before we can ensure that, I think we may only remove MPT part first but still keep gvt device model as part of i915 with config. I'll try to split that out.

Sorry I misread the code that as we always request kvmgt module when gvt init, so it should still apply original method that this isn't a problem. Our current validation result has shown no regression as well.

What does initial mmio state mean? This is something new to me. But as you said in this mail unless I missed something very big it should work the same as before.

It's gvt internal track for all gfx mmio state, and yes with your current change it should still work as before.

...

-static inline void intel_context_unpin(struct intel_context *ce) +static inline void _intel_context_unpin(struct intel_context *ce) { if (!ce->ops->sched_disable) { __intel_context_do_unpin(ce, 1); @@ -150,6 +150,7 @@ static inline void intel_context_unpin(struct intel_context *ce) } } } +void intel_context_unpin(struct intel_context *ce);

Looking at intel_context_unpin/_intel_context_unpin is there really a need to have this inline to start with? It don't see much the compiler could optimize by inlining it.

I'll send patch to i915 for this, and get more comments there.

thanks