Date   

Re: MemoryFence()

Ethin Probst
 

I agree with Ard (I'm not a contributor of EDK2 but thought I'd add my
2 sense anyway). `volatile` should only be used if a pointer
load/store is utilized in a context where you expect the external
environment (e.g.: hardware) to alter the result of such an op. In the
case of fences, volatile, I think, doesn't really do much in terms of
helping. For fences x86 has the LFENCE instruction for load fences and
SFENCE for store fences. If I'm not mistaken, MFENCE, then, is just a
combination of the other two. Would it not then be more appropriate to
use an LFENCE for a load (release?) fence, and SFENCE for a store
(acquire?) fence? (I ask for release/acquire because I might have
these mixed up.) I might of course be entirely wrong too.
I'd love to, perhaps, become a contributor of EDK2 sometime, but I
honestly have no idea where I'd even begin. :-)

On 2/4/21, Ard Biesheuvel <ardb@...> wrote:
On Thu, 4 Feb 2021 at 19:09, Laszlo Ersek <lersek@...> wrote:

Hi All,

here's the declaration of MemoryFence(), from <BaseLib.h>:

/**
Used to serialize load and store operations.

All loads and stores that proceed calls to this function are
guaranteed to be
globally visible when this function returns.

**/
VOID
EFIAPI
MemoryFence (
VOID
);
The IA32 and X64 implementations of MemoryFence() are not memory
barriers, only compiler barriers, however.

This is a problem for two reasons:

- code that believes to add a memory barrier only adds a compiler
barrier, possibly creating an unsafe situation,

- code that knowingly calls MemoryFence() for the purposes of a compiler
barrier -- while it will hopefully work safely, relying on tacit
knowledge / assumptions about the x86 architecture -- results in a
source reading experience that mismatches known lockless patterns.


This topic has repeatedly come up in the past; most recently in the
review of Ankur's v6 series for
<https://bugzilla.tianocore.org/show_bug.cgi?id=3132>, where some
inter-processor communication needs to happen.

I asked Paolo for help with understanding some of the synchronziation
patterns in PiSmmCpuDxeSmm, the reason being that Ankur's series is
going to rely on, and/or imitate, some of those patterns. From this
discussion with Paolo, the following have emerged (I'm going to
liberally steal / paraphrase Paolo's words):


(1) We should introduce finer-grained fence primitives:

ARM AARCH64 i386

CompilerFence() asm("") asm("") asm("")
AcquireMemoryFence() dmb ish dmb ishld asm("")
ReleaseMemoryFence() dmb ish dmb ish asm("")
MemoryFence() dmb ish dmb ish mfence

"where AcquireMemoryFence() is used on the read side (i.e. between
reads) and ReleaseMemoryFence() is used on the write side (i.e. between
writes)".
Acquire semantics typically order writes before reads, not /between/
reads. Similarly, release semantics imply that all outstanding writes
complete before a barrier with acquire semantics is permitted to
complete.

Note that reasoning about this only makes sense in the context of
concurrency, i.e., different CPUs operating on the same memory (or
coherent DMA masters)

For non-coherent DMA, the 'ish' variants are not appropriate, and
given the low likelihood that any of this is creates a performance
bottleneck, I would suggest to only use full barriers on ARM.

(Note that this step would make MemoryFence() heavier-weight on x86 than
it currently is.)


(2) We should audit and rework existent uses (combinations) of
MemoryFence() into matched acquire / release pairs.
It is very important to be *aware* of the acquire/release semantics,
but I don't think it is necessary to use all the fine grained barrier
types in EDK2.

Less importantly, this would restore the x86 behavior (performance) to
the one seen with the current MemoryFence() implementation.

More importantly, it would fix what the code *means*: "it's very
important to stick to known lockless patterns, and having matching
acquire/release fences makes it much more intuitive to understand what's
going on".

--*--

I'm not proposing this as a pre-requisite to merging Ankur's series
(i.e., I don't expect the series to use acquire/release -- it can be
converted later with the rest of the audit).

However, I'd really like us to stop wasting more time on MemoryFence()
doubts such as:

- Hey it's not a memory fence at all, is it safe?
Who decides what 'memory fence' means anyway?

- But on x86 it kinda is, *dependent on* the spinlocks / lock prefixes
we use for synchronization, between the other accesses.

- Is it interchangeable with volatile? Yes? No? which one should we use?
'volatile' affects code generation only, i.e., which loads and stores
are issued. The *order* in which those loads observe stores done by
other masters is what is managed by barrier instructions. So they are
two sides of the same coin: 'volatile' may help to guarantee that
every assignment through a pointer variable results in a store
instruction, but how these are observed by other CPUs still needs to
be managed if it needs to occur in a certain way.

I think volatile is the wrong tool in most cases: the ordering of
accesses are not a property of the type or of the variable, but of the
code sequence. So using asm("") to ensure that an assignment through a
pointer is emitted as a store instruction, followed by a barrier
instruction is a typical pattern. And given the fact that barrier
instructions require asm sequences in most cases anyway, the asm ("")
will often be implied.


In the longer term, multi-processing code like MpInitLib (CpuDxe,
CpuMpPei) and PiSmmCpuDxeSmm should likely abandon all use of
"volatile", and use the explicit barriers instead, for *clarity*.


Step (2) would take quite a bit of thinking (if not much code).

Comments?

Would there be interest in reviewing such work?

Thanks
Laszlo




--
Signed,
Ethin D. Probst


Re: MemoryFence()

Paolo Bonzini <pbonzini@...>
 

Il gio 4 feb 2021, 20:46 Ard Biesheuvel <ardb@...> ha scritto:

(1) We should introduce finer-grained fence primitives:

ARM AARCH64 i386

CompilerFence() asm("") asm("") asm("")
AcquireMemoryFence() dmb ish dmb ishld asm("")
ReleaseMemoryFence() dmb ish dmb ish asm("")
MemoryFence() dmb ish dmb ish mfence

"where AcquireMemoryFence() is used on the read side (i.e. between
reads) and ReleaseMemoryFence() is used on the write side (i.e. between
writes)".
Acquire semantics typically order writes before reads, not /between/
reads. Similarly, release semantics imply that all outstanding writes
complete before a barrier with acquire semantics is permitted to
complete.
Acquire fences are barriers between earlier loads and subsequent loads and
stores; those earlier loads then synchronize with release stores in other
threads.

Release fences are barriers been earlier loads and stores against
subsequent stores, and those subsequent stores synchronize with acquire
loads in other threads.

In both cases, however, fences only make sense between memory operations.
So something like "after reads" and "before writes" would have been more
precise in some sense, but in practice the usual idiom is "between"
reads/writes as Laszlo wrote.

Note that reasoning about this only makes sense in the context of
concurrency, i.e., different CPUs operating on the same memory (or
coherent DMA masters)

For non-coherent DMA, the 'ish' variants are not appropriate, and
given the low likelihood that any of this is creates a performance
bottleneck, I would suggest to only use full barriers on ARM.
Sure, that's a matter of how to implement the primitives. If you think that
non-coherent DMA is important, a full dmb can be used too.

As far as the compiler is concerned, an asm in the macros *should* block
enough optimizations, even without making the accesses volatile.
CompilerFence (or the edk2 equivalent of cpu_relax, whose name escapes me
right now) would be necessary in the body of busy-waiting loops. However
somebody should check the MSVC docs for asm, too.

It is very important to be *aware* of the acquire/release semantics,
but I don't think it is necessary to use all the fine grained barrier
types in EDK2.
I agree as long as the primitives are self-documenting. A single
MemoryFence() does not make it clear in which direction the data is flowing
(whether from other threads to this one, or vice versa).

Paolo


Less importantly, this would restore the x86 behavior (performance) to
the one seen with the current MemoryFence() implementation.

More importantly, it would fix what the code *means*: "it's very
important to stick to known lockless patterns, and having matching
acquire/release fences makes it much more intuitive to understand what's
going on".

--*--

I'm not proposing this as a pre-requisite to merging Ankur's series
(i.e., I don't expect the series to use acquire/release -- it can be
converted later with the rest of the audit).

However, I'd really like us to stop wasting more time on MemoryFence()
doubts such as:

- Hey it's not a memory fence at all, is it safe?
Who decides what 'memory fence' means anyway?

- But on x86 it kinda is, *dependent on* the spinlocks / lock prefixes
we use for synchronization, between the other accesses.

- Is it interchangeable with volatile? Yes? No? which one should we use?
'volatile' affects code generation only, i.e., which loads and stores
are issued. The *order* in which those loads observe stores done by
other masters is what is managed by barrier instructions. So they are
two sides of the same coin: 'volatile' may help to guarantee that
every assignment through a pointer variable results in a store
instruction, but how these are observed by other CPUs still needs to
be managed if it needs to occur in a certain way.

I think volatile is the wrong tool in most cases: the ordering of
accesses are not a property of the type or of the variable, but of the
code sequence. So using asm("") to ensure that an assignment through a
pointer is emitted as a store instruction, followed by a barrier
instruction is a typical pattern. And given the fact that barrier
instructions require asm sequences in most cases anyway, the asm ("")
will often be implied.


In the longer term, multi-processing code like MpInitLib (CpuDxe,
CpuMpPei) and PiSmmCpuDxeSmm should likely abandon all use of
"volatile", and use the explicit barriers instead, for *clarity*.


Step (2) would take quite a bit of thinking (if not much code).

Comments?

Would there be interest in reviewing such work?

Thanks
Laszlo


Re: MemoryFence()

Ard Biesheuvel <ardb@...>
 

On Thu, 4 Feb 2021 at 19:09, Laszlo Ersek <lersek@...> wrote:

Hi All,

here's the declaration of MemoryFence(), from <BaseLib.h>:

/**
Used to serialize load and store operations.

All loads and stores that proceed calls to this function are guaranteed to be
globally visible when this function returns.

**/
VOID
EFIAPI
MemoryFence (
VOID
);
The IA32 and X64 implementations of MemoryFence() are not memory
barriers, only compiler barriers, however.

This is a problem for two reasons:

- code that believes to add a memory barrier only adds a compiler
barrier, possibly creating an unsafe situation,

- code that knowingly calls MemoryFence() for the purposes of a compiler
barrier -- while it will hopefully work safely, relying on tacit
knowledge / assumptions about the x86 architecture -- results in a
source reading experience that mismatches known lockless patterns.


This topic has repeatedly come up in the past; most recently in the
review of Ankur's v6 series for
<https://bugzilla.tianocore.org/show_bug.cgi?id=3132>, where some
inter-processor communication needs to happen.

I asked Paolo for help with understanding some of the synchronziation
patterns in PiSmmCpuDxeSmm, the reason being that Ankur's series is
going to rely on, and/or imitate, some of those patterns. From this
discussion with Paolo, the following have emerged (I'm going to
liberally steal / paraphrase Paolo's words):


(1) We should introduce finer-grained fence primitives:

ARM AARCH64 i386

CompilerFence() asm("") asm("") asm("")
AcquireMemoryFence() dmb ish dmb ishld asm("")
ReleaseMemoryFence() dmb ish dmb ish asm("")
MemoryFence() dmb ish dmb ish mfence

"where AcquireMemoryFence() is used on the read side (i.e. between
reads) and ReleaseMemoryFence() is used on the write side (i.e. between
writes)".
Acquire semantics typically order writes before reads, not /between/
reads. Similarly, release semantics imply that all outstanding writes
complete before a barrier with acquire semantics is permitted to
complete.

Note that reasoning about this only makes sense in the context of
concurrency, i.e., different CPUs operating on the same memory (or
coherent DMA masters)

For non-coherent DMA, the 'ish' variants are not appropriate, and
given the low likelihood that any of this is creates a performance
bottleneck, I would suggest to only use full barriers on ARM.

(Note that this step would make MemoryFence() heavier-weight on x86 than
it currently is.)


(2) We should audit and rework existent uses (combinations) of
MemoryFence() into matched acquire / release pairs.
It is very important to be *aware* of the acquire/release semantics,
but I don't think it is necessary to use all the fine grained barrier
types in EDK2.

Less importantly, this would restore the x86 behavior (performance) to
the one seen with the current MemoryFence() implementation.

More importantly, it would fix what the code *means*: "it's very
important to stick to known lockless patterns, and having matching
acquire/release fences makes it much more intuitive to understand what's
going on".

--*--

I'm not proposing this as a pre-requisite to merging Ankur's series
(i.e., I don't expect the series to use acquire/release -- it can be
converted later with the rest of the audit).

However, I'd really like us to stop wasting more time on MemoryFence()
doubts such as:

- Hey it's not a memory fence at all, is it safe?
Who decides what 'memory fence' means anyway?

- But on x86 it kinda is, *dependent on* the spinlocks / lock prefixes
we use for synchronization, between the other accesses.

- Is it interchangeable with volatile? Yes? No? which one should we use?
'volatile' affects code generation only, i.e., which loads and stores
are issued. The *order* in which those loads observe stores done by
other masters is what is managed by barrier instructions. So they are
two sides of the same coin: 'volatile' may help to guarantee that
every assignment through a pointer variable results in a store
instruction, but how these are observed by other CPUs still needs to
be managed if it needs to occur in a certain way.

I think volatile is the wrong tool in most cases: the ordering of
accesses are not a property of the type or of the variable, but of the
code sequence. So using asm("") to ensure that an assignment through a
pointer is emitted as a store instruction, followed by a barrier
instruction is a typical pattern. And given the fact that barrier
instructions require asm sequences in most cases anyway, the asm ("")
will often be implied.


In the longer term, multi-processing code like MpInitLib (CpuDxe,
CpuMpPei) and PiSmmCpuDxeSmm should likely abandon all use of
"volatile", and use the explicit barriers instead, for *clarity*.


Step (2) would take quite a bit of thinking (if not much code).

Comments?

Would there be interest in reviewing such work?

Thanks
Laszlo


MemoryFence()

Laszlo Ersek
 

Hi All,

here's the declaration of MemoryFence(), from <BaseLib.h>:

/**
Used to serialize load and store operations.

All loads and stores that proceed calls to this function are guaranteed to be
globally visible when this function returns.

**/
VOID
EFIAPI
MemoryFence (
VOID
);
The IA32 and X64 implementations of MemoryFence() are not memory
barriers, only compiler barriers, however.

This is a problem for two reasons:

- code that believes to add a memory barrier only adds a compiler
barrier, possibly creating an unsafe situation,

- code that knowingly calls MemoryFence() for the purposes of a compiler
barrier -- while it will hopefully work safely, relying on tacit
knowledge / assumptions about the x86 architecture -- results in a
source reading experience that mismatches known lockless patterns.


This topic has repeatedly come up in the past; most recently in the
review of Ankur's v6 series for
<https://bugzilla.tianocore.org/show_bug.cgi?id=3132>, where some
inter-processor communication needs to happen.

I asked Paolo for help with understanding some of the synchronziation
patterns in PiSmmCpuDxeSmm, the reason being that Ankur's series is
going to rely on, and/or imitate, some of those patterns. From this
discussion with Paolo, the following have emerged (I'm going to
liberally steal / paraphrase Paolo's words):


(1) We should introduce finer-grained fence primitives:

ARM AARCH64 i386

CompilerFence() asm("") asm("") asm("")
AcquireMemoryFence() dmb ish dmb ishld asm("")
ReleaseMemoryFence() dmb ish dmb ish asm("")
MemoryFence() dmb ish dmb ish mfence

"where AcquireMemoryFence() is used on the read side (i.e. between
reads) and ReleaseMemoryFence() is used on the write side (i.e. between
writes)".

(Note that this step would make MemoryFence() heavier-weight on x86 than
it currently is.)


(2) We should audit and rework existent uses (combinations) of
MemoryFence() into matched acquire / release pairs.

Less importantly, this would restore the x86 behavior (performance) to
the one seen with the current MemoryFence() implementation.

More importantly, it would fix what the code *means*: "it's very
important to stick to known lockless patterns, and having matching
acquire/release fences makes it much more intuitive to understand what's
going on".

--*--

I'm not proposing this as a pre-requisite to merging Ankur's series
(i.e., I don't expect the series to use acquire/release -- it can be
converted later with the rest of the audit).

However, I'd really like us to stop wasting more time on MemoryFence()
doubts such as:

- Hey it's not a memory fence at all, is it safe?

- But on x86 it kinda is, *dependent on* the spinlocks / lock prefixes
we use for synchronization, between the other accesses.

- Is it interchangeable with volatile? Yes? No? which one should we use?

In the longer term, multi-processing code like MpInitLib (CpuDxe,
CpuMpPei) and PiSmmCpuDxeSmm should likely abandon all use of
"volatile", and use the explicit barriers instead, for *clarity*.


Step (2) would take quite a bit of thinking (if not much code).

Comments?

Would there be interest in reviewing such work?

Thanks
Laszlo


Re: [RFC] Update NASM tool version to 2.15.05 for build image

Laszlo Ersek
 

On 02/03/21 08:14, Sheng Wei wrote:
Hi All,
This email is to collect feedback about update NASM version to 2.15.05 for build image.

NASM (https://nasm.us/) is an assembler for the x86 CPU architecture platform.
Current open CI is using NSAM version 2.14.02.
And we have below information in edk2 project.
# - NASM 2.10 or later for use with the GCC toolchain family
# - NASM 2.12.01 or later for use with all other toolchain families
The latest stable NASM version is 2.15.05.
The latest stable NASM version 2.15.05 is released at 2020-08-28.
The first 2.15.x NASM version is released at 2020-06-04.
The last 2.14.x NASM version is released at 2018-12-30.
From NASM 2.14.x to NASM 2.15.05, according to the release history (https://nasm.us/doc/nasmdocc.html), it fixes many bugs and add more new instructions support.
For example, it adds instructions for Intel Control Flow Enforcement Technology (CET) since NASM version 2.15.01.
Actually, we have met such unsupported instruction issue in the development.

There would be 2 possible options:

* Option 1: Update the NASM version to 2.15.05
* 2.15.05 is declared as a stable NASM version.
* New instructions will be supported automatically.
* But need effort to verify new NASM version on the project.
* Option 2: Keep stay at current NASM version
* we have to use hardcode binaries in the source code like "DB xx xx xx xx" for these new instructions.
* It would be not friendly to these new instructions and easy to make mistake in the new feature development.
I suppose option 1 will be better to us in the development.

I'd like to hear the community's feedback about which option is preferred, and let me know if you have any concerns with this change. Thanks!
From a RHEL perspective, I'd be okay with a nasm-2.15.03 version
requirement.

Thanks
Laszlo


[RFC] Update NASM tool version to 2.15.05 for build image

Sheng Wei
 

Hi All,
This email is to collect feedback about update NASM version to 2.15.05 for build image.

NASM (https://nasm.us/) is an assembler for the x86 CPU architecture platform.
Current open CI is using NSAM version 2.14.02.
And we have below information in edk2 project.
# - NASM 2.10 or later for use with the GCC toolchain family
# - NASM 2.12.01 or later for use with all other toolchain families
The latest stable NASM version is 2.15.05.
The latest stable NASM version 2.15.05 is released at 2020-08-28.
The first 2.15.x NASM version is released at 2020-06-04.
The last 2.14.x NASM version is released at 2018-12-30.
From NASM 2.14.x to NASM 2.15.05, according to the release history (https://nasm.us/doc/nasmdocc.html), it fixes many bugs and add more new instructions support.
For example, it adds instructions for Intel Control Flow Enforcement Technology (CET) since NASM version 2.15.01.
Actually, we have met such unsupported instruction issue in the development.

There would be 2 possible options:

* Option 1: Update the NASM version to 2.15.05
* 2.15.05 is declared as a stable NASM version.
* New instructions will be supported automatically.
* But need effort to verify new NASM version on the project.
* Option 2: Keep stay at current NASM version
* we have to use hardcode binaries in the source code like "DB xx xx xx xx" for these new instructions.
* It would be not friendly to these new instructions and easy to make mistake in the new feature development.
I suppose option 1 will be better to us in the development.

I'd like to hear the community's feedback about which option is preferred, and let me know if you have any concerns with this change. Thanks!

Best Regards
Sheng Wei


Re: [edk2-devel] RFC: Adding support for ARM (RNDR etc.) to RngDxe

Sami Mujawar <sami.mujawar@...>
 

Hi All,

I have shared some initial thoughts on the RNG implementation updates at https://edk2.groups.io/g/devel/files/Designs/2021/0116/EDKII%20-%20Proposed%20update%20to%20RNG%20implementation.pdf

Kindly let me know your feedback or if you have any queries.

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca Cran via groups.io
Sent: 14 January 2021 09:05 PM
To: Sami Mujawar <Sami.Mujawar@...>; devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...
Cc: rfc@edk2.groups.io; Jiewen Yao <jiewen.yao@...>; Rahul Kumar <rahul1.kumar@...>; nd <nd@...>
Subject: Re: [edk2-devel] RFC: Adding support for ARM (RNDR etc.) to RngDxe

On 12/10/20 4:26 AM, Sami Mujawar wrote:

I am working on the TRNG FW API interface and will share more details
for the discussion soon.

We had some thoughts about streamlining the RngDxe implementations and
would like to share some diagrams for the discussion.

My diagrams are in Visio that I can export as JPG images. However, I am
open to switching to any other suggested tool.
Hi Sami,

I don't see any further discussions on this. Have you made any progress
with sharing the design documents or scheduling a review?

--
Rebecca Cran


Re: [edk2-devel] RFC: Adding support for ARM (RNDR etc.) to RngDxe

Rebecca Cran <rebecca@...>
 

On 12/10/20 4:26 AM, Sami Mujawar wrote:

I am working on the TRNG FW API interface and will share more details for the discussion soon.
We had some thoughts about streamlining the RngDxe implementations and would like to share some diagrams for the discussion.
My diagrams are in Visio that I can export as JPG images. However, I am open to switching to any other suggested tool.
Hi Sami,

I don't see any further discussions on this. Have you made any progress with sharing the design documents or scheduling a review?

--
Rebecca Cran


回复: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

gaoliming
 

Rebecca:
My new mail address is Liming Gao <gaoliming@...>. It has updated in Maintainers.txt.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+69347+4905953+8761045@groups.io
<bounce+27952+69347+4905953+8761045@groups.io> 代表 Rebecca Cran
发送时间: 2020年12月22日 4:45
收件人: rfc@edk2.groups.io; michael.d.kinney@...; gaoliming
<gaoliming@...>; devel@edk2.groups.io; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Laszlo Ersek' <lersek@...>; 'Gao,
Liming' <liming.gao@...>
主题: Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka
submodule alternatives

On 12/21/20 1:42 PM, Rebecca Cran wrote:
On 12/21/20 1:14 PM, Michael D Kinney wrote:
Rebecca,

Are you ok with using a GitHub mirror?
Yes, that's fine with me.
By the way, I've started getting bounces from Liming's Intel email address.

--
Rebecca Cran





Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Rebecca Cran
 

On 12/21/20 1:42 PM, Rebecca Cran wrote:
On 12/21/20 1:14 PM, Michael D Kinney wrote:
Rebecca,

Are you ok with using a GitHub mirror?
Yes, that's fine with me.
By the way, I've started getting bounces from Liming's Intel email address.

--
Rebecca Cran


Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Rebecca Cran
 

On 12/21/20 1:14 PM, Michael D Kinney wrote:
Rebecca,
Are you ok with using a GitHub mirror?
Yes, that's fine with me.

--
Rebecca Cran


Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Michael D Kinney
 

Rebecca,

Are you ok with using a GitHub mirror?

Mike

-----Original Message-----
From: gaoliming <gaoliming@...>
Sent: Sunday, December 20, 2020 5:18 PM
To: devel@edk2.groups.io; rebecca@...; rfc@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; 'Bret
Barkelew' <Bret.Barkelew@...>; 'Laszlo Ersek' <lersek@...>; 'Gao, Liming' <liming.gao@...>
Subject: 回复: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

I also prefer to fetch all submodules from github. Then, all git repos will have the same behavior.

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+69261+4905953+8761045@groups.io
<bounce+27952+69261+4905953+8761045@groups.io> 代表 Rebecca Cran
发送时间: 2020年12月20日 9:06
收件人: rfc@edk2.groups.io; michael.d.kinney@...;
devel@edk2.groups.io; 'Bret Barkelew' <Bret.Barkelew@...>;
Laszlo Ersek <lersek@...>; Gao, Liming <liming.gao@...>
主题: Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka
submodule alternatives

On 12/19/20 11:58 AM, Michael D Kinney wrote:
There have been a few suggestions to create a mirror of cmocka in
TianoCore
org in GitHub.

I have found a GitHub action that can do a repo sync.

https://github.com/marketplace/actions/github-repo-sync

I have created a temporary mirror of cmocka in my personal GitHub area
that
uses this GitHub action to sync all branches and all tags once a day.

https://github.com/mdkinney/mirror-cmocka

Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.

https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflo
ws/repo-sync.yml

Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in
the
edk2 repo to use edk2-cmocka.
I'd suggest just using the Gitlab mirror. Unlike cryptomilk.org, Gitlab
should be just as reliable as Github and won't introduce another
potential failure point.

--
Rebecca Cran





回复: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

gaoliming
 

I also prefer to fetch all submodules from github. Then, all git repos will have the same behavior.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+69261+4905953+8761045@groups.io
<bounce+27952+69261+4905953+8761045@groups.io> 代表 Rebecca Cran
发送时间: 2020年12月20日 9:06
收件人: rfc@edk2.groups.io; michael.d.kinney@...;
devel@edk2.groups.io; 'Bret Barkelew' <Bret.Barkelew@...>;
Laszlo Ersek <lersek@...>; Gao, Liming <liming.gao@...>
主题: Re: [edk2-devel] [edk2-rfc] [RFC] UnitTestFrameworkPkg cmocka
submodule alternatives

On 12/19/20 11:58 AM, Michael D Kinney wrote:
There have been a few suggestions to create a mirror of cmocka in
TianoCore
org in GitHub.

I have found a GitHub action that can do a repo sync.

https://github.com/marketplace/actions/github-repo-sync

I have created a temporary mirror of cmocka in my personal GitHub area
that
uses this GitHub action to sync all branches and all tags once a day.

https://github.com/mdkinney/mirror-cmocka

Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.

https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflo
ws/repo-sync.yml

Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in
the
edk2 repo to use edk2-cmocka.
I'd suggest just using the Gitlab mirror. Unlike cryptomilk.org, Gitlab
should be just as reliable as Github and won't introduce another
potential failure point.

--
Rebecca Cran





回复: [edk2-rfc] [RFC V3] Create supported branch from edk2-stable* tag (Required to address critical bug BZ3111)

gaoliming
 

Mike:
It is OK to me. I have no other comments.

Thanks
Liming

-----邮件原件-----
发件人: bounce+34240+470+4905953+8776774@groups.io
<bounce+34240+470+4905953+8776774@groups.io> 代表 Michael D
Kinney
发送时间: 2020年12月19日 10:03
收件人: rfc@edk2.groups.io; devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
主题: [edk2-rfc] [RFC V3] Create supported branch from edk2-stable* tag
(Required to address critical bug BZ3111)

Hello,



The following bug has been fixed on edk2/master



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

https://github.com/tianocore/edk2/pull/1226



This bug is also considered a critical bug against edk2-stable202011. The
behavior

of the Variable Lock Protocol was changed in a non-backwards compatible
manner in

edk2-stable202011 and this is impacting some downstream platforms. The
following

2 commits on edk2/master restore the original behavior of the Variable Lock
Protocol.




https://github.com/tianocore/edk2/pull/1226/commits/893cfe2847b83da74f
53858d6acaa15a348bad7c


https://github.com/tianocore/edk2/pull/1226/commits/16491ba6a6e9a91ce
deeed45bc0fbdfde49f7968



The request here is to create a supported branch from edk2-stable202011 tag
and apply

these 2 commits as critical bug fixes on the supported branch.



Since we started using the edk2-stable* tag process, there has not been a
request to create

a supported branch from one of those tags. As a result, there are a couple
opens that

need to be addressed:



1) Supported branch naming convention.



Branch Proposal: stable/<YYYY><MM>

Branch Example: stable/202011



2) CI requirements for supported branches.



Proposal: Update .azurepipelines yml files to trigger on stable/*
branches,

update .mergify configuration file to support merging of stable/*
branches,

and update GitHub branch protection settings to protect stable/*
branches.



3) A stable/* branch is only supported until the next edk2-stable tag release.



4) Release requirements for supported branches.



Proposal: If there are a significant number of critical fixes applied to

a stable/* branch, then a request for a release can be made that would

trigger focused testing of the supported branch and creation of a new

release. If all testing passes, then a tag is created on the stable/*

branch and a release is created on GitHub that summarizes the set of

critical fixes and the testing performed.



Tag Proposal: edk2-stable<YYYY><MM>.<XX>

Tag Example : edk2-stable202011.01



Please let me know if you have any feedback or comments on this proposal.
The goal

is to close on this topic this week.



Thank you,



Mike





Re: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Rebecca Cran
 

On 12/19/20 11:58 AM, Michael D Kinney wrote:
There have been a few suggestions to create a mirror of cmocka in TianoCore
org in GitHub.
I have found a GitHub action that can do a repo sync.
https://github.com/marketplace/actions/github-repo-sync
I have created a temporary mirror of cmocka in my personal GitHub area that
uses this GitHub action to sync all branches and all tags once a day.
https://github.com/mdkinney/mirror-cmocka
Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.
https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflows/repo-sync.yml
Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in the
edk2 repo to use edk2-cmocka.
I'd suggest just using the Gitlab mirror. Unlike cryptomilk.org, Gitlab should be just as reliable as Github and won't introduce another potential failure point.

--
Rebecca Cran


Re: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Bret Barkelew <bret.barkelew@...>
 

I like it.

- Bret

From: Kinney, Michael D<mailto:michael.d.kinney@...>
Sent: Saturday, December 19, 2020 10:59 AM
To: rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Bret Barkelew<mailto:Bret.Barkelew@...>; Laszlo Ersek <lersek@...> (lersek@...)<mailto:lersek@...>; liming.gao<mailto:liming.gao@...>
Subject: [EXTERNAL] RE: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Hello,

There have been a few suggestions to create a mirror of cmocka in TianoCore
org in GitHub.

I have found a GitHub action that can do a repo sync.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmarketplace%2Factions%2Fgithub-repo-sync&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398666049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=AX2EFVoGvtYoOZRtyFjwwTRbZkmQMgOCnjNNhWot7eo%3D&amp;reserved=0

I have created a temporary mirror of cmocka in my personal GitHub area that
uses this GitHub action to sync all branches and all tags once a day.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fmirror-cmocka&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398666049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=U7ThFHC2fsgO9rVTNre3b0dI23b1Iudi1tw%2FjiFEdZc%3D&amp;reserved=0

Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmdkinney%2Fmirror-cmocka%2Fblob%2Frepo-sync%2F.github%2Fworkflows%2Frepo-sync.yml&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398666049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CuxE3Ljy2M7APvXfnuOqb6YPnFCX%2FUkDxsiIGEUHcvY%3D&amp;reserved=0

Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in the
edk2 repo to use edk2-cmocka.

Best regards,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Wednesday, December 16, 2020 10:46 AM
To: rfc@edk2.groups.io; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; 'Bret Barkelew'
<Bret.Barkelew@...>
Subject: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Hello,

We have had at least three incidents in the last year where the link to the
cmocka submodule in the UnitTestFrameworkPkg has not been available, and this
impacted the EDK II CI system. The following submodule link is the one that
is not reliable:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.cryptomilk.org%2Fprojects%2Fcmocka.git&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398666049%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=m4tHEei6OQUwJu6jcldgxycoBJoajqPb9o5aKTDra%2F0%3D&amp;reserved=0

We have identified two potential mirrors of this repo:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fneverware-mirrors%2Fcmocka.git&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398676045%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=19d9nV6kG4FQE3GC3ZzDTmE6%2F7EjNdMMWM%2BbSGT6bSI%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.com%2Fcmocka%2Fcmocka.git&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398676045%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=xgErAgcMsMSKbrLZB9kgCHHl3r%2FzJM%2FJy6jhxpi0ObY%3D&amp;reserved=0

The following patch provided a temporary fix for the EDK II CI agents, but
does not help other consumers of the edk2 repository.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fcommit%2Fbe746104d1766a8c363e74d6063144657820d688&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398676045%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Nyx8mBIwNxMEu4SdHYGiQhBGcpAPxxPHXBgI%2BM0CIU0%3D&amp;reserved=0

I have seen one suggestion that TianoCore create its own
mirror of cmocka. This does require monitoring and maintenance
by the TianoCore community. I would prefer to use a well
maintained mirror in github as long as we do not observe any
issues with the support of that mirror.

I propose we update the submodule in the UnitTestFrameworkPkg
to use the https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fneverware-mirrors%2Fcmocka.git&;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C92da18aaec1443463b2508d8a45023a2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637440011398676045%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=19d9nV6kG4FQE3GC3ZzDTmE6%2F7EjNdMMWM%2BbSGT6bSI%3D&amp;reserved=0 mirror.
By using a mirror in github, we remove one external dependency.

Please provide feedback and comments on this proposal. If there
are no objections, then we will proceed with a patch review for
this update.

Thanks,

Mike


Re: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Michael D Kinney
 

Hello,

There have been a few suggestions to create a mirror of cmocka in TianoCore
org in GitHub.

I have found a GitHub action that can do a repo sync.

https://github.com/marketplace/actions/github-repo-sync

I have created a temporary mirror of cmocka in my personal GitHub area that
uses this GitHub action to sync all branches and all tags once a day.

https://github.com/mdkinney/mirror-cmocka

Here is the GitHub workflow file. It must be in the default branch for the
repo using a branch name that is not present in the repo being mirrored.
In this case, I used a branch name of 'repo-sync'.

https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflows/repo-sync.yml

Please provide feedback on this approach. If we like this approach, then
I suggest we create a new repo in TianoCore called edk2-cmocka that is a
mirror that is synced once a day and we update the cmocka submodule in the
edk2 repo to use edk2-cmocka.

Best regards,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Wednesday, December 16, 2020 10:46 AM
To: rfc@edk2.groups.io; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; 'Bret Barkelew'
<Bret.Barkelew@...>
Subject: [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

Hello,

We have had at least three incidents in the last year where the link to the
cmocka submodule in the UnitTestFrameworkPkg has not been available, and this
impacted the EDK II CI system. The following submodule link is the one that
is not reliable:

https://git.cryptomilk.org/projects/cmocka.git

We have identified two potential mirrors of this repo:

https://github.com/neverware-mirrors/cmocka.git
https://gitlab.com/cmocka/cmocka.git

The following patch provided a temporary fix for the EDK II CI agents, but
does not help other consumers of the edk2 repository.

https://github.com/tianocore/edk2/commit/be746104d1766a8c363e74d6063144657820d688

I have seen one suggestion that TianoCore create its own
mirror of cmocka. This does require monitoring and maintenance
by the TianoCore community. I would prefer to use a well
maintained mirror in github as long as we do not observe any
issues with the support of that mirror.

I propose we update the submodule in the UnitTestFrameworkPkg
to use the https://github.com/neverware-mirrors/cmocka.git mirror.
By using a mirror in github, we remove one external dependency.

Please provide feedback and comments on this proposal. If there
are no objections, then we will proceed with a patch review for
this update.

Thanks,

Mike


[RFC V3] Create supported branch from edk2-stable* tag (Required to address critical bug BZ3111)

Michael D Kinney
 

Hello,



The following bug has been fixed on edk2/master



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

https://github.com/tianocore/edk2/pull/1226



This bug is also considered a critical bug against edk2-stable202011. The behavior

of the Variable Lock Protocol was changed in a non-backwards compatible manner in

edk2-stable202011 and this is impacting some downstream platforms. The following

2 commits on edk2/master restore the original behavior of the Variable Lock Protocol.



https://github.com/tianocore/edk2/pull/1226/commits/893cfe2847b83da74f53858d6acaa15a348bad7c

https://github.com/tianocore/edk2/pull/1226/commits/16491ba6a6e9a91cedeeed45bc0fbdfde49f7968



The request here is to create a supported branch from edk2-stable202011 tag and apply

these 2 commits as critical bug fixes on the supported branch.



Since we started using the edk2-stable* tag process, there has not been a request to create

a supported branch from one of those tags. As a result, there are a couple opens that

need to be addressed:



1) Supported branch naming convention.



Branch Proposal: stable/<YYYY><MM>

Branch Example: stable/202011



2) CI requirements for supported branches.



Proposal: Update .azurepipelines yml files to trigger on stable/* branches,

update .mergify configuration file to support merging of stable/* branches,

and update GitHub branch protection settings to protect stable/* branches.



3) A stable/* branch is only supported until the next edk2-stable tag release.



4) Release requirements for supported branches.



Proposal: If there are a significant number of critical fixes applied to

a stable/* branch, then a request for a release can be made that would

trigger focused testing of the supported branch and creation of a new

release. If all testing passes, then a tag is created on the stable/*

branch and a release is created on GitHub that summarizes the set of

critical fixes and the testing performed.



Tag Proposal: edk2-stable<YYYY><MM>.<XX>

Tag Example : edk2-stable202011.01



Please let me know if you have any feedback or comments on this proposal. The goal

is to close on this topic this week.



Thank you,



Mike


Re: [EXTERNAL] Re: [RFC V2] Create supported branch from edk2-stable* tag (Required to address critical bug BZ3111)

Michael D Kinney
 

I agree that the default policy is to only support a branch until the next
stable tag. My comments were only to address the potential for a request
after that defined support timeline. If a portion of the community wants
to do the work required to support past that point, then I doubt we would
reject the idea.

I will only document the default policy. Anything past that would have
to be raised as a new request.

Mike

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
Sent: Thursday, December 17, 2020 10:46 AM
To: Laszlo Ersek <lersek@...>; Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io;
rfc@edk2.groups.io; gaoliming@...; Andrew Fish (afish@...) <afish@...>; Leif Lindholm
<leif@...>; Sean Brogan <sean.brogan@...>
Subject: Re: [edk2-rfc] [EXTERNAL] Re: [RFC V2] Create supported branch from edk2-stable* tag (Required to address
critical bug BZ3111)

“I agree with Liming that stable branches should have a predefined
lifetime. Keeping stable branches regression-free is very difficult and
ungrateful work, and the community should not have expectations that
we're going to do "LTS" branches.”

Seconded. We actually had to update our release process with this blurb recently:
https://microsoft.github.io/mu/How/release_process/#post-lts-and-archiving

- Bret

From: Laszlo Ersek<mailto:lersek@...>
Sent: Thursday, December 17, 2020 5:50 AM
To: Kinney, Michael D<mailto:michael.d.kinney@...>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>;
rfc@edk2.groups.io<mailto:rfc@edk2.groups.io>; gaoliming@...<mailto:gaoliming@...>; Andrew Fish
(afish@...)<mailto:afish@...>; Leif Lindholm<mailto:leif@...>; Sean
Brogan<mailto:sean.brogan@...>; Bret Barkelew<mailto:Bret.Barkelew@...>
Subject: [EXTERNAL] Re: [RFC V2] Create supported branch from edk2-stable* tag (Required to address critical bug BZ3111)

On 12/16/20 01:24, Kinney, Michael D wrote:
Hello,

The following bug has been fixed on edk2/master

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3111&;da
ta=04%7C01%7Cbret.barkelew%40microsoft.com%7C092cd97468a645f68e4308d8a292a636%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7
C637438098026646126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;
sdata=wE%2B9eA3XrRxS58KUk6DbFZmvX9nvmWopzGoVRN5713k%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1226&;data=04%
7C01%7Cbret.barkelew%40microsoft.com%7C092cd97468a645f68e4308d8a292a636%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63743
8098026646126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=
MnXglF%2FvtUDAouXJvBUIRFq7TuPL2dKXohXwcEuY3zc%3D&amp;reserved=0

This bug is also considered a critical bug against edk2-stable202011. The behavior
of the Variable Lock Protocol was changed in a non-backwards compatible manner in
edk2-stable202011 and this is impacting some downstream platforms. The following
2 commits on edk2/master restore the original behavior of the Variable Lock Protocol.

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1226%2Fcommits%2F
893cfe2847b83da74f53858d6acaa15a348bad7c&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C092cd97468a645f68e4308d8a292a6
36%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637438098026646126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h19eR7KFYlerrva%2FVGMDb7DMVIUihINgAlOh96Hb2xI%3D&amp;reserved=0
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1226%2Fcommits%2F
16491ba6a6e9a91cedeeed45bc0fbdfde49f7968&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C092cd97468a645f68e4308d8a292a6
36%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637438098026656126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=8nCkmGD5jRyfrsMID0ESAcUb8plWrRkFafvhPiS2Zo8%3D&amp;reserved=0

The request here is to create a supported branch from edk2-stable202011 tag and apply
these 2 commits as critical bug fixes on the supported branch.

Since we started using the edk2-stable* tag process, there has not been a request to create
a supported branch from one of those tags. As a result, there are a couple opens that
need to be addressed:

1) Supported branch naming convention.

Proposal: stable/<YYYY><MM>
Example: stable/202011

2) CI requirements for supported branches.

Proposal: Update .azurepipelines yml files to also trigger on stable/* branches
and update GitHub settings so stable/* branches are protected branches.

3) Release requirements for supported branches.

Proposal: If there are a significant number of critical fixes applied to
a stable/edk2-stable* branch, then a request for a release can be made that
would trigger focused testing of the supported branch and creation of a new
release. If all testing passes, then a tag is created on the stable/edk2-stable*
branch and a release is created on GitHub that summarizes the set of critical
fixes and the testing performed.

Proposal: edk2-stable<YYYY><MM>.<XX>
Example : edk2-stable201111.01

Please let me know if you have any feedback or comments on this proposal. The goal
is to close on this topic this week.
- Looks good; just a typo in the example: "edk2-stable201111.01" should
use 2020, not 2011.


- I agree with Liming that stable branches should have a predefined
lifetime. Keeping stable branches regression-free is very difficult and
ungrateful work, and the community should not have expectations that
we're going to do "LTS" branches. That's too resource hungry; companies
have dedicated "maintenance engineer" positions for that.

Here's an example stable process:

https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.qemu.org%2F%3Fp%3Dqemu.git%3Ba%3Dblob_plain%3Bf%3Ddo
cs%2Fdevel%2Fstable-
process.rst%3Bhb%3DHEAD&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C092cd97468a645f68e4308d8a292a636%7C72f988bf86f1
41af91ab2d7cd011db47%7C1%7C0%7C637438098026656126%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bOmSbEHuU%2BqLr3mdmhP%2Foq%2BR7yy%2BVNUWbG367yhFwQE%3D&amp;reserved=0

I would recommend that, initially, we only promise support for the last
stable tag's branch.


- Including a unit test (if it exists) with the actual bugfix on a
stable branch seems important to me.

Thanks
Laszlo





回复: [RFC V2] Create supported branch from edk2-stable* tag (Required to address critical bug BZ3111)

gaoliming
 

Mike:

-----邮件原件-----
发件人: Kinney, Michael D <michael.d.kinney@...>
发送时间: 2020年12月16日 10:52
收件人: gaoliming <gaoliming@...>; devel@edk2.groups.io;
rfc@edk2.groups.io; 'Andrew Fish' <afish@...>; 'Leif Lindholm'
<leif@...>; lersek@...; 'Sean Brogan'
<sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; Kinney, Michael D
<michael.d.kinney@...>
主题: RE: [RFC V2] Create supported branch from edk2-stable* tag (Required
to address critical bug BZ3111)

-----Original Message-----
From: gaoliming <gaoliming@...>
Sent: Tuesday, December 15, 2020 5:19 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; rfc@edk2.groups.io; 'Andrew Fish'
<afish@...>; 'Leif Lindholm' <leif@...>;
lersek@...; 'Sean Brogan' <sean.brogan@...>;
'Bret Barkelew' <Bret.Barkelew@...>
Subject: 回复: [RFC V2] Create supported branch from edk2-stable* tag
(Required to address critical bug BZ3111)

Mike:

-----邮件原件-----
发件人: Kinney, Michael D <michael.d.kinney@...>
发送时间: 2020年12月16日 8:25
收件人: devel@edk2.groups.io; rfc@edk2.groups.io;
gaoliming@...; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>; Laszlo Ersek
<lersek@...> (lersek@...) <lersek@...>; 'Sean
Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; Kinney, Michael D
<michael.d.kinney@...>
主题: [RFC V2] Create supported branch from edk2-stable* tag (Required
to
address critical bug BZ3111)

Hello,

The following bug has been fixed on edk2/master

https://bugzilla.tianocore.org/show_bug.cgi?id=3111
https://github.com/tianocore/edk2/pull/1226

This bug is also considered a critical bug against edk2-stable202011. The
behavior
of the Variable Lock Protocol was changed in a non-backwards compatible
manner in
edk2-stable202011 and this is impacting some downstream platforms.
The
following
2 commits on edk2/master restore the original behavior of the Variable
Lock
Protocol.


https://github.com/tianocore/edk2/pull/1226/commits/893cfe2847b83da74f
53858d6acaa15a348bad7c

https://github.com/tianocore/edk2/pull/1226/commits/16491ba6a6e9a91ce
deeed45bc0fbdfde49f7968
[Liming] This one is for unit test. It is not critical fix. I don't think it is
required.


I agree it is not strictly required for functionality, but the bug fix that is
required
was reviewed and submitted in a PR as a patch series. I think critical bug
fixes should
be applied to a supported branch at the same granularity they were
submitted to the
trunk. Since the EDK II CI system does not evaluate the stability of each
patch in
a patch series, there is a risk to take portions of a patch series.

I suggest when a critical bug fix is identified, that we start with cherry-picking
all the patches in the patch series. If there is a specific concern about taking
the entire patch series, then that can be discussed and potentially a different
patch series can be applied to the supported branch. This would require CI
on
supported branch to make sure the quality and functionality are the same.
This case is clear. One commit is for the bug fix, another is for the unit test.
If the unit test is also important for this fix, it can be cherry-pick. I understand the critical bug fix is for the functionality only.


The request here is to create a supported branch from edk2-stable202011
tag
and apply
these 2 commits as critical bug fixes on the supported branch.

Since we started using the edk2-stable* tag process, there has not been a
request to create
a supported branch from one of those tags. As a result, there are a
couple
opens that
need to be addressed:

1) Supported branch naming convention.

Proposal: stable/<YYYY><MM>
Example: stable/202011
Here is my suggestion on the live period of the stable tag branch.
The stable tag branch will be created only when the critical issue is found in
this stable tag. By default, no stable tag
branch is created.
Now, the quarterly stable tag will be created every three months. So, this
branch will exist for at most three months.
Once next stable tag is created, new stable tag will be used. Previous stable
tag branch will not be maintained.
That means only latest stable tag branch will be maintained if it is created.
It is hard to predict how downstream platforms use a stable tag or a
supported branch.
If a downstream consumer identifies a critical bug in a previous stable tag or
a supported branch, then that bug report needs to be evaluated and
determine if
the bug fix needs to be applied to a stable branch or not. I do not think we
should
reject all requests just because there is a more recent stable tag. We need
to evaluate each request.
If stable branch requires long term maintain, the stable branches will become more and more,
the branch maintain effort will be more and more. It may not be workable in open source edk2 community
unless there is the dedicated branch maintainers.

So, I suggest that downstream consumers maintain their downstream branch to include the hot fix.
Edk2 stable branch is the reference for the downstream users.

Thanks
Liming
We do want to encourage all platforms under development to use the latest
stable
tag. But once a platform is released as a product using a specific stable tag
they may prefer to continue to use that stable tag for long term maintenance
of that platform.



2) CI requirements for supported branches.

Proposal: Update .azurepipelines yml files to also trigger on stable/*
branches
and update GitHub settings so stable/* branches are protected
branches.
The patch has been verified in master. CI test may not be necessary.
In the general case where more than one critical bug may be fixed in a
stable branch, CI will help make sure that the combination of fixes
work together. For this first case, I agree that CI may not be required.


3) Release requirements for supported branches.

Proposal: If there are a significant number of critical fixes applied to
a stable/edk2-stable* branch, then a request for a release can be
made
that
would trigger focused testing of the supported branch and creation of
a
new
release. If all testing passes, then a tag is created on the
stable/edk2-stable*
branch and a release is created on GitHub that summarizes the set of
critical
fixes and the testing performed.

Proposal: edk2-stable<YYYY><MM>.<XX>
Example : edk2-stable201111.01
It is OK to create new stable tag per the request. The platform can use
stable branch.

Thank you. I will start work on a patch for review.


Besides, there are few new issues. I have cancelled the bug triage meeting.

Thanks
Liming
Please let me know if you have any feedback or comments on this
proposal.
The goal
is to close on this topic this week.

Thank you,

Mike

301 - 320 of 786