Date   

Re: Does EmulatorPkg support source-level debug?

Liming Gao
 

Yes. You can use VS do source level debug. Please see https://edk2.groups.io/g/devel/message/49972

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Yeh
Sent: Tuesday, December 17, 2019 4:25 PM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] Does EmulatorPkg support source-level debug?

HI,

I am wondering if EmulatorPkg supports source-level debug on VS2017 build.
Can I know if there is any documentation for that?

Thanks,
Yeh


Does EmulatorPkg support source-level debug?

Yeh
 

HI,

I am wondering if EmulatorPkg supports source-level debug on VS2017 build.
Can I know if there is any documentation for that?

Thanks,
Yeh


Re: Lock BootOrder variable

Wang, Sunny (HPS SW)
 

Thanks, guys.

Hi Samer,
Thanks for checking this and giving great information. I just knew that NIST 800-193 has some guidelines for locking down the UEFI boot order. The requirements and guidelines you added further proved the need of locking boot order.

Hi Sean,
Actually, I did check the Project Mu VariablePolicy protocol when you guys proposed this in the design meeting. This is definitely a good enhancement because I also ran into the problems mentioned in slides.
- https://edk2.groups.io/g/devel/files/Designs/2019/0516
However, It looks like we would still run into the OS installation failure with the current VariablePolicy protocol implementation. I didn't deeply look into the VariablePolicy protocol or give it a try, so I may overlook the solution in your implementation. Could you help point out where the code for solving OS installation failure is? The problem here is that OS doesn't gracefully deal with the locked UEFI variable, so we may not be able to return EFI_WRITE_PROTECTED to OS for the writes to BootOrder.


Hi All,
I think we all agree with the following points:
1. There is a need to lock BootOrder like the information brought by Samer and NIST 800-193 guidelines.
2. OS needs to gracefully deal with the locked BootOrder without causing failure during the installation and operations.
3. We will at least need to update the UEFI specification for asking OS to gracefully deal with the locked variable. If no one is currently working on this, I will bring it to USWG and work on it.

Regards,
Sunny Wang

-----Original Message-----
From: tim.lewis@insyde.com [mailto:tim.lewis@insyde.com]
Sent: Friday, December 13, 2019 10:26 AM
To: discuss@edk2.groups.io; sean.brogan@microsoft.com; phlamorim@riseup.net; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Subject: RE: [edk2-discuss] Lock BootOrder variable

Sean --

Since you already have published code and it is already publicly available, then I suggest that you bring it to USWG now, rather than later.

Thanks,

Tim

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Sean via Groups.Io
Sent: Thursday, December 12, 2019 5:31 PM
To: discuss@edk2.groups.io; phlamorim@riseup.net; sunnywang@hpe.com
Subject: Re: [edk2-discuss] Lock BootOrder variable

Sunny,

There are two other public, non-uefi spec solutions I am aware of.

1. Edk2 VariableLock protocol: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Protocol/VariableLock.h
A relatively limited solution with hard coded lock points tied to edk2 SMM variable store.

2. Project Mu VariablePolicy protocol: https://github.com/microsoft/mu_basecore/blob/release/201911/MdeModulePkg/Include/Protocol/VariablePolicy.h
Flexible policy based locking that can be implemented in various hardware architectures.

My team will be proposing the VariablePolicy protocol (potentially as a "code-first" effort) in the coming months and working to upstream this feature into edk2. The reality is some users and use cases want higher assurance for their platform settings and this can include the boot order. Doing this thru a well-defined and auditable protocol is better than an ad-hoc solutions. As you know locking some variables may break assumptions (or spec definition) that other code may have but that tradeoff is best evaluated by the use case.

Thanks
Sean


-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Paulo Henrique Lacerda de Amorim via Groups.Io
Sent: Thursday, December 12, 2019 12:53 PM
To: discuss@edk2.groups.io; sunnywang@hpe.com
Subject: [EXTERNAL] Re: [edk2-discuss] Lock BootOrder variable

The UEFI define the BootOrder variable with NV+BS+RT attributes, so its not possible to lock this variable, you can try to delete the BootOrder variable and then set the variable with AT attribute, which will probably will result in an undefined behavior. OVMF just recreates the the BootOrder with NV+BS+RT again, then any code which can call Runtime Services will be able to change BootOrder again.

A possible method is too use a signed loader which have your own 'BootOrder' hardcoded.

-----Original Message-----
From: Samer El-Haj-Mahmoud [mailto:Samer.El-Haj-Mahmoud@arm.com]
Sent: Friday, December 13, 2019 12:30 AM
To: discuss@edk2.groups.io; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wiginton, Scott <scott.wiginton@hpe.com>; Bodner, James <james.bodner@hpe.com>; Haskell, Darrell <darrell.haskell@hpe.com>
Subject: RE: [edk2-discuss] Lock BootOrder variable

Sunny,

Looks like this is a Windows Hardware Compatibility Specification requirement: https://go.microsoft.com/fwlink/?linkid=2086856 , in Systems.pdf, under System.Fundamentals.Security.DGCG.DeviceGuard - Firmware BIOS lockdown

I am aware of some proprietary implementations of "Lock Boot Order" as a firmware/UEFI setting, but not a standard method defined in the spec.

Also, NSA has some guidelines on locking down UEFI boot order using whatever firmware settings to disable any undesired boot sources (such as externally available USB or network ports): https://www.nsa.gov/Portals/70/documents/what-we-do/cybersecurity/professional-resources/csi-uefi-lockdown.pdf .

Thanks,

Em 12/12/2019 05:49, Wang, Sunny (HPS SW) escreveu:
Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang




Re: Lock BootOrder variable

Tim Lewis
 

Sean --

Since you already have published code and it is already publicly available, then I suggest that you bring it to USWG now, rather than later.

Thanks,

Tim

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Sean via Groups.Io
Sent: Thursday, December 12, 2019 5:31 PM
To: discuss@edk2.groups.io; phlamorim@riseup.net; sunnywang@hpe.com
Subject: Re: [edk2-discuss] Lock BootOrder variable

Sunny,

There are two other public, non-uefi spec solutions I am aware of.

1. Edk2 VariableLock protocol: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Protocol/VariableLock.h
A relatively limited solution with hard coded lock points tied to edk2 SMM variable store.

2. Project Mu VariablePolicy protocol: https://github.com/microsoft/mu_basecore/blob/release/201911/MdeModulePkg/Include/Protocol/VariablePolicy.h
Flexible policy based locking that can be implemented in various hardware architectures.

My team will be proposing the VariablePolicy protocol (potentially as a "code-first" effort) in the coming months and working to upstream this feature into edk2. The reality is some users and use cases want higher assurance for their platform settings and this can include the boot order. Doing this thru a well-defined and auditable protocol is better than an ad-hoc solutions. As you know locking some variables may break assumptions (or spec definition) that other code may have but that tradeoff is best evaluated by the use case.

Thanks
Sean


-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Paulo Henrique Lacerda de Amorim via Groups.Io
Sent: Thursday, December 12, 2019 12:53 PM
To: discuss@edk2.groups.io; sunnywang@hpe.com
Subject: [EXTERNAL] Re: [edk2-discuss] Lock BootOrder variable

The UEFI define the BootOrder variable with NV+BS+RT attributes, so its not possible to lock this variable, you can try to delete the BootOrder variable and then set the variable with AT attribute, which will probably will result in an undefined behavior. OVMF just recreates the the BootOrder with NV+BS+RT again, then any code which can call Runtime Services will be able to change BootOrder again.

A possible method is too use a signed loader which have your own 'BootOrder' hardcoded.

Em 12/12/2019 05:49, Wang, Sunny (HPS SW) escreveu:
Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang




Re: Lock BootOrder variable

Sean
 

Sunny,

There are two other public, non-uefi spec solutions I am aware of.

1. Edk2 VariableLock protocol: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Protocol/VariableLock.h
A relatively limited solution with hard coded lock points tied to edk2 SMM variable store.

2. Project Mu VariablePolicy protocol: https://github.com/microsoft/mu_basecore/blob/release/201911/MdeModulePkg/Include/Protocol/VariablePolicy.h
Flexible policy based locking that can be implemented in various hardware architectures.

My team will be proposing the VariablePolicy protocol (potentially as a "code-first" effort) in the coming months and working to upstream this feature into edk2. The reality is some users and use cases want higher assurance for their platform settings and this can include the boot order. Doing this thru a well-defined and auditable protocol is better than an ad-hoc solutions. As you know locking some variables may break assumptions (or spec definition) that other code may have but that tradeoff is best evaluated by the use case.

Thanks
Sean

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Paulo Henrique Lacerda de Amorim via Groups.Io
Sent: Thursday, December 12, 2019 12:53 PM
To: discuss@edk2.groups.io; sunnywang@hpe.com
Subject: [EXTERNAL] Re: [edk2-discuss] Lock BootOrder variable

The UEFI define the BootOrder variable with NV+BS+RT attributes, so its not possible to lock this variable, you can try to delete the BootOrder variable and then set the variable with AT attribute, which will probably will result in an undefined behavior. OVMF just recreates the the BootOrder with NV+BS+RT again, then any code which can call Runtime Services will be able to change BootOrder again.

A possible method is too use a signed loader which have your own 'BootOrder' hardcoded.

Em 12/12/2019 05:49, Wang, Sunny (HPS SW) escreveu:
Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang




Re: Lock BootOrder variable

Paulo Henrique Lacerda de Amorim
 

The UEFI define the BootOrder variable with NV+BS+RT attributes, so its
not possible to lock this variable, you can try to delete the BootOrder
variable and then set the variable with AT attribute, which will
probably will result in an undefined behavior. OVMF just recreates the
the BootOrder with NV+BS+RT again, then any code which can call Runtime
Services will be able to change BootOrder again.

A possible method is too use a signed loader which have your own
'BootOrder' hardcoded.

Em 12/12/2019 05:49, Wang, Sunny (HPS SW) escreveu:

Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang




Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Jeff Brasen
 

Thanks for the summary Ray, for the problem summary only thing I would add would be that platform also wants to create/modify boot options when full enumeration is requested as well.

For solutions I prefer option 2 as we don't have to put the same logic everywhere of how to modify the default enumerated list. And if we do that 2b makes more sense as then we don't have to modify all of the existing platforms.

I see two things the platform would need to do.

1. Update list created in BmEnumerateBootOptions
2. Delete any no longer valid platform created boot options


Thanks,

Jeff

________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, December 11, 2019 7:00 AM
To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>; afish@apple.com <afish@apple.com>; discuss@edk2.groups.io <discuss@edk2.groups.io>
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

External email: Use caution opening links or attachments


Jeff,

Tom from AMD booked the meeting for SEV discussion months ago. I am afraid there is no time for this discussion.

Let’s try to resolve it in mails.



Firstly, let me rephase the problem and your proposed solutions here (subjective + verb + objective). Sunny’s input is also included. Hope Mike K and others can provide inputs.

Personally, I agree with 2.b. It helps us to gradually migrate the PlatformBootManagerLib to PlatformBootManager protocol. Protocol with Revision field helps to reduce the impact to old platforms with new APIs added.



**Problem:

Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don’t cause Boot#### created. It’s a need of platform customization.



**Details:

Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:

1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering “Boot Manager” page and “Boot Maintenance Manager” page.



Platform can change Platform Boot Manager to remove the unneeded Boot#### in case #1.

But platform has no way to remove the Boot#### created in case #2 .



**Potential solutions:

1. Update UiApp
* Define a new PCD and a new event group.

If PCD is TRUE, UiApp signals the event. Event callback creates the Boot####. Otherwise, EfiBootManagerRefreshAllBootOptions() is called.

* Add a new PlatformBootManagerLib API (implemented by platform).

UiApp calls the new API instead of EfiBootManagerRefreshAllBootOption. (need to coordinate rollout with updates to all platforms).

* Add a new protocol (implemented by platform).

UiApp calls the new protocol if it exists otherwise calls EfiBootManagerRefreshAllBootOption.



1. Update EfiBootManagerRefreshAllBootOptions()
* Add a new library class (implemented by platform).

EfiBootManagerRefreshAllBootOption() calls the new library class.

* Add a new PlatformBootManager protocol (implemented by platform).

EfiBootManagerRefreshAllBootOption() calls the new protocol if it exists.



Thanks,

Ray



From: Jeff Brasen <jbrasen@nvidia.com>
Sent: Wednesday, December 11, 2019 4:46 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; discuss@edk2.groups.io
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Can we discuss this at the design meeting this week (12/12)?



Thanks,

Jeff

________________________________

From: Jeff Brasen
Sent: Thursday, November 14, 2019 10:04 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Yes, I think that would be good.



Summarizing everything in this thread



Problem: Platform needs to customize the boot options, this can be done for normal boot but the UiApp calls EfiBootManagerRefreshAllBootOption in a couple places.



Potential solutions:

1 – Define new PCD and event group if PCD is set true then signal event instead of calling EfiBootManagerRefreshAllBootOption in UiApp

2 – Add new function to boot manager library and replace call to EfiBootManagerRefreshAllBootOption in UiApp (need to coordinate rollout with updates to all platform.

3 – Add new protocol with new function, if supported call this otherwise call EfiBootManagerRefreshAllBootOption as is done now

4 – For 2/3 use generic function so we don’t need new APIs for future expansion

5 – Update EfiBootManagerRefreshAllBootOption to call platform specific function.



Thanks,
Jeff





From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Wednesday, November 13, 2019 7:09 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

I think it’s a good topic that we could discuss in the open design meeting.

Are you ok to present the problem you have and discuss the potential solutions in that meeting?

https://github.com/tianocore/tianocore.github.io/wiki/Design-Meeting



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Thursday, November 14, 2019 2:43 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Thinking about this more I think we could do this with a PCD and a new group event without having to define any new function interfaces.



We could change UiApp and BootManagerMenuApp (and any others that are relevant) from



EfiBootManagerRefreshAllBootOption ();



to



if (FeaturePcdGet (PcdEventBasedRefreshAllBootOptionSupport) {

EFI_EVENT Event;

gBS->CreateEventEx ( 0, 0, NULL, NULL, &gEventBasedRefreshGuid, &Event );

gBS->SignalEvent (Event);

gBS->CloseEvent (Event);

} else {

EfiBootManagerRefreshAllBootOption ();

}



Then a platform that wants to do this on its own would just set this pcd and create a group event and do what it needs to do there.



Thanks,

Jeff

________________________________

From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: Monday, November 11, 2019 5:00 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



I am not sure a PCD would work (unless I am missing something) We do want to do a connect all and re-enumerate in UiApp but we need the platform code to be involved in that process.



Thanks,

Jeff

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Monday, November 11, 2019 4:58 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

If adding a PCD to control UiApp can meet the real needs, I prefer to do in that way instead of adding new APIs to PlatformBootManagerLib.



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Tuesday, November 12, 2019 6:58 AM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



If we are concerned about deploying this and breaking builds we could do this via a new protocol instead. In that case though we would leave the old default behavior in the code to handle platforms that didn't implement the new protocol, so this might not be the cleanest way to deploy this.



We could also look at adding a generic platform boot hook function (either as a library function or protocol) if we wanted to limit the number of disruption on new customization hooks. Something like



EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextData OPTIONAL)



Where Notification type describes where we are that we want platform to potentially handle and ContextData is per type caller allocated data that provides additional in/out data. This has the same issue of leaving the current default behavior in place for unsupported types as well as being a less than specific function to describe.



Thanks,

Jeff



________________________________

From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



On 11/07/19 18:46, Jeff Brasen wrote:
Fixing UiApp seems reasonable, I do think we would want a hook to the platform library in here as the enumeration that occurs in the UiApp is intended to do a full enumeration of the system and there may be platform specifics to how that occurs.
Fully agreed -- entering UiApp should expose everything bootable in the
system, unless (perhaps) PlatformBootManagerLib specifically thinks
otherwise.

Of course, then we arrive (again) at the problem that a call in
UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break
tens of out-of-tree platforms. :)

I think that can be prevented, as follows; but it will take quite some time:

- introduce the new function declaration in "PlatformBootManagerLib.h",
- modify all platforms (in tree and out of tree) to implement (define)
the new function,
- call the new function from UefiBootManagerLib

For some history / background on this kind of problem, I suggest reading
through:

https://bugzilla.tianocore.org/show_bug.cgi?id=982

Thanks,
Laszlo

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, November 7, 2019 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

I treat the issue in this way:

1. Platform Boot Manager library does a good job. It doesn't always call RefreshAll() API to auto-create the boot options
2. But UiApp doesn't. It constantly call RefreshAll().

Do you think that we can fix UiApp instead? For example, introducing a PCD to control the boot option refresh behavior?

Thanks,
Ray

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>> On Behalf Of Jeff Brasen
Sent: Thursday, November 7, 2019 3:02 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; afish@apple.com<mailto:afish@apple.com>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

The issue is there are some auto created options we do not want on our platform.
Get Outlook for Android<https://aka.ms/ghei36>

________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Sent: Wednesday, November 6, 2019 11:59:31 PM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Jeff,

RefreshAllBootOption() only modifies/creates the auto-created boot options. For the boot options created by platform boot manager library, they stay with no one touches. And all auto-created boot options are appended in the end of boot option list (through BootOrder).



From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>
Sent: Thursday, November 7, 2019 12:13 PM
To: afish@apple.com<mailto:afish@apple.com>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



As the suggestions below made sense, we updated our platform boot manager library to behave in this manner and for normal boots everything works well. However the UiApp and boot maintenance applications in EDK2 also call EfiBootManagerRefreshAllBootOption() when ever a user goes into the menu which will re-create the skipped boot options with no place for the platform code to intervene.



What about a solution where we add a new Platform library function that allows for override of the behavior of BmEnumerateBootOptions? For example, either a function or protocol that takes the same parameters as this function and only if it returns NULL then we continue to the default enumeration code. Or a function call inserted at the end that would modify the load option array after the system does the standard enumeration.



-Jeff



From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 9:20 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Mike Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Ray,



Is there an obvious hook point we could point Jeff and Ashish at?



Long term it would be a good idea to have a Wiki page to give some guidance on how to customize the BDS.



Thanks,



Andrew Fish



On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>> wrote:



Andrew,

I agree with your opinion.

It's expected that Platform Boot Manager lib calls EfiBootManagerRefreshAllBootOption() only in full configuration boot path.

The full configuration boot path is chosen when hardware changes happen. So it's not expected EfiBootManagerRefresh...() be
called in every boot.

So you could:

1. Delete the auto-created option pointing to LoadFile instance
2. Create your own one with customized description.





From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 10:47 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration







On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>> wrote:



Wouldn't having a variable that we create and delete on every boot put unnecessary stress on the SPI-NOR that the variable store lives on?
What about the alternative approach where we allow the platform code to modify the attributes of the auto created variable to disable it with hidden/!active but still match for detection purposes so that it doesn't delete and recreate the modified variable each boot? That way all the logic on what to disable can still be in the platform code and all the existing logic in the boot manager can stay basically the same?

What changes every boot that forces the variable to need to get modified?

I would assume the NOR driver is smart enough to not update a variable that is not changing.

The custom BDS could could only create the variable for this device if it does not exist.

[JB] The current flow with no changes in the boot manager would be as follows



1. Scan for instance of the boot option in the variables
2. It will not be found, so create a new boot option store it to a variable and update BootOrder
3. Platform code runs creates the options for the boot option it wants and writes those to variable store
4. Delete/disable the boot option in the variable store



When you reboot it won't find the variable so 1/2/4 will re-occur



The code that does this (1/2) is EfiBootManagerRefreshAllBootOption in BmBoot.c



If you modify the variable to disable it with hidden/not active it would delete that and create a new one as well as the code wouldn't recognize that is the same boot option.



If however we modify EfiBootManagerFindLoadOption to not compare the attributes (at least allow for differences in active and hidden) then the when it refreshes every thing it would see the match and not delete/create a new variable in the store and thus we wouldn't have changes every boot.





Jeff,



Sorry if I'm a little off on the sequence of things as the platform I work on day to day has a custom BDS and does not use this library..... I though the patch changed BmEnumerateBootOptions(), so that is going to change how EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch as given is invalid as it changed the behavior of the public library API for EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it would need to change the comments to reflect the new behavior. This is kind of what Laszlo's technical debt comment was about.



I think Laszlo advocated having the BDS platform specific code make sure the boot variables are in the correct state. That should happen before the Boot Manager code runs, and it is not clear to me why the Boot Manager could would need to run if you have a valid EFI nvram variable to boot from.



I think the question is how is your use case different than the boot variable that Windows installs? If it works kind of the same way then the answer is to have the BDS platform specific code write the boot variable.





[1]

/**

The function creates boot options for all possible bootable medias in the following order:

1. Removable BlockIo - The boot option only points to the removable media

device, like USB key, DVD, Floppy etc.

2. Fixed BlockIo - The boot option only points to a Fixed blockIo device,

like HardDisk.

3. Non-BlockIo SimpleFileSystem - The boot option points to a device supporting

SimpleFileSystem Protocol, but not supporting BlockIo

protocol.

4. LoadFile - The boot option points to the media supporting

LoadFile protocol.

Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior



The function won't delete the boot option not added by itself.

**/

VOID

EFIAPI

EfiBootManagerRefreshAllBootOption (

VOID

);



Thanks,



Andrew Fish



Thanks,

Andrew Fish

Thanks,

Jeff

________________________________

This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.

________________________________


Re: Lock BootOrder variable

Samer El-Haj-Mahmoud
 

Sunny,

Looks like this is a Windows Hardware Compatibility Specification requirement: https://go.microsoft.com/fwlink/?linkid=2086856 , in Systems.pdf, under System.Fundamentals.Security.DGCG.DeviceGuard - Firmware BIOS lockdown

I am aware of some proprietary implementations of "Lock Boot Order" as a firmware/UEFI setting, but not a standard method defined in the spec.

Also, NSA has some guidelines on locking down UEFI boot order using whatever firmware settings to disable any undesired boot sources (such as externally available USB or network ports): https://www.nsa.gov/Portals/70/documents/what-we-do/cybersecurity/professional-resources/csi-uefi-lockdown.pdf .

Thanks,
--Samer

-----Original Message-----
From: discuss@edk2.groups.io <discuss@edk2.groups.io> On Behalf Of Wang, Sunny (HPS SW) via Groups.Io
Sent: Thursday, December 12, 2019 3:50 AM
To: discuss@edk2.groups.io
Cc: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; Spottswood, Jason <jason.spottswood@hpe.com>; Wiginton, Scott <scott.wiginton@hpe.com>; Bodner, James <james.bodner@hpe.com>; Haskell, Darrell <darrell.haskell@hpe.com>
Subject: [edk2-discuss] Lock BootOrder variable

Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang




IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Lock BootOrder variable

Wang, Sunny (HPS SW)
 

Hi All,

Is there any spec'd way that we can use to lock some UEFI variables like BootOrder without breaking OS installation and OS functionalities?

For some security reasons and customer use cases, we need to let system firmware completely own some UEFI variables like BootOrder. In other words, we don't want some UEFI variables to be controlled by the OS using the UEFI runtime service SetVariable. In addition, we tried to lock the BootOrder variable, but it would break OS installation or some OS functionalities.

By the way, we will bring this need to USWG if there is no existing spec'd way for satisfying this need.

Regards,
Sunny Wang


Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Ni, Ray
 

Jeff,

Tom from AMD booked the meeting for SEV discussion months ago. I am afraid there is no time for this discussion.

Let's try to resolve it in mails.



Firstly, let me rephase the problem and your proposed solutions here (subjective + verb + objective). Sunny's input is also included. Hope Mike K and others can provide inputs.

Personally, I agree with 2.b. It helps us to gradually migrate the PlatformBootManagerLib to PlatformBootManager protocol. Protocol with Revision field helps to reduce the impact to old platforms with new APIs added.



**Problem:

Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need of platform customization.



**Details:

Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:

1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.



Platform can change Platform Boot Manager to remove the unneeded Boot#### in case #1.

But platform has no way to remove the Boot#### created in case #2 .



**Potential solutions:

1. Update UiApp
* Define a new PCD and a new event group.

If PCD is TRUE, UiApp signals the event. Event callback creates the Boot####. Otherwise, EfiBootManagerRefreshAllBootOptions() is called.

* Add a new PlatformBootManagerLib API (implemented by platform).

UiApp calls the new API instead of EfiBootManagerRefreshAllBootOption. (need to coordinate rollout with updates to all platforms).

* Add a new protocol (implemented by platform).

UiApp calls the new protocol if it exists otherwise calls EfiBootManagerRefreshAllBootOption.



1. Update EfiBootManagerRefreshAllBootOptions()
* Add a new library class (implemented by platform).

EfiBootManagerRefreshAllBootOption() calls the new library class.

* Add a new PlatformBootManager protocol (implemented by platform).

EfiBootManagerRefreshAllBootOption() calls the new protocol if it exists.

Thanks,
Ray

From: Jeff Brasen <jbrasen@nvidia.com>
Sent: Wednesday, December 11, 2019 4:46 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; discuss@edk2.groups.io
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Can we discuss this at the design meeting this week (12/12)?


Thanks,

Jeff

________________________________
From: Jeff Brasen
Sent: Thursday, November 14, 2019 10:04 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Yes, I think that would be good.



Summarizing everything in this thread



Problem: Platform needs to customize the boot options, this can be done for normal boot but the UiApp calls EfiBootManagerRefreshAllBootOption in a couple places.



Potential solutions:

1 - Define new PCD and event group if PCD is set true then signal event instead of calling EfiBootManagerRefreshAllBootOption in UiApp

2 - Add new function to boot manager library and replace call to EfiBootManagerRefreshAllBootOption in UiApp (need to coordinate rollout with updates to all platform.

3 - Add new protocol with new function, if supported call this otherwise call EfiBootManagerRefreshAllBootOption as is done now

4 - For 2/3 use generic function so we don't need new APIs for future expansion

5 - Update EfiBootManagerRefreshAllBootOption to call platform specific function.



Thanks,
Jeff





From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Wednesday, November 13, 2019 7:09 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

I think it's a good topic that we could discuss in the open design meeting.

Are you ok to present the problem you have and discuss the potential solutions in that meeting?

https://github.com/tianocore/tianocore.github.io/wiki/Design-Meeting



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Thursday, November 14, 2019 2:43 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Thinking about this more I think we could do this with a PCD and a new group event without having to define any new function interfaces.



We could change UiApp and BootManagerMenuApp (and any others that are relevant) from



EfiBootManagerRefreshAllBootOption ();



to



if (FeaturePcdGet (PcdEventBasedRefreshAllBootOptionSupport) {

EFI_EVENT Event;

gBS->CreateEventEx ( 0, 0, NULL, NULL, &gEventBasedRefreshGuid, &Event );

gBS->SignalEvent (Event);

gBS->CloseEvent (Event);

} else {

EfiBootManagerRefreshAllBootOption ();

}



Then a platform that wants to do this on its own would just set this pcd and create a group event and do what it needs to do there.



Thanks,

Jeff

________________________________

From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: Monday, November 11, 2019 5:00 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



I am not sure a PCD would work (unless I am missing something) We do want to do a connect all and re-enumerate in UiApp but we need the platform code to be involved in that process.



Thanks,

Jeff

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Monday, November 11, 2019 4:58 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

If adding a PCD to control UiApp can meet the real needs, I prefer to do in that way instead of adding new APIs to PlatformBootManagerLib.



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Tuesday, November 12, 2019 6:58 AM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



If we are concerned about deploying this and breaking builds we could do this via a new protocol instead. In that case though we would leave the old default behavior in the code to handle platforms that didn't implement the new protocol, so this might not be the cleanest way to deploy this.



We could also look at adding a generic platform boot hook function (either as a library function or protocol) if we wanted to limit the number of disruption on new customization hooks. Something like



EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextData OPTIONAL)



Where Notification type describes where we are that we want platform to potentially handle and ContextData is per type caller allocated data that provides additional in/out data. This has the same issue of leaving the current default behavior in place for unsupported types as well as being a less than specific function to describe.



Thanks,

Jeff



________________________________

From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



On 11/07/19 18:46, Jeff Brasen wrote:
Fixing UiApp seems reasonable, I do think we would want a hook to the platform library in here as the enumeration that occurs in the UiApp is intended to do a full enumeration of the system and there may be platform specifics to how that occurs.
Fully agreed -- entering UiApp should expose everything bootable in the
system, unless (perhaps) PlatformBootManagerLib specifically thinks
otherwise.

Of course, then we arrive (again) at the problem that a call in
UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break
tens of out-of-tree platforms. :)

I think that can be prevented, as follows; but it will take quite some time:

- introduce the new function declaration in "PlatformBootManagerLib.h",
- modify all platforms (in tree and out of tree) to implement (define)
the new function,
- call the new function from UefiBootManagerLib

For some history / background on this kind of problem, I suggest reading
through:

https://bugzilla.tianocore.org/show_bug.cgi?id=982

Thanks,
Laszlo

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, November 7, 2019 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

I treat the issue in this way:

1. Platform Boot Manager library does a good job. It doesn't always call RefreshAll() API to auto-create the boot options
2. But UiApp doesn't. It constantly call RefreshAll().

Do you think that we can fix UiApp instead? For example, introducing a PCD to control the boot option refresh behavior?

Thanks,
Ray

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>> On Behalf Of Jeff Brasen
Sent: Thursday, November 7, 2019 3:02 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; afish@apple.com<mailto:afish@apple.com>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

The issue is there are some auto created options we do not want on our platform.
Get Outlook for Android<https://aka.ms/ghei36>

________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Sent: Wednesday, November 6, 2019 11:59:31 PM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Jeff,

RefreshAllBootOption() only modifies/creates the auto-created boot options. For the boot options created by platform boot manager library, they stay with no one touches. And all auto-created boot options are appended in the end of boot option list (through BootOrder).



From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>
Sent: Thursday, November 7, 2019 12:13 PM
To: afish@apple.com<mailto:afish@apple.com>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



As the suggestions below made sense, we updated our platform boot manager library to behave in this manner and for normal boots everything works well. However the UiApp and boot maintenance applications in EDK2 also call EfiBootManagerRefreshAllBootOption() when ever a user goes into the menu which will re-create the skipped boot options with no place for the platform code to intervene.



What about a solution where we add a new Platform library function that allows for override of the behavior of BmEnumerateBootOptions? For example, either a function or protocol that takes the same parameters as this function and only if it returns NULL then we continue to the default enumeration code. Or a function call inserted at the end that would modify the load option array after the system does the standard enumeration.



-Jeff



From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 9:20 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Mike Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Ray,



Is there an obvious hook point we could point Jeff and Ashish at?



Long term it would be a good idea to have a Wiki page to give some guidance on how to customize the BDS.



Thanks,



Andrew Fish



On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>> wrote:



Andrew,

I agree with your opinion.

It's expected that Platform Boot Manager lib calls EfiBootManagerRefreshAllBootOption() only in full configuration boot path.

The full configuration boot path is chosen when hardware changes happen. So it's not expected EfiBootManagerRefresh...() be
called in every boot.

So you could:

1. Delete the auto-created option pointing to LoadFile instance
2. Create your own one with customized description.





From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 10:47 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration







On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>> wrote:



Wouldn't having a variable that we create and delete on every boot put unnecessary stress on the SPI-NOR that the variable store lives on?
What about the alternative approach where we allow the platform code to modify the attributes of the auto created variable to disable it with hidden/!active but still match for detection purposes so that it doesn't delete and recreate the modified variable each boot? That way all the logic on what to disable can still be in the platform code and all the existing logic in the boot manager can stay basically the same?

What changes every boot that forces the variable to need to get modified?

I would assume the NOR driver is smart enough to not update a variable that is not changing.

The custom BDS could could only create the variable for this device if it does not exist.

[JB] The current flow with no changes in the boot manager would be as follows



1. Scan for instance of the boot option in the variables
2. It will not be found, so create a new boot option store it to a variable and update BootOrder
3. Platform code runs creates the options for the boot option it wants and writes those to variable store
4. Delete/disable the boot option in the variable store



When you reboot it won't find the variable so 1/2/4 will re-occur



The code that does this (1/2) is EfiBootManagerRefreshAllBootOption in BmBoot.c



If you modify the variable to disable it with hidden/not active it would delete that and create a new one as well as the code wouldn't recognize that is the same boot option.



If however we modify EfiBootManagerFindLoadOption to not compare the attributes (at least allow for differences in active and hidden) then the when it refreshes every thing it would see the match and not delete/create a new variable in the store and thus we wouldn't have changes every boot.





Jeff,



Sorry if I'm a little off on the sequence of things as the platform I work on day to day has a custom BDS and does not use this library..... I though the patch changed BmEnumerateBootOptions(), so that is going to change how EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch as given is invalid as it changed the behavior of the public library API for EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it would need to change the comments to reflect the new behavior. This is kind of what Laszlo's technical debt comment was about.



I think Laszlo advocated having the BDS platform specific code make sure the boot variables are in the correct state. That should happen before the Boot Manager code runs, and it is not clear to me why the Boot Manager could would need to run if you have a valid EFI nvram variable to boot from.



I think the question is how is your use case different than the boot variable that Windows installs? If it works kind of the same way then the answer is to have the BDS platform specific code write the boot variable.





[1]

/**

The function creates boot options for all possible bootable medias in the following order:

1. Removable BlockIo - The boot option only points to the removable media

device, like USB key, DVD, Floppy etc.

2. Fixed BlockIo - The boot option only points to a Fixed blockIo device,

like HardDisk.

3. Non-BlockIo SimpleFileSystem - The boot option points to a device supporting

SimpleFileSystem Protocol, but not supporting BlockIo

protocol.

4. LoadFile - The boot option points to the media supporting

LoadFile protocol.

Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior



The function won't delete the boot option not added by itself.

**/

VOID

EFIAPI

EfiBootManagerRefreshAllBootOption (

VOID

);



Thanks,



Andrew Fish



Thanks,

Andrew Fish

Thanks,

Jeff

________________________________

This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.

________________________________


Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Wang, Sunny (HPS SW)
 

Hi all,

Since I can't attend the design meeting (EMEA), I would like to add some use cases and a suggestion for your guys' reference before this topic being discussed:

We had similar needs (use cases) as Ashish like the following, so making this code change would be definitely good for system vendors like us.
- Disabling a specific boot option like a PXE boot option for a specific NIC port.
- Hiding a partition that is used for special purpose rather than booting.

If we don't have a strong reason to prevent creating a platform hook function (solution 5), I will prefer to use it because it is the simplest solution and can be easily extended and maintained. Also, solution 5 is our current implementation. We can even contribute our implementation to EDK2 if you guys need it. Moreover, if we finally choose solution 5, I will prefer to create a new platform library instead of adding a new function into the PlatformBootManagerLib, and let this new platform library only be called by UefiBootManagerLib. This would avoid a potential circular dependency problem.

For solutions 1, 2, 3, and 4, if I understand them correctly, they all require making changes in the EfiBootManagerRefreshAllBootOption callers. UiApp may not be the only application to call EfiBootManagerRefreshAllBootOption, so this may cause additional maintenance effort. However, if we still need to choose a solution other than solution 5, can we make the change directly in EfiBootManagerRefreshAllBootOption instead of UiApp?

By the way, if we still need to discuss this further at a meeting, I will be at the other design meeting (APAC).

Regards,
Sunny Wang

-----Original Message-----
From: discuss@edk2.groups.io [mailto:discuss@edk2.groups.io] On Behalf Of Jeff Brasen
Sent: Wednesday, December 11, 2019 4:46 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; discuss@edk2.groups.io
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-discuss] [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Can we discuss this at the design meeting this week (12/12)?


Thanks,

Jeff

________________________________
From: Jeff Brasen
Sent: Thursday, November 14, 2019 10:04 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>; afish@apple.com <afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Yes, I think that would be good.



Summarizing everything in this thread



Problem: Platform needs to customize the boot options, this can be done for normal boot but the UiApp calls EfiBootManagerRefreshAllBootOption in a couple places.



Potential solutions:

1 - Define new PCD and event group if PCD is set true then signal event instead of calling EfiBootManagerRefreshAllBootOption in UiApp

2 - Add new function to boot manager library and replace call to EfiBootManagerRefreshAllBootOption in UiApp (need to coordinate rollout with updates to all platform.

3 - Add new protocol with new function, if supported call this otherwise call EfiBootManagerRefreshAllBootOption as is done now

4 - For 2/3 use generic function so we don't need new APIs for future expansion

5 - Update EfiBootManagerRefreshAllBootOption to call platform specific function.



Thanks,
Jeff





From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, November 13, 2019 7:09 PM
To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Laszlo Ersek <lersek@redhat.com>; afish@apple.com
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

I think it's a good topic that we could discuss in the open design meeting.

Are you ok to present the problem you have and discuss the potential solutions in that meeting?

https://github.com/tianocore/tianocore.github.io/wiki/Design-Meeting



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Thursday, November 14, 2019 2:43 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Thinking about this more I think we could do this with a PCD and a new group event without having to define any new function interfaces.



We could change UiApp and BootManagerMenuApp (and any others that are relevant) from



EfiBootManagerRefreshAllBootOption ();



to



if (FeaturePcdGet (PcdEventBasedRefreshAllBootOptionSupport) {

EFI_EVENT Event;

gBS->CreateEventEx ( 0, 0, NULL, NULL, &gEventBasedRefreshGuid, &Event );

gBS->SignalEvent (Event);

gBS->CloseEvent (Event);

} else {

EfiBootManagerRefreshAllBootOption ();

}



Then a platform that wants to do this on its own would just set this pcd and create a group event and do what it needs to do there.



Thanks,

Jeff

________________________________

From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: Monday, November 11, 2019 5:00 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



I am not sure a PCD would work (unless I am missing something) We do want to do a connect all and re-enumerate in UiApp but we need the platform code to be involved in that process.



Thanks,

Jeff

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Monday, November 11, 2019 4:58 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

If adding a PCD to control UiApp can meet the real needs, I prefer to do in that way instead of adding new APIs to PlatformBootManagerLib.



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Tuesday, November 12, 2019 6:58 AM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



If we are concerned about deploying this and breaking builds we could do this via a new protocol instead. In that case though we would leave the old default behavior in the code to handle platforms that didn't implement the new protocol, so this might not be the cleanest way to deploy this.



We could also look at adding a generic platform boot hook function (either as a library function or protocol) if we wanted to limit the number of disruption on new customization hooks. Something like



EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextData OPTIONAL)



Where Notification type describes where we are that we want platform to potentially handle and ContextData is per type caller allocated data that provides additional in/out data. This has the same issue of leaving the current default behavior in place for unsupported types as well as being a less than specific function to describe.



Thanks,

Jeff



________________________________

From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



On 11/07/19 18:46, Jeff Brasen wrote:
Fixing UiApp seems reasonable, I do think we would want a hook to the platform library in here as the enumeration that occurs in the UiApp is intended to do a full enumeration of the system and there may be platform specifics to how that occurs.
Fully agreed -- entering UiApp should expose everything bootable in the system, unless (perhaps) PlatformBootManagerLib specifically thinks otherwise.

Of course, then we arrive (again) at the problem that a call in UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break tens of out-of-tree platforms. :)

I think that can be prevented, as follows; but it will take quite some time:

- introduce the new function declaration in "PlatformBootManagerLib.h",
- modify all platforms (in tree and out of tree) to implement (define) the new function,
- call the new function from UefiBootManagerLib

For some history / background on this kind of problem, I suggest reading
through:

https://bugzilla.tianocore.org/show_bug.cgi?id=982

Thanks,
Laszlo

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, November 7, 2019 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen
<jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>;
afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Laszlo
Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael
D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration

I treat the issue in this way:

1. Platform Boot Manager library does a good job. It doesn't always call RefreshAll() API to auto-create the boot options
2. But UiApp doesn't. It constantly call RefreshAll().

Do you think that we can fix UiApp instead? For example, introducing a PCD to control the boot option refresh behavior?

Thanks,
Ray

From:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>
<devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gr
oups.io%3cmailto:devel@edk2.groups.io>>> On Behalf Of Jeff Brasen
Sent: Thursday, November 7, 2019 3:02 PM
To: Ni, Ray
<ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
ilto:ray.ni@intel.com>>>; afish@apple.com<mailto:afish@apple.com>
Cc:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
<lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
cmailto:lersek@redhat.com>>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration

The issue is there are some auto created options we do not want on our platform.
Get Outlook for
Android<INVALID URI REMOVED
ei36&d=DwIF-g&c=C5b8zRQO1miGmBeVZ2LFWg&r=Z9cLgEMdGZCI1_R0bW6KqOAGzCXLJ
UR24f8N3205AYw&m=tNOWNYcrAfOGKuSMyGZvQ46qeU3yiXGlPkDQK70Nshw&s=V7Nc8Wt
ostFCvj8E2KL2RcggpHj8sblqsBrdrabOuoE&e= >

________________________________
From: Ni, Ray
<ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
ilto:ray.ni@intel.com>>>
Sent: Wednesday, November 6, 2019 11:59:31 PM
To: Jeff Brasen
<jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
m%3cmailto:jbrasen@nvidia.com>>>;
afish@apple.com<mailto:afish@apple.com>
<afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
o:afish@apple.com>>>
Cc:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>
<devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gr
oups.io%3cmailto:devel@edk2.groups.io>>>; Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
<lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
cmailto:lersek@redhat.com>>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration


Jeff,

RefreshAllBootOption() only modifies/creates the auto-created boot options. For the boot options created by platform boot manager library, they stay with no one touches. And all auto-created boot options are appended in the end of boot option list (through BootOrder).



From: Jeff Brasen
<jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
m%3cmailto:jbrasen@nvidia.com>>>
Sent: Thursday, November 7, 2019 12:13 PM
To: afish@apple.com<mailto:afish@apple.com>; Ni, Ray
<ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
ilto:ray.ni@intel.com>>>
Cc:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
<lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
cmailto:lersek@redhat.com>>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration



As the suggestions below made sense, we updated our platform boot manager library to behave in this manner and for normal boots everything works well. However the UiApp and boot maintenance applications in EDK2 also call EfiBootManagerRefreshAllBootOption() when ever a user goes into the menu which will re-create the skipped boot options with no place for the platform code to intervene.



What about a solution where we add a new Platform library function that allows for override of the behavior of BmEnumerateBootOptions? For example, either a function or protocol that takes the same parameters as this function and only if it returns NULL then we continue to the default enumeration code. Or a function call inserted at the end that would modify the load option array after the system does the standard enumeration.



-Jeff



From: afish@apple.com<mailto:afish@apple.com>
<afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
o:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 9:20 AM
To: Ni, Ray
<ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
ilto:ray.ni@intel.com>>>
Cc:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>; Jeff Brasen
<jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.co
m%3cmailto:jbrasen@nvidia.com>>>; Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
<lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
cmailto:lersek@redhat.com>>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
@intel.com%3cmailto:zhichao.gao@intel.com>>>; Mike Kinney
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration



Ray,



Is there an obvious hook point we could point Jeff and Ashish at?



Long term it would be a good idea to have a Wiki page to give some guidance on how to customize the BDS.



Thanks,



Andrew Fish



On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>> wrote:



Andrew,

I agree with your opinion.

It's expected that Platform Boot Manager lib calls EfiBootManagerRefreshAllBootOption() only in full configuration boot path.

The full configuration boot path is chosen when hardware changes
happen. So it's not expected EfiBootManagerRefresh...() be called in every boot.

So you could:

1. Delete the auto-created option pointing to LoadFile instance
2. Create your own one with customized description.





From: afish@apple.com<mailto:afish@apple.com>
<afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailt
o:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 10:47 AM
To:
devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.gro
ups.io%3cmailto:devel@edk2.groups.io>>;
jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>
Cc: Ashish Singhal
<ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishs
ingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek
<lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3
cmailto:lersek@redhat.com>>>; Ni, Ray
<ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cma
ilto:ray.ni@intel.com>>>; Wang, Jian J
<jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang
@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A
<hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.co
m%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao
<zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao
@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D
<michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:m
ichael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM
enumeration







On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>> wrote:



Wouldn't having a variable that we create and delete on every boot put unnecessary stress on the SPI-NOR that the variable store lives on?
What about the alternative approach where we allow the platform code to modify the attributes of the auto created variable to disable it with hidden/!active but still match for detection purposes so that it doesn't delete and recreate the modified variable each boot? That way all the logic on what to disable can still be in the platform code and all the existing logic in the boot manager can stay basically the same?

What changes every boot that forces the variable to need to get modified?

I would assume the NOR driver is smart enough to not update a variable that is not changing.

The custom BDS could could only create the variable for this device if it does not exist.

[JB] The current flow with no changes in the boot manager would be as
follows



1. Scan for instance of the boot option in the variables
2. It will not be found, so create a new boot option store it to a variable and update BootOrder
3. Platform code runs creates the options for the boot option it wants and writes those to variable store
4. Delete/disable the boot option in the variable store



When you reboot it won't find the variable so 1/2/4 will re-occur



The code that does this (1/2) is EfiBootManagerRefreshAllBootOption in
BmBoot.c



If you modify the variable to disable it with hidden/not active it would delete that and create a new one as well as the code wouldn't recognize that is the same boot option.



If however we modify EfiBootManagerFindLoadOption to not compare the attributes (at least allow for differences in active and hidden) then the when it refreshes every thing it would see the match and not delete/create a new variable in the store and thus we wouldn't have changes every boot.





Jeff,



Sorry if I'm a little off on the sequence of things as the platform I work on day to day has a custom BDS and does not use this library..... I though the patch changed BmEnumerateBootOptions(), so that is going to change how EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch as given is invalid as it changed the behavior of the public library API for EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it would need to change the comments to reflect the new behavior. This is kind of what Laszlo's technical debt comment was about.



I think Laszlo advocated having the BDS platform specific code make sure the boot variables are in the correct state. That should happen before the Boot Manager code runs, and it is not clear to me why the Boot Manager could would need to run if you have a valid EFI nvram variable to boot from.



I think the question is how is your use case different than the boot variable that Windows installs? If it works kind of the same way then the answer is to have the BDS platform specific code write the boot variable.





[1]

/**

The function creates boot options for all possible bootable medias in the following order:

1. Removable BlockIo - The boot option only points to the removable media

device, like USB key, DVD, Floppy etc.

2. Fixed BlockIo - The boot option only points to a Fixed blockIo device,

like HardDisk.

3. Non-BlockIo SimpleFileSystem - The boot option points to a device
supporting

SimpleFileSystem Protocol, but not
supporting BlockIo

protocol.

4. LoadFile - The boot option points to the media supporting

LoadFile protocol.

Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot
Behavior



The function won't delete the boot option not added by itself.

**/

VOID

EFIAPI

EfiBootManagerRefreshAllBootOption (

VOID

);



Thanks,



Andrew Fish



Thanks,

Andrew Fish

Thanks,

Jeff

________________________________

This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.

________________________________


Re: Problems in running EmulatorPkg.

Lin, Derek (HPS SW)
 

This seems like an old thread. https://edk2.groups.io/g/devel/topic/30916071

I remember I also saw segmentation fault as well. Disabling optimization works for me.

EmulatorPkg.dsc
MdeModulePkg/Core/Pei/PeiMain.inf {
<BuildOptions>
GCC:*_*_*_CC_FLAGS = -O0
}


Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

Jeff Brasen
 

Can we discuss this at the design meeting this week (12/12)?


Thanks,

Jeff

________________________________
From: Jeff Brasen
Sent: Thursday, November 14, 2019 10:04 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com>; afish@apple.com <afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Yes, I think that would be good.



Summarizing everything in this thread



Problem: Platform needs to customize the boot options, this can be done for normal boot but the UiApp calls EfiBootManagerRefreshAllBootOption in a couple places.



Potential solutions:

1 – Define new PCD and event group if PCD is set true then signal event instead of calling EfiBootManagerRefreshAllBootOption in UiApp

2 – Add new function to boot manager library and replace call to EfiBootManagerRefreshAllBootOption in UiApp (need to coordinate rollout with updates to all platform.

3 – Add new protocol with new function, if supported call this otherwise call EfiBootManagerRefreshAllBootOption as is done now

4 – For 2/3 use generic function so we don’t need new APIs for future expansion

5 – Update EfiBootManagerRefreshAllBootOption to call platform specific function.



Thanks,
Jeff





From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, November 13, 2019 7:09 PM
To: devel@edk2.groups.io; Jeff Brasen <jbrasen@nvidia.com>; Laszlo Ersek <lersek@redhat.com>; afish@apple.com
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

I think it’s a good topic that we could discuss in the open design meeting.

Are you ok to present the problem you have and discuss the potential solutions in that meeting?

https://github.com/tianocore/tianocore.github.io/wiki/Design-Meeting



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Thursday, November 14, 2019 2:43 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Thinking about this more I think we could do this with a PCD and a new group event without having to define any new function interfaces.



We could change UiApp and BootManagerMenuApp (and any others that are relevant) from



EfiBootManagerRefreshAllBootOption ();



to



if (FeaturePcdGet (PcdEventBasedRefreshAllBootOptionSupport) {

EFI_EVENT Event;

gBS->CreateEventEx ( 0, 0, NULL, NULL, &gEventBasedRefreshGuid, &Event );

gBS->SignalEvent (Event);

gBS->CloseEvent (Event);

} else {

EfiBootManagerRefreshAllBootOption ();

}



Then a platform that wants to do this on its own would just set this pcd and create a group event and do what it needs to do there.



Thanks,

Jeff

________________________________

From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: Monday, November 11, 2019 5:00 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



I am not sure a PCD would work (unless I am missing something) We do want to do a connect all and re-enumerate in UiApp but we need the platform code to be involved in that process.



Thanks,

Jeff

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Monday, November 11, 2019 4:58 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Jeff,

If adding a PCD to control UiApp can meet the real needs, I prefer to do in that way instead of adding new APIs to PlatformBootManagerLib.



Thanks,

Ray



From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Jeff Brasen
Sent: Tuesday, November 12, 2019 6:58 AM
To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



If we are concerned about deploying this and breaking builds we could do this via a new protocol instead. In that case though we would leave the old default behavior in the code to handle platforms that didn't implement the new protocol, so this might not be the cleanest way to deploy this.



We could also look at adding a generic platform boot hook function (either as a library function or protocol) if we wanted to limit the number of disruption on new customization hooks. Something like



EFI_STATUS PlatformBootNotify (CONST EFI_GUID *NotificationType, VOID *ContextData OPTIONAL)



Where Notification type describes where we are that we want platform to potentially handle and ContextData is per type caller allocated data that provides additional in/out data. This has the same issue of leaving the current default behavior in place for unsupported types as well as being a less than specific function to describe.



Thanks,

Jeff



________________________________

From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
Sent: Friday, November 8, 2019 9:37 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com>>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



On 11/07/19 18:46, Jeff Brasen wrote:
Fixing UiApp seems reasonable, I do think we would want a hook to the platform library in here as the enumeration that occurs in the UiApp is intended to do a full enumeration of the system and there may be platform specifics to how that occurs.
Fully agreed -- entering UiApp should expose everything bootable in the
system, unless (perhaps) PlatformBootManagerLib specifically thinks
otherwise.

Of course, then we arrive (again) at the problem that a call in
UefiBootManagerLib, to a *new* PlatformBootManagerLib API, will break
tens of out-of-tree platforms. :)

I think that can be prevented, as follows; but it will take quite some time:

- introduce the new function declaration in "PlatformBootManagerLib.h",
- modify all platforms (in tree and out of tree) to implement (define)
the new function,
- call the new function from UefiBootManagerLib

For some history / background on this kind of problem, I suggest reading
through:

https://bugzilla.tianocore.org/show_bug.cgi?id=982

Thanks,
Laszlo

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, November 7, 2019 12:21 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; afish@apple.com<mailto:afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

I treat the issue in this way:

1. Platform Boot Manager library does a good job. It doesn't always call RefreshAll() API to auto-create the boot options
2. But UiApp doesn't. It constantly call RefreshAll().

Do you think that we can fix UiApp instead? For example, introducing a PCD to control the boot option refresh behavior?

Thanks,
Ray

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>> On Behalf Of Jeff Brasen
Sent: Thursday, November 7, 2019 3:02 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; afish@apple.com<mailto:afish@apple.com>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration

The issue is there are some auto created options we do not want on our platform.
Get Outlook for Android<https://aka.ms/ghei36>

________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Sent: Wednesday, November 6, 2019 11:59:31 PM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>> <devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration


Jeff,

RefreshAllBootOption() only modifies/creates the auto-created boot options. For the boot options created by platform boot manager library, they stay with no one touches. And all auto-created boot options are appended in the end of boot option list (through BootOrder).



From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>
Sent: Thursday, November 7, 2019 12:13 PM
To: afish@apple.com<mailto:afish@apple.com>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: RE: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



As the suggestions below made sense, we updated our platform boot manager library to behave in this manner and for normal boots everything works well. However the UiApp and boot maintenance applications in EDK2 also call EfiBootManagerRefreshAllBootOption() when ever a user goes into the menu which will re-create the skipped boot options with no place for the platform code to intervene.



What about a solution where we add a new Platform library function that allows for override of the behavior of BmEnumerateBootOptions? For example, either a function or protocol that takes the same parameters as this function and only if it returns NULL then we continue to the default enumeration code. Or a function call inserted at the end that would modify the load option array after the system does the standard enumeration.



-Jeff



From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 9:20 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Mike Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration



Ray,



Is there an obvious hook point we could point Jeff and Ashish at?



Long term it would be a good idea to have a Wiki page to give some guidance on how to customize the BDS.



Thanks,



Andrew Fish



On Nov 5, 2019, at 9:20 PM, Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>> wrote:



Andrew,

I agree with your opinion.

It's expected that Platform Boot Manager lib calls EfiBootManagerRefreshAllBootOption() only in full configuration boot path.

The full configuration boot path is chosen when hardware changes happen. So it's not expected EfiBootManagerRefresh...() be
called in every boot.

So you could:

1. Delete the auto-created option pointing to LoadFile instance
2. Create your own one with customized description.





From: afish@apple.com<mailto:afish@apple.com> <afish@apple.com<mailto:afish@apple.com<mailto:afish@apple.com%3cmailto:afish@apple.com>>>
Sent: Wednesday, November 6, 2019 10:47 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io%3cmailto:devel@edk2.groups.io>>; jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com%3cmailto:ashishsingha@nvidia.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com<mailto:ray.ni@intel.com%3cmailto:ray.ni@intel.com>>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com<mailto:jian.j.wang@intel.com%3cmailto:jian.j.wang@intel.com>>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com<mailto:hao.a.wu@intel.com%3cmailto:hao.a.wu@intel.com>>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com<mailto:zhichao.gao@intel.com%3cmailto:zhichao.gao@intel.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
Subject: Re: [edk2-devel] [PATCH] Support skipping automatic BM enumeration







On Nov 5, 2019, at 7:34 PM, Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com<mailto:jbrasen@nvidia.com%3cmailto:jbrasen@nvidia.com>>> wrote:



Wouldn't having a variable that we create and delete on every boot put unnecessary stress on the SPI-NOR that the variable store lives on?
What about the alternative approach where we allow the platform code to modify the attributes of the auto created variable to disable it with hidden/!active but still match for detection purposes so that it doesn't delete and recreate the modified variable each boot? That way all the logic on what to disable can still be in the platform code and all the existing logic in the boot manager can stay basically the same?

What changes every boot that forces the variable to need to get modified?

I would assume the NOR driver is smart enough to not update a variable that is not changing.

The custom BDS could could only create the variable for this device if it does not exist.

[JB] The current flow with no changes in the boot manager would be as follows



1. Scan for instance of the boot option in the variables
2. It will not be found, so create a new boot option store it to a variable and update BootOrder
3. Platform code runs creates the options for the boot option it wants and writes those to variable store
4. Delete/disable the boot option in the variable store



When you reboot it won't find the variable so 1/2/4 will re-occur



The code that does this (1/2) is EfiBootManagerRefreshAllBootOption in BmBoot.c



If you modify the variable to disable it with hidden/not active it would delete that and create a new one as well as the code wouldn't recognize that is the same boot option.



If however we modify EfiBootManagerFindLoadOption to not compare the attributes (at least allow for differences in active and hidden) then the when it refreshes every thing it would see the match and not delete/create a new variable in the store and thus we wouldn't have changes every boot.





Jeff,



Sorry if I'm a little off on the sequence of things as the platform I work on day to day has a custom BDS and does not use this library..... I though the patch changed BmEnumerateBootOptions(), so that is going to change how EfiBootManagerRefreshAllBootOption() works. I'd also point out the patch as given is invalid as it changed the behavior of the public library API for EfiBootManagerRefreshAllBootOption() [1] so for the patch to be valid it would need to change the comments to reflect the new behavior. This is kind of what Laszlo's technical debt comment was about.



I think Laszlo advocated having the BDS platform specific code make sure the boot variables are in the correct state. That should happen before the Boot Manager code runs, and it is not clear to me why the Boot Manager could would need to run if you have a valid EFI nvram variable to boot from.



I think the question is how is your use case different than the boot variable that Windows installs? If it works kind of the same way then the answer is to have the BDS platform specific code write the boot variable.





[1]

/**

The function creates boot options for all possible bootable medias in the following order:

1. Removable BlockIo - The boot option only points to the removable media

device, like USB key, DVD, Floppy etc.

2. Fixed BlockIo - The boot option only points to a Fixed blockIo device,

like HardDisk.

3. Non-BlockIo SimpleFileSystem - The boot option points to a device supporting

SimpleFileSystem Protocol, but not supporting BlockIo

protocol.

4. LoadFile - The boot option points to the media supporting

LoadFile protocol.

Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior



The function won't delete the boot option not added by itself.

**/

VOID

EFIAPI

EfiBootManagerRefreshAllBootOption (

VOID

);



Thanks,



Andrew Fish



Thanks,

Andrew Fish

Thanks,

Jeff

________________________________

This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.

________________________________


Problems in running EmulatorPkg.

Diego.Marcelino@...
 

I am facing some struggles trying running EmulatorPkg.

I am using a x86-64 machine using Ubuntu 18.04 and GCC5, install the dependencies libx11-dev libxext-dev according to https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg.

When directly (./Host in Build folder) running Host I got the following output:

EDK II UNIX Host Emulation Environment from http://www.tianocore.org/edk2/
BootMode 0x00
OS Emulator passing in 128 KB of temp RAM at 0x40000000 to SEC
FD loaded from ../FV/FV_RECOVERY.fd at 0x102000000 contains SEC Core

0x102000400 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/EmulatorPkg/Sec/Sec/DEBUG/EmuSec.dll with entry point 0x10200163f
SEC Has Started
0x102002780 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/MdeModulePkg/Core/Pei/PeiMain/DEBUG/PeiCore.dll with entry point 0x10200c4cf
0x102014200 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei/DEBUG/ReportStatusCodeRouterPei.dll with entry point 0x102014fdb
0x102015c80 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei/DEBUG/StatusCodeHandlerPei.dll with entry point 0x10201767a
PROGRESS CODE: V03020003 I0
0x10200ed00 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/MdeModulePkg/Universal/PCD/Pei/Pcd/DEBUG/PcdPeim.dll with entry point 0x102012c0a
Loading PEIM at 0x0010200EAC0 EntryPoint=0x00102012C0A PcdPeim.efi
PROGRESS CODE: V03020002 I0
Install PPI: 06E81C58-4AD7-44BC-8390-F10265F72480
Install PPI: 01F34D25-4DE2-23AD-3FF3-36353FF323F1
Install PPI: 4D8B155B-C059-4C8F-8926-06FD4331DB8A
Install PPI: A60C6B59-E459-425D-9C69-0BCC9CB27D81
Register PPI Notify: 605EA650-C65C-42E1-BA80-91A52AB618C6
PROGRESS CODE: V03020003 I0
0x102018800 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/EmulatorPkg/BootModePei/BootModePei/DEBUG/BootModePei.dll with entry point 0x10201908b
Loading PEIM at 0x001020185C0 EntryPoint=0x0010201908B BootModePei.efi
PROGRESS CODE: V03020002 I0
Emu Boot Mode PEIM Loaded
Install PPI: 7408D748-FC8C-4EE6-9288-C4BEC092A410
PROGRESS CODE: V03020003 I0
0x102019b00 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/EmulatorPkg/AutoScanPei/AutoScanPei/DEBUG/AutoScanPei.dll with entry point 0x10201a4b3
Loading PEIM at 0x001020198C0 EntryPoint=0x0010201A4B3 AutoScanPei.efi
PROGRESS CODE: V03020002 I0
Emu Autoscan PEIM Loaded
PeiInstallPeiMemory MemoryBegin 0x41000000, MemoryLength 0x4000000
PROGRESS CODE: V03020003 I0
Temp Stack : BaseAddress=0x40000000 Length=0x10000
Temp Heap : BaseAddress=0x40010000 Length=0x10000
Total temporary memory: 131072 bytes.
temporary memory stack ever used: 65532 bytes.
temporary memory heap used for HobList: 5240 bytes.
temporary memory heap occupied by memory pages: 0 bytes.
Old Stack size 65536, New stack size 131072
Stack Hob: BaseAddress=0x41000000 Length=0x20000
Heap Offset = 0x1010000 Stack Offset = 0x1010000
0x44ff2240 Loading /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/MdeModulePkg/Core/Pei/PeiMain/DEBUG/PeiCore.dll with entry point 0x44ffbf8f
Segmentation fault (core dumped)

-------------------------------------------------------------------------------------------------------------------------------------------

When running using EmulatorPkg/build.sh run, it opens gdb terminal and the application is not running:

Initializing workspace
/home/diego/Desktop/edk2/BaseTools
Loading previous configuration from /home/diego/Desktop/edk2/Conf/BuildEnv.sh
WORKSPACE: /home/diego/Desktop/edk2
EDK_TOOLS_PATH: /home/diego/Desktop/edk2/BaseTools
CONF_PATH: /home/diego/Desktop/edk2/Conf
using prebuilt tools
Reading symbols from /home/diego/Desktop/edk2/Build/Emulator/DEBUG_GCC5/X64/Host...(no debugging symbols found)...done.
/home/diego/Desktop/edk2/EmulatorPkg/Unix/GdbRun:79: Error in sourced command file:
No symbol table is loaded. Use the "file" command.
(gdb) c
The program is not being run.
(gdb)

-------------------------------------------------------------------------------------------------------------------------------------------

I tried in both master branch and tag vUDK2018.


[EDK2] OS booting from SPI flash device on Leaf Hill-based new board

violet7027@...
 

Hi.
Could you please help to take a look at my problem?
I have designed Leaf Hill-based new board and testing now.
BIOS booting was done successfully, memory and most of peripherals are listed on the efi shell.
There are two SPI Flash memory on the board, the one is for OS and other one is for image files.

Q1. SPI flash for OS and image files are not accessible.(FS0, FS1)
Efi shell displays no map files
How can I get access to SPI flash?

Q2. How can I boot from SPI flash?
Booting was done successfully from USB memory stick, making file system and copying to SPI flash the OS image was done.
But I couldn't boot from SPI flash and there are no options in the boot order set up menu.
How can I make boot OS image from SPI flash?
Any help would be appreciated.

Thank you.


How to determine/confirm whether SCT test being run is against the intended driver

kusumakaralthi@...
 

Hi,
I am trying to run SCT test on a NIC(Network Interface Card) Option Rom/Driver. But I am not sure that the test is performed against my particular Option Rom driver or any other UEFI driver
I tried one debug driver by adding debug messages at multiple locations and tried SCT, but none of those debug messages are being printed during the test
Q1) How to confirm if the SCT is being run against my driver?
Q2) Can we provide Driver Handle to UEFI SCT test to test against?
Q3) I could not find much documentation on SCT in the UEFI forums. I want to look at specific documentation that if SCT throw warning/Error, which line of code/method in driver this Error is thrown at. Can you point me to the documentation?

--
Thanks,
Kusumakar Althi
Broadcom


Re: Examples opening and reading/writing a file with EDK2

Gao, Zhichao
 

Hi Alejadro,

I am trying to answer your questions. If anything missed, please feel free to let me know.

1. First I think CapsuleApp is a good example to consume the file protocol.
2. File handle like the standard C's file pointer, it has multi required protocol install on it, such as device path protocol, simple file protocol and so on.
We can do some operation on the file thru the file handle.
3. We can use DevicePathFromHandle to get the file's path from the file handle. And use ConvertDevicePathToText to convert the path to text mode to view.

All the above can be found in the CapsuleApp.

Thanks,
Zhichao

-----Original Message-----
From: discuss@edk2.groups.io [mailto:discuss@edk2.groups.io] On Behalf Of
alejandro.estay@gmail.com
Sent: Friday, November 22, 2019 9:54 AM
To: discuss@edk2.groups.io
Subject: [edk2-discuss] Examples opening and reading/writing a file with EDK2

Hi, I'm making a little UEFI app, just for check basic functionality of the firmware.
inside this app I want to load, read and write a file, binary or text. However I
can't find a "complete explanation" or examples about the use of the
procedures (EFI_FILE_PROTOCOL.Open(), EFI_FILE_PROTOCOL.Read()) from the
UEFI API (steps, what to check). The only thing I found was some little Uefi Shell
apps doing this using the shell API. However I would like to do it using the "bare
firmware" instead of loading the shell. For me, the most confusing part, is when
the program has to check the handle database to find the particular handle of
the file that is being opened. Also I have some doubts about how to check,
without the shell, what volume or partition would have the exact file I'm looking
for (i.e. what if 2 volumes have simmilar, or even identical root directories).

Thanks in advance


Re: [OVMF] resource assignment fails for passthrough PCI GPU

Eduardo Habkost <ehabkost@...>
 

(+Jiri, +libvir-list)

On Fri, Nov 22, 2019 at 04:58:25PM +0000, Dr. David Alan Gilbert wrote:
* Laszlo Ersek (lersek@redhat.com) wrote:
(+Dave, +Eduardo)

On 11/22/19 00:00, dann frazier wrote:
On Tue, Nov 19, 2019 at 06:06:15AM +0100, Laszlo Ersek wrote:
On 11/19/19 01:54, dann frazier wrote:
On Fri, Nov 15, 2019 at 11:51:18PM +0100, Laszlo Ersek wrote:
On 11/15/19 19:56, dann frazier wrote:
Hi,
I'm trying to passthrough an Nvidia GPU to a q35 KVM guest, but UEFI
is failing to allocate resources for it. I have no issues if I boot w/
a legacy BIOS, and it works fine if I tell the linux guest to do the
allocation itself - but I'm looking for a way to make this work w/
OVMF by default.

I posted a debug log here:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563/+attachment/5305740/+files/q35-uefidbg.log

Linux guest lspci output is also available for both seabios/OVMF boots here:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563
By default, OVMF exposes such a 64-bit MMIO aperture for PCI MMIO BAR
allocation that is 32GB in size. The generic PciBusDxe driver collects,
orders, and assigns / allocates the MMIO BARs, but it can work only out
of the aperture that platform code advertizes.

Your GPU's region 1 is itself 32GB in size. Given that there are further
PCI devices in the system with further 64-bit MMIO BARs, the default
aperture cannot accommodate everything. In such an event, PciBusDxe
avoids assigning the largest BARs (to my knowledge), in order to
conserve the most aperture possible, for other devices -- hence break
the fewest possible PCI devices.

You can control the aperture size from the QEMU command line. You can
also do it from the libvirt domain XML, technically speaking. The knob
is experimental, so no stability or compatibility guarantees are made.
(That's also the reason why it's a bit of a hack in the libvirt domain XML.)

The QEMU cmdline options is described in the following edk2 commit message:

https://github.com/tianocore/edk2/commit/7e5b1b670c38
Hi Laszlo,

Thanks for taking the time to describe this in detail! The -fw_cfg
option did avoid the problem for me.
Good to hear, thanks.

I also noticed that the above
commit message mentions the existence of a 24GB card as a reasoning
behind choosing the 32GB default aperture. From what you say below, I
understand that bumping this above 64GB could break hosts w/ <= 37
physical address bits.
Right.

What would be the downside of bumping the
default aperture to, say, 48GB?
The placement of the aperture is not trivial (please see the code
comments in the linked commit). The base address of the aperture is
chosen so that the largest BAR that can fit in the aperture may be
naturally aligned. (BARs are whole powers of two.)

The largest BAR that can fit in a 48 GB aperture is 32 GB. Therefore
such an aperture would be aligned at 32 GB -- the lowest base address
(dependent on guest RAM size) would be 32 GB. Meaning that the aperture
would end at 32 + 48 = 80 GB. That still breaches the 36-bit phys
address width.

32 GB is the largest aperture size that can work with 36-bit phys
address width; that's the aperture that ends at 64 GB exactly.
Thanks, yeah - now that I read the code comments that is clear (as
clear as it can be w/ my low level of base knowledge). In the commit you
mention Gerd (CC'd) had suggested a heuristic-based approach for
sizing the aperture. When you say "PCPU address width" - is that a
function of the available physical bits?
"PCPU address width" is not a "function" of the available physical bits
-- it *is* the available physical bits. "PCPU" simply stands for
"physical CPU".

IOW, would that approach
allow OVMF to automatically grow the aperture to the max ^2 supported
by the host CPU?
Maybe.

The current logic in OVMF works from the guest-physical address space
size -- as deduced from multiple factors, such as the 64-bit MMIO
aperture size, and others -- towards the guest-CPU (aka VCPU) address
width. The VCPU address width is important for a bunch of other purposes
in the firmware, so OVMF has to calculate it no matter what.

Again, the current logic is to calculate the highest guest-physical
address, and then deduce the VCPU address width from that (and then
expose it to the rest of the firmware).

Your suggestion would require passing the PCPU (physical CPU) address
width from QEMU/KVM into the guest, and reversing the direction of the
calculation. The PCPU address width would determine the VCPU address
width directly, and then the 64-bit PCI MMIO aperture would be
calculated from that.

However, there are two caveats.

(1) The larger your guest-phys address space (as exposed through the
VCPU address width to the rest of the firmware), the more guest RAM you
need for page tables. Because, just before entering the DXE phase, the
firmware builds 1:1 mapping page tables for the entire guest-phys
address space. This is necessary e.g. so you can access any PCI MMIO BAR.

Now consider that you have a huge beefy virtualization host with say 46
phys address bits, and a wimpy guest with say 1.5GB of guest RAM. Do you
absolutely want tens of *terabytes* for your 64-bit PCI MMIO aperture?
Do you really want to pay for the necessary page tables with that meager
guest RAM?

(Such machines do exist BTW, for example:

http://mid.mail-archive.com/9BD73EA91F8E404F851CF3F519B14AA8036C67B5@DGGEMI521-MBX.china.huawei.com
)

In other words, you'd need some kind of knob anyway, because otherwise
your aperture could grow too *large*.


(2) Exposing the PCPU address width to the guest may have nasty
consequences at the QEMU/KVM level, regardless of guest firmware. For
example, that kind of "guest enlightenment" could interfere with migration.

If you boot a guest let's say with 16GB of RAM, and tell it "hey friend,
have 40 bits of phys address width!", then you'll have a difficult time
migrating that guest to a host with a CPU that only has 36-bits wide
physical addresses -- even if the destination host has plenty of RAM
otherwise, such as a full 64GB.

There could be other QEMU/KVM / libvirt issues that I m unaware of
(hence the CC to Dave and Eduardo).
host physical address width gets messy. There are differences as well
between upstream qemu behaviour, and some downstreams.
I think the story is that:

a) Qemu default: 40 bits on any host
b) -cpu blah,host-phys-bits=true to follow the host.
c) RHEL has host-phys-bits=true by default

As you say, the only real problem with host-phys-bits is migration -
between say an E3 and an E5 xeon with different widths. The magic 40's
is generally wrong as well - I think it came from some ancient AMD,
but it's the default on QEMU TCG as well.
Yes, and because it affects live migration ability, we have two
constraints:
1) It needs to be exposed in the libvirt domain XML;
2) QEMU and libvirt can't choose a value that works for everybody
(because neither QEMU or libvirt know where the VM might be
migrated later).

Which is why the BZ below is important:


I don't think there's a way to set it in libvirt;
https://bugzilla.redhat.com/show_bug.cgi?id=1578278 is a bz asking for
that.

IMHO host-phys-bits is actually pretty safe; and makes most sense in a
lot of cases.
Yeah, it is mostly safe and makes sense, but messy if you try to
migrate to a host with a different size.


Dave


Thanks,
Laszlo


-dann

For example, to set a 64GB aperture, pass:

-fw_cfg name=opt/ovmf/X-PciMmio64Mb,string=65536

The libvirt domain XML syntax is a bit tricky (and it might "taint" your
domain, as it goes outside of the QEMU features that libvirt directly
maps to):

<domain
type='kvm'
xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
<qemu:commandline>
<qemu:arg value='-fw_cfg'/>
<qemu:arg value='opt/ovmf/X-PciMmio64Mb,string=65536'/>
</qemu:commandline>
</domain>

Some notes:

(1) The "xmlns:qemu" namespace definition attribute in the <domain> root
element is important. You have to add it manually when you add
<qemu:commandline> and <qemu:arg> too. Without the namespace
definition, the latter elements will make no sense, and libvirt will
delete them immediately.

(2) The above change will grow your guest's physical address space to
more than 64GB. As a consequence, on your *host*, *if* your physical CPU
supports nested paging (called "ept" on Intel and "npt" on AMD), *then*
the CPU will have to support at least 37 physical address bits too, for
the guest to work. Otherwise, the guest will break, hard.

Here's how to verify (on the host):

(2a) run "egrep -w 'npt|ept' /proc/cpuinfo" --> if this does not produce
output, then stop reading here; things should work. Your CPU does not
support nested paging, so KVM will use shadow paging, which is slower,
but at least you don't have to care about the CPU's phys address width.

(2b) otherwise (i.e. when you do have nested paging), run "grep 'bits
physical' /proc/cpuinfo" --> if the physical address width is >=37,
you're good.

(2c) if you have nested paging but exactly 36 phys address bits, then
you'll have to forcibly disable nested paging (assuming you want to run
a guest with larger than 64GB guest-phys address space, that is). On
Intel, issue:

rmmod kvm_intel
modprobe kvm_intel ept=N

On AMD, go with:

rmmod kvm_amd
modprobe kvm_amd npt=N

Hope this helps,
Laszlo
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Eduardo


Re: [OVMF] resource assignment fails for passthrough PCI GPU

Laszlo Ersek
 

On 11/22/19 17:58, Dr. David Alan Gilbert wrote:
* Laszlo Ersek (lersek@redhat.com) wrote:
(+Dave, +Eduardo)

On 11/22/19 00:00, dann frazier wrote:
On Tue, Nov 19, 2019 at 06:06:15AM +0100, Laszlo Ersek wrote:
On 11/19/19 01:54, dann frazier wrote:
On Fri, Nov 15, 2019 at 11:51:18PM +0100, Laszlo Ersek wrote:
On 11/15/19 19:56, dann frazier wrote:
Hi,
I'm trying to passthrough an Nvidia GPU to a q35 KVM guest, but UEFI
is failing to allocate resources for it. I have no issues if I boot w/
a legacy BIOS, and it works fine if I tell the linux guest to do the
allocation itself - but I'm looking for a way to make this work w/
OVMF by default.

I posted a debug log here:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563/+attachment/5305740/+files/q35-uefidbg.log

Linux guest lspci output is also available for both seabios/OVMF boots here:
https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563
By default, OVMF exposes such a 64-bit MMIO aperture for PCI MMIO BAR
allocation that is 32GB in size. The generic PciBusDxe driver collects,
orders, and assigns / allocates the MMIO BARs, but it can work only out
of the aperture that platform code advertizes.

Your GPU's region 1 is itself 32GB in size. Given that there are further
PCI devices in the system with further 64-bit MMIO BARs, the default
aperture cannot accommodate everything. In such an event, PciBusDxe
avoids assigning the largest BARs (to my knowledge), in order to
conserve the most aperture possible, for other devices -- hence break
the fewest possible PCI devices.

You can control the aperture size from the QEMU command line. You can
also do it from the libvirt domain XML, technically speaking. The knob
is experimental, so no stability or compatibility guarantees are made.
(That's also the reason why it's a bit of a hack in the libvirt domain XML.)

The QEMU cmdline options is described in the following edk2 commit message:

https://github.com/tianocore/edk2/commit/7e5b1b670c38
Hi Laszlo,

Thanks for taking the time to describe this in detail! The -fw_cfg
option did avoid the problem for me.
Good to hear, thanks.

I also noticed that the above
commit message mentions the existence of a 24GB card as a reasoning
behind choosing the 32GB default aperture. From what you say below, I
understand that bumping this above 64GB could break hosts w/ <= 37
physical address bits.
Right.

What would be the downside of bumping the
default aperture to, say, 48GB?
The placement of the aperture is not trivial (please see the code
comments in the linked commit). The base address of the aperture is
chosen so that the largest BAR that can fit in the aperture may be
naturally aligned. (BARs are whole powers of two.)

The largest BAR that can fit in a 48 GB aperture is 32 GB. Therefore
such an aperture would be aligned at 32 GB -- the lowest base address
(dependent on guest RAM size) would be 32 GB. Meaning that the aperture
would end at 32 + 48 = 80 GB. That still breaches the 36-bit phys
address width.

32 GB is the largest aperture size that can work with 36-bit phys
address width; that's the aperture that ends at 64 GB exactly.
Thanks, yeah - now that I read the code comments that is clear (as
clear as it can be w/ my low level of base knowledge). In the commit you
mention Gerd (CC'd) had suggested a heuristic-based approach for
sizing the aperture. When you say "PCPU address width" - is that a
function of the available physical bits?
"PCPU address width" is not a "function" of the available physical bits
-- it *is* the available physical bits. "PCPU" simply stands for
"physical CPU".

IOW, would that approach
allow OVMF to automatically grow the aperture to the max ^2 supported
by the host CPU?
Maybe.

The current logic in OVMF works from the guest-physical address space
size -- as deduced from multiple factors, such as the 64-bit MMIO
aperture size, and others -- towards the guest-CPU (aka VCPU) address
width. The VCPU address width is important for a bunch of other purposes
in the firmware, so OVMF has to calculate it no matter what.

Again, the current logic is to calculate the highest guest-physical
address, and then deduce the VCPU address width from that (and then
expose it to the rest of the firmware).

Your suggestion would require passing the PCPU (physical CPU) address
width from QEMU/KVM into the guest, and reversing the direction of the
calculation. The PCPU address width would determine the VCPU address
width directly, and then the 64-bit PCI MMIO aperture would be
calculated from that.

However, there are two caveats.

(1) The larger your guest-phys address space (as exposed through the
VCPU address width to the rest of the firmware), the more guest RAM you
need for page tables. Because, just before entering the DXE phase, the
firmware builds 1:1 mapping page tables for the entire guest-phys
address space. This is necessary e.g. so you can access any PCI MMIO BAR.

Now consider that you have a huge beefy virtualization host with say 46
phys address bits, and a wimpy guest with say 1.5GB of guest RAM. Do you
absolutely want tens of *terabytes* for your 64-bit PCI MMIO aperture?
Do you really want to pay for the necessary page tables with that meager
guest RAM?

(Such machines do exist BTW, for example:

http://mid.mail-archive.com/9BD73EA91F8E404F851CF3F519B14AA8036C67B5@DGGEMI521-MBX.china.huawei.com
)

In other words, you'd need some kind of knob anyway, because otherwise
your aperture could grow too *large*.


(2) Exposing the PCPU address width to the guest may have nasty
consequences at the QEMU/KVM level, regardless of guest firmware. For
example, that kind of "guest enlightenment" could interfere with migration.

If you boot a guest let's say with 16GB of RAM, and tell it "hey friend,
have 40 bits of phys address width!", then you'll have a difficult time
migrating that guest to a host with a CPU that only has 36-bits wide
physical addresses -- even if the destination host has plenty of RAM
otherwise, such as a full 64GB.

There could be other QEMU/KVM / libvirt issues that I m unaware of
(hence the CC to Dave and Eduardo).
host physical address width gets messy. There are differences as well
between upstream qemu behaviour, and some downstreams.
I think the story is that:

a) Qemu default: 40 bits on any host
b) -cpu blah,host-phys-bits=true to follow the host.
c) RHEL has host-phys-bits=true by default

As you say, the only real problem with host-phys-bits is migration -
between say an E3 and an E5 xeon with different widths. The magic 40's
is generally wrong as well - I think it came from some ancient AMD,
but it's the default on QEMU TCG as well.

I don't think there's a way to set it in libvirt;
https://bugzilla.redhat.com/show_bug.cgi?id=1578278 is a bz asking for
that.

IMHO host-phys-bits is actually pretty safe; and makes most sense in a
lot of cases.
Thanks -- this is a useful piece of the puzzle to know. It seems that
the guest can learn about the guest-phys address width via CPUID.
(cpu_x86_cpuid() in "target/i386/cpu.c" consumes "cpu->phys_bits", which
seems to be set in x86_cpu_realizefn().)

Cheers!
Laszlo


Re: Examples opening and reading/writing a file with EDK2

Laszlo Ersek
 

On 11/22/19 02:54, alejandro.estay@gmail.com wrote:
Hi, I'm making a little UEFI app, just for check basic functionality
of the firmware. inside this app I want to load, read and write a
file, binary or text. However I can't find a "complete explanation" or
examples about the use of the procedures (EFI_FILE_PROTOCOL.Open(),
EFI_FILE_PROTOCOL.Read()) from the UEFI API (steps, what to check).
The only thing I found was some little Uefi Shell apps doing this
using the shell API. However I would like to do it using the "bare
firmware" instead of loading the shell. For me, the most confusing
part, is when the program has to check the handle database to find
the particular handle of the file that is being opened. Also I have
some doubts about how to check, without the shell, what volume or
partition would have the exact file I'm looking for (i.e. what if 2
volumes have simmilar, or even identical root directories).
First, you need to find the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL instance in
the protocol database that is right for your purposes. You could locate
this protocol instance for example with the LocateDevicePath() boot
service. There could be other ways for you to locate the right handle,
and then open the Simple File System protocol interface on that handle.

This really depends on your use case. It's your application that has to
know on what device (such as, what PCI(e) controller, what SCSI disk,
what ATAPI disk, what partition, etc) to look for the interesting file.

For example, if you simply check every EFI_SIMPLE_FILE_SYSTEM_PROTOCOL
in the protocol database, and among those, you cannot distinguish two
from each other (because both look suitable), then you'll have to
investigate the device path protocol installed on each handle. You might
be able to make a decision based on the structure / semantics of the
device paths themselves. Alternatively, you might have to traverse the
device paths node by node, and open further protocol interfaces on the
corresponding handles, to ultimately pick the right
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.


Once you have the EFI_SIMPLE_FILE_SYSTEM_PROTOCOL interface open, you
need to call its OpenVolume() member function. See the UEFI spec for
details please. It will give you the EFI_FILE_PROTOCOL for the root
directory of that file system.

Once you got the EFI_FILE_PROTOCOL interface for the root directory, you
can call the Open() member function for opening files or directories
relative to the root directory. Either way, you'll get a new
EFI_FILE_PROTOCOL interface for the opened object (file or directory).
If you've opened a directory previously, then you can issue further
Open() calls for opening files or directories relative to *that*
(sub)directory.

In case you start with an EFI_DEVICE_PATH_PROTOCOL instance that
identifies a particular file in a particular filesystem, then the device
path protocol will contain *at least one* File Path Media Device Path
node. It is important that there may be more than one such device path
node, and the full pathname (within the filesystem), from the root
directory to the particular file, may be split over a number of device
path nodes.

For example, you could have just one File Path node containing
"\dir1\dir2\hello.txt". Or you could have three File Path nodes
containing "dir1", "dir2", "hello.txt", respectively. Or you could have
two File Path nodes containing "dir1\dir2\" and "hello.txt",
respectively.

In these cases, you'd need one, three, or two, EFI_FILE_PROTOCOL.Open()
calls, accordingly.

Alternatively, you'd need to concatenate the pathname fragments into a
whole pathname, making sure that there be precisely one backslash
separator between each pair of pathname components, and then issue a
single EFI_FILE_PROTOCOL.Open() call in the end.


You can find a helper function called EfiOpenFileByDevicePath() in
"MdePkg/Library/UefiLib/UefiLib.c".

A somewhat similar function is GetFileBufferByFilePath(), in
"MdePkg/Library/DxeServicesLib/DxeServicesLib.c".

Thanks,
Laszlo

801 - 820 of 888