From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web12.821.1616119700540541175 for ; Thu, 18 Mar 2021 19:08:20 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: colin.xu@intel.com) IronPort-SDR: qlvnO/RajlLJu940877y3WLHKJsIAH8gvDmr0rTTRvplRgBDC3sgvJPbRTYAvTn/tjlBvOnu3m vWFLd+tZTn+Q== X-IronPort-AV: E=McAfee;i="6000,8403,9927"; a="187451904" X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="187451904" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Mar 2021 19:08:19 -0700 IronPort-SDR: 9j6GT/zYN2bJHks4a4MuqlfpfanZI6l+LONpNhfOBhSL9bk5FkIkiE+68Fn7OiT5DinZRP8Xgw uw9s3ARFaIUw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,259,1610438400"; d="scan'208";a="606425905" Received: from unknown (HELO coxu-arch-shz) ([10.239.160.25]) by fmsmga005.fm.intel.com with ESMTP; 18 Mar 2021 19:08:17 -0700 Date: Fri, 19 Mar 2021 10:08:16 +0800 (CST) From: "Colin Xu" X-X-Sender: coxu_arch@coxu-arch-shz To: Gerd Hoffmann , zhenyuw@linux.intel.com cc: Colin Xu , devel@edk2.groups.io, lersek@redhat.com, alex.williamson@redhat.com, rebecca@bsdio.com, zhenyuw@linux.intel.com Subject: Re: [edk2-devel] [PATCH v2 1/2] OvmfPkg/IntelGvtGopDxe: Intel GVT-g GOP Implementation. In-Reply-To: <20210312123544.uarnizkane27phcl@sirius.home.kraxel.org> Message-ID: References: <714af4f188644d03cbb93eb7621c34a6386dff32.1614924813.git.colin.xu@intel.com> <60a24d26-e332-c907-61fe-648957708793@redhat.com> <20210312123544.uarnizkane27phcl@sirius.home.kraxel.org> User-Agent: Alpine 2.22 (LNX 419 2020-04-12) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed 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