Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-14 Thread Ni, Ray 
Laszlo,
I tend to make it a platform local implementation first.
Your proposals impact a broader scope which we could
consider later if it's hard for a platform to handle the issue.

Thanks,
Ray

> -Original Message-
> From: Laszlo Ersek 
> Sent: Saturday, January 11, 2020 12:24 AM
> To: Ni, Ray ; 'devel@edk2.groups.io'
> ; 'ard.biesheu...@linaro.org'
> 
> Cc: 'Leif Lindholm' ; Gao, Zhichao
> ; Ma, Maurice ; Dong,
> Guo ; You, Benjamin 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> 
> 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 fails
> 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
> variables in BeforeConsole()
> > and expect these drivers are processed in the same boot (not the next
> boot). I don't have the real
> > examples. But today's flow allows this. With your change, Driver will
> not be processed in the first
> > boot. The change impacts these platforms.
> >
> > One backward compatible approach can be: processing Driver before
> BeforeConsole() and processing the new Driver (added by
> BeforeConsole()) after BeforeConsole().
> >
> > But another problem arises: If BeforeConsole() removes a Driver,
> platform'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
> and process them?
> > Right now, it’s the time after BeforeConsole(). Just as right now
> BeforeConsole() updates ConOut
> > in your case, BeforeConsole() is the place where platform updates all UEFI
> defined boot related
> > variables. Processing Driver before BeforeConsole() requires platform
> 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
> (ConOut) to re-connect ConOut.
> > It's a bit ugly I agree.
> 
> Let me raise three other ideas (alternatives to each other, and to the above),
> with varying levels of annoyance. :)
> 
> 
> (1) Keep the following logic (i.e. the subject of this patch):
> 
>   //
>   // Execute Driver Options
>   //
>   LoadOptions = EfiBootManagerGetLoadOptions (,
> LoadOptionTypeDriver);
>   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 branches, 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
> EfiBootManagerRefreshAllBootOption() function, handles that return value
> well (it does nothing).
> 
> The idea would be to bump the Revision field, and add a new member
> function. Then call this member function (if it exists) in the spot where the
> current patch proposes to move the Driver dispatch logic to.
> 
> This is almost like a new PlatformBootManagerLib interface, except it does
> not require existent lib instances to be updated.
> 
> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL
> implementation side, RefreshAllBootOptions() would return
> EFI_UNSUPPORTED. (Because that is irrelevant 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
> specified to be ignored for Driver options. So,
> 
> - either invent the standardese to let us use LOAD_OPTION_CATEGORY for
> Driver#

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-14 Thread Ni, Ray 
> -Original Message-
> From: af...@apple.com 
> Sent: Wednesday, January 15, 2020 1:45 AM
> To: devel@edk2.groups.io; Ard Biesheuvel 
> Cc: Laszlo Ersek ; Ni, Ray ; Leif
> Lindholm ; Gao, Zhichao ;
> Ma, Maurice ; Dong, Guo ;
> You, Benjamin 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> 
> 
> 
> > On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel 
> wrote:
> >
> > 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 devices 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 make 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?
> >
> 
> Ard,
> 
> I was thinking this specific case was no active GOP driver in the system due 
> to
> the emulation of the ROM. I was saying there could be a platform policy to
> require GOP, and there could be a  platform hook point to check for no GOP
> and take action. This is not really a Driver path, but a fallback path 
> for no
> GOP. So that is kind of cheating :).
> 
> 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.
> 
> 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.

Andrew,
TianoCore BDS in this part is very simple. The BDS core expects a platform
hook (PlatformBootManagerBeforeConsole()) to initialize every BDS related UEFI
variables like ConIn, ConOut, Driver, Boot, DriverOrder, BootOrder, etc.
Then BDS core consumes these variables and acts accordingly.
I think the "platform hook" you suggested is similar to what I suggested 
earlier,
except that you think that putting GOP in Driver is unnecessary.

Ard,
Assuming you've put GOP driver path to Driver,
you could put below code in the beginning of PlatformBootManagerAfterConsole():
1. Find the GFX controller handle
2. EfiBootManagerConnectVideoController (GFXHandle)

Thanks,
Ray


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53255): https://edk2.groups.io/g/devel/message/53255
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-14 Thread Andrew Fish via Groups.Io 



> On Jan 14, 2020, at 8:46 AM, Ard Biesheuvel  wrote:
> 
> 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 devices 
>> 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 make 
>> 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?
> 

Ard,

I was thinking this specific case was no active GOP driver in the system due to 
the emulation of the ROM. I was saying there could be a platform policy to 
require GOP, and there could be a  platform hook point to check for no GOP and 
take action. This is not really a Driver path, but a fallback path for no 
GOP. So that is kind of cheating :). 

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. 

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

> 
> 
>> 
>> 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 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 fails 
>> 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 variables 
>> in BeforeConsole()
>> and expect these drivers are processed in the same boot (not the next boot). 
>> I don't have the real
>> examples. But today's flow allows this. With your change, Driver will 
>> not be processed in the first
>> boot. The change impacts these platforms.
>> 
>> One backward compatible approach can be: processing Driver before 
>> BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
>> after BeforeConsole().
>> 
>> But another problem arises: If BeforeConsole() removes a Driver, 
>> platform'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 and 
>> process them?
>> Right now, it’s the time after BeforeConsole(). Just as right now 
>> BeforeConsole() updates ConOut
>> in your case, BeforeConsole() is the place where platform updates all UEFI 
>> defined boot related
>> variables. Processing Driver before BeforeConsole() requires platform 
>> 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 (ConOut) 
>> to re-connect ConOut.
>> It's a bit ugly I agree.
>> 
>> 
>> Let me raise three other ideas (alternatives to each other, and to the 
>> above), 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 = EfiBootManagerGetLoadOptions (, 
>> LoadOptionTypeDriver);
>> 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 branches, 
>> 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 
>> EfiBootManagerRefreshAllBootOption() function, handles that return value 
>> well (it does nothing).
>> 
>> The idea would be to bump the Revision field, and add a new member function. 
>> Then call this member function (if it exists) in the spot where the current 
>> patch proposes to

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-14 Thread Ard Biesheuvel 
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 devices 
> 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 make 
> 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 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 fails 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 variables in 
> BeforeConsole()
> and expect these drivers are processed in the same boot (not the next boot). 
> I don't have the real
> examples. But today's flow allows this. With your change, Driver will not 
> be processed in the first
> boot. The change impacts these platforms.
>
> One backward compatible approach can be: processing Driver before 
> BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
> after BeforeConsole().
>
> But another problem arises: If BeforeConsole() removes a Driver, 
> platform'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 and 
> process them?
> Right now, it’s the time after BeforeConsole(). Just as right now 
> BeforeConsole() updates ConOut
> in your case, BeforeConsole() is the place where platform updates all UEFI 
> defined boot related
> variables. Processing Driver before BeforeConsole() requires platform 
> 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 (ConOut) to 
> re-connect ConOut.
> It's a bit ugly I agree.
>
>
> Let me raise three other ideas (alternatives to each other, and to the 
> above), 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 = EfiBootManagerGetLoadOptions (, 
> LoadOptionTypeDriver);
>  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 branches, 
> 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 
> EfiBootManagerRefreshAllBootOption() function, handles that return value well 
> (it does nothing).
>
> The idea would be to bump the Revision field, and add a new member function. 
> Then call this member function (if it exists) in the spot where the current 
> patch proposes to move the Driver dispatch logic to.
>
> This is almost like a new PlatformBootManagerLib interface, except it does 
> not require existent lib instances to be updated.
>
> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, 
> RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is 
> irrelevant 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 
> specified to be ignored for Driver options. So,
>
> - 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_DRIVER_CATEGORY.
>
> Whichever

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-13 Thread Andrew Fish via Groups.Io 
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 devices 
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 make the 
ConSpliter see it?

Thanks,

Andrew Fish

> 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 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 fails 
>>> 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 variables 
>>> in BeforeConsole()
>>> and expect these drivers are processed in the same boot (not the next 
>>> boot). I don't have the real
>>> examples. But today's flow allows this. With your change, Driver will 
>>> not be processed in the first
>>> boot. The change impacts these platforms.
>>> 
>>> One backward compatible approach can be: processing Driver before 
>>> BeforeConsole() and processing the new Driver (added by 
>>> BeforeConsole()) after BeforeConsole().
>>> 
>>> But another problem arises: If BeforeConsole() removes a Driver, 
>>> platform'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 and 
>>> process them?
>>> Right now, it’s the time after BeforeConsole(). Just as right now 
>>> BeforeConsole() updates ConOut
>>> in your case, BeforeConsole() is the place where platform updates all UEFI 
>>> defined boot related
>>> variables. Processing Driver before BeforeConsole() requires platform 
>>> 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 
>>> (ConOut) to re-connect ConOut.
>>> It's a bit ugly I agree.
>> 
>> Let me raise three other ideas (alternatives to each other, and to the 
>> above), 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 = EfiBootManagerGetLoadOptions (, 
>> LoadOptionTypeDriver);
>>  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 branches, 
>> 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 
>> EfiBootManagerRefreshAllBootOption() function, handles that return value 
>> well (it does nothing).
>> 
>> The idea would be to bump the Revision field, and add a new member function. 
>> Then call this member function (if it exists) in the spot where the current 
>> patch proposes to move the Driver dispatch logic to.
>> 
>> This is almost like a new PlatformBootManagerLib interface, except it does 
>> not require existent lib instances to be updated.
>> 
>> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, 
>> RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is 
>> irrelevant 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 
>> specified to be ignored for Driver options. So,
>> 
>> - 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_DRIVER_CATEGORY.
>> 
>> Whichever category

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-13 Thread Ard Biesheuvel 
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 fails 
> > 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 variables 
> > in BeforeConsole()
> > and expect these drivers are processed in the same boot (not the next 
> > boot). I don't have the real
> > examples. But today's flow allows this. With your change, Driver will 
> > not be processed in the first
> > boot. The change impacts these platforms.
> >
> > One backward compatible approach can be: processing Driver before 
> > BeforeConsole() and processing the new Driver (added by 
> > BeforeConsole()) after BeforeConsole().
> >
> > But another problem arises: If BeforeConsole() removes a Driver, 
> > platform'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 and 
> > process them?
> > Right now, it’s the time after BeforeConsole(). Just as right now 
> > BeforeConsole() updates ConOut
> > in your case, BeforeConsole() is the place where platform updates all UEFI 
> > defined boot related
> > variables. Processing Driver before BeforeConsole() requires platform 
> > 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 
> > (ConOut) to re-connect ConOut.
> > It's a bit ugly I agree.
>
> Let me raise three other ideas (alternatives to each other, and to the 
> above), 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 = EfiBootManagerGetLoadOptions (, 
> LoadOptionTypeDriver);
>   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 branches, 
> 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 
> EfiBootManagerRefreshAllBootOption() function, handles that return value well 
> (it does nothing).
>
> The idea would be to bump the Revision field, and add a new member function. 
> Then call this member function (if it exists) in the spot where the current 
> patch proposes to move the Driver dispatch logic to.
>
> This is almost like a new PlatformBootManagerLib interface, except it does 
> not require existent lib instances to be updated.
>
> And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, 
> RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is 
> irrelevant 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 
> specified to be ignored for Driver options. So,
>
> - 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_DRIVER_CATEGORY.
>
> Whichever category bitfield proves acceptable, introduce a new "driver 
> category bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE.
>
> Specify that, if any Driver option has the 
> LOAD_OPTION_DRIVER_CATEGORY_CONSOLE attribute set, then Driver options 
> will be processed in two passes. In both passes, DriverOrder is to be 
> honored, but the first pass will only consider options with the attribute 
> set, and the second pass will only consider options with the attribute clear.
>
> Implement this in BdsEntry /

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-10 Thread Laszlo Ersek 
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 fails 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 variables in 
> BeforeConsole()
> and expect these drivers are processed in the same boot (not the next boot). 
> I don't have the real
> examples. But today's flow allows this. With your change, Driver will not 
> be processed in the first
> boot. The change impacts these platforms.
> 
> One backward compatible approach can be: processing Driver before 
> BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
> after BeforeConsole().
> 
> But another problem arises: If BeforeConsole() removes a Driver, 
> platform'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 and 
> process them?
> Right now, it’s the time after BeforeConsole(). Just as right now 
> BeforeConsole() updates ConOut
> in your case, BeforeConsole() is the place where platform updates all UEFI 
> defined boot related
> variables. Processing Driver before BeforeConsole() requires platform 
> 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 (ConOut) to 
> re-connect ConOut.
> It's a bit ugly I agree.

Let me raise three other ideas (alternatives to each other, and to the above), 
with varying levels of annoyance. :)


(1) Keep the following logic (i.e. the subject of this patch):

  //
  // Execute Driver Options
  //
  LoadOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeDriver);
  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 branches, 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 
EfiBootManagerRefreshAllBootOption() function, handles that return value well 
(it does nothing).

The idea would be to bump the Revision field, and add a new member function. 
Then call this member function (if it exists) in the spot where the current 
patch proposes to move the Driver dispatch logic to.

This is almost like a new PlatformBootManagerLib interface, except it does not 
require existent lib instances to be updated.

And, on the EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL implementation side, 
RefreshAllBootOptions() would return EFI_UNSUPPORTED. (Because that is 
irrelevant 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 
specified to be ignored for Driver options. So,

- 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_DRIVER_CATEGORY.

Whichever category bitfield proves acceptable, introduce a new "driver category 
bit" in that bitfield, called LOAD_OPTION_DRIVER_CATEGORY_CONSOLE.

Specify that, if any Driver option has the 
LOAD_OPTION_DRIVER_CATEGORY_CONSOLE attribute set, then Driver options will 
be processed in two passes. In both passes, DriverOrder is to be honored, but 
the first pass will only consider options with the attribute set, and the 
second pass will only consider options with the attribute clear.

Implement this in BdsEntry / UefiBootManagerLib.

... Maybe don't even introduce LOAD_OPTION_DRIVER_CATEGORY*; just a bitfield 
that is expressly vendor specific?

Thanks
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53163): https://edk2.groups.io/g/devel/message/53163
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-10 Thread Ni, Ray 
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 fails 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 variables in 
BeforeConsole()
and expect these drivers are processed in the same boot (not the next boot). I 
don't have the real
examples. But today's flow allows this. With your change, Driver will not 
be processed in the first
boot. The change impacts these platforms.

One backward compatible approach can be: processing Driver before 
BeforeConsole() and processing the new Driver (added by BeforeConsole()) 
after BeforeConsole().

But another problem arises: If BeforeConsole() removes a Driver, platform'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 and 
process them?
Right now, it’s the time after BeforeConsole(). Just as right now 
BeforeConsole() updates ConOut
in your case, BeforeConsole() is the place where platform updates all UEFI 
defined boot related
variables. Processing Driver before BeforeConsole() requires platform 
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 (ConOut) to 
re-connect ConOut.
It's a bit ugly I agree.

Thanks,
Ray


> -Original Message-
> From: Ni, Ray
> Sent: Friday, January 10, 2020 9:20 AM
> To: devel@edk2.groups.io; ard.biesheu...@linaro.org
> Cc: Laszlo Ersek ; Leif Lindholm
> ; Gao, Zhichao ; Ma,
> Maurice ; Dong, Guo ; You,
> Benjamin 
> Subject: RE: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> 
> Ard,
> I will give update on this by end of this week. sorry about the delay.
> 
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ard
> Biesheuvel
> > Sent: Thursday, January 9, 2020 8:54 PM
> > To: Ni, Ray 
> > Cc: devel@edk2.groups.io; Laszlo Ersek ; Leif Lindholm
> ; Gao, Zhichao
> > ; Ma, Maurice ; Dong, Guo
> ; You, Benjamin
> > 
> > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow
> consoles on drivers loaded via Driver
> >
> > On Mon, 23 Dec 2019 at 15:09, Ard Biesheuvel 
> wrote:
> > >
> > > On Thu, 12 Dec 2019 at 10:05, Ard Biesheuvel 
> wrote:
> > > >
> > > > On Thu, 12 Dec 2019 at 09:59, Ni, Ray  wrote:
> > > > >
> > > > > Ard,
> > > > > I still cannot understand it.
> > > > >
> > > > > Since Driver are processed (process = LoadImage + StartImage) 
> > > > > after
> EndOfDxe, they are not deferred.
> > > > > It means the Driver entrypoints are called directly.
> > > > > Inside the entrypoints, driverbinding protocols are installed.
> > > > >
> > > > > After that, EfiBootManagerConnectAllDefaultConsoles () uses driver
> model logic to start the proper GOP driver.
> > > > >
> > > >
> > > > It only does that if the GOP console is already in the
> > > > ConIn/ConOut/ConErr variables, no?
> > > >
> > > > Today, we have code in the PlatformBdsLib to traverse the PCI
> > > > hierarchy to populate those ConXXX variables if we encounter any PCI
> > > > graphics cards, but this is done in
> > > > PlatformBootManagerBeforeConsole(), so if the Driver is loaded
> > > > *after* PlatformBootManagerBeforeConsole(), it will not be added to
> > > > ConXXX. So we need to process Driver before calling
> > > > PlatformBootManagerBeforeConsole(), so that the driver is dispatches
> > > > right after we call DispatchDeferredImages() but before we do the
> > > > traversal and populate the COnXXX variables.
> > > >
> > > > How else do you propose we could make PCI graphics controllers
> > > > supported by Driver options appear in COnXXX before
> > > > EfiBootManagerConnectAllDefaultConsoles() is called?
> > > >
> > >
> > > Ping?
> > >
> > >
> >
> > Ping again?
> >
> > > >
> > > >
>

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-09 Thread Ni, Ray 
Ard,
I will give update on this by end of this week. sorry about the delay.

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ard Biesheuvel
> Sent: Thursday, January 9, 2020 8:54 PM
> To: Ni, Ray 
> Cc: devel@edk2.groups.io; Laszlo Ersek ; Leif Lindholm 
> ; Gao, Zhichao
> ; Ma, Maurice ; Dong, Guo 
> ; You, Benjamin
> 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles 
> on drivers loaded via Driver
> 
> On Mon, 23 Dec 2019 at 15:09, Ard Biesheuvel  
> wrote:
> >
> > On Thu, 12 Dec 2019 at 10:05, Ard Biesheuvel  
> > wrote:
> > >
> > > On Thu, 12 Dec 2019 at 09:59, Ni, Ray  wrote:
> > > >
> > > > Ard,
> > > > I still cannot understand it.
> > > >
> > > > Since Driver are processed (process = LoadImage + StartImage) after 
> > > > EndOfDxe, they are not deferred.
> > > > It means the Driver entrypoints are called directly.
> > > > Inside the entrypoints, driverbinding protocols are installed.
> > > >
> > > > After that, EfiBootManagerConnectAllDefaultConsoles () uses driver 
> > > > model logic to start the proper GOP driver.
> > > >
> > >
> > > It only does that if the GOP console is already in the
> > > ConIn/ConOut/ConErr variables, no?
> > >
> > > Today, we have code in the PlatformBdsLib to traverse the PCI
> > > hierarchy to populate those ConXXX variables if we encounter any PCI
> > > graphics cards, but this is done in
> > > PlatformBootManagerBeforeConsole(), so if the Driver is loaded
> > > *after* PlatformBootManagerBeforeConsole(), it will not be added to
> > > ConXXX. So we need to process Driver before calling
> > > PlatformBootManagerBeforeConsole(), so that the driver is dispatches
> > > right after we call DispatchDeferredImages() but before we do the
> > > traversal and populate the COnXXX variables.
> > >
> > > How else do you propose we could make PCI graphics controllers
> > > supported by Driver options appear in COnXXX before
> > > EfiBootManagerConnectAllDefaultConsoles() is called?
> > >
> >
> > Ping?
> >
> >
> 
> Ping again?
> 
> > >
> > >
> > > >
> > > > > -----Original Message-
> > > > > From: devel@edk2.groups.io  On Behalf Of Ard 
> > > > > Biesheuvel
> > > > > Sent: Wednesday, December 11, 2019 6:40 PM
> > > > > To: Ni, Ray 
> > > > > Cc: Laszlo Ersek ; edk2-devel-groups-io 
> > > > > ; Leif Lindholm
> > > > > ; Gao, Zhichao ; Ma, 
> > > > > Maurice ; Dong,
> Guo
> > > > > ; You, Benjamin 
> > > > > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow 
> > > > > consoles on drivers loaded via Driver
> > > > >
> > > > > On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel 
> > > > >  wrote:
> > > > > >
> > > > > > On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> > > > > > >
> > > > > > > > Exactly. This flow is identical to how option ROMs are 
> > > > > > > > processed if
> > > > > > > > they are discovered before EndOfDxe signalling completes (which 
> > > > > > > > is why
> > > > > > > > the Juno platform was broken without the call to
> > > > > > > > EfiBootManagerDispatchDeferredImages() in
> > > > > > > > PlatformBootManagerBeforeConsole())
> > > > > > > >
> > > > > > >
> > > > > > > Ard,
> > > > > > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > > > > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > > > > > >
> > > > > >
> > > > > > It does. We just added this in 
> > > > > > 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> > > > > >
> > > > > > > The deferred image dispatch mechanism assumes the platform
> > > > > > > needs to call the *DispatchDeferredImages() after signaling 
> > > > > > > EndOfDxe.
> > > > > > >
> > > > > >
> > > > > > Indeed.
> > > > > >
> > > > > > > I don't understand why the deferred image can be loaded with your 
> > > > > > > patch.
> > > > > > > They are still deferred because the loading time is before 
> > > > > > > EndOfDxe.
> > > > > > >
> > > > > >
> > > > > > Yes, but because PlatformBootManagerBeforeConsole () does all of 
> > > > > > this,
> > > > > > the only way to get Driver to work for consoles on GOP drivers, 
> > > > > > we
> > > > > > need to move it before that call.
> > > > >
> > > > > Any further comments on this patch?
> > > > >
> > > > >
> > > >
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53109): https://edk2.groups.io/g/devel/message/53109
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2020-01-09 Thread Ard Biesheuvel 
On Mon, 23 Dec 2019 at 15:09, Ard Biesheuvel  wrote:
>
> On Thu, 12 Dec 2019 at 10:05, Ard Biesheuvel  
> wrote:
> >
> > On Thu, 12 Dec 2019 at 09:59, Ni, Ray  wrote:
> > >
> > > Ard,
> > > I still cannot understand it.
> > >
> > > Since Driver are processed (process = LoadImage + StartImage) after 
> > > EndOfDxe, they are not deferred.
> > > It means the Driver entrypoints are called directly.
> > > Inside the entrypoints, driverbinding protocols are installed.
> > >
> > > After that, EfiBootManagerConnectAllDefaultConsoles () uses driver model 
> > > logic to start the proper GOP driver.
> > >
> >
> > It only does that if the GOP console is already in the
> > ConIn/ConOut/ConErr variables, no?
> >
> > Today, we have code in the PlatformBdsLib to traverse the PCI
> > hierarchy to populate those ConXXX variables if we encounter any PCI
> > graphics cards, but this is done in
> > PlatformBootManagerBeforeConsole(), so if the Driver is loaded
> > *after* PlatformBootManagerBeforeConsole(), it will not be added to
> > ConXXX. So we need to process Driver before calling
> > PlatformBootManagerBeforeConsole(), so that the driver is dispatches
> > right after we call DispatchDeferredImages() but before we do the
> > traversal and populate the COnXXX variables.
> >
> > How else do you propose we could make PCI graphics controllers
> > supported by Driver options appear in COnXXX before
> > EfiBootManagerConnectAllDefaultConsoles() is called?
> >
>
> Ping?
>
>

Ping again?

> >
> >
> > >
> > > > -Original Message-
> > > > From: devel@edk2.groups.io  On Behalf Of Ard 
> > > > Biesheuvel
> > > > Sent: Wednesday, December 11, 2019 6:40 PM
> > > > To: Ni, Ray 
> > > > Cc: Laszlo Ersek ; edk2-devel-groups-io 
> > > > ; Leif Lindholm
> > > > ; Gao, Zhichao ; Ma, 
> > > > Maurice ; Dong, Guo
> > > > ; You, Benjamin 
> > > > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow 
> > > > consoles on drivers loaded via Driver
> > > >
> > > > On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel  
> > > > wrote:
> > > > >
> > > > > On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> > > > > >
> > > > > > > Exactly. This flow is identical to how option ROMs are processed 
> > > > > > > if
> > > > > > > they are discovered before EndOfDxe signalling completes (which 
> > > > > > > is why
> > > > > > > the Juno platform was broken without the call to
> > > > > > > EfiBootManagerDispatchDeferredImages() in
> > > > > > > PlatformBootManagerBeforeConsole())
> > > > > > >
> > > > > >
> > > > > > Ard,
> > > > > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > > > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > > > > >
> > > > >
> > > > > It does. We just added this in 
> > > > > 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> > > > >
> > > > > > The deferred image dispatch mechanism assumes the platform
> > > > > > needs to call the *DispatchDeferredImages() after signaling 
> > > > > > EndOfDxe.
> > > > > >
> > > > >
> > > > > Indeed.
> > > > >
> > > > > > I don't understand why the deferred image can be loaded with your 
> > > > > > patch.
> > > > > > They are still deferred because the loading time is before EndOfDxe.
> > > > > >
> > > > >
> > > > > Yes, but because PlatformBootManagerBeforeConsole () does all of this,
> > > > > the only way to get Driver to work for consoles on GOP drivers, we
> > > > > need to move it before that call.
> > > >
> > > > Any further comments on this patch?
> > > >
> > > > 
> > >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53076): https://edk2.groups.io/g/devel/message/53076
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-23 Thread Ard Biesheuvel 
On Thu, 12 Dec 2019 at 10:05, Ard Biesheuvel  wrote:
>
> On Thu, 12 Dec 2019 at 09:59, Ni, Ray  wrote:
> >
> > Ard,
> > I still cannot understand it.
> >
> > Since Driver are processed (process = LoadImage + StartImage) after 
> > EndOfDxe, they are not deferred.
> > It means the Driver entrypoints are called directly.
> > Inside the entrypoints, driverbinding protocols are installed.
> >
> > After that, EfiBootManagerConnectAllDefaultConsoles () uses driver model 
> > logic to start the proper GOP driver.
> >
>
> It only does that if the GOP console is already in the
> ConIn/ConOut/ConErr variables, no?
>
> Today, we have code in the PlatformBdsLib to traverse the PCI
> hierarchy to populate those ConXXX variables if we encounter any PCI
> graphics cards, but this is done in
> PlatformBootManagerBeforeConsole(), so if the Driver is loaded
> *after* PlatformBootManagerBeforeConsole(), it will not be added to
> ConXXX. So we need to process Driver before calling
> PlatformBootManagerBeforeConsole(), so that the driver is dispatches
> right after we call DispatchDeferredImages() but before we do the
> traversal and populate the COnXXX variables.
>
> How else do you propose we could make PCI graphics controllers
> supported by Driver options appear in COnXXX before
> EfiBootManagerConnectAllDefaultConsoles() is called?
>

Ping?


>
>
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io  On Behalf Of Ard 
> > > Biesheuvel
> > > Sent: Wednesday, December 11, 2019 6:40 PM
> > > To: Ni, Ray 
> > > Cc: Laszlo Ersek ; edk2-devel-groups-io 
> > > ; Leif Lindholm
> > > ; Gao, Zhichao ; Ma, 
> > > Maurice ; Dong, Guo
> > > ; You, Benjamin 
> > > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow 
> > > consoles on drivers loaded via Driver
> > >
> > > On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel  
> > > wrote:
> > > >
> > > > On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> > > > >
> > > > > > Exactly. This flow is identical to how option ROMs are processed if
> > > > > > they are discovered before EndOfDxe signalling completes (which is 
> > > > > > why
> > > > > > the Juno platform was broken without the call to
> > > > > > EfiBootManagerDispatchDeferredImages() in
> > > > > > PlatformBootManagerBeforeConsole())
> > > > > >
> > > > >
> > > > > Ard,
> > > > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > > > >
> > > >
> > > > It does. We just added this in 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> > > >
> > > > > The deferred image dispatch mechanism assumes the platform
> > > > > needs to call the *DispatchDeferredImages() after signaling EndOfDxe.
> > > > >
> > > >
> > > > Indeed.
> > > >
> > > > > I don't understand why the deferred image can be loaded with your 
> > > > > patch.
> > > > > They are still deferred because the loading time is before EndOfDxe.
> > > > >
> > > >
> > > > Yes, but because PlatformBootManagerBeforeConsole () does all of this,
> > > > the only way to get Driver to work for consoles on GOP drivers, we
> > > > need to move it before that call.
> > >
> > > Any further comments on this patch?
> > >
> > > 
> >

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52507): https://edk2.groups.io/g/devel/message/52507
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-12 Thread Ard Biesheuvel 
On Thu, 12 Dec 2019 at 09:59, Ni, Ray  wrote:
>
> Ard,
> I still cannot understand it.
>
> Since Driver are processed (process = LoadImage + StartImage) after 
> EndOfDxe, they are not deferred.
> It means the Driver entrypoints are called directly.
> Inside the entrypoints, driverbinding protocols are installed.
>
> After that, EfiBootManagerConnectAllDefaultConsoles () uses driver model 
> logic to start the proper GOP driver.
>

It only does that if the GOP console is already in the
ConIn/ConOut/ConErr variables, no?

Today, we have code in the PlatformBdsLib to traverse the PCI
hierarchy to populate those ConXXX variables if we encounter any PCI
graphics cards, but this is done in
PlatformBootManagerBeforeConsole(), so if the Driver is loaded
*after* PlatformBootManagerBeforeConsole(), it will not be added to
ConXXX. So we need to process Driver before calling
PlatformBootManagerBeforeConsole(), so that the driver is dispatches
right after we call DispatchDeferredImages() but before we do the
traversal and populate the COnXXX variables.

How else do you propose we could make PCI graphics controllers
supported by Driver options appear in COnXXX before
EfiBootManagerConnectAllDefaultConsoles() is called?



>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Ard 
> > Biesheuvel
> > Sent: Wednesday, December 11, 2019 6:40 PM
> > To: Ni, Ray 
> > Cc: Laszlo Ersek ; edk2-devel-groups-io 
> > ; Leif Lindholm
> > ; Gao, Zhichao ; Ma, 
> > Maurice ; Dong, Guo
> > ; You, Benjamin 
> > Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow 
> > consoles on drivers loaded via Driver
> >
> > On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel  
> > wrote:
> > >
> > > On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> > > >
> > > > > Exactly. This flow is identical to how option ROMs are processed if
> > > > > they are discovered before EndOfDxe signalling completes (which is why
> > > > > the Juno platform was broken without the call to
> > > > > EfiBootManagerDispatchDeferredImages() in
> > > > > PlatformBootManagerBeforeConsole())
> > > > >
> > > >
> > > > Ard,
> > > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > > >
> > >
> > > It does. We just added this in 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> > >
> > > > The deferred image dispatch mechanism assumes the platform
> > > > needs to call the *DispatchDeferredImages() after signaling EndOfDxe.
> > > >
> > >
> > > Indeed.
> > >
> > > > I don't understand why the deferred image can be loaded with your patch.
> > > > They are still deferred because the loading time is before EndOfDxe.
> > > >
> > >
> > > Yes, but because PlatformBootManagerBeforeConsole () does all of this,
> > > the only way to get Driver to work for consoles on GOP drivers, we
> > > need to move it before that call.
> >
> > Any further comments on this patch?
> >
> > 
>

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52157): https://edk2.groups.io/g/devel/message/52157
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-12 Thread Ni, Ray 
Ard,
I still cannot understand it.

Since Driver are processed (process = LoadImage + StartImage) after 
EndOfDxe, they are not deferred.
It means the Driver entrypoints are called directly.
Inside the entrypoints, driverbinding protocols are installed.

After that, EfiBootManagerConnectAllDefaultConsoles () uses driver model logic 
to start the proper GOP driver.

Thanks,
Ray

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Ard Biesheuvel
> Sent: Wednesday, December 11, 2019 6:40 PM
> To: Ni, Ray 
> Cc: Laszlo Ersek ; edk2-devel-groups-io 
> ; Leif Lindholm
> ; Gao, Zhichao ; Ma, Maurice 
> ; Dong, Guo
> ; You, Benjamin 
> Subject: Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles 
> on drivers loaded via Driver
> 
> On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel  wrote:
> >
> > On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> > >
> > > > Exactly. This flow is identical to how option ROMs are processed if
> > > > they are discovered before EndOfDxe signalling completes (which is why
> > > > the Juno platform was broken without the call to
> > > > EfiBootManagerDispatchDeferredImages() in
> > > > PlatformBootManagerBeforeConsole())
> > > >
> > >
> > > Ard,
> > > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > > call *DispatchDeferredImages() after signaling EndOfDxe.
> > >
> >
> > It does. We just added this in 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
> >
> > > The deferred image dispatch mechanism assumes the platform
> > > needs to call the *DispatchDeferredImages() after signaling EndOfDxe.
> > >
> >
> > Indeed.
> >
> > > I don't understand why the deferred image can be loaded with your patch.
> > > They are still deferred because the loading time is before EndOfDxe.
> > >
> >
> > Yes, but because PlatformBootManagerBeforeConsole () does all of this,
> > the only way to get Driver to work for consoles on GOP drivers, we
> > need to move it before that call.
> 
> Any further comments on this patch?
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52156): https://edk2.groups.io/g/devel/message/52156
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-11 Thread Ard Biesheuvel 
On Mon, 9 Dec 2019 at 09:42, Ard Biesheuvel  wrote:
>
> On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
> >
> > > Exactly. This flow is identical to how option ROMs are processed if
> > > they are discovered before EndOfDxe signalling completes (which is why
> > > the Juno platform was broken without the call to
> > > EfiBootManagerDispatchDeferredImages() in
> > > PlatformBootManagerBeforeConsole())
> > >
> >
> > Ard,
> > I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> > call *DispatchDeferredImages() after signaling EndOfDxe.
> >
>
> It does. We just added this in 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069
>
> > The deferred image dispatch mechanism assumes the platform
> > needs to call the *DispatchDeferredImages() after signaling EndOfDxe.
> >
>
> Indeed.
>
> > I don't understand why the deferred image can be loaded with your patch.
> > They are still deferred because the loading time is before EndOfDxe.
> >
>
> Yes, but because PlatformBootManagerBeforeConsole () does all of this,
> the only way to get Driver to work for consoles on GOP drivers, we
> need to move it before that call.

Any further comments on this patch?

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52107): https://edk2.groups.io/g/devel/message/52107
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-09 Thread Ard Biesheuvel 
On Mon, 9 Dec 2019 at 03:12, Ni, Ray  wrote:
>
> > Exactly. This flow is identical to how option ROMs are processed if
> > they are discovered before EndOfDxe signalling completes (which is why
> > the Juno platform was broken without the call to
> > EfiBootManagerDispatchDeferredImages() in
> > PlatformBootManagerBeforeConsole())
> >
>
> Ard,
> I checked ArmPkg's PlatformBootManagerLib and found it doesn't
> call *DispatchDeferredImages() after signaling EndOfDxe.
>

It does. We just added this in 0f9395d7c5cc6ae2beaa2d87008fe158d04a8069

> The deferred image dispatch mechanism assumes the platform
> needs to call the *DispatchDeferredImages() after signaling EndOfDxe.
>

Indeed.

> I don't understand why the deferred image can be loaded with your patch.
> They are still deferred because the loading time is before EndOfDxe.
>

Yes, but because PlatformBootManagerBeforeConsole () does all of this,
the only way to get Driver to work for consoles on GOP drivers, we
need to move it before that call.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52038): https://edk2.groups.io/g/devel/message/52038
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-08 Thread Ni, Ray 
> Exactly. This flow is identical to how option ROMs are processed if
> they are discovered before EndOfDxe signalling completes (which is why
> the Juno platform was broken without the call to
> EfiBootManagerDispatchDeferredImages() in
> PlatformBootManagerBeforeConsole())
> 

Ard,
I checked ArmPkg's PlatformBootManagerLib and found it doesn't
call *DispatchDeferredImages() after signaling EndOfDxe.

The deferred image dispatch mechanism assumes the platform
needs to call the *DispatchDeferredImages() after signaling EndOfDxe.

I don't understand why the deferred image can be loaded with your patch.
They are still deferred because the loading time is before EndOfDxe.

Thanks,
Ray

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#52034): https://edk2.groups.io/g/devel/message/52034
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-06 Thread Laszlo Ersek 
On 12/06/19 15:31, Ard Biesheuvel wrote:
> Currently, we dispatch drivers specified via Driver entries after
> calling PlatformBootManagerBeforeConsole(), which prevents us from
> loading drivers that provide consoles via Driver entries.

I'd put this as:

  ... which prevents PlatformBootManagerBeforeConsole() from adding such
  consoles to ConIn, ConOut and StdErr that are produced by drivers
  loaded via Driver options.

> This is particularly annoying on AArch64 systems, given that it prevents
> us from loading AArch64 drivers from the ESP for PCIe graphics cards
> that shipped with x86 drivers in the option ROM, which is the common
> case today.
>
> So let's reorder the handling of the Driver entries with the
> invocation of PlatformBootManagerBeforeConsole(), which results in
> Driver entries being dispatched in the same way as option ROM
> images.

Ah, so you are saying the drivers will be added unconditionally to the
deferred list, with this patch (because, at the new ProcessLoadOptions()
call site, the platform cannot have signaled EndOfDxe yet).

Then in PlatformBootManagerBeforeConsole(), the platform can signal
EndOfDxe, call EfiBootManagerDispatchDeferredImages(), connect GOP
drivers, see what new handles / device paths are created, and *then*
update ConIn / ConOut / ConErr.

Can you point out where the deferral happens, inside
ProcessLoadOptions()?

(If there is no deferral, then we can't / shouldn't do this, as drivers
from the ESP should not be loaded before EndOfDxe, I think.)

Hmmm... it looks like:

BdsEntry()[MdeModulePkg/Universal/BdsDxe/BdsEntry.c]
  ProcessLoadOptions()[MdeModulePkg/Universal/BdsDxe/BdsEntry.c]
EfiBootManagerProcessLoadOption() 
[MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c]
  CoreLoadImage() [MdeModulePkg/Core/Dxe/Image/Image.c] -- 
via gBS->LoadImage()
CoreLoadImageCommon() [MdeModulePkg/Core/Dxe/Image/Image.c]
  Security2StubAuthenticate() 
[MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c] -- via 
gSecurity2->FileAuthentication()
Defer3rdPartyImageLoad()  
[MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c]
  // checks "mEndOfDxe"
  QueueImage()
[MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c]

OK.

I think this series makes sense.

I defer to Ray, but from my POV, it should be OK.

With the patch#2 commit message updated:

series
Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

>
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
> b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 1fb04dcbbcda..ad4c4c0406f6 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -850,6 +850,14 @@ BdsEntry (
>  }
>}
>
> +  //
> +  // Execute Driver Options. Images will be loaded but dispatch will be
> +  // deferred for 3rd party images until EndOfDxe is signalled.
> +  //
> +  LoadOptions = EfiBootManagerGetLoadOptions (, 
> LoadOptionTypeDriver);
> +  ProcessLoadOptions (LoadOptions, LoadOptionCount);
> +  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
> +
>//
>// Do the platform init, can be customized by OEM/IBV
>// Possible things that can be done in PlatformBootManagerBeforeConsole:
> @@ -868,13 +876,6 @@ BdsEntry (
>//
>EfiBootManagerStartHotkeyService ();
>
> -  //
> -  // Execute Driver Options
> -  //
> -  LoadOptions = EfiBootManagerGetLoadOptions (, 
> LoadOptionTypeDriver);
> -  ProcessLoadOptions (LoadOptions, LoadOptionCount);
> -  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
> -
>//
>// Connect consoles
>//
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51976): https://edk2.groups.io/g/devel/message/51976
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [RFC PATCH 2/2] MdeModulePkg/BdsDxe: allow consoles on drivers loaded via Driver####

2019-12-06 Thread Ard Biesheuvel 
Currently, we dispatch drivers specified via Driver entries after
calling PlatformBootManagerBeforeConsole(), which prevents us from
loading drivers that provide consoles via Driver entries.

This is particularly annoying on AArch64 systems, given that it prevents
us from loading AArch64 drivers from the ESP for PCIe graphics cards
that shipped with x86 drivers in the option ROM, which is the common
case today.

So let's reorder the handling of the Driver entries with the
invocation of PlatformBootManagerBeforeConsole(), which results in
Driver entries being dispatched in the same way as option ROM
images.

Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 1fb04dcbbcda..ad4c4c0406f6 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -850,6 +850,14 @@ BdsEntry (
 }
   }
 
+  //
+  // Execute Driver Options. Images will be loaded but dispatch will be
+  // deferred for 3rd party images until EndOfDxe is signalled.
+  //
+  LoadOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeDriver);
+  ProcessLoadOptions (LoadOptions, LoadOptionCount);
+  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
+
   //
   // Do the platform init, can be customized by OEM/IBV
   // Possible things that can be done in PlatformBootManagerBeforeConsole:
@@ -868,13 +876,6 @@ BdsEntry (
   //
   EfiBootManagerStartHotkeyService ();
 
-  //
-  // Execute Driver Options
-  //
-  LoadOptions = EfiBootManagerGetLoadOptions (, 
LoadOptionTypeDriver);
-  ProcessLoadOptions (LoadOptions, LoadOptionCount);
-  EfiBootManagerFreeLoadOptions (LoadOptions, LoadOptionCount);
-
   //
   // Connect consoles
   //
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51966): https://edk2.groups.io/g/devel/message/51966
Mute This Topic: https://groups.io/mt/67470372/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-