Re: [edk2-devel] [PATCH v2 1/2] OvmfPkg/IntelGvtGopDxe: Intel GVT-g GOP Implementation.

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> Thanks a lot for your comment, Laszlo! See my reply also end of the patch.
> Hi Gerd, Alex, would you mind share your thoughts as well?

> > (2) If the fw_cfg file in question is not found, the patch simply
> > ignores it. We log a debug message about it (not even an error message),
> > but then proceed with the rest of the code as if everything was OK.
> > 
> > Is that intentional?
> 
> It's absolutely OK doing in this way. etc/igd-opregion is indeed not clear
> for OVMF. Assume future guest driver decouple itself with igd-opregion,
> which some guest driver doesn't rely on it already, the intention of this
> patch is to have a common solution for Intel GVT-g vGPU.

For reference, on seabios the opregion workflow looks like this:

  (1) i915 gvt kernel driver generates opregion content.
  (2) opregion is exported as special vfio region.
  (3) qemu copies over the content to etc/igd-opregion fw_cfg file.
  (4) seabios loads etc/igd-opregion, stores it in ram, writes the
      address to a register.
  (5) guest os driver reads the register to find the opregion.

So, first question is how dynamic is the opregion content?

On physical hardware it clearly is, it carries information on how
outputs are wired to physical DVI / HDMI / DP plugs and for laptops how
to drive the lvds panel built in.  For virtual hardware this is fixed
and a simple "all outputs are displayport".

What else is in there?  Anything which might need changes when the gvt
driver is updated?  Given that some guest drivers do not depend on the
opregion I assume this is not the case.

So I'm wondering whenever there is a good reason in the first place to
generate the opregion in the i915 gvt kernel driver.  Is it an option to
generate the opregion in this IntelGvtGopDxe driver instead?  That would
clearly be the easiest solution.

Failing that we should at least take fw_cfg out of the loop.  It is just
a middle man here and not needed at all.  As discussed before the pci
rom bar can do the job instead.  That will remove any platform
dependencies from the driver, it only need to talk to the device itself
then.

take care,
  Gerd