Laszlo: Thanks for your comments. I will create SamplePkg to include this usage. And, I will document the combined behavior on DEPEX in branch Readme.MD. The DEPEX will be combined together with AND when the driver will be combined.
Thanks Liming > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Monday, June 12, 2017 10:17 PM > To: Gao, Liming <liming....@intel.com>; edk2-devel@lists.01.org > Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com> > Subject: Re: [edk2] [PATCH staging][BaseToolsOpt 0/4] Enable multiple driver > combination > > (CC Mike) > > On 06/10/17 15:49, Gao, Liming wrote: > > Laszlo: > > I really create one example to show the combined driver usage. I > > don't plan to push this change into OvmfPkg master. > > Ah, I see. > > > If this patch brings confuse to you, I will create one SamplePkg to > > include it. > I'd just like to understand the workflow intended for the BaseToolsOpt > branch. After all, you did modify OvmfPkg on that branch, to provide an > example. But, what is going to happen to this change later on? > > I mean, patch #1 is the BaseTools feature itself, so you surely want to > bring that to edk2 master at some point, right? How can we tell later > that patch #1 should be merged into edk2 master but patch #2 and patch > #3 (the OvmfPkg example code) should not? > > My understanding was that staging branches would be *merged* into edk2 > master, with a git merge operation, pulling in all the changes from the > staging branch. If that's the case, I don't think we can realistically > separate out OvmfPkg (or similar) platform code at the time of merge. > > If you are going to do a final rebase to edk2 master (instead of a > merge), when the BaseToolsOpt changes are ready for edk2 master, then > you can indeed drop the OvmfPkg patches at that point. But then the > example code will be lost (it will never go beyond the mailing list and > the BaseToolsOpt staging branch). > > ... Historically, in this message: > <http://mid.mail-archive.com/E92EE9817A31E24EB0585FDF735412F56485F2E7@ORSMSX113.amr.corp.intel.com>, > Mike seems to have suggested a final rebase / repost, for the HTTPS-TLS > feature. A rebase certainly makes it possible to drop the OvmfPkg > example code from the final version of this set, but then the example > code -- which *is* valuable -- will never be part of edk2 master. That's > not optimal IMO (unless you add the same example to the DSC spec). > > So, I think SamplePkg is a good idea. You can provide a long-term > example in that package, even in edk2 master, without disturbing current > platforms. > > (Please correct me if I'm incorrect about the staging branch workflow. > CC'ing Mike just to be sure.) > > > And, I don't want to limit this feature into the specific driver > > type. I would like platform developer make the decision. If user > > combines some PEIM or some DXE drivers in its Platform.dsc, it can > > specify the combined driver in APRIOR list to make sure it be > > dispatched correctly. > I agree, but should we document what happens to the DEPEX sections that > were defined in the original (separate) INF files? Are they dropped? Are > they combined in some way (like, are they AND-ed together)? > > It's fine if the platform developer makes the decision, but they need to > understand the DEPEX behavior to decide about it. > > (My apologies if the DEPEX behavior is already documented for combined > drivers, I missed it then.) > > Thanks! > Laszlo > > > > > Thanks > > Liming > >> -----Original Message----- > >> From: Laszlo Ersek [mailto:ler...@redhat.com] > >> Sent: Friday, June 9, 2017 5:03 AM > >> To: Gao, Liming <liming....@intel.com>; edk2-devel@lists.01.org > >> Cc: Justen, Jordan L <jordan.l.jus...@intel.com> > >> Subject: Re: [edk2] [PATCH staging][BaseToolsOpt 0/4] Enable multiple > >> driver combination > >> > >> 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel