public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marvin Häuser" <mhaeuser@outlook.de>
To: Kilian Kegel <kilian_kegel@outlook.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"Richardson, Brian" <brian.richardson@intel.com>
Subject: Re: [staging/branch]: CdePkg - some more details
Date: Tue, 10 Dec 2019 17:35:41 +0100	[thread overview]
Message-ID: <AM6PR07MB5859419B3DE56809A6DA7BEBAC5B0@AM6PR07MB5859.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <VI1PR0502MB3968094EB99A981F344211A9EB5B0@VI1PR0502MB3968.eurprd05.prod.outlook.com>

Good day Kilian,

Comments are inline.
Thank you very much for your very comprehensive explainations!

Please do not understand my sceptical tone as if I didn't want this 
project to succeed, my concerns are a lot more organisation-centered 
than on the work on the library itself.

I wish you the best of luck with your project!

Best regards,
Marvin

Am 10.12.2019 um 08:23 schrieb Kilian Kegel:
> Resend to <devel@edk2.groups.io>
> 
> Hi Marvin, hi UEFI community,
> 
> sorry for being late with my reply,
> 
>> As far as I understood it, it aims to provide an actual full C standard library implementation
> 
> Yes, full with known limitations regarding file access, console access 
> and <locale.h> related requirements
> 
> (e.g. all *f...()* functions or *system(“echo Hello world”) *won’t be 
> run in POST)

Got it, thanks!

> 
> Here is a list of 110 ANSI C functions that already run in PEI, DXE 
> using CdePkg and UEFI Shell using Torito C Library:
> 
> https://github.com/tianocore/edk2-staging/blob/CdePkg/implemented.md#validation-status
> 
>> Is this planned to land in edk2
> 
> This would be my proposal for future UEFI based firmware BIOS products 
> like modernFW.
> 
> As UEFI implements a filesystem “FAT32”,
> 
> I propose to implement additionally a “C” interface to modernFW to:
> 
>   * make it trustable for security, finance, military and government
>     applications/platforms/products

I do not get your point here - a standard compliant interface itself is 
supposed to make it more trustable? I think I misunderstand you here.

>   * demystifying UEFI BIOS firmware by providing a “known” API >
> that shall be extended to secure/bounds checking interface of C11 e.g. 
> snprintf*_s*()
> 
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf Annex K
> 
>   * improve maintainability by using “standard” functions known to the
>     compiler and to static code analysis tools
> 
> (that warns at  parameter mismatch e.g. on sprintf() at buildtime)
> 
>   * allow validation by C validation suites CVS from third party vendors
>   * make it usable and understandable for “normal” IT professionals and
>     application developers, 

The points about existing validation suites, static analysis support and 
such are all good.

> 
> writing shell apps and OPTION ROMs; developers are forced to understand 
> a lot of the EDK buildprocess
> 
> and EDK libraries before they can write a simple flash tool… (at 
> companies like Broadcom, Realtek, LSI, Emulex,
> 
> Infineon, PC manufacturers…)
> 
>> and if so, in what ways will one be encouraged to use it?
> 
> EDK-STAGING is the place on tianocore.org where new features that are 
> not ready for product
> 
> integration can be checked in for evaluation by the EDK II community 
> prior to
> 
> adding to the edk2 trunk…
> 
> https://github.com/tianocore/edk2-staging

Thank you, but I am aware of this. I am more interested in the future 
mainline usage, i.e. the final goal of this initiative.

> 
>> Usually it's considered best practice to keep the code quantity in low levels to a minimum,
> 
> this might be true for small microcontrollers.
> 
> On x86 architecture UEFI runs on 64 Bit processors that start in 32 Bit 
> mode and use CAR Cash As RAM to get PEI
> 
> in C running before memory detection. Usually those platforms  have a 
> multi megabyte cache available for CAR.

This point was not about code size or resource consumption, but that is 
also a concern. When making a point about such, you cannot take the best 
case (x86-based products which are optimised for performance and 
support) but must take the worst case (e.g. I have seen edk2 has been 
ported to Raspberry devices and other embedded systems), where the 
resources are far more limited. For CDE itself, I'm not worrying about 
the resource consumption though, at most so when combining it with edk2.

The actual point is about validation. We should not strive for a 
feature-rich framework to get everyone happy with the interface, but for 
a rock-solid fundament that is easy to verify and validate. 110 ANSI C 
functions which overlap with edk2 functions mean 220 functions to 
validate and maintain, and this, in my opinion, is disastrous. I do not 
know about how well C standard functions work out in firmware 
implementations, but if they work out as well as edk2-specific stuff, 
I'd strongly suggest and favour deprecating edk2 concepts in favour of 
ANSI C step by step.

I have also seen efforts to allow Rust support for edk2. *Please* engage 
with the different people proposing different modernisations for the 
project to not end up with a suboptimal middle ground, a lot of scrapped 
work, or, at worst, many alternatives. Supporting traditional edk2, CDE 
*and* Rust at once sounds absolutely horrible to me. Currently, it 
sounds a bit chaotic to me where edk2 is heading.

This does make me wonder, why was edk2 designed with this set of 
proprietary functions and types, manifested in even the specification, 
in the first place? Is there a practical point regarding ANSI C design 
or just the compiler situation when the original EDK was written? If 
former, that should surely be considered in the CDE design.

> 
> For those machines ANSI C shall be considered as the lowest common 
> denominatorrequired for string processing <string.h> / <wchar.h>,
> 
> character handling <ctype.h> / <wctype.h>, time and date serviceing 
> <time.h>, string to number conversion and vice versa <stdio.h>,
> 
> memory allocation,  global jumps and exit() services <stdlib.h>…
> 
> Most of that functionality is already provided in UEFI in ANSI and 
> UNICODE representation each as a
> 
> proprietary interface and  lacks compatibility with ANSI C.

I'm all for moving to ANSI C for all *existing* concepts. A full C 
standard library consists of a lot of concepts that we probably do not 
want in firmware, including but not limited to advanced (or any really) 
floating point support, math operations, signals, advanced 
multithreading, advanced file and console I/O, advanced environment APIs 
(e.g. locale, time). I know a bunch of those are optional and were 
probably not considered in the first place, but I wanted to stress to 
keep it as simple as possible - and if that means to deviate or just 
ignore parts of the standard (library), I think that should be done.

But just for the basic stuff, I would really appreciate ANSI C support. 
This is for basic APIs (malloc, str*, etc) and especially types. Integer 
Promotion is an essential aspect of C, I don't know how one can just 
banish that type from their vocabulary like that. This can make things 
like the print functions a bit inconvenient. I do not see how UEFI's 
types make any sense over standard fixed-width types either.
Did any of the edk2 designers (or Stewards? Sorry, I don't understand 
who is in charge of what very well) comment on this kind of stuff before?

> 
>> and I do not think even most kernel spaces implement a *full* C standard library for this reason.
> 
> If you consider BIOS flash space requirements, it is delightfully small, 
> much smaller than you expect, and
> 
> probably smaller than the current implementation.
> 
> Because it is consistently divided into wrapper library and worker driver.
> 
> The “big” worker code is present once per PEI/DXE/SMM phase only.
> 
> The code from the wrapper library is linked into the drivers binary only 
> on demand.
> 
> This is the “traditional” library concept.
> 
> The full functionality (CdeServices) needed for 
> malloc()/free()/realloc(), entire  printf()-family (narrow and wide),
> 
> entire  scanf()-family (narrow and wide), most  string processing 
> functions (narrow and wide) needs
> 
> much less than 20KB (DXE) / 15KB (PEI) and could also serve as a 
> C-Library-driver for UEFI Shell programs (DXE driver).
> 
> https://github.com/KilianKegel/CdeBinPkg/blob/master/CdeServices/CdeServicesDxe64.efi
> 
> https://github.com/KilianKegel/CdeBinPkg/blob/master/CdeServices/CdeServicesPei32.efi
> 
> Let’s have a look into some ANSI C functions that interact with the 
> *CdeServices* driver:
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/stdio_h/Vsscanf.c#L50
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/stdio_h/Printf.c#L34
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/stdlib_h/Realloc.c#L21
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/stdlib_h/strtol.c#L58
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/string_h/StrTok.c#L25
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/time_h/clock.c#L37
> 
> On function entrance they all fetch the application interface, that 
> contains the pointer to
> 
> the “*CdeServices*” protocol:
> 
> **
> 
> *CDE_APP_IF *pCdeAppIf = _GetCdeAppIf();*
> 
> **
> 
> That, itself is allocated once during drivers runthrough 
> driverentrypoint/CRT0():
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/OSInterface/OS_DXE/osifUefiDxeEntryPoint.c#L125
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/OSInterface/OS_PEI/osifUefiPeiEntryPoint.c#L97
> 
> A lot of functions defined in the ANSI C library don’t need any space 
> optimizing, because they are that small by nature:
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/ctype_h/islower.c#L38
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/ctype_h/isalpha.c#L39
> 
> https://github.com/KilianKegel/CdeBinPkgSrc/blob/master/CdeLib/Library/string_h/StrLen.c#L16
> 
> That is true for DXE/(SMM) and PEI.
> 
>>and is there a chance the rest of edk2 is (very slowly) transfered to a standard C
> 
> That´d be my idea, but it depends on the acceptance.

That was my biggest concern, really. I think this project can do a lot 
of damage if it is not fully accepted but only provided as a mere 
alternative by choice. It makes review (quantity) and understanding the 
control flow (two separate API designs) harder, overcomplicates the 
codebase (e.g. folder layout) and may lead to multiple functions per 
purpose (MDE *and* CDE) per module, which will degrade performance 
(binary size -> code cache, load time, etc).

But I also think it can do a lot of good if it is accepted and executed 
well, so I hope this gains some actual traction. So far I only have seen 
you posting about it, and that worries me. Are there disucussions going 
on in the background?

> 
> I will provide a demonstration on how to convert an existing MdePkg UEFI 
> driver to
> 
> CdePkg extention soon.
> 
>> Or will it "just" be a handy set of libraries for porting purposes?
> 
> I am talking about one single C library CdeLib (as each normal  C 
> library or LIBC is always
> 
> provided as one single library file).
> 
> CdePkg does not need multiple libraries. There is one driver and one 
> library per POST phase

Sorry, this was unclear, I did not mean it in a edk2 module sense, but 
the separation into groups (stdio, stddef, stdlib, ...). The actual 
point was the "just", as in an alternative, as discussed above.

> 
> only (the command line driver is a temporary addition).
> 
> All  built out of the same source code.
> 
> EDK2 is only the firmware fundament for final PC products (server, 
> desktop, notebook, industry PC, Tablet)
> 
> e.g. 
> https://www.fujitsu.com/global/products/computing/peripheral/mainboards/
> 
> The final products needs lots of improvements and extensions to 
> withstand the
> 
> market requirements and the competitors offerings and to match the 
> customer demands.
> 
> PC products for business applications implements many additional
> features e.g.
> 
>   * LAN wakeup for different NICs Marvell, BroadCom, Realtek, Intel…
>   * keyboard power button wake out of S5
>   * LAN Boot
>   * USB Port Protection/Disable for security purpose
>   * authentification via CardReader in POST
>   * power fail recovery including setup wake resources after power fail
>   * console redirection for AMT or DASH
>   * hard disk security around security freeze lock
>   * boot device selection by boot menu / hot key
>   * HDD SMART
>   * HSTI Hardware Security Test Interface
>   * TPM
>   * OA OEM activation
>   * (Server runtime) error logging
>   * (remote) BIOS configuration / setup settings,
>   * graphical setup
>   * Server RAS features
>   * IPMI support
>   * NVRAM backup
>   * LVDS display support for industrial applications
>   * brightness slider support
>   * password protection for boot/ HDD/ setup,
>   * fan control
>   * industry requirements: 
> 
> forced preference of “signed” USB flash/CDROM/HDD drives in
> 
> the BIOS boot order for maintenance and service
> 
>   * runtime counter
>   * exchangeable customer logo
> 
> (this is a selection of features I worked on the past 10 years doing 
> UEFI BIOS projects).
> 
>> Especially in latter case, this, to me intuitively, sounds like a maintenance and reviewing nightmare
> 
> Absolutely not, because it is easy to compare each function´s behavior 
> against its corresponding
> 
> Microsoft LIBCMT.LIB pendant, once you have started the 
> */CdeValidationPkg.sln /*Visual Studio solution:
> 
> cid:image004.png@01D5AEB1.24B7B000
> 
> Each sample provided in the CdeValidationPkg can be translated for and 
> run on Windows Console, UEFI Shell, DXE and PEI
> 
> (except the SystemInterfaceDxe/PEI projects)
> 
> All implemented ANSI C functions are already tested and validated 
> comprehensively that way.

Well, yes, but this seems to be basically unit testing, I meant in a 
sense of formal review. The problem is not the characteristics of your 
project, but the fact that it duplicates existing concepts. If you 
believe in the advantages of this approach, I hope you aim to deprecate 
existing edk2 pendants step by step.

> 
> Therefore I am pretty sure, you´ll have to try hard to find one single 
> bug beside
> 
> https://github.com/KilianKegel/torito-C-Library#known-bugs
> 
> if any…
> 
> To get the validation modules  running in POST you have to use the 
> traditional EDK2 build process:
> 
>  1. clone the *edk2-staging* repository
>  2. checkout *CdePkg*
>  3. run *LAUNCH.BAT*
>  4. run |*build -p EmulatorPkg\EmulatorPkg.dsc -t VS2015x86 -a IA32*|
>  5. run|*DBGEMU.BAT *||to start emulation||*(EmulatorPkg)*|
>  6. run|*build -a IA32 -a X64 -n 5 -t VS2015x86 -b DEBUG -p
>     Vlv2TbltDevicePkg\PlatformPkgX64.dsc*|
>  7. |update MinnowBoard
>     with||*Build/Vlv2TbltDevicePkgX64\DEBUG_VS2015x86\FV\VLV.fd*|
> 
>>curious about its purpose and future
> 
> with a “C” interface provided as CdeServices it would:
> 
>  1. allow Shell apps / DXE/SMM/PEI driver to share the same sourcecode
>     (beside API specific parts)
>  2. Share or reuse source code with open source
>  3. allow use of automatically generated source code by
>     syntactical/lexical analysis tools (lex/yacc)
>  4. ease programming tasks that deal with text processing (e.g. device
>     path, setup strings), time and date handling…
> 
> because ANSI C is standardized
> 
>  5. allow prototyping as a UEFI Shell application or as a Windows
>     Console application to  be debugged with
> 
> superb Windows debug tools
> 
>  6. dispense the need of reading the source code to get an idea about
>     exact behavior of a particular function as
> 
> (https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePrintLib/PrintLib.c#L26
> 
> Thanks for your curiosity and best regards,
> 
> Kilian
> 
> 
> 

      reply	other threads:[~2019-12-10 16:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-10  7:23 [staging/branch]: CdePkg - some more details Kilian Kegel
2019-12-10 16:35 ` Marvin Häuser [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM6PR07MB5859419B3DE56809A6DA7BEBAC5B0@AM6PR07MB5859.eurprd07.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox