From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web12.41681.1578936530891180691 for ; Mon, 13 Jan 2020 09:28:51 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=nvqiqPEG; spf=pass (domain: linaro.org, ip: 209.85.221.65, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f65.google.com with SMTP id q10so9461020wrm.11 for ; Mon, 13 Jan 2020 09:28:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=awZqXot82d0wWw+RhuGN+V85zfTqop1VtFKu43jD+vk=; b=nvqiqPEGIBPbGV7T7rxjoRWUyKwD/6y+8ajfX4hd9N30R1lKAOCR2KVzf3xjHRiUjY pdyEACptiWljUaavocOnonbfS8AUCEgWRW9j2rgM6DNwRcHo5kUkyY2XtiVnJpfc1Dtp JciYhS48f9oOR9CsHevSE6FpAtIo7vUAkoKaBsGpVXc8eja9/hRg9H2/SF260SZAR8zy UUOpFDkUaTUaeoRXzv3rhk0GmHDpeHxQ9eIWy3V5xorwSEmJWVek5K1DIdrW/MqDBN2m dNBpFK33+D0UzGqEhDcDtM4rOvqhSJFUJDeqHPDg43oCQEHI3tmk6JJnQLKKr1VAtTvl IERA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=awZqXot82d0wWw+RhuGN+V85zfTqop1VtFKu43jD+vk=; b=He/kpwfeMYH50oztB5rm7vx0yyE+L+fBDcMgGBjEc3LRNE7KApYPjmqE0YM+xwZcxz +g5oliC3Xfc0H/YRkSO2FUr7ExVp0ldzruc2AZT6p0mFU0C4tLox1IE4tQ1S0Su0cqBe Rqo73UqMdAZ7UxK1Z05LCbhjowIhCcH5fjU53ZzikcE/K0O193o/G1SlIvGgV9EEC+iC AwPJAwjj5oU98W7/QBoXyQxjvM8AXBSMc9wRT8v/Fg/eAKxpwe2JYLhYs3tgZKFdQyhh hcJzqOeAGBXL2geLKL2nI1QT4ohrawOzo6CXTRRVJSaIZSQhkcbAghVNMLXV1Zsn1GGN uKHw== X-Gm-Message-State: APjAAAVcJeV4B/Lv08aAPg0DSggAwa3HdWzjFkKaV2ua5Oy72+wl/oq9 T5Dzmi5dRRZfyAFPOiQk4bEhAF+2EgwbRYI31DEVyA== X-Google-Smtp-Source: APXvYqw5eYlevhyXjNsl2ATm+CIsnN4oVFOaYpA88CL1lpUg+OMQE6UYsMnqtmidrAC/RHLJCUeaqBZJq5nKbjAoQbw= X-Received: by 2002:adf:cf0a:: with SMTP id o10mr19143146wrj.325.1578936529084; Mon, 13 Jan 2020 09:28:49 -0800 (PST) MIME-Version: 1.0 References: <20191206143128.19371-1-ard.biesheuvel@linaro.org> <20191206143128.19371-3-ard.biesheuvel@linaro.org> <8d49cca6-b1b9-cddc-0590-4e4a84643e4f@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C39604F@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C399C9C@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C3E2179@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C3EFAF4@SHSMSX104.ccr.corp.intel.com> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 13 Jan 2020 18:28:37 +0100 Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver#### To: Laszlo Ersek Cc: "Ni, Ray" , "devel@edk2.groups.io" , Leif Lindholm , "Gao, Zhichao" , "Ma, Maurice" , "Dong, Guo" , "You, Benjamin" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 10 Jan 2020 at 17:23, Laszlo Ersek wrote: > > On 01/10/20 15:37, Ni, Ray wrote: > > Ard, > > I understand now that: BeforeConsole() needs to connect Gfx to get the = GOP device path > > for ConOut updating. But the GOP driver is specified in Driver#### and = it will be loaded after > > BeforeConsole() in today's code. This order makes the Gfx connection fa= ils to start the GOP driver > > resulting ConOut not updated. > > > > Moving Driver#### processing before BeforeConsole() helps in your case.= But it may impact > > other platforms: There might be platforms that update Driver#### variab= les in BeforeConsole() > > and expect these drivers are processed in the same boot (not the next b= oot). I don't have the real > > examples. But today's flow allows this. With your change, Driver#### wi= ll not be processed in the first > > boot. The change impacts these platforms. > > > > One backward compatible approach can be: processing Driver#### before B= eforeConsole() and processing the new Driver#### (added by BeforeConsole())= after BeforeConsole(). > > > > But another problem arises: If BeforeConsole() removes a Driver####, pl= atform's expectation is that deleted Driver#### doesn't run. But that drive= r already runs. > > > > So actually the question is: when BDS core can consume the Driver#### a= nd process them? > > Right now, it=E2=80=99s the time after BeforeConsole(). Just as right n= ow BeforeConsole() updates ConOut > > in your case, BeforeConsole() is the place where platform updates all U= EFI defined boot related > > variables. Processing Driver#### before BeforeConsole() requires platfo= rm updates Driver#### > > in other places. It's a new requirement to the platform. > > > > With all what I explained in above, I cannot agree with the changes. > > > > Another approach is: > > Platform could connect the GFX in AfterConsole() and update the ConOut.= Then platform could manually call EfiBootManagerConnectConsoleVariable (Co= nOut) to re-connect ConOut. > > It's a bit ugly I agree. > > Let me raise three other ideas (alternatives to each other, and to the ab= ove), with varying levels of annoyance. :) > Thanks Laszlo Ray, given your objection to my approach, could you please consider the below and give feedback on which approach is suitable to address the issue I am trying to fix? > > (1) Keep the following logic (i.e. the subject of this patch): > > // > // Execute Driver Options > // > LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOpt= ionTypeDriver); > ProcessLoadOptions (LoadOptions, LoadOptionCount); > EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); > > in *both* places, but gate each one with a bit in a new bitmask PCD. > > (Note: it's probably not the best for any platform to permit both branche= s, as driver images would be loaded twice.) > > > (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been added to edk2.= It looks like a well-designed (extensible) protocol, for two reasons: > > - the protocol structure has a Revision field, > > - the only current member function, RefreshAllBootOptions(), is permitted= to return EFI_UNSUPPORTED -- and the single call site, in the EfiBootManag= erRefreshAllBootOption() function, handles that return value well (it does = nothing). > > The idea would be to bump the Revision field, and add a new member functi= on. Then call this member function (if it exists) in the spot where the cur= rent patch proposes to move the Driver#### dispatch logic to. > > This is almost like a new PlatformBootManagerLib interface, except it doe= s not require existent lib instances to be updated. > > And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, Ref= reshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is irrelev= ant to Ard's use case.) > > Perhaps add yet another member function that can disable the Driver#### o= ption processing in the current location. > > > (3) Extend the UEFI specification, section "3.1.3 Load Options". > > The LOAD_OPTION_CATEGORY bit-field is almost what we want, except it's sp= ecified to be ignored for Driver#### options. So, > > - either invent the standardese to let us use LOAD_OPTION_CATEGORY for Dr= iver#### options too, without breaking existing platforms, *or* > - introduce a new (not too wide) distinct bitfield called LOAD_OPTION_DRI= VER_CATEGORY. > > Whichever category bitfield proves acceptable, introduce a new "driver ca= tegory bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE. > > Specify that, if any Driver#### option has the LOAD_OPTION_DRIVER_CATEGOR= Y_CONSOLE attribute set, then Driver#### options will be processed in two p= asses. In both passes, DriverOrder is to be honored, but the first pass wil= l only consider options with the attribute set, and the second pass will on= ly consider options with the attribute clear. > > Implement this in BdsEntry / UefiBootManagerLib. > > ... Maybe don't even introduce LOAD_OPTION_DRIVER_CATEGORY*; just a bitfi= eld that is expressly vendor specific? > > Thanks > Laszlo >