From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web08.9287.1616744235459551743 for ; Fri, 26 Mar 2021 00:37:15 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: colin.xu@intel.com) IronPort-SDR: nat1knMAG1eYZ7Lz+h8GgOIL6WVVZbKg4przoAFMe8/TnuRo3KCw/1D8PaZ5jJKHwR8kXmg0pl Iq7hErwNMIQw== X-IronPort-AV: E=McAfee;i="6000,8403,9934"; a="171085524" X-IronPort-AV: E=Sophos;i="5.81,279,1610438400"; d="scan'208";a="171085524" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Mar 2021 00:37:14 -0700 IronPort-SDR: r5bl74w9oA5eQMRq2YLExfv19ReC/fXYmP7OdkQIOhHJz/bWk6WaxU+VvqSCcMmdyEzC3yUAOZ GuZobBgOwMGA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,279,1610438400"; d="scan'208";a="443213348" Received: from unknown (HELO coxu-arch-shz) ([10.239.160.25]) by fmsmga002.fm.intel.com with ESMTP; 26 Mar 2021 00:37:12 -0700 Date: Fri, 26 Mar 2021 15:37:11 +0800 (CST) From: "Colin Xu" X-X-Sender: coxu_arch@coxu-arch-shz To: Gerd Hoffmann cc: Colin Xu , zhenyuw@linux.intel.com, devel@edk2.groups.io, lersek@redhat.com, alex.williamson@redhat.com, rebecca@bsdio.com Subject: Re: [edk2-devel] [PATCH v2 1/2] OvmfPkg/IntelGvtGopDxe: Intel GVT-g GOP Implementation. In-Reply-To: <20210319090641.zx7ly4ev7ileticc@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> <20210319090641.zx7ly4ev7ileticc@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, 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