From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by mx.groups.io with SMTP id smtpd.web10.1042.1579020379395660839 for ; Tue, 14 Jan 2020 08:46:19 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=j9eIDonB; spf=pass (domain: linaro.org, ip: 209.85.221.68, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f68.google.com with SMTP id c14so12868638wrn.7 for ; Tue, 14 Jan 2020 08:46:19 -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=++Z29f3HF7xvTKkDZRnB9AYTOzkyBBuCEfZNuO01ANo=; b=j9eIDonBCTvtHLB9DyFBLU+1ar22Aur5EDhQ0gr3UpU9SigVlZyHL2Mioh2RRUS1/x cgmAzvYePgRf4cl/YNvphK34No70SJKt+wuF9UNA2E8ovCBRkIJSugOvrvE+pCAEhuGU CuxoP4tOzc286Jo7YucyJ2Z146xgkwZCUEgZOBjy9ePihCdgdebl7L/L+PPZr1NMr9Wr w233xAnLYFbjYCl87YCnrE7GqnGuU0lbaW9h1T303ZZ+7UstcpHDFxAlM9xiskocqQnA W0mGdqrkDL0iEZkhF67ge1FHK8TipP1NkC+NbRdUQ6fV1Pxc+0ETPqk2ZZSHaEb+Cg6B t5Cg== 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=++Z29f3HF7xvTKkDZRnB9AYTOzkyBBuCEfZNuO01ANo=; b=TC2KnV4lXE/oRhKjmE4ZVyTpILv1YY29Ug5wXMc8Jq6aEse8gIRoMP6ItCCH1GxCqB jnwTXhwIpCKyy0MvMxxowLxJlQOrWE5tpcldRFv7DzoWVfsh31JKZM8UgfxqAD98jz/9 /ieNbE7UKuAXT2eNY3aLAY1RjniUoR65WCYRupU5iPfbcs4m4UM4Mj09aYXqVIcCqRom L4HgZK2LdiCH9an/H33BwtnFe6XgLXdJtCPFDulaocllmON3vbyE3v7VLAb1U6s+7Ptd XobdTws/yD9gS27L4EbkYxDtqa6IlpVG0qtdWO1V3NxD5TBaTMrP80PfSeeFXqODSNaT 201Q== X-Gm-Message-State: APjAAAVZe0TJSErv/J4D2DIMUK/QXE4ifyBPfbcJPV4lfqbxBEIq9SjJ x6t5cZq6DafYKzx3ibaRtzfRO+ijclt2t2g1cRmNaYyXsBM= X-Google-Smtp-Source: APXvYqyNEadQtqc3nIauhZJh3YGT9e0q+ttWDYEHBN8Bx38tvKWS467J7qG+7hg3eM08y3QEa4GBhwp7mYBRVP7FqCM= X-Received: by 2002:a5d:46c1:: with SMTP id g1mr25521990wrs.200.1579020377340; Tue, 14 Jan 2020 08:46:17 -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> <40949126-F9E5-4B0A-B07D-D9CBF7A1EE3B@apple.com> In-Reply-To: <40949126-F9E5-4B0A-B07D-D9CBF7A1EE3B@apple.com> From: "Ard Biesheuvel" Date: Tue, 14 Jan 2020 17:46:06 +0100 Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver#### To: edk2-devel-groups-io , Andrew Fish Cc: Laszlo Ersek , "Ni, Ray" , Leif Lindholm , "Gao, Zhichao" , "Ma, Maurice" , "Dong, Guo" , "You, Benjamin" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io wrote: > > Ard, > > Is the problem GFX console? Would it be possible to have a PCD to assume= graphics console, and if non was found on the boot connect those PCI devic= es and update the NVRAM to cause a console to connect. You might have to do= a 2nd connect on the GOP handle after the nvram variable was written to ma= ke the ConSpliter see it? > I'm not sure I follow. Do you mean update the console variable if it doesn't contain the GOP produced by the Driver#### option and reboot? > > On Jan 13, 2020, at 9:28 AM, Ard Biesheuvel = wrote: > > 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 G= OP device path > for ConOut updating. But the GOP driver is specified in Driver#### and i= t will be loaded after > BeforeConsole() in today's code. This order makes the Gfx connection fai= ls 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#### variabl= es in BeforeConsole() > and expect these drivers are processed in the same boot (not the next bo= ot). I don't have the real > examples. But today's flow allows this. With your change, Driver#### wil= l not be processed in the first > boot. The change impacts these platforms. > > One backward compatible approach can be: processing Driver#### before Be= foreConsole() and processing the new Driver#### (added by BeforeConsole()) = after BeforeConsole(). > > But another problem arises: If BeforeConsole() removes a Driver####, pla= tform's expectation is that deleted Driver#### doesn't run. But that driver= already runs. > > So actually the question is: when BDS core can consume the Driver#### an= d process them? > Right now, it=E2=80=99s the time after BeforeConsole(). Just as right no= w BeforeConsole() updates ConOut > in your case, BeforeConsole() is the place where platform updates all UE= FI defined boot related > variables. Processing Driver#### before BeforeConsole() requires platfor= m 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 (Con= Out) to re-connect ConOut. > It's a bit ugly I agree. > > > Let me raise three other ideas (alternatives to each other, and to the a= bove), 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 branch= es, 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 permitte= d to return EFI_UNSUPPORTED -- and the single call site, in the EfiBootMana= gerRefreshAllBootOption() function, handles that return value well (it does= nothing). > > The idea would be to bump the Revision field, and add a new member funct= ion. Then call this member function (if it exists) in the spot where the cu= rrent patch proposes to move the Driver#### dispatch logic to. > > This is almost like a new PlatformBootManagerLib interface, except it do= es not require existent lib instances to be updated. > > And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, Re= freshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is irrele= vant to Ard's use case.) > > Perhaps add yet another member function that can disable the Driver#### = option 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 s= pecified to be ignored for Driver#### options. So, > > - either invent the standardese to let us use LOAD_OPTION_CATEGORY for D= river#### options too, without breaking existing platforms, *or* > - introduce a new (not too wide) distinct bitfield called LOAD_OPTION_DR= IVER_CATEGORY. > > Whichever category bitfield proves acceptable, introduce a new "driver c= ategory bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE. > > Specify that, if any Driver#### option has the LOAD_OPTION_DRIVER_CATEGO= RY_CONSOLE attribute set, then Driver#### options will be processed in two = passes. In both passes, DriverOrder is to be honored, but the first pass wi= ll only consider options with the attribute set, and the second pass will o= nly consider options with the attribute clear. > > Implement this in BdsEntry / UefiBootManagerLib. > > ... Maybe don't even introduce LOAD_OPTION_DRIVER_CATEGORY*; just a bitf= ield that is expressly vendor specific? > > Thanks > Laszlo > > > >=20