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

["Colin Xu" <colin.xu@intel.com>]

On Fri, 12 Mar 2021, Gerd Hoffmann wrote:

>  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.
>
It's absolutely true that generating opregion in IntelGvtGopDxe could be 
an option. I thought the reason that i915/gvt generating the opregion is 
due to the design that how to emulate a GPU in a layered hierarchy. On 
physical hardware, OpRegion is part of firmware, loaded and comsumed 
by GPU SW stack. Firmware itself is always platform specific. So the most 
close virtualization is let gvt virtualize GPU HW, and firmware virtualize 
opregion.

However in QEMU world, it looks like those non-simulated device related 
opreations are left to other component as most as possible, like VFIO 
region, so that QEMU itself could force less on physical device specific 
operations but handle the common logic. In this opregion case, QEMU and 
seabios only copies the raw data, but the contents are left to gvt to fill 
so that gvt could fill different data in different cases. Otherwise any 
change requires SEABIOS update, instead of dyncamilly filled by gvt.

One benefit OVMF overcomes SEABIOS is that IntelGvtGopDxe can be loaded
as a standalone driver. So it maybe possible that in OVMF case, both gvt 
and IntelGvtGopDxe can act part of Intel vGPU virtualization component. 
However the behavior of gvt+seabios and gvt+ovmf will be inconsistent. May 
need update gvt logic so that if IntelGvtGopDxe generate opregion, gvt can 
still update it before any opregion consumer using it. Then, loading 
fw_cfg can be dropped without preventing guest OS driver find it.

I'm adding gvt maintainer here for more comments.

> 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
>
>

--
Best Regards,
Colin Xu