From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web12.7605.1615552553969022618 for ; Fri, 12 Mar 2021 04:35:54 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=VGl+60Gt; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1615552553; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ig68PvIvVfppGrS129ZI840MTxOKfGOv4hBV3JCbUSI=; b=VGl+60GtP3iem9I7ncp3+pLXvJkCPXF1VurTcPlSwQWCDGEa0dMsfPxcDt6LD7zfTUGSw1 ZCIS77fMGsiyKAFCe83pkBgBP/RE1bC0UJphExZ1Jdvvh6Tvs2k+DBoTG09Y64sNj+UxQL L7WlMcXyKrNJc8o2KwWtsO4MoT6s01I= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-332-FUfukg0BP8aka1IDyaiDXg-1; Fri, 12 Mar 2021 07:35:50 -0500 X-MC-Unique: FUfukg0BP8aka1IDyaiDXg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 76F6D1966349; Fri, 12 Mar 2021 12:35:49 +0000 (UTC) Received: from sirius.home.kraxel.org (ovpn-112-141.ams2.redhat.com [10.36.112.141]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 00DB22BFE9; Fri, 12 Mar 2021 12:35:45 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 67E7F18000B4; Fri, 12 Mar 2021 13:35:44 +0100 (CET) Date: Fri, 12 Mar 2021 13:35:44 +0100 From: Gerd Hoffmann To: Colin Xu Cc: 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. Message-ID: <20210312123544.uarnizkane27phcl@sirius.home.kraxel.org> References: <714af4f188644d03cbb93eb7621c34a6386dff32.1614924813.git.colin.xu@intel.com> <60a24d26-e332-c907-61fe-648957708793@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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