From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 335FB21A143CD for ; Thu, 8 Jun 2017 14:01:59 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2305A7E9E6; Thu, 8 Jun 2017 21:03:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2305A7E9E6 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 2305A7E9E6 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-122.phx2.redhat.com [10.3.116.122]) by smtp.corp.redhat.com (Postfix) with ESMTP id 512F617134; Thu, 8 Jun 2017 21:03:07 +0000 (UTC) To: Liming Gao , edk2-devel@lists.01.org References: <1496904940-11364-1-git-send-email-liming.gao@intel.com> From: Laszlo Ersek Cc: "Jordan Justen (Intel address)" Message-ID: Date: Thu, 8 Jun 2017 23:03:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <1496904940-11364-1-git-send-email-liming.gao@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 08 Jun 2017 21:03:08 +0000 (UTC) Subject: Re: [PATCH staging][BaseToolsOpt 0/4] Enable multiple driver combination X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Jun 2017 21:01:59 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Hi Liming, (CC Jordan) On 06/08/17 08:55, Liming Gao wrote: > Combine more drivers into the single one can reduce the image size and > compile link time. This patch adds this support in BaseTools. > > Liming Gao (4): > BaseTools: Merge multiple drivers into one for size and link > performance > OvmfPkg: Update QemuVideo and VirtioGpuDxe to use NULL as > DriverBindingHandle > OvmfPkg: Combine QemuVideoDxe and VirtioGpuDxe to one driver > Update Readme.MD to include multiple driver combination. > > BaseTools/Source/Python/AutoGen/GenC.py | 24 +++++++++++++++++++----- > OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++-- > OvmfPkg/OvmfPkgIa32X64.fdf | 2 +- > OvmfPkg/QemuVideoDxe/Driver.c | 2 +- > OvmfPkg/VirtioGpuDxe/DriverBinding.c | 2 +- > Readme.MD | 1 + > 6 files changed, 27 insertions(+), 10 deletions(-) > I don't have a lot of hands-on practice with staging branches, but I believe that ultimately such changes are meant to be merged into the edk2 master branch. Is that right? If that's the case, then I don't think we should do this in OvmfPkg (on the master branch). Instead, I think example usage should be shown in: - the commit message (it's already there, so that's great), - in the DSC specification. If you absolutely need an in-tree platform to use this BaseTools feature, and you think OvmfPkg is better for this purpose than, say, EmulatorPkg or Nt32Pkg, then: (1) Please make this dependent on a new build flag (-D), which should default to FALSE. (2) Please make the same change to all three DSC files under OvmfPkg. (3) Please introduce a new FeaturePCD which corresponds to the new build flag, and controls the handle value in the entry points of the affected drivers. In particular I'm asking for (3) because the UEFI Driver Writer's Guide (Version 1.01, 03/08/2012) says: 6.1.4 Device drivers with one driver binding protocol [...] The driver entry point is responsible for installing the Driver Binding Protocol onto the driver’s image handle. [...] 6.1.5 Device drivers with multiple driver binding protocols [...] The first instance of EFI_DRIVER_BINDING_PROTOCOL is installed onto the driver’s image handle, and the additional instances of the Driver Binding Protocol are installed onto newly created driver binding handles. [...] So, in order to follow these recommendations, - when the drivers are not combined, each driver should stick with its non-NULL image handle, - when the drivers are combined, one driver should stick with its image handle, and the rest should use NULL (based on the feature PCD) Regarding the BaseTools feature itself, I think it should be restricted to UEFI_DRIVER modules (maybe it is already restricted, but then the documentation should say it). I'm suggesting that becasue UEFI_DRIVERs are supposed to have identical DEPEXes. With DXE_DRIVER modules for example, their DEPEXes could be different, and I couldn't say how those should be combined. Thanks, Laszlo