From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp02.apple.com (ma1-aaemail-dr-lapp02.apple.com [17.171.2.68]) by mx.groups.io with SMTP id smtpd.web11.3977.1602037072313436721 for ; Tue, 06 Oct 2020 19:17:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=GGuiBP2r; spf=pass (domain: apple.com, ip: 17.171.2.68, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp02.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp02.apple.com (8.16.0.42/8.16.0.42) with SMTP id 0972CbnH028727; Tue, 6 Oct 2020 19:17:51 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=JesXGQ+YuoN/tN3YYewf6dIx3/8H4vBbqhnJQkQ72r0=; b=GGuiBP2rTctkUmYRtf+1PuSZnDtxUsb6G8/LGf8bopTz2O2a8zFC5k0v9CfYLAzi5eLf w5hT9QF+PH8tvMUR05dJX9AzgF1Dc/BnDABdKAvrpZDX1NK/HIDcMOpJm9HCEK2lobbH 9DY+2/s8JatsRao2+DEFrZ/ZrtgRF+cRz9rxbMV5ECjV2F6c44Qlust5QVuIajPZnU/C ihESOiSyDPnYVEFq1skNseQLTRgImyilyEuA0Uws0RF1H/2v6Mkrq2gVD6HliXZRI74J Jog930TxPbAy8bMnsvZ9sR1XbEf+6j7joXB5olaySImLypZymMajtMy9m97MWoFel4X6 7A== Received: from rn-mailsvcp-mta-lapp02.rno.apple.com (rn-mailsvcp-mta-lapp02.rno.apple.com [10.225.203.150]) by ma1-aaemail-dr-lapp02.apple.com with ESMTP id 33xnyshu0f-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 06 Oct 2020 19:17:51 -0700 Received: from rn-mailsvcp-mmp-lapp01.rno.apple.com (rn-mailsvcp-mmp-lapp01.rno.apple.com [17.179.253.14]) by rn-mailsvcp-mta-lapp02.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPS id <0QHT00SIK7PLU1H0@rn-mailsvcp-mta-lapp02.rno.apple.com>; Tue, 06 Oct 2020 19:17:45 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp01.rno.apple.com by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) id <0QHT00M007DNWN00@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Tue, 06 Oct 2020 19:17:45 -0700 (PDT) X-Va-A: X-Va-T-CD: 70a38c3f5b1d46c4b8dccb3b011be358 X-Va-E-CD: b6fdcc266045f0dce41293d449be90e1 X-Va-R-CD: befe6021ea6c6af6b3b07c34d0379a2c X-Va-CD: 0 X-Va-ID: b097d78b-7c57-4d19-b548-bbff676a7ce6 X-V-A: X-V-T-CD: 70a38c3f5b1d46c4b8dccb3b011be358 X-V-E-CD: b6fdcc266045f0dce41293d449be90e1 X-V-R-CD: befe6021ea6c6af6b3b07c34d0379a2c X-V-CD: 0 X-V-ID: 603d0f96-f553-4ba8-8ffd-48e75974fcb6 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-10-07_03:2020-10-06,2020-10-07 signatures=0 Received: from [17.235.57.94] (unknown [17.235.57.94]) by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPSA id <0QHT00SA67PJ2Q00@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Tue, 06 Oct 2020 19:17:44 -0700 (PDT) From: "Andrew Fish" Message-id: <38FD7E16-93BD-45D1-985A-702A9776D3D3@apple.com> MIME-version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) Subject: Re: [edk2-devel] VariablePolicy: Final Changes Thread 1 - TPL Ordering Date: Tue, 06 Oct 2020 19:17:43 -0700 In-reply-to: Cc: "bret.barkelew@microsoft.com" To: edk2-devel-groups-io , Mike Kinney References: X-Mailer: Apple Mail (2.3608.80.23.2.2) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-10-07_03:2020-10-06,2020-10-07 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_F5D814C9-49BA-43DF-A3B7-AD95A46CF006" --Apple-Mail=_F5D814C9-49BA-43DF-A3B7-AD95A46CF006 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Mike, When I=E2=80=99ve done things like this in the past I think of making them= configurable like via a PCD.=20 In terms of the #defines I think it makes more sense to just do math on th= e spec defined values. So TPL_CALLBACK + 1 or TPL_CALLBACK - 1 etc. I=E2= =80=99ve got an lldb type formatter for TPL and it prints out [+ ] as I think this is the clearest way to do it.=20 Thanks, Andrew Fish > On Oct 6, 2020, at 6:54 PM, Michael D Kinney wrote: >=20 > Bret, > > It is likely best to go with the first approach. The discussion on TPL = levels can continue and you could adopt it in the future if a general solut= ion is identified. > > TPL 17..30 are reserved by the UEFI Spec for firmware interrupts. So TP= L_NOTIFY_HIGH as defined would not be allowed. > > I agree that the use of TPL values other than those defined by the UEFI = Spec can potentially be used by DXE. However, that DXE usage must be flexi= ble enough to handle a future extension to the UEFI Spec for new TPL levels= without a collision. > > Instead of defining specific TPL values, you could add a DXE scoped serv= ice to allocate the use of a new TPL level that is not being used by UEFI o= r other DXE drivers. I will point out that these approaches (defining new = TPL levels or allocating unused TPL levels) just moves the same problem. Y= ou can solve it for the first driver that needs something special. As soon= as there is more than one driver that need something special at the same T= PL level, the potential for a race condition for ordering will show up agai= n. So I do not consider adding TPL levels to be a good general solution to= this problem. > > Best regards, > > Mike > > From: devel@edk2.groups.io > On Behalf Of Bret Barkelew via groups= .io > Sent: Tuesday, October 6, 2020 5:24 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] VariablePolicy: Final Changes Thread 1 - TPL Order= ing > > As many will be aware, I=E2=80=99m in the final stages of having Variabl= e Policy ready for commit. However, after moving the =E2=80=9CLock=E2=80=9D= event back to EndOfDxe, this exposed a race condition in variable services= . > > A quick synopsis of the problem: > Previously, MorLock abused a privileged position by being tightly couple= d to Variable Services, and its lock callback was called directly so that i= t could be strongly ordered with the internal property lock that disables f= uture RequestToLock calls. > VariablePolicy attempted to decouple this (without realizing it was a pr= oblem) and go through formalized interfaces that could ultimately be broken= out of Variable Services altogether. > However, doing so triggered the race condition, causing an ASSERT when M= orLock attempted to lock its variables. > I current have a reimplementation of the strong ordering workaround that= can be previewed at the link below. I have tested that it works the same a= s the OLD system. > Take a stab at solving the lock ordering problem =C2=B7 corthon/edk2@e7d= 0164 (github.com) > > However, replacing one bad design with another is not what this communit= y is about (when we can help it), so we=E2=80=99d like to take a moment to = revisit a conversation that has come up before: expanding the number of sup= ported TPL levels. > > Now, I know that the current TPL levels are defined in the UEFI spec and= we don=E2=80=99t want to have to change those, but there=E2=80=99s no reas= on that we can come up with not to add some more granularity in the PI spec= , dedicated to platform and implementation ordering (the UEFI spec events w= ill have to remain on UEFI spec TPLs). Basically there would be a set of DX= E Services that allow WaitForEvent, CheckEvent, Event registration at TPLs = other than notify/callback. The UEFI system table versions of the function= s would still have this restriction but code built with the platform could = use the DXE Services. Right now, any attempt to use a non-UEFI TPL will ASS= ERT, so we can keep that in place on the SystemTable interface, but allow t= he platform to go around it with DXE Services. Similar functionality would = have to be provided by the Mmst, but that=E2=80=99s already platform-specif= ic and can probably allow it in all cases. > > We=E2=80=99re suggesting something like the below: > > // > // Task priority level > // > #define TPL_APPLICATION 4 > #define TPL_CALLBACK_LOW 7 > #define TPL_CALLBACK 8 > #define TPL_CALLBACK_HIGH 9 > #define TPL_NOTIFY_LOW 15 > #define TPL_NOTIFY 16 > #define TPL_NOTIFY_HIGH 17 > #define TPL_HIGH_LEVEL 31 > > There=E2=80=99s already a long-in-the-tooth bug tracking a similar issue= : > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1676 > > This proposal is simpler than what=E2=80=99s in that bug, and would grea= tly simplify some of our event ordering (and code). > > Thoughts? > > If this conversation takes too long, I will publish a set of patches tha= t just go with the lesser solution posted above, but I=E2=80=99d much rathe= r work the problem this way. > > - Bret >=20 --Apple-Mail=_F5D814C9-49BA-43DF-A3B7-AD95A46CF006 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Mike,

When I=E2=80=99ve done things like this in= the past I think of making them configurable like via a PCD. 

In terms of the #defines= I think it makes more sense to just do math on the spec defined values. So=  TP= L_CALLBACK + 1 or TPL_CALLBACK - 1 etc.  I=E2=80=99ve got an l= ldb type formatter for TPL and it prints out <UEFI Spec T= PL> [+ <extra>] as I think this is the clearest way to do it. = ;

Thanks,<= /div>

Andrew Fish

= On Oct 6, 2020, at 6:54 PM, Michael D Kinney <michael.d.kinney@intel.com> wrote:
Bret,
 
It is likely best to go with= the first approach.  The discussion on TPL levels can continue and you= could adopt it in the future if a general solution is identified.
 
TPL 17..30 are reserved by the UEFI Spec for firmware interrupts.  So TPL_NOTIFY_HIGH as defined would not be allowed.
=  
I agr= ee that the use of TPL values other than those defined by the UEFI Spec can= potentially be used by DXE.  However, that DXE usage must be flexible = enough to handle a future extension to the UEFI Spec for new TPL levels wit= hout a collision.
 
Instead of defining specific TPL values= , you could add a DXE scoped service to allocate the use of a new TPL level= that is not being used by UEFI or other DXE drivers. = ; I will point out= that these approaches (defining new TPL levels or allocating unused TPL le= vels) just moves the same problem.  You can solve it for the first driv= er that needs something special.  As soon as there is more than one dri= ver that need something special at the same TPL level, the potential for a = race condition for ordering will show up again.  So I do not consider a= dding TPL levels to be a good general solution to this problem.
 
Best regards,
 
Mike
=
 
From:&nb= sp;devel@edk2.groups.io<= span class=3D"Apple-converted-space"> <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
Sent: <= /span>Tuesday, October 6, 2020 5:24 PM
To:<= span class=3D"Apple-converted-space"> devel@edk2.groups.io
Subject= : [edk2-devel] Variab= lePolicy: Final Changes Thread 1 - TPL Ordering
 =
As many will be aware, I=E2=80=99m in the= final stages of having Variable Policy ready for commit. However, after mo= ving the =E2=80=9CLock=E2=80=9D event back to EndOfDxe, this exposed a race= condition in variable services.
 
= A quick synopsis of the problem:
  • Previously, MorLock abused a = privileged position by being tightly coupled to Variable Services, and its = lock callback was called directly so that it could be strongly ordered with= the internal property lock that disables future RequestToLock calls.
  • VariablePolicy attempted to decouple this (without realizing i= t was a problem) and go through formalized interfaces that could ultimately= be broken out of Variable Services altogether.
  • However, = doing so triggered the race condition, causing an ASSERT when MorLock attem= pted to lock its variables.
  • I current have a reimplementa= tion of the strong ordering workaround that can be previewed at the link be= low. I have tested that it works the same as the OLD system.
 
However, replacing one bad design with another= is not what this community is about (when we can help it), so we=E2=80=99d= like to take a moment to revisit a conversation that has come up before: e= xpanding the number of supported TPL levels.
 
Now, I know that the current TPL levels are defined in the UEFI= spec and we don=E2=80=99t want to have to change those, but there=E2=80=99= s no reason that we can come up with not to add some more granularity in th= e PI spec, dedicated to platform and implementation ordering (the UEFI spec= events will have to remain on UEFI spec TPLs). Basically there would be a = set of DXE Services that allow WaitForEvent, CheckEvent, Event registration= at TPLs other than notify/callback.  The UEFI system table versions o= f the functions would still have this restriction but code built with the p= latform could use the DXE Services. Right now, any attempt to use a non-UEF= I TPL will ASSERT, so we can keep that in place on the SystemTable interfac= e, but allow the platform to go around it with DXE Services. Similar functi= onality would have to be provided by the Mmst, but that=E2=80=99s already p= latform-specific and can probably allow it in all cases.
 
We=E2=80=99re suggesting something like the below:<= o:p class=3D"">
&nbs= p;
//
// Task priority l= evel
//
#define TPL_APPLICATION       4
#define TPL_CALLBACK_LOW   &n= bsp;  7
= #define TPL_CALLBACK = ;         8
#define TPL_CALLBACK_HIGH     9
#define TPL_NOTIFY_LOW    =     15
#define TPL_NOTI= FY            16
#define TPL_NOTIFY_HIGH   =     17
#define TPL_HIGH= _LEVEL        31
 
There=E2=80=99s already a long-in-the-tooth bug t= racking a similar issue:
http= s://bugzilla.tianocore.org/show_bug.cgi?id=3D1676=
 
This proposal is simpler than what=E2=80=99s in that b= ug, and would greatly simplify some of our event ordering (and code).
 
Thoughts?
<= div style=3D"margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibr= i, sans-serif;" class=3D""> 
If this conversation takes too long, I will publish a set of = patches that just go with the lesser solution posted above, but I=E2=80=99d= much rather work the problem this way.
 
- Bret


--Apple-Mail=_F5D814C9-49BA-43DF-A3B7-AD95A46CF006--