Date   

Re: [RFCv2] code-first process for UEFI-forum specifications

Ni, Ray
 


## Github
New repositories will be added for holding the text changes and the source code.

Specification text changes will be held within the affected source repository,
in the Github flavour of markdown, in a file (or split across several files)
with .md suffix.
What's the case when multiple .MD files are needed?

(This one may break down where we have a specification change affecting multiple
specifications, but at that point we can track it with multiple BZ entries)


## Source code
In order to ensure draft code does not accidentally leak into production use,
and to signify when the changeover from draft to final happens, *all* new or
modified[1] identifiers need to be prefixed with the relevant BZ####.

[1] Modified in a non-backwards-compatible way. If, for example, a statically
sized array is grown - this does not need to be prefixed. But a tag in a
comment would be *highly* recommended.
If a protocol is enhanced to provide more interfaces with increased revision number,
would you like the protocol name to be prefixed with BZ####?
Or just the new interfaces added to the protocol are prefixed the BZ####?
I think just prefixing the new interfaces can meet the purpose.

But the protocol definition is changed, it also needs to be prefixed according to this flow.
Can you clarify a bit more?


### File names
New public header files need the prefix. I.e. `Bz1234MyNewProtocol.h`
Private header files do not need the prefix.

### Contents

The tagging must follow the coding style used by each affected codebase.
Examples:

| Released in spec | Draft version in tree | Comment |
| --- | --- | --- |
| `FunctionName` | `Bz1234FunctionName` | |
| `HEADER_MACRO` | `BZ1234_HEADER_MACRO` | |
If FunctionName or HEADER_MACRO is defined in non-public header files, I don't
think they require the prefix. Do you agree?

For data structures or enums, any new or non-backwards-compatible structs or
fields require a prefix. As above, growing an existing array in an existing
struct requires no prefix.

| `typedef SOME_STRUCT` | `BZ1234_SOME_STRUCT` | Typedef only [2] |
| `StructField` | `Bz1234StructField` | In existing struct[3] |
| `typedef SOME_ENUM` | `BZ1234_SOME_ENUM` | Typedef only [2] |

[2] If the struct or enum definition is separate from the typedef in the public
header, the definition does not need the prefix.
What does "separate" mean?
Does it mean "struct or enum in the public header BzXXX.h don't need the prefix"?
If yes, then I think macros defined in BzXXX.h also don't need the prefix.

[3] Individual fields in newly added typedefd struct do not need prefix, the
struct already carried the prefix.

Variable prefixes indicating global scope ('g' or 'm') go before the BZ prefix.

| `gSomeGuid` | `gBz1234SomeGuid` | |

Local identifiers, including module-global ones (m-prefixed) do not require a
BZ prefix.
I think only the names (struct type name, enum type name, interface name, protocol/ppi name)
defined in public header files need the BZ prefix when the public header doesn't have prefix.
Right?


Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Wu, Hao A
 

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Brian
J. Johnson
Sent: Tuesday, March 24, 2020 12:39 AM
To: devel@edk2.groups.io; Ni, Ray; Laszlo Ersek; Wu, Hao A;
rfc@edk2.groups.io
Cc: Dong, Eric; Kinney, Michael D; Zeng, Star
Subject: Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce
AP status check interval

On 3/23/20 9:37 AM, Ni, Ray wrote:
Laszlo,
Adding a PCD means platform integrators need to consider which value to
set.
Most of the time, they may just use the default PCD value.
Then, why not we add the PCD later when a real case is met?
The patch changes existent behavior; it is not for a newly introduced
feature.

Because most platforms are not in the edk2 tree, we don't know what
platforms could be regressed by increasing the polling frequency
tenfold. (And remember that the polling action has O(n) cost, where "n"
is the logical processor count.)

Under your suggestion, the expression "real case is met" amounts to
"someone reports a regression" (possibly after the next stable tag,
even). I don't think that's a good idea.
In particular, the patch is motivated by RegisterCpuFeaturesLib -- the
CpuFeaturesInitialize() function -- on some platform(s) that Hao uses.
But there are platforms that don't use RegisterCpuFeaturesLib, and still
use MpInitLib.
OK. I agree with your suggestion.
Thank you. I agree with Laszlo: MP initialization is tricky to scale,
and platform dependent. My group builds real machines with thousands of
APs, and we have had to do various tweaks to the MP init. code. Having
a PCD to adjust this timeout will be very useful.

Thanks all for the feedbacks. Please grant me some time to prepare a new
version of the patch.

Best Regards,
Hao Wu



Thanks,
--
Brian J. Johnson
Enterprise X86 Lab

Hewlett Packard Enterprise
brian.johnson@...



[RFCv2] code-first process for UEFI-forum specifications

Leif Lindholm
 

Changes to v2 of this proposal:
- Feedback from Laszlo[a] and Mike[b] incorporated.
- I opted to view Mike's responses to Laszlo's questions as
accepted, as no follow-up was made.

Feedback from Felix[c] *not* incorporated, as while I agree with all
of it, I am not convinced that information should go here, but should
instead reside with the UEFI Forum. (This text documents the public
part of the process - it would cause me slight impedance mismatch to
have it also document the non-public part.)

[a] https://edk2.groups.io/g/devel/message/53422
[b] https://edk2.groups.io/g/devel/message/53738
[c] https://edk2.groups.io/g/devel/message/54453

/
Leif

---

This is a proposal for a process by which new features can be added to UEFI
forum specifications after first having been designed and prototyped in the
open.

This process lets changes and the development of new features happen in the
open, without violating the UEFI forum bylaws which prevent publication of
code for in-draft features/changes.

The process does not in fact change the UEFI bylaws - the change is that the
development (of both specification and code) happens in the open. The resulting
specification update is then submitted to the appropriate working goup as an
Engineering Change Request (ECR), and voted on. For the UEFI Forum, this is a
change in workflow, not a change in process.

ECRs are tracked in a UEFI Forum Mantis instance, access restricted to UEFI
Forum Members. TianoCore will enable this new process by providing areas on
https://bugzilla.tianocore.org/ to track both specification updates and
reference implementations and new repositories under
https://github.com/tianocore/ dedicated to hold "code first".


## Bugzilla

bugzilla.tianocore.org will have a product category each for
* ACPI Specification
* PI Specification
* UEFI Specification

Each product category will have a separate components for
* Specification
* Reference implementation


## Github
New repositories will be added for holding the text changes and the source code.

Specification text changes will be held within the affected source repository,
in the Github flavour of markdown, in a file (or split across several files)
with .md suffix.
(This one may break down where we have a specification change affecting multiple
specifications, but at that point we can track it with multiple BZ entries)

Reference implementations targeting EDK2 will be held in branches on
edk2-staging.
Additional repositories for implementing reference features in
additional open source projects can be added in the future, as required.


## Intended workflow
The entity initiating a specifiation update raises a Bugzilla in the appropriate
area in bugzilla.tianocore.org. This entry contains the outline of the change,
and the full initial draft text is attached.

If multiple specification updates are interdependent, especially if between
different specifications, then multiple bugzilla entries should be created.
These bugzilla entries *must* be linked together with dependencies.

After the BZs have been created, new branches should be created in the relevant
repositories for each bugzilla - the branch names should be BZ####, where ####
describes the bugzilla ID assigned, optionally followed by a '-' and something
more descriptive. If multiple bugzilla entries must coexist on a single branch,
one of them is designated the 'top-level', with dependencies properly tracked.
That BZ will be the one naming the branch.


## Source code
In order to ensure draft code does not accidentally leak into production use,
and to signify when the changeover from draft to final happens, *all* new or
modified[1] identifiers need to be prefixed with the relevant BZ####.

[1] Modified in a non-backwards-compatible way. If, for example, a statically
sized array is grown - this does not need to be prefixed. But a tag in a
comment would be *highly* recommended.

### File names
New public header files need the prefix. I.e. `Bz1234MyNewProtocol.h`
Private header files do not need the prefix.

### Contents

The tagging must follow the coding style used by each affected codebase.
Examples:

| Released in spec | Draft version in tree | Comment |
| --- | --- | --- |
| `FunctionName` | `Bz1234FunctionName` | |
| `HEADER_MACRO` | `BZ1234_HEADER_MACRO` | |

For data structures or enums, any new or non-backwards-compatible structs or
fields require a prefix. As above, growing an existing array in an existing
struct requires no prefix.

| `typedef SOME_STRUCT` | `BZ1234_SOME_STRUCT` | Typedef only [2] |
| `StructField` | `Bz1234StructField` | In existing struct[3] |
| `typedef SOME_ENUM` | `BZ1234_SOME_ENUM` | Typedef only [2] |

[2] If the struct or enum definition is separate from the typedef in the public
header, the definition does not need the prefix.
[3] Individual fields in newly added typedefd struct do not need prefix, the
struct already carried the prefix.

Variable prefixes indicating global scope ('g' or 'm') go before the BZ prefix.

| `gSomeGuid` | `gBz1234SomeGuid` | |

Local identifiers, including module-global ones (m-prefixed) do not require a
BZ prefix.


Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Ni, Ray
 

Laszlo,
Adding a PCD means platform integrators need to consider which value to set.
Most of the time, they may just use the default PCD value.
Then, why not we add the PCD later when a real case is met?
The patch changes existent behavior; it is not for a newly introduced
feature.

Because most platforms are not in the edk2 tree, we don't know what
platforms could be regressed by increasing the polling frequency
tenfold. (And remember that the polling action has O(n) cost, where "n"
is the logical processor count.)

Under your suggestion, the expression "real case is met" amounts to
"someone reports a regression" (possibly after the next stable tag,
even). I don't think that's a good idea.
In particular, the patch is motivated by RegisterCpuFeaturesLib -- the
CpuFeaturesInitialize() function -- on some platform(s) that Hao uses.
But there are platforms that don't use RegisterCpuFeaturesLib, and still
use MpInitLib.
OK. I agree with your suggestion.


Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Laszlo Ersek
 

On 03/16/20 02:37, Ni, Ray wrote:
The use case is valid, IMO. And the commit message is helpful.

But I really think this constant should be PCD. Here's why I think a
platform might want to control it:

- The best (finest) possible resolution for timer events is platform
dependent, IIUC. The duration of the "idle tick" is platform-specific.
And, it likely makes no sense to set AP_CHECK_INTERVAL to a duration
that's around, or under, what the arch timer resolution allows for.

- In the other direction, CheckAndUpdateApsStatus() contains a loop that
counts up to CpuMpData->CpuCount. In a very large system (hundreds or
maybe thousands of APs) this function may have non-negligible cost.

I suggest introducing a PCD for this (measured in msecs) and using 100
msecs as the default.
Laszlo,
Adding a PCD means platform integrators need to consider which value to set.
Most of the time, they may just use the default PCD value.
Then, why not we add the PCD later when a real case is met?
The patch changes existent behavior; it is not for a newly introduced
feature.

Because most platforms are not in the edk2 tree, we don't know what
platforms could be regressed by increasing the polling frequency
tenfold. (And remember that the polling action has O(n) cost, where "n"
is the logical processor count.)

Under your suggestion, the expression "real case is met" amounts to
"someone reports a regression" (possibly after the next stable tag,
even). I don't think that's a good idea.

In particular, the patch is motivated by RegisterCpuFeaturesLib -- the
CpuFeaturesInitialize() function -- on some platform(s) that Hao uses.
But there are platforms that don't use RegisterCpuFeaturesLib, and still
use MpInitLib.

Thanks,
Laszlo


Re: [edk2-devel] [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Ni, Ray
 

The use case is valid, IMO. And the commit message is helpful.

But I really think this constant should be PCD. Here's why I think a
platform might want to control it:

- The best (finest) possible resolution for timer events is platform
dependent, IIUC. The duration of the "idle tick" is platform-specific.
And, it likely makes no sense to set AP_CHECK_INTERVAL to a duration
that's around, or under, what the arch timer resolution allows for.

- In the other direction, CheckAndUpdateApsStatus() contains a loop that
counts up to CpuMpData->CpuCount. In a very large system (hundreds or
maybe thousands of APs) this function may have non-negligible cost.

I suggest introducing a PCD for this (measured in msecs) and using 100
msecs as the default.
Laszlo,
Adding a PCD means platform integrators need to consider which value to set.
Most of the time, they may just use the default PCD value.
Then, why not we add the PCD later when a real case is met?

Thanks,
Ray


Re: [RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Laszlo Ersek
 

Hi Hao,

On 03/13/20 14:22, Hao A Wu wrote:
This commit will reduce the interval of the AP status check event from 100
milliseconds to 10 milliseconds.

(I searched the history of the 100ms interval, it seems no comment or log
message was mentioned for the choice of this value. Looks like the value
is selected by experience.)

The purpose is to reduce the response time when the BSP calls below
EFI_MP_SERVICES_PROTOCOL services in a non-blocking manner:

* StartupAllAPs()
* StartupThisAP()

Reducing the check interval will benefit the performance for the case when
the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait for
AP(s) to complete the task, especially when the task can be finished
considerably fast on AP(s).

An example is within function CpuFeaturesInitialize() under
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
where BSP will perform the same task with APs and requires all the
processors to finish the task before BSP proceeds to its next task.

Impact:
A. The impact is minimal when there is no non-blocking calls of the
StartupAllAPs/StartupThisAp MP services, because the check function
CheckAndUpdateApsStatus() will return directly when there is no
registered wait event (i.e. no non-blocking request).

B. There will be a performance tradeoff when BSP continues to proceed
other tasks after submitting a non-blocking StartupAllAPs/StartupThisAP
request. If the AP status check takes a good portion of the shortened
interval, BSP will have less time slice working on its own task before
all the APs complete their tasks.

My investigation for Impact B is that it is a rare scenario in the edk2
code base.

Unitests:
A. OS boot successfully.
B. System (with 24 threads) boot time reduced. Almost all the saved time
comes from the above-mentioned case in CpuFeaturesInitialize().

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Hao A Wu <hao.a.wu@...>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index a987c32109..9ba886e8ed 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -15,7 +15,7 @@

#include <Protocol/Timer.h>

-#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (100))
+#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (10))
#define AP_SAFE_STACK_SIZE 128

CPU_MP_DATA *mCpuMpData = NULL;
The use case is valid, IMO. And the commit message is helpful.

But I really think this constant should be PCD. Here's why I think a
platform might want to control it:

- The best (finest) possible resolution for timer events is platform
dependent, IIUC. The duration of the "idle tick" is platform-specific.
And, it likely makes no sense to set AP_CHECK_INTERVAL to a duration
that's around, or under, what the arch timer resolution allows for.

- In the other direction, CheckAndUpdateApsStatus() contains a loop that
counts up to CpuMpData->CpuCount. In a very large system (hundreds or
maybe thousands of APs) this function may have non-negligible cost.

I suggest introducing a PCD for this (measured in msecs) and using 100
msecs as the default.

Thanks
Laszlo


[RFC][PATCH v1] UefiCpuPkg/MpInitLib DXE: Reduce AP status check interval

Wu, Hao A
 

This commit will reduce the interval of the AP status check event from 100
milliseconds to 10 milliseconds.

(I searched the history of the 100ms interval, it seems no comment or log
message was mentioned for the choice of this value. Looks like the value
is selected by experience.)

The purpose is to reduce the response time when the BSP calls below
EFI_MP_SERVICES_PROTOCOL services in a non-blocking manner:

* StartupAllAPs()
* StartupThisAP()

Reducing the check interval will benefit the performance for the case when
the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait for
AP(s) to complete the task, especially when the task can be finished
considerably fast on AP(s).

An example is within function CpuFeaturesInitialize() under
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
where BSP will perform the same task with APs and requires all the
processors to finish the task before BSP proceeds to its next task.

Impact:
A. The impact is minimal when there is no non-blocking calls of the
StartupAllAPs/StartupThisAp MP services, because the check function
CheckAndUpdateApsStatus() will return directly when there is no
registered wait event (i.e. no non-blocking request).

B. There will be a performance tradeoff when BSP continues to proceed
other tasks after submitting a non-blocking StartupAllAPs/StartupThisAP
request. If the AP status check takes a good portion of the shortened
interval, BSP will have less time slice working on its own task before
all the APs complete their tasks.

My investigation for Impact B is that it is a rare scenario in the edk2
code base.

Unitests:
A. OS boot successfully.
B. System (with 24 threads) boot time reduced. Almost all the saved time
comes from the above-mentioned case in CpuFeaturesInitialize().

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Star Zeng <star.zeng@...>
Signed-off-by: Hao A Wu <hao.a.wu@...>
---
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index a987c32109..9ba886e8ed 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -15,7 +15,7 @@

#include <Protocol/Timer.h>

-#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (100))
+#define AP_CHECK_INTERVAL (EFI_TIMER_PERIOD_MILLISECONDS (10))
#define AP_SAFE_STACK_SIZE 128

CPU_MP_DATA *mCpuMpData = NULL;
--
2.12.0.windows.1


Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Samer El-Haj-Mahmoud
 

Thanks Leif.

As far as I know, the main feedback I heard is "when will this start?"... So, the sooner the better .. Thanks for taking the lead and driving!

-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: Wednesday, March 11, 2020 12:03 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Cc: devel@edk2.groups.io; Felixp@...; rfc@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Hi Samer,

I had, perhaps excessively, been waiting for more feedback.

I promised Mike yesterday that I will rework based on feedback and send out next week at the latest. If people have no further comments then, we can adopt the process and start using it.

Regards,

Leif

On Wed, Mar 11, 2020 at 15:52:07 +0000, Samer El-Haj-Mahmoud wrote:
Has there been any progress on this "code-first process" proposal? Any timeline on when we should expect it to be launched?

Thanks,
--Samer


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Felix
Polyudov via Groups.Io
Sent: Friday, February 14, 2020 10:30 AM
To: rfc@edk2.groups.io; 'leif@...' <leif@...>;
devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for
UEFI-forum specifications

Leif,

The process does not in fact change the UEFI bylaws - the change is
that the development (of both specification and code) happens in the
open. The resulting specification update is then submitted to the
appropriate working goup as an Engineering Change Request (ECR), and
voted on. For the UEFI Forum, this is a change in workflow, not a change in process.
I think it would be good to add more details regarding the interaction between edk2 and UEFI forum.
Here is what I suggest:
Each specification update Bugzilla ticket must have a sponsor. A sponsor is a person or a company that will be presenting change request to the UEFI forum.
A sponsor has to be identified early in the process. Preferably along with Buzilla ticket creation.
It is sponsor's responsibility to officially submit ECR to the UEFI forum by creating a mantis ticket with a Bugzilla link.
There are two reasons to create mantis ticket early in the process:
- Creation of the ticket exposes the effort to the UEFI forum thus enabling early feedback from the members(via Bugzilla), which may reduce number of iterations in the
implement --> get feedback --> re-implement cycle.
- edk2 effort will be taken into consideration while scheduling future
specification releases


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.



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


Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Samer El-Haj-Mahmoud
 

Has there been any progress on this "code-first process" proposal? Any timeline on when we should expect it to be launched?

Thanks,
--Samer

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Felix Polyudov via Groups.Io
Sent: Friday, February 14, 2020 10:30 AM
To: rfc@edk2.groups.io; 'leif@...' <leif@...>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Leif,

The process does not in fact change the UEFI bylaws - the change is
that the development (of both specification and code) happens in the
open. The resulting specification update is then submitted to the
appropriate working goup as an Engineering Change Request (ECR), and
voted on. For the UEFI Forum, this is a change in workflow, not a change in process.
I think it would be good to add more details regarding the interaction between edk2 and UEFI forum.
Here is what I suggest:
Each specification update Bugzilla ticket must have a sponsor. A sponsor is a person or a company that will be presenting change request to the UEFI forum.
A sponsor has to be identified early in the process. Preferably along with Buzilla ticket creation.
It is sponsor's responsibility to officially submit ECR to the UEFI forum by creating a mantis ticket with a Bugzilla link.
There are two reasons to create mantis ticket early in the process:
- Creation of the ticket exposes the effort to the UEFI forum thus enabling early feedback from the members(via Bugzilla), which may reduce number of iterations in the
implement --> get feedback --> re-implement cycle.
- edk2 effort will be taken into consideration while scheduling future specification releases


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.



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.


Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Doran, Mark <mark.doran@...>
 

The one thing we don't have in hand for this is a template into which we can put prospective ECR submission material that is collected as part of the code-first process.

We could just use the ECR template that is used for regular proposals with the UEFI Work Groups. However, the thought was to add some verbiage that template which effectively says: anything put in the proposed ECR template comes with permission to give ownership and rights in that content to the UEFI Forum so that it can publish the material as part of the specs under the same rights as all the other spec material there.

Since TianoCore doesn't have a legal team per se, we're having Intel legal work something like that with a view to contributing that to the community for review and hopefully adoption in short order.

That said, I don't think we need to wait on the template to begin using a code-first approach for things we would ultimately like to end up in the formal specs once code is "done". We really only need that template at the point that the ECR content is also done, meaning reflective of sufficiently completed code work.

If we do start assembling content that looks like ECR fodder in parallel with code development, I'd suggest that we keep track of who is putting words into that fodder so that when it gets moved into the template prior to formal submission as an ECR we can get assurance from those people that they are OK with the T's & C's in the template.

Fundamentally we just need to make sure that everyone contributing to the development of one of these code first things is OK with spec description stuff being incorporated into a UEFI document under UEFI T's & C's as the end product. UEFI T's & C's are designed to make sure that anyone can read the spec just for the asking and that there are no undesired IP complications (as there have been in some other standards) for people that choose to implement to the specs.

--
Cheers,

Mark.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer El-Haj-Mahmoud
Sent: Wednesday, March 11, 2020 9:32 AM
To: Leif Lindholm <leif@...>
Cc: devel@edk2.groups.io; Felixp@...; rfc@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Thanks Leif.

As far as I know, the main feedback I heard is "when will this start?"... So, the sooner the better .. Thanks for taking the lead and driving!



-----Original Message-----
From: Leif Lindholm <leif@...>
Sent: Wednesday, March 11, 2020 12:03 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Cc: devel@edk2.groups.io; Felixp@...; rfc@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Hi Samer,

I had, perhaps excessively, been waiting for more feedback.

I promised Mike yesterday that I will rework based on feedback and send out next week at the latest. If people have no further comments then, we can adopt the process and start using it.

Regards,

Leif

On Wed, Mar 11, 2020 at 15:52:07 +0000, Samer El-Haj-Mahmoud wrote:
Has there been any progress on this "code-first process" proposal? Any timeline on when we should expect it to be launched?

Thanks,
--Samer


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Felix
Polyudov via Groups.Io
Sent: Friday, February 14, 2020 10:30 AM
To: rfc@edk2.groups.io; 'leif@...' <leif@...>;
devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for
UEFI-forum specifications

Leif,

The process does not in fact change the UEFI bylaws - the change is
that the development (of both specification and code) happens in the
open. The resulting specification update is then submitted to the
appropriate working goup as an Engineering Change Request (ECR), and
voted on. For the UEFI Forum, this is a change in workflow, not a change in process.
I think it would be good to add more details regarding the interaction between edk2 and UEFI forum.
Here is what I suggest:
Each specification update Bugzilla ticket must have a sponsor. A sponsor is a person or a company that will be presenting change request to the UEFI forum.
A sponsor has to be identified early in the process. Preferably along with Buzilla ticket creation.
It is sponsor's responsibility to officially submit ECR to the UEFI forum by creating a mantis ticket with a Bugzilla link.
There are two reasons to create mantis ticket early in the process:
- Creation of the ticket exposes the effort to the UEFI forum thus enabling early feedback from the members(via Bugzilla), which may reduce number of iterations in the
implement --> get feedback --> re-implement cycle.
- edk2 effort will be taken into consideration while scheduling future
specification releases


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.



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


Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Leif Lindholm
 

Hi Samer,

I had, perhaps excessively, been waiting for more feedback.

I promised Mike yesterday that I will rework based on feedback and
send out next week at the latest. If people have no further comments
then, we can adopt the process and start using it.

Regards,

Leif

On Wed, Mar 11, 2020 at 15:52:07 +0000, Samer El-Haj-Mahmoud wrote:
Has there been any progress on this "code-first process" proposal? Any timeline on when we should expect it to be launched?

Thanks,
--Samer


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Felix Polyudov via Groups.Io
Sent: Friday, February 14, 2020 10:30 AM
To: rfc@edk2.groups.io; 'leif@...' <leif@...>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [RFC] code-first process for UEFI-forum specifications

Leif,

The process does not in fact change the UEFI bylaws - the change is
that the development (of both specification and code) happens in the
open. The resulting specification update is then submitted to the
appropriate working goup as an Engineering Change Request (ECR), and
voted on. For the UEFI Forum, this is a change in workflow, not a change in process.
I think it would be good to add more details regarding the interaction between edk2 and UEFI forum.
Here is what I suggest:
Each specification update Bugzilla ticket must have a sponsor. A sponsor is a person or a company that will be presenting change request to the UEFI forum.
A sponsor has to be identified early in the process. Preferably along with Buzilla ticket creation.
It is sponsor's responsibility to officially submit ECR to the UEFI forum by creating a mantis ticket with a Bugzilla link.
There are two reasons to create mantis ticket early in the process:
- Creation of the ticket exposes the effort to the UEFI forum thus enabling early feedback from the members(via Bugzilla), which may reduce number of iterations in the
implement --> get feedback --> re-implement cycle.
- edk2 effort will be taken into consideration while scheduling future specification releases


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.



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.


Re: [RFC MdeModulePkg/Variable v1 0/2] Fix two issue in variable

Ming Huang
 

Any comment?

On 2/15/2020 4:54 PM, Ming Huang wrote:
There are two infrequent issues in variable.

Ming Huang (2):
MdeModulePkg/Variable: Remove some debug print for runtime
MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable

.../Universal/Variable/RuntimeDxe/Variable.c | 44 ++++++++++----------
1 file changed, 21 insertions(+), 23 deletions(-)


Re: [RFC] code-first process for UEFI-forum specifications

Felix Polyudov
 

Leif,

The process does not in fact change the UEFI bylaws - the change is that the
development (of both specification and code) happens in the open. The resulting
specification update is then submitted to the appropriate working goup as an
Engineering Change Request (ECR), and voted on. For the UEFI Forum, this is a
change in workflow, not a change in process.
I think it would be good to add more details regarding the interaction between edk2 and UEFI forum.
Here is what I suggest:
Each specification update Bugzilla ticket must have a sponsor. A sponsor is a person or a company that will be presenting change request to the UEFI forum.
A sponsor has to be identified early in the process. Preferably along with Buzilla ticket creation.
It is sponsor's responsibility to officially submit ECR to the UEFI forum by creating a mantis ticket with a Bugzilla link.
There are two reasons to create mantis ticket early in the process:
- Creation of the ticket exposes the effort to the UEFI forum thus enabling early feedback from the members(via Bugzilla), which may reduce number of iterations in the
implement --> get feedback --> re-implement cycle.
- edk2 effort will be taken into consideration while scheduling future specification releases


Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


Re: Q: Side effects of incrementing gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size?

Aaron Young
 

Thanks Laszlo! Very helpful.

-Aaron

On 02/11/2020 01:15 AM, Laszlo Ersek wrote:
-fw_cfg name=opt/ovmf/X-PciMmio64Mb,string=65536


Re: Q: Side effects of incrementing gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size?

Laszlo Ersek
 

Hi Aaron,

On 02/10/20 21:15, aaron.young@... wrote:

 Hello, After adding some GPU cards to our system (each containing a
16GB BAR), we ran out of MEM64 BAR space as indicated by the following
OVMF DEBUG output:

--------
[...]
-----------

 Incrementing gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size from the
default value (0x800000000) to 0x2000000000 fixed the issue and should
allow enough space to add several (7 or so) GPUs to the system.

 However, we are concerned about side-effects of such a change.

 So, my question is: Could incrementing PcdPciMmio64Size cause any
negative side effects?

 Thanks in advance for any help/comments/suggestions...
Please refer to the following thread:

* [edk2-discuss]
[OVMF] resource assignment fails for passthrough PCI GPU

https://edk2.groups.io/g/discuss/message/59

The gist is that

(a) you can increase the 64-bit MMIO aperture without rebuilding OVMF,
like this (on the QEMU command line):

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

(This sets a 64GiB aperture)

(b) Implementing such a change in the DSC file(s) statically is not
recommended, as it could prevent guests from booting on hosts that
support EPT (= nested paging on Intel CPUs) but only support 36 physical
address bits.

(c) Sizing the aperture in OVMF dynamically to whatever the host CPU's
phys address width supports would also not be ideal, due to guest RAM
consumption and possible VM migration problems. QEMU doesn't even report
the host CPU's phys address width accurately to the guest, at the
moment, AIUI.

Thanks
Laszlo


Q: Side effects of incrementing gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size?

Aaron Young
 

Hello, After adding some GPU cards to our system (each containing a 16GB BAR), we ran out of MEM64 BAR space as indicated by the following OVMF DEBUG output:

--------
PciHostBridge: SubmitResources for PciRoot(0x0)
I/O: Granularity/SpecificFlag = 0 / 01
Length/Alignment = 0x1000 / 0xFFF
Mem: Granularity/SpecificFlag = 32 / 00
Length/Alignment = 0x3100000 / 0xFFFFFF
Mem: Granularity/SpecificFlag = 64 / 00
Length/Alignment = 0x804000000 / 0x3FFFFFFFF
PciBus: HostBridge->SubmitResources() - Success
PciHostBridge: NotifyPhase (AllocateResources)
RootBridge: PciRoot(0x0)
* Mem64: Base/Length/Alignment =**
**FFFFFFFFFFFFFFFF/804000000/3FFFFFFFF - Out Of**
**Resource!*
Mem: Base/Length/Alignment = C0000000/3100000/FFFFFF - Success
I/O: Base/Length/Alignment = C000/1000/FFF - Success
Call PciHostBridgeResourceConflict().
PciHostBridge: Resource conflict happens!
RootBridge[0]:
I/O: Length/Alignment = 0x1000 / 0xFFF
Mem: Length/Alignment = 0x3100000 / 0xFFFFFF
Granularity/SpecificFlag = 32 / 00
Mem: Length/Alignment = 0x0 / 0x0
Granularity/SpecificFlag = 32 / 06 (Prefetchable)
Mem: Length/Alignment = 0x804000000 / 0x3FFFFFFFF
Granularity/SpecificFlag = 64 / 00
Mem: Length/Alignment = 0x0 / 0x0
Granularity/SpecificFlag = 64 / 06 (Prefetchable)
Bus: Length/Alignment = 0x1 / 0x0
PciBus: HostBridge->NotifyPhase(AllocateResources) -
Out of Resources

-----------

Incrementing gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size from the default value (0x800000000) to 0x2000000000 fixed the issue and should allow enough space to add several (7 or so) GPUs to the system.

However, we are concerned about side-effects of such a change.

So, my question is: Could incrementing PcdPciMmio64Size cause any negative side effects?

Thanks in advance for any help/comments/suggestions...

-Aaron Young


Enhancement to CryptoPkg/BaseHashApiLib

Sukerkar, Amol N
 

Hi Mike, Jian and Jiewen,

Based on our discussion, I have compiled the enhancements/changes to be made to BaseHashApiLib instance in CryptoPkg in the Bugzilla, https://bugzilla.tianocore.org/show_bug.cgi?id=2511.

Please go over the changes and provide comments/feedback. Once I have confirmation that I have captured all the identified changes, I will begin implementation.

Thanks,
Amol


Re: [RFC] VariablePolicy - Protocol, Libraries, and Implementation for VariableLock Alternative

Wang, Sunny (HPS SW)
 

Hi Bret,

Your proposal looks good to me, and most of my questions/concerns were already answered/solved by the GOOD discussion between you and Jiewen.

For now, I just have one remaining question. Can we also make DumpVariablePolicy as a platform choice?
DumpVariablePolicy would also expose the information about how to pass the check to the attacker. If my understanding is correct, the attacker can even use the information to unlock the variable locked by LOCK_ON_VAR_STATE type policy. Therefore, If the DumpVariablePolicy is just to allow platform tests to audit the entire policy list, can we add a PCD or a build flag check to disable it in release build? Of course, I also agree with Jiewen's point about making DisableVarPolicy as a platform choice.

By the way, I had the other question that I already got the answer from the code and your document. Since it hasn't been discussed, I think it would be good to share the answer with others. The question is how VariablePolicy deals with the multiple RegisterVariablePolicy calls with the same variable name and GUID. The answer is that the first registered policy will be the winner. The later calls (from attackers) with the same name and GUID will just get EFI_ALREADY_STARTED and fail to register the compromised policy. Therefore, RegisterVariablePolicy looks good to me as well.


Regards,
Sunny Wang

-----Original Message-----
From: rfc@edk2.groups.io [mailto:rfc@edk2.groups.io] On Behalf Of Bret Barkelew via Groups.Io
Sent: Wednesday, January 29, 2020 9:36 AM
To: rfc@edk2.groups.io
Subject: [edk2-rfc] [RFC] VariablePolicy - Protocol, Libraries, and Implementation for VariableLock Alternative

All,

VariablePolicy is our proposal for an expanded "VarLock-like" interface to constrain and govern platform variables.
I brought this up back in May to get initial comments on the interface and implications of the interface and the approach. We implemented it in Mu over the summer and it is not our defacto variable solution. It plugs in easily to the existing variable infrastructure, but does want to control some of the things that are currently managed by VarLock.

There are also some tweaks that would be needed if this were desired to be 100% optional code, but that's no different than the current VarLock implementation which has implementation code directly tied to some of the common variable code.

I've structured this RFC in two pieces:

* The Core piece represents the minimum changes needed to implement Variable Policy and integrate it into Variable Services. It contains core driver code, central libraries and headers, and DXE driver for the protocol interface.
* The Extras piece contains recommended code for a full-feature implementation including a replacement for the VarLock protocol that enables existing code to continue functioning as-is. It also contains unit and integration tests. And as a bonus, it has a Rust implementation of the core business logic for Variable Policy.

The code can be found in the following two branches:
https://github.com/corthon/edk2/tree/personal/brbarkel/var_policy_rfc_core
https://github.com/corthon/edk2/tree/personal/brbarkel/var_policy_rfc_extra

A convenient way to see all the changes in one place is to look at a comparison:
https://github.com/corthon/edk2/compare/master...corthon:personal/brbarkel/var_policy_rfc_core
https://github.com/corthon/edk2/compare/personal/brbarkel/var_policy_rfc_core...corthon:personal/brbarkel/var_policy_rfc_extra

There's additional documentation in the PPT and DOC files in the core branch:
https://github.com/corthon/edk2/blob/personal/brbarkel/var_policy_rfc_core/RFC%20VariablePolicy%20Proposal%20Presentation.pptx https://github.com/corthon/edk2/blob/personal/brbarkel/var_policy_rfc_core/RFC%20VariablePolicy%20Whitepaper.docx
(You'd need to download those to view.)

My ultimate intention for this is to submit it as a series of patches for acceptance into EDK2 as a replacement for VarLock. For now, I'm just looking for initial feedback on any broad changes that might be needed to get this into shape for more detailed code review on the devel list.

Thanks!

- Bret


Re: [edk2-devel] [RFC] VariablePolicy - Protocol, Libraries, and Implementation for VariableLock Alternative

Yao, Jiewen
 

Add devel@edk2.groups.io.

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@...>
Sent: Friday, February 7, 2020 6:13 AM
To: brbarkel@...; rfc@edk2.groups.io
Cc: edk2-devel@...
Subject: RE: [edk2-rfc] [edk2-devel] [RFC] VariablePolicy - Protocol, Libraries,
and Implementation for VariableLock Alternative

Thanks, Bret.

Comments below.

-----Original Message-----
From: brbarkel via [] <brbarkel=microsoft.com@[]>
Sent: Friday, February 7, 2020 5:49 AM
To: Yao; Yao, Jiewen <jiewen.yao@...>; rfc@edk2.groups.io
Subject: Re: [edk2-rfc] [edk2-devel] [RFC] VariablePolicy - Protocol, Libraries,
and Implementation for VariableLock Alternative

Sorry, Jiewen. I missed this email when it came in...

1. We have 2 variable related protocol - EDKII_VARIABLE_LOCK_PROTOCOL
and EDKII_VAR_CHECK_PROTOCOL. Do you want to deprecate both? Or only
deprecate EDKII_VARIABLE_LOCK_PROTOCOL?

I think we would recommend deprecating both. Is there much consumption of
the VarCheck protocol? A platform could always opt to include both (or all 3!),
but they wouldn't be able to immediately tell which engine rejected the
SetVariable.
[Jiewen] Right. That is my understanding- deprecate both.
I saw you only mention to deprecate VarLock, but not VarCheck. That is why I
get confused.

I don’t recommend we have 3 engines. Too burden to maintain the code.

One possible option is to implement VarLockProtocol and VarCheckProtocol on
top of VarPolicyProtocol. (like a thunk)
As such, there is no impact on the existing private platform code, with the
benefit that one central engine controls everything.
Later, we can check platform one by one. After the last one is migrated, we can
remove VarLock and VarCheck thunk.




2. The Function - DumpVariablePolicy() - makes me confused in the
beginning.
In my thought, "Dump" means to show some debug message. But here, you
want to return the policy. Can we change the name to GetVariablePolicy()?

I see your point about possible confusion. I think we would try to clarify this in
the function documentation. Due to the code-first approach we've taken on
this,
we already have partners who have implemented the policy with
DumpVariablePolicy as the name and it doesn't seem like a huge problem.

3. The function - DisableVariablePolicy(). Does it disable current policy
engine?
Does it disable any future policy engine? Does it block RegisterVariablePolicy()
call?

Disable turns off the enforcement. You should still be able to register new
policies for auditing purposes (they will still be returned by
DumpVariablePolicy),
but no SetVariables will be rejected.

4. The function - LockVariablePolicy() - Can it lock the DisableVariablePolicy()
call?

Correct. Once the interface is locked, it cannot be disabled. Locking should be
performed at EndOfDxe or ReadyToBoot or wherever the platform TCB is.

5. The use case "In MFG Mode Variable Policy Engine is disabled, thus these
VPD variables can be created. These variables are locked with lock policy type
LockNow, so that these variables can't be tampered with in Customer Mode."

Correct. The MfgMode is just a suggestion and is entirely up to the platform.
Our
MfgMode (which has not been open-sourced yet, but we're working on it) will
call DisableVariablePolicy because our threat model indicates that
compromising
MfgMode is a total compromise (there are many other attack vectors once
MfgMode is compromised). As such, we protect it extensively. But there is
nothing in the core or extra code that would automatically disable the policy
engine for any platform that didn't want that. It is up to the platform, however,
to make sure they lock the policy engine (similar to locking SMM).
[Jiewen] OK, if my understanding is correct, the DisablePolicy() should only be
called in some special environment, but NOT a normal feature involved in the
normal boot.

Then I am feeling we should separate the DisablePolicy from this protocol,
because this interface is very dangerous. I don’t want any driver call it by
mistake.

Can we split the VARIABLE_POLICY to VARIABLE_POLICY (normal boot usage) +
VARIABLE_POLICY_CONTROL (very special usage)

Like below:

typedef struct {
UINT64 Revision;
REGISTER_VARIABLE_POLICY RegisterVariablePolicy;
DUMP_VARIABLE_POLICY DumpVariablePolicy;
LOCK_VARIABLE_POLICY LockVariablePolicy;
} _VARIABLE_POLICY_PROTOCOL;

typedef struct {
UINT64 Revision;
DISABLE_VARIABLE_POLICY DisableVariablePolicy;
IS_VARIABLE_POLICY_ENABLED IsVariablePolicyEnabled;
} _VARIABLE_POLICY_CONTROL_PROTOCOL;

VARIABLE_POLICY_PROTOCOL is always installed.

In normal boot, the VARIABLE_POLICY_CONTROL_PROTOCOL is NOT installed.
As such, the policy is always enforced.
The VARIABLE_POLICY_CONTROL_PROTOCOL is only installed in some special
mode, controlled by PCD - maybe.
A platform may choose to:
1) Use static PCD to always disable VARIABLE_POLICY_CONTROL_PROTOCOL
installation.
2) Use dynamic PCD to control VARIABLE_POLICY_CONTROL_PROTOCOL
installation only in special mode.


Thank you
Yao Jiewen

521 - 540 of 780