From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp01.apple.com (ma1-aaemail-dr-lapp01.apple.com [17.171.2.60]) by mx.groups.io with SMTP id smtpd.web09.16.1583267280254443978 for ; Tue, 03 Mar 2020 12:28:00 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=Za+K69uS; spf=pass (domain: apple.com, ip: 17.171.2.60, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp01.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp01.apple.com (8.16.0.27/8.16.0.27) with SMTP id 023KHOQQ062522; Tue, 3 Mar 2020 12:27:58 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=20180706; bh=LuutfuXtf3YtzyQA0eX8Pk5UtiUkQUG9G9lIfHsYj4E=; b=Za+K69uSd8fGnd8wquuCjJ47DpdoSdccxoqfUFSLgS+dG3mJszaSKvCbytyriYIgnBgC 2Gze+AVs8aSCfT3+jzlQrK7s6Bldxxe/nf/tS2EKWsweemjGO0MMPUvc0uhmkTCXwPx2 ciiyNK6JXuOaX85d1SgJwRKeUkOWB2F6UvpWonkArSap5I4Lpl7BOeu/JdsVLWWXsO/m HCxRlTvB2CHI1irdk3Au5dkewIktUgffZIjebQwMSlQDKqoasln5Cipg/yCo7IR5GsP3 h8oltodfQQRXGFrkbpdNhHm1Pj8AS523QLLvilgu+4rqi7ZBZ1w5iOu/h87nDn2RRXal SA== Received: from rn-mailsvcp-mta-lapp04.rno.apple.com (rn-mailsvcp-mta-lapp04.rno.apple.com [10.225.203.152]) by ma1-aaemail-dr-lapp01.apple.com with ESMTP id 2yfqg68wrn-11 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Tue, 03 Mar 2020 12:27:58 -0800 Received: from nwk-mmpp-sz10.apple.com (nwk-mmpp-sz10.apple.com [17.128.115.122]) by rn-mailsvcp-mta-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.1.20190704 64bit (built Jul 4 2019)) with ESMTPS id <0Q6M00XCTWUK5C00@rn-mailsvcp-mta-lapp04.rno.apple.com>; Tue, 03 Mar 2020 12:27:56 -0800 (PST) Received: from process_milters-daemon.nwk-mmpp-sz10.apple.com by nwk-mmpp-sz10.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0Q6M00800W9R4700@nwk-mmpp-sz10.apple.com>; Tue, 03 Mar 2020 12:27:56 -0800 (PST) X-Va-A: X-Va-T-CD: 5ba617998c06f710a1787aac0b6b40c1 X-Va-E-CD: ce6e59d19e41f9f978af9e8b590c623d X-Va-R-CD: 4ad19a74e8868beceae55112f7f03bd9 X-Va-CD: 0 X-Va-ID: 9bb8d834-dc6a-4fd3-a9ce-a58168fe2682 X-V-A: X-V-T-CD: 5ba617998c06f710a1787aac0b6b40c1 X-V-E-CD: ce6e59d19e41f9f978af9e8b590c623d X-V-R-CD: 4ad19a74e8868beceae55112f7f03bd9 X-V-CD: 0 X-V-ID: db0c9aaf-64c5-4583-8d05-db9f5267859f X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572 definitions=2020-03-03_06:2020-03-03,2020-03-03 signatures=0 Received: from [17.235.58.36] by nwk-mmpp-sz10.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0Q6M006RZWUFQP10@nwk-mmpp-sz10.apple.com>; Tue, 03 Mar 2020 12:27:53 -0800 (PST) Sender: afish@apple.com MIME-version: 1.0 (Mac OS X Mail 13.0 \(3594.4.17\)) Subject: Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe string constraint assertions From: "Andrew Fish" In-reply-to: Date: Tue, 03 Mar 2020 12:27:51 -0800 Cc: Mike Kinney , vit9696 , "Gao, Liming" , "Gao, Zhichao" , =?utf-8?Q?Marvin_H=C3=A4user?= , Laszlo Ersek Message-id: <3A1AF3A8-D148-4EA6-B2FE-C75B41B4E589@apple.com> References: <20200103171242.63839-1-vit9696@protonmail.com> <492A300B-1CED-4FF0-98F8-20D1E116DCC0@protonmail.com> <2719704F-6DB4-444F-97FE-DBF71D39B698@apple.com> <3D7EB7E9-9B94-40AD-A0E9-BFDEB787F6DA@apple.com> To: devel@edk2.groups.io, mhaeuser@outlook.de X-Mailer: Apple Mail (2.3594.4.17) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572 definitions=2020-03-03_06:2020-03-03,2020-03-03 signatures=0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: quoted-printable Marvin, If no one else objects to your proposal I don't want to block forward prog= ress. My main intent was to have a conversation about this topic.=20 > On Feb 20, 2020, at 2:18 AM, Marvin H=C3=A4user wr= ote: >=20 > Hey Andrew, >=20 > Thanks once again for your comments, mine are inline. >=20 > Best regards, > Marvin >=20 > Am 20.02.2020 um 00:55 schrieb Andrew Fish: >>=20 >>=20 >>> On Feb 17, 2020, at 12:26 AM, Marvin H=C3=A4user >> > wrote: >>>=20 >>> Good day Andrew, >>>=20 >>> First of all, thank you very much for putting this amount of thought >>> into the situation. I definitely agree with the problem you see, but I >>> could also live with Vitaly's proposal. However, I think you are >>> overcomplicating the situation a little. So, we do agree the caller >>> needs control of the behaviour, however your proposal needs "support" >>> from both the caller (choose the handler) and the callee (call the >>> handler) and is neither thread-safe nor event-safe as Vitaly already >>> pointed out. Fixing these problems would only worsen the complexity >>> situation in my opinion. >>=20 >> Well EFI does not have Threads but it does have Events. I agree if an= =20 >=20 > Sorry, we were refering to the UEFI Multi-Processor services, so threads= = =20 > in a physical sense. I think very similar concerns apply regarding these= = =20 > services as with software multithreading in this context, please correct= = =20 > me if I'm wrong. It is not legal to do anything EFI on the APs. The locks in the DXE Core a= re TPL based so that is a software construct that only exists on the BSP.= =20 Technically speaking from a LibraryClass definition you should not run any= libs that have ASSERT or DEBUG macros on the AP given the DebugLib Library= Class is not MP safe. In reality it is possible to have an MP safe version= of the DebugLib, and it is likely people are just getting away with MdePkg= libs since it is not an issue unless they ASSERT or DEBUG print.=20 I'd also point out that most code does not do work on the APs. On a typica= l x86 firmware it is Microcode, SMM, and the driver producing the MP Servi= ces than run code on the APs. While most other code does not.=20 Note: EFI runs on the BSP, the other CPUs in the systems are called APs.= =20 Thanks, Andrew Fish >=20 >> Event handler changes the constraint handler it would need to restore i= t=20 >> so my proposal is missing an API. >>=20 >> CONSTRAINT_HANDLER * >> GetConstraintHandler ( >> VOID >> ) >> { >> return gActiveConstraintHandler; >> } >>=20 >> You an always use the standard EFI >>=20 >> It is important to remember that all the EFI images=20 >> (drivers/applications) are statically linked and they carry a unique=20 >> instance of the Debug (or new Constraint) lib. So in your driver or App= = =20 >> is very self contained. Also a lot of the events are phase related=20 >> callbacks, and since there are no threads your drivers main thread is= =20 >> only really running when your driver, or app, dispatches. A lot of the= =20 >> callbacks are via protocols your driver publishes, but they have strict= = =20 >> TPL rules so you can prevent reentrancy in general. So there is a lot= =20 >> more determinism in EFI vs. generic threaded coding. >=20 > This is true, but yet all edge-cases should be covered (event=20 > "interruption", actual (processor) interrupts, Multi-Processor=20 > execution), so developers do not run into unexpected (seemingly)=20 > undeterministic behaviour. However, I agree it is easier to ensure for= =20 > something like UEFI than for a full-fledged OS of course. >=20 >>=20 >>>=20 >>> Diverging from both of your concepts entirely and keeping it very >>> simple, what keeps us from simply ASSERTing based on return value? We >>> could introduce a CONSTRAINT_ASSERT or similar, but it would be >>> completely different from Vitaly's proposal. It would be a caller macr= o >>> to ASSERT based on a pre-defined list of return statuses. >>> To achieve this, we would order all statuses by types (classes). At th= e >>> moment, I can only think of two: "environmental" and "constraint". For >>> example, "Out Of Resources" would be environmental and "Invalid >>> Parameter" would be constraint. We could define environment statuses >>> explicitly (as it is less likely new environment statuses are >>> introduced) and define constraint statuses as "not environmental" (to >>> silently support new constraint statuses, however of course it is a >>> little error-prone with forwards-compatibility). As a bonus, it forces >>> developers to use truly appropiate error codes and correctly propagate >>> them from callees for this to work. :) >>> This solution would be backwards-compatible in a compilation sense as = it >>> can simply be a new macro, however for its possible complexity (switch >>> cases) I might actually prefer a function. But it would not be >>> backwards-compatible with the current functionality, which none of the >>> "caller-based" concepts can be anyway (the caller needs explicit code)= . >>>=20 >>=20 >> It is an interesting idea to add granularity, but at the end of the=20 >> knowledge of the correct behavior of the lower level library code reall= y=20 >> lives in code that calls this. I will admit I can see some value to=20 >> making RETURN_INVALID_PARAMETER different than RETURN_BUFFER_TOO_SMALL. >=20 > So far, I prefer this idea as the only requirement is that function=20 > contracts are well-designed (i.e. reflect environmental vs constraint=20 > errors by using sensical return values). >=20 >>=20 >> I kind of went down the C11 path (thanks for mentioning the event issue= )=20 >> but there are other ways to empower the caller. >=20 > It's definitely always a good idea to stick to standards and=20 > conventions. It appears my proposal is not conventional at all. However,= = =20 > I also believe there sometimes is a more pragmatic approach to things=20 > without imposing significant disadvantages, especially by limiting the= =20 > scope. >=20 > To be honest, I'm now interested for which cases the C11 path is=20 > advantagous over my approach, except that setting the constraint handler= = =20 > is a one-time thing if it remains consistent throughout execution. I'd= =20 > imagine for some more complex handling logic I cannot think of an=20 > example of (all we really want to do is ASSERT conditionally afterall),= =20 > and I hope this is not something edk2 would ever run into - keep it=20 > simple in my opinion. >=20 >>=20 >> We could let the caller decide the behavior. That implies passing=20 >> CHECK_DEFAULT (PCD), CHECK_ASSERT, or CHECK_NULL. >>=20 >> Then keep backward compatibility. >> #define StrCpyS(Destination,DestMax,Source)StrCpuSC(Destination,=20 >> DestMax, Source, CHECK_DEFAULT) >>=20 >> But that seems like a lot of thrash to the BaseLib and a lot of new=20 >> magic to teach developers. >=20 > Fully agreed with the last sentence. That was one of the approaches I=20 > mentioned before my proposal, but I included it only for completeness'= =20 > sake so we have a full overview. This requires prototype adaption for=20 > literally every function that may be used in such a way (which scopes=20 > beyond just BaseLib), ugly macros and more. Please don't do that. :) >=20 >>=20 >> I would guess that the most common usage of my library would be to turn= = =20 >> Constraint Checking off in the entire driver or app and then handle the= = =20 >> errors manually. >=20 > I'm afraid that's what most platforms will end up with. Our main concern= = =20 > was we may not allow such constraint violations to ASSERT, that is our= =20 > main objective. However, if it can be made advantageous to future coding= = =20 > practice, it would be nice to have a decent solution for the caller to= =20 > have some freedom. More about that below. >=20 >>=20 >> On driver/app entry do: >>=20 >> SetConstraintHandler (IgnoreConstraintHandler); >>=20 >> Then handle the errors you care about. >>=20 >> Status =3D StrCpyS (Destination,DestMax,Source); >> if (EFI_ERROR (Status)) { >> ASSERT_EFI_ERROR (Status); >> return Status; >> } >>=20 >> or >>=20 >> Status =3D StrCpyS (Destination,DestMax,Source); >> if (Status =3D=3D RETURN_INVALID_PARAMETER) { >> ASSERT_EFI_ERROR (Status); >> return Status; >> } >>=20 >> At the end of the day I've code my driver and I know the rules I coded= =20 >> to and I want predictable behavior. >=20 > Yes, agreed. We want predictable behaviour of "not ASSERTing" for=20 > untrusted input, because we *know* it is untrusted and the function=20 > *can* (and should be able to) handle it - it is simply not a preconditio= n. >=20 >>=20 >> I can see a Si vendor developing with ASSERT Constraint off and then=20 >> when the customer turns it on stuff starts randomly breaking. >=20 > A lot of Si and core (DxeCore, PeiCore, related libraries) code seems to= = =20 > ASSERT (frequently *only* ASSERT with no actual handling, especially for= = =20 > AllocatePool() =3D=3D NULL) when there are valid error situations possib= le,=20 > so I hope this approach would be a handy tool to satisfy their needs=20 > without this absolutely terrible practice. :) >=20 >=20 >=20 > I think I understood all your comment individually, but I'm afraid I'm= =20 > not sure what your suggested solution is right now. You commented=20 > semi-positively on my proposal, you did not revoke your C11 path, and=20 > you furthermore introduced CHECK_* - this is brainstorming, and I think= =20 > it's a great thing, but I'm simply not sure what your exact goal or=20 > prefered route is right now. >=20 > My proposal is a bit unconventional and I agree it sounds hacky, but so= =20 > far I'm not convinced it would be bad practice at all. It boils down to= =20 > sensefully using the information a function returns. This requires=20 > nothing but the function to return sensical information, which every=20 > function should anyway. >=20 > Your proposal is close to the C standard, and that is a good thing first= = =20 > of all. However, with the rest of edk2 not being very compliant, I don't= = =20 > think it's an instant win as in "we just comply to standards" because=20 > this choice does not suddenly make porting existing code significantly= =20 > easier compared to the total lack of standard library support for=20 > anything but Shell apps. > Pragmatically, I think it solves the same problem equally well, but at a= = =20 > higher cost (ensuring the implementation is safe and, strictly speaking,= = =20 > an ever-so-slight runtime overhead). >=20 >>=20 >>> However, CONSTRAINT_ASSERT might actually be too specific. Maybe we wa= nt >>> something like "ASSERT_RETURN" and have the classes "ASSERT_CONSTRAINT= " >>> and "ASSERT_ENVIRONMENT" (bitfield, like with DEBUG). The reason for >>> this is embedded systems where the environment is trusted/known and th= e >>> firmware is supposed to be well-matched. Think of a SoC with soldered >>> RAM - the firmware can very well make assumption about memory capacity >>> (considering usage across all modules in the worst case) and might >>> actually want ASSERTs on something like "Out Of Resources" because it'= s >>> supposed to be impossible within the bounds of this specific design. >>> It's possible this would need a new PCD's involvement, for example to >>> tell DxeCore whether to ASSERT on environmental aspects too. There cou= ld >>> be an argument-less macro that uses PCD config as base, and an >>> argument-based macro that uses only the parameters. Of course this >>> cannot cover everyone's very specific preferences as it's a per-module >>> switch, but it's the best I can think of right now. >>>=20 >>> Something a bit unrelated now, but it would make this easier. You >>> mentioned PeCoffLib as example, and I think it's a good one, however i= t >>> has its own return status type within the context[1] which I will most >>> definitely be getting rid of as part of my upcoming (in the far-ish >>> future :) ) RFC. Could we expand RETURN_STATUS to have (very high?) >>> reserved values for caller-defined error codes to have all RETURN_STAT= US >>> macros apply to those special return values too? We'd need to define >>> them categorically as "constraint status", but I don't see how a libra= ry >>> would declare a new environment status anyway. >>>=20 >>> Regarding MdePkgCompatability.dsc.inc, I think one can override librar= y >>> classes from the inc by declaring them after the !include statement, >>> please correct me if I'm wrong. If so, I strongly agree and think it >>> should be the case for all packages, so one only overrides the default= s >>> when something specific (debugging, performance-optimized versions, ..= .) >>> is required - easier to read, easier to maintain. The content is >>> needed/there anyway as the libraries are declared in the package's own= = =20 >>> DSC. >>>=20 >>=20 >> Yes it the new .inc DSC file could be overridden by the platform DSC=20 >> that includes it. >>=20 >> I think this is a more general idea than something for this given patch= .=20 >> It is something we could make sure we update every stable tag or so, bu= t=20 >> I guess someone has to go 1st :). It would make it easier on platforms= =20 >> when they update the edk2 version. >=20 > Agreed. >=20 >>=20 >>=20 >>=20 >>=20 >>> It would be nice if you had comments regarding every aspect I just >>> mentioned, it was just something coming to my mind this morning. Thank= s >>> for the input so far, it's nice to see some movement now! >>>=20 >>=20 >> Sorry for the delay >=20 > Sure, thanks for taking time to respond! >=20 >>=20 >> Thanks, >>=20 >> Andrew Fish >>=20 >>> Best regards, >>> Marvin >>>=20 >>> [1] >>> https://github.com/tianocore/edk2/blob/f1d78c489a39971b5aac5d2fc8a39bf= a925c3c5d/MdePkg/Include/Library/PeCoffLib.h#L20-L31 >>> https://github.com/tianocore/edk2/blob/f1d78c489a39971b5aac5d2fc8a39bf= a925c3c5d/MdePkg/Include/Library/PeCoffLib.h#L159 >>>=20 >>> Am 16.02.2020 um 22:25 schrieb Andrew Fish via Groups.Io: >>>> Mike, >>>>=20 >>>> Sorry I don't think I totally understood what felt wrong to me, so I = did >>>> a bad job explaining my concerns. Also I don't think I was thinking >>>> enough in the context of the C11 StdLib. >>>>=20 >>>> I think my concern is still the global scope of the constraint, even = if >>>> we tune it per module. For example the DXE Core can interact with >>>> PE/COFF images and other data that could have indirectly come from th= e >>>> disk. So conceptually you may want to ASSERT on some constraints and = not >>>> on others. I think that is why I ratholed on expanding the error >>>> handling macros as that was more in the vein of having the caller dea= l >>>> with it, so that is kind of like what you would do pre C11. Also the >>>> DebugLib is probably one of the MdePkg Libs that has the most instanc= es >>>> floating around, so I think we should change it in a non backwards wa= y >>>> very carefully. >>>>=20 >>>> So after reading up on the C11 implementation of Constraints I think = my >>>> alternate proposal is for us to add a ConstraintLib modeled after C11 >>>> vs. updating the DebugLib. This would solve the 2 things that made me >>>> uncomfortable: 1) Extending the DebugLib API; 2) Giving the caller >>>> control of the ASSERT behavior. It would still have the down side of >>>> breaking builds as the BaseLib would get a new dependency, so we coul= d >>>> talk about adding these functions to the DebugLib as the cost of >>>> replicating code. >>>>=20 >>>> C11 defines constraint_handler_t and set_constraint_handler_s as a wa= y >>>> for the caller to configure the behavior for bounds checked functions= . I >>>> think that is the behavior I prefer. So if we are going to make a cha= nge >>>> that impacts DebugLib compatibility I just want to make sure we have = a >>>> conversation about all the options. My primary goal is we have the >>>> conversation, and if folks don't agree with me that is fine at least = we >>>> talked about it. >>>>=20 >>>> What I'm thinking about is as simply exposing an API to control the >>>> Constraint handler like C11. This could be done via an ConstrainLib o= r >>>> extending the DebugLib. >>>>=20 >>>> The basic implementation of the lib would look like: >>>>=20 >>>> typedef >>>> VOID >>>> (EFIAPI *CONSTRAINT_HANDLER) ( >>>> IN CONST CHAR8 *FileName, >>>> IN UINTN LineNumber, >>>> IN CONST CHAR8 *Description, >>>> IN EFI_STATUS Status >>>> ); >>>>=20 >>>>=20 >>>> // Default to AssertConstraintHandler to make it easier to implement >>>> Base and XIP libs. >>>> // We could have a PCD that also sets the default handler in a Lib >>>> Constructor. The default handler is implementation defined in C11. >>>> CONSTRAINT_HANDLER gDefaultConstraintHandler =3D AssertConstraintHand= ler; >>>> CONSTRAINT_HANDLER gActiveConstraintHandler =3D gDefaultConstraintHan= dler; >>>>=20 >>>> BOOLEAN >>>> EFIAPI >>>> ConstraintAssertEnabled ( >>>> VOID >>>> ) >>>> { >>>> return (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) & >>>> DEBUG_PROPERTY_DEBUG_CONSTRAINT_ENABLED) !=3D 0); >>>> } >>>>=20 >>>> EFI_STATUS >>>> EFIAPI >>>> SetConstraintHandler ( >>>> IN CONSTRAINT_HANDLER Handler >>>> ) >>>> { >>>> if (Handler =3D=3D NULL) { >>>> gActiveConstraintHandler =3D gDefaultConstraintHandler; >>>> } else { >>>> gActiveConstraintHandler =3D Handler; >>>> } >>>> } >>>>=20 >>>> VOID >>>> AssertConstraintHandler ( >>>> IN CONST CHAR8 *FileName, >>>> IN UINTN LineNumber, >>>> IN CONST CHAR8 *Description, >>>> IN EFI_STATUS Status >>>> ) >>>> { >>>> if (ConstraintAssertEnabled ()) { >>>> DEBUG ((EFI_D_ERROR, "\Constraint ASSERT (Status =3D %r): ", Sta= tus)); >>>> DebugAssert (FileName, LineNumber, Description) >>>> } >>>>=20 >>>> return; >>>> } >>>>=20 >>>> VOID >>>> IgnoreConstraintHandler ( >>>> IN CONST CHAR8 *FileName, >>>> IN UINTN LineNumber, >>>> IN CONST CHAR8 *Description, >>>> IN EFI_STATUS Status >>>> ) >>>> { >>>> return; >>>> } >>>>=20 >>>> We could add macros for the code in the lib to call: >>>>=20 >>>> #define CONSTRAINT_CHECK(Expression, Status) \ >>>> do { \ >>>> if (!(Expression)) { \ >>>> gActiveConstraintHandler (__FILE__, __LINE__, Expression, Status); \ >>>> return Status; \ >>>> } \ >>>> } while (FALSE) >>>>=20 >>>> #define CONSTRAINT_REQUIRE(Expression, Status, Label) \ >>>> do { \ >>>> if (!(Expression)) { \ >>>> gActiveConstraintHandler (__FILE__, __LINE__, Expression, Status); \ >>>> goto Label; \ >>>> } \ >>>> } while (FALSE) >>>>=20 >>>>=20 >>>> As a caller we have now have control: >>>> EFI_STATUS Status; >>>> CHAR16 Dst[2]; >>>>=20 >>>> SetConstraintHandler (IgnoreConstraintHandler); >>>> Status =3D StrCpyS (Dst, sizeof (Dst), L"Too Long"); >>>> Print (L"Dst =3D%s (%r)\n", Dst, Status); >>>>=20 >>>> SetConstraintHandler (AssertConstraintHandler); >>>> StrCpyS (Dst, sizeof (Dst), L"Too Long"); >>>>=20 >>>> Thanks, >>>>=20 >>>> Andrew Fish >>>>=20 >>>> PS Since I'm on a crazy idea roll another idea would be to add a >>>> MdePkgCompatability.dsc.inc file that could be used to future proof >>>> adding dependent libs to existing MdePkg libs. So a platform could >>>> include this .DSC and that would give them the default library mappin= g >>>> to keep code compiling. It will only work after other platforms start >>>> including it, but after that it would give default mappings for >>>> dependent libs. >>>>=20 >>>> In our above example we could have added this and then existing build= s >>>> that included MdePkgCompatability.dsc.inc would keep compiling. >>>>=20 >>>> [LibraryClasses] >>>>=20 >>>> DebugConstraintLib|MdePkg/Library/DebugConstraintLib/DebugConstraintL= ib.inf >>>>=20 >>>>=20 >>>>=20 >>>>> On Feb 15, 2020, at 11:38 AM, Michael D Kinney >>>>> >>>> >=20 >>>>> wrote: >>>>>=20 >>>>> Andrew, >>>>> I do not think of this as a globally scoped PCD.It can be set in >>>>> global scope in DSC.But is can also be set to values based on module >>>>> type or for specific modules.In the case of the safe string function= s, >>>>> I think there is a desire to disable the constraint asserts when >>>>> building a UEFI App or UEFI Driver and implement those modules to >>>>> handle the error return values.Enabling the constraint asserts for P= EI >>>>> Core, DXE Core, SMM/MM Core, PEIM, DXE, SMM/MM modules makes sense t= o >>>>> find incorrect input to these functions from modules that can >>>>> guarantee the inputs would never return an error and catch these as >>>>> part of dev/debug/validation builds. >>>>> I would not expect disabling on a module by module basis to be commo= n. >>>>> I think the rule for API implementations is to only use >>>>> CONSTRAINT_ASSERT() for conditions that are also checked and return = an >>>>> error or fail with predicable behavior that allows the system to >>>>> continue to function.ASSERT() is for conditions that the systemcan >>>>> notcontinue. >>>>> Best regards, >>>>> Mike >>>>> *From:*afish@apple.com=20 >>>>> >>>> >>>>> > >>>>> *Sent:*Friday, February 14, 2020 10:27 PM >>>>> *To:*vit9696 >>>> > >>>>> *Cc:*devel@edk2.groups.io=20 >>>>> ; Kinney, >>>>> Michael D >>>> >>>>> >; Gao, Liming >>>>> >>>> >; Gao,=20 >>>>> Zhichao >>>>> >>>> >;=20 >>>>> Marvin H=C3=A4user >>>>> >>>> >; >>>>> Laszlo Ersek >>>> > >>>>> *Subject:*Re: [edk2-devel] [PATCH v3 0/1] Add PCD to disable safe >>>>> string constraint assertions >>>>> Vitaly, >>>>> Sorry after I sent the mail I realized it may come across as me >>>>> asking you to do work and that was not my intent. >>>>> I will point out though that a non backward compatible change to >>>>> something as fundamental as the DebugLib is a very big deal. I've go= t >>>>> a few different custom implementations that would break with this >>>>> change as Mike proposed. Given breaking every one's debug lib is suc= h >>>>> a big deal maybe it is something that we should do as a long term pl= an >>>>> vs. some incremental fix. So my intent was to start a conversation >>>>> about what else we might want to change if we are going to break the >>>>> world. The only think worse than breaking the world is breaking the >>>>> world frequently. >>>>> I'm also a little worried that we are taking things that are today >>>>> locally scoped like SAFE_STRING_CONSTRAINT_CHECK and >>>>> SAFE_PRINT_CONSTRAINT_CHECK and making them global constructs. I thi= nk >>>>> the way others have dealt with things like this is to make them be >>>>> DEBUG prints vs. ASSERTs. Also even something as simple as >>>>> SAFE_STRING_CONSTRAINT_CHECK could be called from code that wants >>>>> ASSERT and CONSTRAINT_ASSERT behavior. It is not clear to me that th= e >>>>> low level code knows the right thing to do in a global sense even if >>>>> there is a PCD. It almost seems like we should have wrappers for th= e >>>>> Safe string functions that implement the behavior you want as a >>>>> caller. I'm not sure about that, but it seems like it is worth talki= ng >>>>> about? >>>>> Thanks, >>>>> Andrew Fish >>>>>=20 >>>>>=20 >>>>> On Feb 14, 2020, at 7:31 PM, vit9696 >>>> >>>>> > wrote: >>>>> Hi Andrew, >>>>> While your suggestions look interesting, I am afraid they are not >>>>> particularly what we want to cover with this discussion at the=20 >>>>> moment. >>>>> Making assertions go through DEBUG printing functions/macros is >>>>> what we have to strongly disagree about. Assertions and debug >>>>> prints are separate things configurable by separate PCDs. We >>>>> should not mix them. Introducing constraint assertions is a >>>>> logical step forward in understanding that different software >>>>> works in different environments. >>>>>=20 >>>>> * There are normal, or, as I call them, invariant >>>>> assertions (e.g. preconditions), for places where the functio= n >>>>> cannot work unless the assertion is satisfied. This is where >>>>> we ASSERT. >>>>> * There are constraint assertions, which signalise that bad dat= a >>>>> came through the function, even though the function was calle= d >>>>> from a trusted source. This is where we call CONSTRAINT_ASSER= T. >>>>>=20 >>>>> The thing we need is to have the latter separable >>>>> and configurable, because not every piece of software works in a >>>>> trusted environment. Other than that, constraint assertions, when >>>>> enabled, are not anyhow different from normal assertions in the >>>>> sense of action taken. Assertions have configurable breakpoints >>>>> and deadloops, and DEBUG prints go through a different route in >>>>> DebugLib that may cause entirely different effects. For example, >>>>> we halt execution upon printing to DEBUG_ERROR in our DebugLib >>>>> even in release builds. >>>>>=20 >>>>> =3DTo make it clear, currently I plan to add the following interf= ace: >>>>> #define CONSTRAINT_ASSERT(Expression) \ >>>>> do { \ >>>>> if (DebugConstraintAssertEnabled ()) { \ >>>>> if (!(Expression)) { \ >>>>> _ASSERT (Expression); \ >>>>> ANALYZER_UNREACHABLE (); \ >>>>> } \ >>>>> } \ >>>>> } while (FALSE) >>>>> with DebugConstraintAssertEnabled implemented as >>>>> (BOOLEAN) ((PcdGet8(PcdDebugPropertyMask) & >>>>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED | >>>>> DEBUG_PROPERTY_CONTRAINT_ASSERT_DISABLED) =3D=3D >>>>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) >>>>> Your suggestion with require macros looks interesting indeed, but >>>>> I believe it is in fact parallel to this discussion. The change w= e >>>>> discuss introduces a new assertion primitive =E2=80=94 constraint >>>>> assertions, while REQUIRE macros are mostly about advanced syntax >>>>> sugar and higher level assertion primitives on top of existing >>>>> ones. Perhaps we can have this and make a good use of it, >>>>> especially given that it brought some practical benefit in Apple, >>>>> but I would rather discuss this later once constraint assertions >>>>> are merged into EDK II tree. >>>>> Best wishes, >>>>> Vitaly >>>>> On Sat, Feb 15, 2020 at 03:02, Andrew Fish >>>> >>>>> > wrote: >>>>>=20 >>>>>=20 >>>>>=20 >>>>> On Feb 14, 2020, at 2:50 PM, Michael D Kinney >>>>> >>>> >>>>> > wrote: >>>>> Hi Vitaly, >>>>> I agree that this proposal makes a lot of sense. We >>>>> recently added a new assert type called STATIC_ASSERT() >>>>> for assert conditions that can be tested at build time. >>>>> A new assert type for checks that can be removed and the >>>>> API still guarantees that it fails gracefully with a >>>>> proper return code is a good idea. Given we have >>>>> STATIC_ASSERT(), how about naming the new macro >>>>> CONSTRAINT_ASSERT()? >>>>> We also want the default to be enabled. The current use o= f >>>>> bit 0x40 inPcdDebugPropertyMask is always clear, so we >>>>> want the asserts enabled when 0x40 is clear. We can chang= e >>>>> the name of the define bit to >>>>> DEBUG_PROPERTY_CONTRAINT_ASSERT_DISABLED so bit 0x40 need= s >>>>> to be set inPcdDebugPropertyMaskto disable these types of >>>>> asserts. >>>>> This approach does require all theDebugLibimplementations >>>>> to be updated with the newDebugConstraintAssertDisabled()= = =20 >>>>> API. >>>>>=20 >>>>> Mike, >>>>> If you wanted to be backward compatible you could just >>>>> use DebugAssertEnabled () but in place of _ASSERT() use >>>>> _CONSTRAINT_ASSERT >>>>> #define _ASSERT(Expression) DebugAssert (__FILE__, __LINE__, >>>>> #Expression) >>>>> #define _CONSTRAINT_ASSERT(Expression) DebugPrint >>>>> (DEBUG_ERROR, "ASSERT %a(%d): %a\n",, __FILE__, __LINE__, >>>>> #Expression) >>>>> Not as elegant as the non backward compatible change, but I >>>>> thought I'd throw it out there. >>>>> There are some ancient Apple C ASSERT macros [AssertMacros.h] >>>>> that also have the concept of require. Require includes an >>>>> exception label (goto label). It is like a CONSTRAINT_ASSERT(= ) >>>>> but with the label. On release builds the DEBUG prints are >>>>> skipped. >>>>> So we could do something like: >>>>> EFI_STATUS Status =3D EFI_INVALID_PARAMETER; >>>>> REQUIRE(Arg1 !=3D NULL, ErrorExit); >>>>> REQUIRE(Arg2 !=3D NULL, ErrorExit); >>>>> REQUIRE(Arg3 !=3D NULL, ErrorExit); >>>>> ErrorExit: >>>>> return Status; >>>>> There is another form that allows an ACTION (a statement to >>>>> execute. So you could have: >>>>> EFI_STATUS Status; >>>>> REQUIRE_ACTION(Arg1 !=3D NULL, ErrorExit, Status =3D >>>>> EFI_INVALID_PARAMETER); >>>>> REQUIRE_ACTION(Arg2 !=3D NULL, ErrorExit, Status =3D >>>>> EFI_INVALID_PARAMETER); >>>>> REQUIRE_ACTION(Arg3 !=3D NULL, ErrorExit, Status =3D >>>>> EFI_INVALID_PARAMETER); >>>>> ErrorExit: >>>>> return Status; >>>>> If you use CONSTRAINT_ASSERT(); >>>>> if (Arg1 =3D=3D NULL || Arg2 =3D=3D NULL || Arg3 =3D=3D NUL= L) { >>>>> CONSTRAINT_ASSERT (Arg1 !=3D NULL); >>>>> CONSTRAINT_ASSERT (Arg2 !=3D NULL); >>>>> CONSTRAINT_ASSERT (Arg3 !=3D NULL); >>>>> return EFI_INVALID_PARAMETER; >>>>> } >>>>> I'd note error processing args on entry is the simplest case. >>>>> In a more complex case when cleanup is required the goto >>>>> label is more useful. >>>>> I guess we could argue for less typing and more symmetry and >>>>> say use ASSERT, CONSTRAINT, and REQUIRE. I guess you could ad= d >>>>> an ASSERT_ACTION too. >>>>> The AssertMacros.h versions also support _quiet (skip the >>>>> print) and _string (add a string to the print) so you end up= =20 >>>>> with: >>>>> REQUIRE >>>>> REQUIRE_STRING >>>>> REQUIRE_QUIET >>>>> REQUIRE_ACTION >>>>> REQUIRE_ACTION_STRING >>>>> REQUIRE_ACTION_QUIET >>>>> We could also end up with >>>>> CONSTRAINT >>>>> CONSTRAINT_STRING >>>>> CONSTRAINT_QUIET >>>>> I think the main idea behind _QUIET is you can silence things >>>>> that are too noisy, and you can easily make noise things show >>>>> up by removing the _QUIET to debug. >>>>> I'd thought I throw out the other forms for folks to think >>>>> about. I'm probably biased as I used to looking at code and >>>>> seeing things like require_action_string(Arg1 !=3D NULL, >>>>> ErrorExit, Status =3D EFI_INVALID_PARAMETER, "1st Arg1 check"= ); >>>>> Thanks, >>>>> Andrew Fish >>>>> PS The old debug macros had 2 versions of CONSTRAINT check an= d >>>>> verify. The check version was compiled out on a release build= , >>>>> the verify version always does the check and just skips the >>>>> DEBUG print. >>>>>=20 >>>>> Mike >>>>> *From:*vit9696 >>>> >>>>> > >>>>> *Sent:*Friday, February 14, 2020 9:38 AM >>>>> *To:*Kinney, Michael D >>>> >>>>> > >>>>> *Cc:*devel@edk2.groups.io=20 >>>>> ; >>>>> Gao, Liming >>>> >>>>> >; Gao, Zhichao >>>>> >>>> >; >>>>> Marvin H=C3=A4user >>>> >>>>> >; Laszlo Ersek >>>>> >>>> > >>>>> *Subject:*Re: [edk2-devel] [PATCH v3 0/1] Add PCD to >>>>> disable safe string constraint assertions >>>>> Michael, >>>>> Generalising the approach makes good sense to me, but we >>>>> need to make an obvious distinguishable difference betwee= n: >>>>> - precondition and invariant assertions (i.e. assertions, >>>>> where function will NOT work if they are violated) >>>>> - constraint asserts (i.e. assertions, which allow us to >>>>> spot unintentional behaviour when parsing untrusted data, >>>>> but which do not break function behaviour). >>>>> As we want to use this outside of SafeString, I suggest >>>>> the following: >>>>> - Introduce DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED 0x40 >>>>> bit for PcdDebugPropertyMask instead >>>>> of PcdAssertOnSafeStringConstraints. >>>>> - Introduce DebugAssertConstraintEnabled DebugLib functio= n >>>>> to check for DEBUG_PROPERTY_ASSERT_CONSTRAINT_ENABLED. >>>>> - Introduce ASSERT_CONSTRAINT macro, that will assert onl= y >>>>> if DebugConstraintAssertEnabled returns true. >>>>> - Change SafeString ASSERTS >>>>> in SAFE_STRING_CONSTRAINT_CHECK to ASSERT_CONSTRAINTs. >>>>> - Use ASSERT_CONSTRAINT in other places where necessary. >>>>>=20 >>>>> I believe this way lines best with EDK II design. If ther= e >>>>> are no objections, I can submit the patch in the beginnin= g >>>>> of next week. >>>>>=20 >>>>> Best wishes, >>>>> Vitaly >>>>>=20 >>>>> 14 =D1=84=D0=B5=D0=B2=D1=80. 2020 =D0=B3., =D0=B2 20:= 00, Kinney, Michael D >>>>> >>>> >>>>> > =D0=BD=D0=B0=D0= =BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>>>> Vitaly, >>>>> I want to make sure a feature PCD can be used to >>>>> disable ASSERT() behavior in more than just safe >>>>> string functions inBaseLib. >>>>> Can we consider changing the name and description >>>>> ofPcdAssertOnSafeStringConstraintsto be more generic, >>>>> so if we find other lib APIs, the name will make sens= e? >>>>> Maybe something like:PcdEnableLibraryAssertChecks? >>>>> Default is TRUE. Can set to FALSE in DSC file to >>>>> disable ASSERT() checks. >>>>> Thanks, >>>>> Mike >>>>> *From:*devel@edk2.groups.io >>>>> >>>> >>>>> >*On Behalf Of*Vitaly >>>>> Cheptsov via Groups.Io >>>>> *Sent:*Friday, February 14, 2020 3:55 AM >>>>> *To:*Kinney, Michael D >>>> >>>>> >; Gao, Liming >>>>> >>>> >; >>>>> Gao, Zhichao >>>> >>>>> >;devel@edk2.groups.io= =20 >>>>> >>>>> >>>>> *Cc:*Marvin H=C3=A4user >>>> >>>>> >; Laszlo Ersek >>>>> >>>> > >>>>> *Subject:*Re: [edk2-devel] [PATCH v3 0/1] Add PCD to >>>>> disable safe string constraint assertions >>>>> Replying as per Liming's request for this to be merge= d >>>>> into edk2-stable202002. >>>>> On Mon, Feb 10, 2020 at 14:12, vit9696 >>>>> >>>>> > wrote: >>>>>=20 >>>>> Hello, >>>>>=20 >>>>> It has been quite some time since we submitted th= e >>>>> patch with so far no negative response. As I >>>>> mentioned previously, my team will strongly >>>>> benefit from its landing in EDK II mainline. Sinc= e >>>>> it does not add any regressions and can be viewed >>>>> as a feature implementation for the rest of EDK I= I >>>>> users, I request this to be merged upstream in >>>>> edk2-stable202002. >>>>>=20 >>>>> Best wishes, >>>>> Vitaly >>>>>=20 >>>>>> 27 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 12:47, vit9696 >>>>> >>>> >>>>> > =D0=BD=D0=B0=D0= =BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>>>>>=20 >>>>>>=20 >>>>>> Hi Mike, >>>>>>=20 >>>>>> Any progress with this? We would really benefit >>>>> from this landing in the next stable release. >>>>>>=20 >>>>>> Best, >>>>>> Vitaly >>>>>>=20 >>>>>>> 8 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 19:35, Kinney, Michael = D >>>>> >>>> >>>>> > =D0=BD=D0=B0= = =D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>>>>>>=20 >>>>>>>=20 >>>>>>> Hi Vitaly, >>>>>>>=20 >>>>>>> Thanks for the additional background. I would like >>>>>>> a couple extra day to review the PCD name and >>>>> the places >>>>>>> the PCD might potentially be used. >>>>>>>=20 >>>>>>> If we find other APIs where ASSERT() behavior >>>>> is only >>>>>>> valuable during dev/debug to quickly identify >>>>> misuse >>>>>>> with trusted data and the API provides predicable >>>>>>> return behavior when ASSERT() is disabled, then >>>>> I would >>>>>>> like to have a pattern we can potentially apply >>>>> to all >>>>>>> these APIs across all packages. >>>>>>>=20 >>>>>>> Thanks, >>>>>>>=20 >>>>>>> Mike >>>>>>>=20 >>>>>>>> -----Original Message----- >>>>>>>> From:devel@edk2.groups.io >>>>> >>>>> > On >>>>>>>> Behalf Of Vitaly Cheptsov via Groups.Io >>>>>>>> Sent: Monday, January 6, 2020 10:44 AM >>>>>>>> To: Kinney, Michael D >>>>> >>>> >>>>> > >>>>>>>> Cc:devel@edk2.groups.io >>>>> >>>>>>>> Subject: Re: [edk2-devel] [PATCH v3 0/1] Add >>>>> PCD to >>>>>>>> disable safe string constraint assertions >>>>>>>>=20 >>>>>>>> Hi Mike, >>>>>>>>=20 >>>>>>>> Yes, the primary use case is for UEFI >>>>> Applications. We >>>>>>>> do not want to disable ASSERT=E2=80=99s completely, as >>>>>>>> assertions that make sense, i.e. the ones >>>>> signalising >>>>>>>> about interface misuse, are helpful for debugging. >>>>>>>>=20 >>>>>>>> I have already explained in the BZ that >>>>> basically all >>>>>>>> safe string constraint assertions make no >>>>> sense for >>>>>>>> handling untrusted data. We find this use case >>>>> very >>>>>>>> logical, as these functions behave properly with >>>>>>>> assertions disabled and cover all these error >>>>>>>> conditions by the return statuses. In such >>>>> situation is >>>>>>>> not useful for these functions to assert, as >>>>> we end up >>>>>>>> inefficiently reimplementing the logic. I >>>>> would have >>>>>>>> liked the approach of discussing the interfaces >>>>>>>> individually, but I struggle to find any that >>>>> makes >>>>>>>> sense from this point of view. >>>>>>>>=20 >>>>>>>> AsciiStrToGuid will ASSERT when the length of the >>>>>>>> passed string is odd. Functions that cannot, ahem, >>>>>>>> parse, for us are pretty much useless. >>>>>>>> AsciiStrCatS will ASSERT when the appended >>>>> string does >>>>>>>> not fit the buffer. For us this logic makes this >>>>>>>> function pretty much equivalent to deprecated >>>>> and thus >>>>>>>> unavailable AsciiStrCat, except it is also slower. >>>>>>>>=20 >>>>>>>> My original suggestion was to remove the >>>>> assertions >>>>>>>> entirely, but several people here said that >>>>> they use >>>>>>>> them to verify usage errors when handling >>>>> trusted data. >>>>>>>> This makes good sense to me, so we suggest to >>>>> support >>>>>>>> both cases by introducing a PCD in this patch. >>>>>>>>=20 >>>>>>>> Best wishes, >>>>>>>> Vitaly >>>>>>>>=20 >>>>>>>>> 6 =D1=8F=D0=BD=D0=B2. 2020 =D0=B3., =D0=B2 21:28, Kinney, Michae= l D >>>>>>>> >>>>> > =D0=BD=D0=B0= = =D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0): >>>>>>>>>=20 >>>>>>>>>=20 >>>>>>>>> Hi Vitaly, >>>>>>>>>=20 >>>>>>>>> Is the use case for UEFI Applications? >>>>>>>>>=20 >>>>>>>>> There is a different mechanism to disable all >>>>>>>> ASSERT() >>>>>>>>> statements within a UEFI Application. >>>>>>>>>=20 >>>>>>>>> If a component is consuming data from an >>>>> untrusted >>>>>>>> source, >>>>>>>>> then that component is required to verify the >>>>>>>> untrusted >>>>>>>>> data before passing it to a function that clearly >>>>>>>> documents >>>>>>>>> is input requirements. If this approach is >>>>> followed, >>>>>>>> then >>>>>>>>> the BaseLib functions can be used "as is" as >>>>> long as >>>>>>>> the >>>>>>>>> ASSERT() conditions are verified before calling. >>>>>>>>>=20 >>>>>>>>> If there are some APIs that currently >>>>> document their >>>>>>>> ASSERT() >>>>>>>>> behavior and we think that ASSERT() behavior is >>>>>>>> incorrect and >>>>>>>>> should be handled by an existing error return >>>>> value, >>>>>>>> then we >>>>>>>>> should discuss each of those APIs individually. >>>>>>>>>=20 >>>>>>>>> Mike >>>>>>>>>=20 >>>>>>>>>=20 >>>>>>>>>> -----Original Message----- >>>>>>>>>> From:devel@edk2.groups.io >>>>> >>>>> > On >>>>>>>>>> Behalf Of Vitaly Cheptsov via Groups.Io >>>>>>>>>> Sent: Friday, January 3, 2020 9:13 AM >>>>>>>>>> To:devel@edk2.groups.io >>>>> >>>>>>>>>> Subject: [edk2-devel] [PATCH v3 0/1] Add PCD to >>>>>>>> disable >>>>>>>>>> safe string constraint assertions >>>>>>>>>>=20 >>>>>>>>>> REF: >>>>>>>>>> https://bugzilla.tianocore.org/show_bug.cgi?id=3D2054 >>>>>>>>>>=20 >>>>>>>>>> Requesting for merge in edk2-stable202002. >>>>>>>>>>=20 >>>>>>>>>> Changes since V1: >>>>>>>>>> - Enable assertions by default to preserve the >>>>>>>> original >>>>>>>>>> behaviour >>>>>>>>>> - Fix bugzilla reference link >>>>>>>>>> - Update documentation in BaseLib.h >>>>>>>>>>=20 >>>>>>>>>> Vitaly Cheptsov (1): >>>>>>>>>> MdePkg: Add PCD to disable safe string >>>>> constraint >>>>>>>>>> assertions >>>>>>>>>>=20 >>>>>>>>>> MdePkg/MdePkg.dec | 6 ++ >>>>>>>>>> MdePkg/Library/BaseLib/BaseLib.inf | 11 +-- >>>>>>>>>> MdePkg/Include/Library/BaseLib.h | 74 >>>>>>>>>> +++++++++++++------- >>>>>>>>>> MdePkg/Library/BaseLib/SafeString.c | 4 +- >>>>>>>>>> MdePkg/MdePkg.uni | 6 ++ >>>>>>>>>> 5 files changed, 71 insertions(+), 30 >>>>> deletions(-) >>>>>>>>>>=20 >>>>>>>>>> -- >>>>>>>>>> 2.21.0 (Apple Git-122.2) >>>>>>>>>>=20 >>>>>>>>>>=20 >>>>>>>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>>>>>>> Groups.io Links: You >>>>> receive all messages sent to >>>>>>>> this >>>>>>>>>> group. >>>>>>>>>>=20 >>>>>>>>>> View/Reply Online (#52837): >>>>>>>>>> https://edk2.groups.io/g/devel/message/52837 >>>>>>>>>> Mute This Topic: >>>>>>>> https://groups.io/mt/69401948/1643496 >>>>>>>>>> Group Owner:devel+owner@edk2.groups.io=20 >>>>>>>>>> >>>>> >>>>>>>>>> Unsubscribe:https://edk2.groups.io/g/devel/unsub >>>>>>>>>> [michael.d.kinney@intel.com >>>>> ] >>>>>>>>>> -=3D-=3D-=3D-=3D-=3D-=3D >>>>>>>>>=20 >>>>>>>>=20 >>>>>>>>=20 >>>>>>>>=20 >>>>>>>=20 >>>>>>=20 >>>>>=20 >>>>=20 >>>>=20 >>>=20 >>>=20 >>=20 >=20 >=20 >=20