+ Thomas, Maarten and Matt

(Also, Zhi and Zhenyu, please see down)

Quoting Christoph Hellwig (2021-11-02 09:05:38)

Instead of having an option to build the gvt code into the main i915 module, just move it into the kvmgt.ko module. This only requires a new struct with three entries that the KVMGT modules needs to register with the main i915 module, and a proper list of GVT-enabled devices instead of global device pointer.

Signed-off-by: Christoph Hellwig hch@lst.de

<SNIP>

+/*

• • Exported here so that the exports only get created when GVT support is
• • actually enabled.
• */

+EXPORT_SYMBOL_NS_GPL(i915_gem_object_alloc, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_object_create_shmem, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_object_init, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_object_ggtt_pin_ww, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_object_pin_map, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_object_set_to_cpu_domain, I915_GVT); +EXPORT_SYMBOL_NS_GPL(__i915_gem_object_flush_map, I915_GVT); +EXPORT_SYMBOL_NS_GPL(__i915_gem_object_set_pages, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_gtt_insert, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_prime_export, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_ww_ctx_init, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_ww_ctx_backoff, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_gem_ww_ctx_fini, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_ppgtt_create, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_request_add, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_request_create, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_request_wait, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_reserve_fence, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_unreserve_fence, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_vm_release, I915_GVT); +EXPORT_SYMBOL_NS_GPL(i915_vma_move_to_active, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_context_create, I915_GVT); +EXPORT_SYMBOL_NS_GPL(__intel_context_do_pin, I915_GVT); +EXPORT_SYMBOL_NS_GPL(__intel_context_do_unpin, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_ring_begin, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_get, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_runtime_pm_put_unchecked, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_for_reg, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_get, I915_GVT); +EXPORT_SYMBOL_NS_GPL(intel_uncore_forcewake_put, I915_GVT); +EXPORT_SYMBOL_NS_GPL(shmem_pin_map, I915_GVT); +EXPORT_SYMBOL_NS_GPL(shmem_unpin_map, I915_GVT); +EXPORT_SYMBOL_NS_GPL(__px_dma, I915_GVT);

This is where the module conversion got stuck last time.

Conditionally exporting is kind of a partial remedy. Previously the intention for the rewrite was to define a clear interface between the two modules which would be well defined enough that we could avoid frequent clashes and hopefully get GVT into drm-tip, too.

The absolute minimum requirement is not to have any of the double underscore symbols in this list. As that convention is specifically used to mark functions which are expected to have reduced error checking because of the exact point they are being called from. With that done we should be where we can enable the exports by default and modprobe would be all that is required.

I think Zhenyu and Zhi took an AR back in time to see how small they could shrink this list. Would we have some followup patches available to shrink this interface?

Also, the golden MMIO state capture remains something that leaks the hypervisor state into the guests. For example the fact which bug W/A are applied in hypervisor, and I would rather have that code path isolated before enabling by default.

Regards, Joonas