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

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

On Fri, 19 Mar 2021, Gerd Hoffmann wrote:

>  Hi,
>
>> 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.
>
> Yes, gvt could do that.
>
> The big question is: does that actually happen?  When looking at
> drivers/gpu/drm/i915/gvt/opregion.c it seems the opregion is pretty
> static.
>
> Are there plausible reasons for that capability being needed in the
> future?
>
The current situation is static indeed. However there are still two cases 
requires run-time adjustment.
1) Current opregion/vbt has fixed display mappings, and many virtual GPU 
registers' value are inherited from host. However host opregion/vbt is 
platform specific. So if guest driver (like i915) do some strict sanity 
check it will found some display registers are incorrect. The proper way 
is that gvt generate vbt based on the virutal display type it emulated, 
and initialze related vREGs by following VESA and Intel specs.
2) Some downstream project may use physical display as an output, thus 
also need generate VBT based on host opregion/vbt.

>> Otherwise any change requires SEABIOS update, instead of dyncamilly
>> filled by gvt.
>
> That clearly isn't an option.
>
Yes, so based on such reason, gvt should still be able to update it 
dynamically even ovmf/seabios can generate the initial value.
>> One benefit OVMF overcomes SEABIOS is that IntelGvtGopDxe can be loaded
>> as a standalone driver.
>
> Well, seabios allows standalone drivers too.  It's called vgabios ;)
>
Like ATI VGABIOS in seabios repo. Unfortunately Intel hasn't implemented 
yet.
>> 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.
>
> Well, in case the opregion is not static there is still the option to
> use the pci rom bar instead of fw_cfg to deliver the opregion content
> to the guest.
>
>> May need
>> update gvt logic so that if IntelGvtGopDxe generate opregion, gvt can still
>> update it before any opregion consumer using it.
>
> Hmm.  Can gvt trap pci config space writes?  IIRC pci config space is
> simply yet another vfio region, so that should be possible.  gvt could
> update the opregion then when the firmware updates the opregion address.
>
> If that actually works there is little reason for IntelGvtGopDxe (and
> seabios) to fill the opregion in the first place though.  We could
> change the opregion initialization protocol to something like this:
>
>  (1) firmware allocates memory for the opregion
>  (2) firmware fills signature and size
>  (3) firmware writes opregion address to pci config space
>  (4) gvt goes fill opregion at the given address in guest memory.
>
> Maybe it would be better to reserve a paravirtual mmio register for that
> purpose to make the whole thing a bit more robust.
>
Yes gvt can trap PCI cfg read/write, the suggested protocol is exact the 
same as I'm considering. Let me refine this big patch to smaller ones and 
send out again. Thanks very much!

>> Then, loading fw_cfg can be
>> dropped without preventing guest OS driver find it.
>
> I guess we have to keep that for a while for backward compatibility
> reasons.
Sure. That will be the correct way.
>
> take care,
>  Gerd
>
>

--
Best Regards,
Colin Xu