From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from nwk-aaemail-lapp02.apple.com (nwk-aaemail-lapp02.apple.com [17.151.62.67]) by mx.groups.io with SMTP id smtpd.web10.655.1579023917981300866 for ; Tue, 14 Jan 2020 09:45:18 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=AwaXzw67; spf=pass (domain: apple.com, ip: 17.151.62.67, mailfrom: afish@apple.com) Received: from pps.filterd (nwk-aaemail-lapp02.apple.com [127.0.0.1]) by nwk-aaemail-lapp02.apple.com (8.16.0.27/8.16.0.27) with SMTP id 00EHgMqO050279; Tue, 14 Jan 2020 09:45:16 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=20180706; bh=pVlNaKBLa1bJwAEsvtbhXZEezexcCiKW2H/Ie6t+46g=; b=AwaXzw67fo1H9YfOtNJaF8deMmI4Ozl6ZwLJ10RjDeoGXG2cOtJsp+wJ4HRbHM3Gpwqc eCLWb4uOVGaow/jl2X9uKvVv/Iv4uSg61Rcmi5Gm52JC3lxBwkJoNXyiApOu50yMx2gV uyqT1+qhfCt1vRsUVVjcndIRi5SSEBMTthShhmW+GI+g3KRCitmV2ISRkPxV8NGUzmsH XDgNrsXemj2AemXJsd7X9A2jz6dsqdQWolDWD1xxX8WUNsi9e9HK1oV29IxuaMjVlDxw M/Pi639WzerLFvxy4kDP3zyKbmVGyXf/aEYB+Ee1YX6AQUveQq5wqC9te+BGpz7BIN1t PQ== Received: from ma1-mtap-s03.corp.apple.com (ma1-mtap-s03.corp.apple.com [17.40.76.7]) by nwk-aaemail-lapp02.apple.com with ESMTP id 2xfbtmv0gh-8 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 14 Jan 2020 09:45:16 -0800 Received: from nwk-mmpp-sz12.apple.com (nwk-mmpp-sz12.apple.com [17.128.115.204]) by ma1-mtap-s03.corp.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPS id <0Q4300K4KYNA5240@ma1-mtap-s03.corp.apple.com>; Tue, 14 Jan 2020 09:45:15 -0800 (PST) Received: from process_milters-daemon.nwk-mmpp-sz12.apple.com by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0Q4300I00YGT4X00@nwk-mmpp-sz12.apple.com>; Tue, 14 Jan 2020 09:45:13 -0800 (PST) X-Va-A: X-Va-T-CD: 650d61ef17a2247d3ba567199bcad85e X-Va-E-CD: a8eebd4dd0e994b0e21db70f44624896 X-Va-R-CD: faabe6adb46224738b5248b7e4027d4b X-Va-CD: 0 X-Va-ID: c9ae641f-d14b-4d76-9a5c-20ece021b388 X-V-A: X-V-T-CD: 650d61ef17a2247d3ba567199bcad85e X-V-E-CD: a8eebd4dd0e994b0e21db70f44624896 X-V-R-CD: faabe6adb46224738b5248b7e4027d4b X-V-CD: 0 X-V-ID: d93885b9-e818-431e-83f0-927a852555f4 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2020-01-14_06:,, signatures=0 Received: from da0602a-dhcp109.apple.com ([17.226.23.109]) by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0Q43009HCYN7DU40@nwk-mmpp-sz12.apple.com>; Tue, 14 Jan 2020 09:45:08 -0800 (PST) Sender: afish@apple.com MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver#### From: "Andrew Fish" In-reply-to: Date: Tue, 14 Jan 2020 09:45:06 -0800 Cc: Laszlo Ersek , "Ni, Ray" , Leif Lindholm , "Gao, Zhichao" , "Ma, Maurice" , "Dong, Guo" , "You, Benjamin" Message-id: 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> To: devel@edk2.groups.io, Ard Biesheuvel X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2020-01-14_06:,, signatures=0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: quoted-printable > On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel = wrote: >=20 > On Mon, 13 Jan 2020 at 18:57, Andrew Fish via Groups.Io > wrote: >>=20 >> Ard, >>=20 >> Is the problem GFX console? Would it be possible to have a PCD to assum= e graphics console, and if non was found on the boot connect those PCI devi= ces and update the NVRAM to cause a console to connect. You might have to d= o a 2nd connect on the GOP handle after the nvram variable was written to m= ake the ConSpliter see it? >>=20 >=20 > 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? >=20 Ard, I was thinking this specific case was no active GOP driver in the system d= ue to the emulation of the ROM. I was saying there could be a platform poli= cy to require GOP, and there could be a platform hook point to check for n= o GOP and take action. This is not really a Driver#### path, but a fallback= path for no GOP. So that is kind of cheating :).=20 Also I was thinking the ConSpliter would activate the new GOP if you do a = 2nd gBS->ConnectController on the Graphics PCI or GOP handle after updating= the console nvram variable.=20 Sorry I'm not really familiar with the modern TianoCore BDS, as we have a = custom BDS on Macs, so I'm talking more hypothetically about how BDS could = work. Thanks, Andrew Fish >=20 >=20 >>=20 >> On Jan 13, 2020, at 9:28 AM, Ard Biesheuvel = wrote: >>=20 >> On Fri, 10 Jan 2020 at 17:23, Laszlo Ersek wrote: >>=20 >>=20 >> On 01/10/20 15:37, Ni, Ray wrote: >>=20 >> 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. >>=20 >> 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. >>=20 >> One backward compatible approach can be: processing Driver#### before B= eforeConsole() and processing the new Driver#### (added by BeforeConsole())= after BeforeConsole(). >>=20 >> 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. >>=20 >> 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. >>=20 >> With all what I explained in above, I cannot agree with the changes. >>=20 >> 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. >>=20 >>=20 >> Let me raise three other ideas (alternatives to each other, and to the = above), with varying levels of annoyance. :) >>=20 >>=20 >> Thanks Laszlo >>=20 >> 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? >>=20 >>=20 >> (1) Keep the following logic (i.e. the subject of this patch): >>=20 >> // >> // Execute Driver Options >> // >> LoadOptions =3D EfiBootManagerGetLoadOptions (&LoadOptionCount, LoadOpt= ionTypeDriver); >> ProcessLoadOptions (LoadOptions, LoadOptionCount); >> EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount); >>=20 >> in *both* places, but gate each one with a bit in a new bitmask PCD. >>=20 >> (Note: it's probably not the best for any platform to permit both branc= hes, as driver images would be loaded twice.) >>=20 >>=20 >> (2) EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL has recently been added to edk= 2. It looks like a well-designed (extensible) protocol, for two reasons: >>=20 >> - the protocol structure has a Revision field, >>=20 >> - the only current member function, RefreshAllBootOptions(), is permitt= ed to return EFI_UNSUPPORTED -- and the single call site, in the EfiBootMan= agerRefreshAllBootOption() function, handles that return value well (it doe= s nothing). >>=20 >> The idea would be to bump the Revision field, and add a new member func= tion. Then call this member function (if it exists) in the spot where the c= urrent patch proposes to move the Driver#### dispatch logic to. >>=20 >> This is almost like a new PlatformBootManagerLib interface, except it d= oes not require existent lib instances to be updated. >>=20 >> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, R= efreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is irrel= evant to Ard's use case.) >>=20 >> Perhaps add yet another member function that can disable the Driver####= option processing in the current location. >>=20 >>=20 >> (3) Extend the UEFI specification, section "3.1.3 Load Options". >>=20 >> The LOAD_OPTION_CATEGORY bit-field is almost what we want, except it's = specified to be ignored for Driver#### options. So, >>=20 >> - either invent the standardese to let us use LOAD_OPTION_CATEGORY for = Driver#### options too, without breaking existing platforms, *or* >> - introduce a new (not too wide) distinct bitfield called LOAD_OPTION_D= RIVER_CATEGORY. >>=20 >> Whichever category bitfield proves acceptable, introduce a new "driver = category bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE. >>=20 >> Specify that, if any Driver#### option has the LOAD_OPTION_DRIVER_CATEG= ORY_CONSOLE attribute set, then Driver#### options will be processed in two= passes. In both passes, DriverOrder is to be honored, but the first pass w= ill only consider options with the attribute set, and the second pass will = only consider options with the attribute clear. >>=20 >> Implement this in BdsEntry / UefiBootManagerLib. >>=20 >> ... Maybe don't even introduce LOAD_OPTION_DRIVER_CATEGORY*; just a bit= field that is expressly vendor specific? >>=20 >> Thanks >> Laszlo >>=20 >>=20 >>=20 >>=20 >=20 >=20 >=20