public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* DSC nightmare
@ 2023-02-20  1:36 Benjamin Mordaunt
  2023-02-23  1:19 ` [edk2-devel] " Andrew Fish
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Mordaunt @ 2023-02-20  1:36 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

Hi,

I am working on implementing a new Arm platform in edk2-platforms, and have reached the stage of tackling writing a DSC to describe it, along with a "dsc.inc" which contains defines etc. common to platforms sharing the same SoC. I'm using, as reference the platforms currently present in the repository, such as RPi, Beagleboard and HiKey.

However, it's a horrible process and I can't wrap my head around why I need to list hundreds of libraries (in the LibraryClasses.X) sections, seemingly by hand. By the looks of it, most of them pull from ArmPkg, ArmPlatformPkg, MdePkg etc., being either the default implementations of these libraries, or "Null" versions which stub out the functionality. Only a few are platform specific, such as some drivers I've implemented (display, serial etc.).

Not only that, I then need to list them all over again for each EFI boot stage, SEC, PEI_CORE, PEIM and so on... why?! Why can't I just say "Hey, this is an aarch64 platform with the following drivers/libraries/quirks for my platform. It's a huge pain, not to mention a maintenance nightmare...

Am I looking at this wrong? This seems like an obvious problem and greatly increases the labour and maintenance effort required to implement a new platform.

---

Ben

[-- Attachment #2: Type: text/html, Size: 1322 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] DSC nightmare
  2023-02-20  1:36 DSC nightmare Benjamin Mordaunt
@ 2023-02-23  1:19 ` Andrew Fish
       [not found]   ` <CANtEDzSL1hxwhACgHNMRd_c8_4YVF0sbX64ROTnLNPYFaqeiHA@mail.gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Fish @ 2023-02-23  1:19 UTC (permalink / raw)
  To: edk2-devel-groups-io, crawford.benjamin15

[-- Attachment #1: Type: text/plain, Size: 7782 bytes --]

Ben,

I’d say the tools are optimized to be general purpose. So they are not really designed to be the easy button.

In terms of things like SEC/PEI or DXE it is very common for this code on x86 to be compiled for different architectures. SEC/PEI is usually i386 and DXE is x86_64. Crazy I know but historically on x86 you can’t be in 64-bit mode without paging being enabled, and you can’t (legally) put page tables in ROM. So the early code at the reset vector (SEC) and the code to turn on memory (PEI) is 32-bit i386. PEI got a lot bigger than we envisioned mainly due to it taking a PhD in antenna design to turn on memory on an Intel chipset. Then all the power on engineers got good at writing PEI code and moved more code to PEI, but I digress….

OK let us leave behind binary compatibility and talk about the different contracts that exist in the different phases. For example how to allocate memory. There is no standard way in SEC (People use PHIT HOB), there is a PEI specific API[1], and a DXE/UEFI specific API[2]. So you can end up with different instances of the same library API coded with the same API, but implemented differently. This was a conscious choice on our part to make it much easier to port code between the different worlds. We also support the concept of multiple instances of a give library that use different hardware, or are optimized for different things. For example we have some libs that produce our version of memcpy etc that have different instances for PEI[3] and DXE[4]. Seems when you are running from ROM vs. running from memory the optimal optimization schemes are different. Thus we give the user a lot of power over which instance of a library they can pick. When you need it, it is very handy. You also don’t have other people randomly changing the rules on you, which can be important for things like security reviews etc. 

So a little lingo sometimes helps. A library class is the contract, or the header file that defines the public interface for the library. The library instance is the implementation, as I mentioned above there can be more than one for many reasons. Base means the code does not depend on any of the boot phases. SEC/PEI/DXE etc are phases defined by the UEFI PI Spec [1].

I guess we could try to combine libraries of different phases together to simplify things, but at this point we have lots of customers with (as you point out) complex DSC files that would all break if we refactored the libs. So kind of makes fixing things a little harder. 

In terms of tips I find it I useful to use a little git foo to find instances of libraries. The library INF file defines which library class the library implements so you can filter by LIBRARY_CLASS to get the instances (implementations) of a given library class. 

/Volumes/Case/edk2(master)>git grep BaseMemoryLib -- \*.inf | grep LIBRARY_CLASS
MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf:20:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:79:  LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION
MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf:20:  LIBRARY_CLASS                  = BaseMemoryLib
MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib|PEIM
MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER

/Volumes/Case/edk2(master)>git grep SerialPortLib  -- \*.inf | grep LIBRARY_CLASS
ArmPkg/Library/SemiHostingSerialPortLib/SemiHostingSerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib
ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib
ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib|SEC PEI_CORE PEIM
ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib|DXE_CORE DXE_DRIVER UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
EmulatorPkg/Library/DxeEmuSerialPortLib/DxeEmuSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE
EmulatorPkg/Library/DxeEmuStdErrSerialPortLib/DxeEmuStdErrSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE
EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| PEI_CORE PEIM SEC
MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf:18:  LIBRARY_CLASS                  = SerialPortLib
MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf:18:  LIBRARY_CLASS                  = SerialPortLib
OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib
PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf:16:  LIBRARY_CLASS                  = SerialPortLib
UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf:16:  LIBRARY_CLASS                  = SerialPortLib
UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf:15:  LIBRARY_CLASS                  = SerialPortLib


[1] https://github.com/tianocore/edk2/blob/master/MdePkg/Library/PeiMemoryAllocationLib/MemoryAllocationLib.c#L373
[2] https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c#L376

[3] https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptPei
[4] https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptDxe

[5] https://uefi.org/specifications

Thanks,

Andrew Fish

> On Feb 19, 2023, at 5:36 PM, Benjamin Mordaunt <crawford.benjamin15@gmail.com> wrote:
> 
> Hi,
> 
> I am working on implementing a new Arm platform in edk2-platforms, and have reached the stage of tackling writing a DSC to describe it, along with a "dsc.inc" which contains defines etc. common to platforms sharing the same SoC. I'm using, as reference the platforms currently present in the repository, such as RPi, Beagleboard and HiKey.
> 
> However, it's a horrible process and I can't wrap my head around why I need to list hundreds of libraries (in the LibraryClasses.X) sections, seemingly by hand. By the looks of it, most of them pull from ArmPkg, ArmPlatformPkg, MdePkg etc., being either the default implementations of these libraries, or "Null" versions which stub out the functionality. Only a few are platform specific, such as some drivers I've implemented (display, serial etc.).
> 
> Not only that, I then need to list them all over again for each EFI boot stage, SEC, PEI_CORE, PEIM and so on... why?! Why can't I just say "Hey, this is an aarch64 platform with the following drivers/libraries/quirks for my platform. It's a huge pain, not to mention a maintenance nightmare...
> 
> Am I looking at this wrong? This seems like an obvious problem and greatly increases the labour and maintenance effort required to implement a new platform.
> 
> ---
> 
> Ben
> 


[-- Attachment #2: Type: text/html, Size: 21982 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [edk2-devel] DSC nightmare
       [not found]   ` <CANtEDzSL1hxwhACgHNMRd_c8_4YVF0sbX64ROTnLNPYFaqeiHA@mail.gmail.com>
@ 2023-02-24 11:25     ` Benjamin Mordaunt
  2023-03-01 22:49       ` Nate DeSimone
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Mordaunt @ 2023-02-24 11:25 UTC (permalink / raw)
  To: devel

[-- Attachment #1: Type: text/plain, Size: 10215 bytes --]

Hi Andrew,

(I appreciate this is the wrong group - I subbed to the discussion thread
on groups.io, but it decided to post here instead - oh well).

Thank you for the comprehensive response, I understand there is lots of X86
legacy embedded in all parts of the EFI architecture.
Perhaps you could shed some light on the following questions/points:

   - You state that the flexibility in specification of library instances
   is a handy feature and has been designed that way. From a safety
   certification perspective, I certainly can see how explicit specification
   helps. Folks in the automotive space still stick to Device Tree
   specification and typically stay away from UEFI/ACPI with a 10 foot barge
   pole for exactly this reason, but I digress. I still take issue on the
   practical point though - looking through most of the platforms in
   edk2-platforms, you can clearly see that most of the DSCs are copy-pastes
   of the same common base. Most of the comments contain the same spelling
   errors and a good 80% or so of the lines are common between
   implementations. This really irks me. There is no way this *isn't* causing
   a maintenance headache. There (IMHO) needs to be a significant increase in
   the use of includes and dsc.inc files, for example. So there seems to be
   two conflicting things here - the narrative and the reality.
   - How am I supposed to discover all of the libraries I need for my
   implementation, if dependencies are not pulled in automatically and I need
   to go out and figure it all out myself? There are some really obscure
   libraries/library classes and, unless I look at an existing platform
   specification, I have no chance. It feels like this is reinventing the
   wheel a little. Toolchain designers have had this sort of system figured
   out since forever with linkers and related infrastructure.

I would be interested to hear your (or others') comments on this.

Kind regards,

---
Ben

On Thu, Feb 23, 2023 at 1:19 AM Andrew (EFI) Fish <afish@apple.com> wrote:

> Ben,
>
> I’d say the tools are optimized to be general purpose. So they are not
> really designed to be the easy button.
>
> In terms of things like SEC/PEI or DXE it is very common for this code on
> x86 to be compiled for different architectures. SEC/PEI is usually i386 and
> DXE is x86_64. Crazy I know but historically on x86 you can’t be in 64-bit
> mode without paging being enabled, and you can’t (legally) put page tables
> in ROM. So the early code at the reset vector (SEC) and the code to turn on
> memory (PEI) is 32-bit i386. PEI got a lot bigger than we envisioned mainly
> due to it taking a PhD in antenna design to turn on memory on an Intel
> chipset. Then all the power on engineers got good at writing PEI code and
> moved more code to PEI, but I digress….
>
> OK let us leave behind binary compatibility and talk about the different
> contracts that exist in the different phases. For example how to allocate
> memory. There is no standard way in SEC (People use PHIT HOB), there is a
> PEI specific API[1], and a DXE/UEFI specific API[2]. So you can end up with
> different instances of the same library API coded with the same API, but
> implemented differently. This was a conscious choice on our part to make it
> much easier to port code between the different worlds. We also support the
> concept of multiple instances of a give library that use different
> hardware, or are optimized for different things. For example we have some
> libs that produce our version of memcpy etc that have different instances
> for PEI[3] and DXE[4]. Seems when you are running from ROM vs. running from
> memory the optimal optimization schemes are different. Thus we give the
> user a lot of power over which instance of a library they can pick. When
> you need it, it is very handy. You also don’t have other people randomly
> changing the rules on you, which can be important for things like security
> reviews etc.
>
> So a little lingo sometimes helps. A library class is the contract, or the
> header file that defines the public interface for the library. The library
> instance is the implementation, as I mentioned above there can be more than
> one for many reasons. Base means the code does not depend on any of the
> boot phases. SEC/PEI/DXE etc are phases defined by the UEFI PI Spec [1].
>
> I guess we could try to combine libraries of different phases together to
> simplify things, but at this point we have lots of customers with (as you
> point out) complex DSC files that would all break if we refactored the
> libs. So kind of makes fixing things a little harder.
>
> In terms of tips I find it I useful to use a little git foo to find
> instances of libraries. The library INF file defines which library class
> the library implements so you can filter by LIBRARY_CLASS to get the
> instances (implementations) of a given library class.
>
> /Volumes/Case/edk2(master)*>*git grep BaseMemoryLib -- \*.inf | grep
> LIBRARY_CLASS
>
> MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf:20:  LIBRARY_CLASS
>         = BaseMemoryLib
>
> MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf:21:  LIBRARY_CLASS
>               = BaseMemoryLib
>
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:21:
> LIBRARY_CLASS                  = BaseMemoryLib
>
> MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:79:
> LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER
> UEFI_DRIVER UEFI_APPLICATION
>
> MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf:21:
> LIBRARY_CLASS                  = BaseMemoryLib
>
> MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf:21:
> LIBRARY_CLASS                  = BaseMemoryLib
>
> MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf:20:  LIBRARY_CLASS
>                 = BaseMemoryLib
>
> MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf:21:  LIBRARY_CLASS
>       = BaseMemoryLib|PEIM
>
> MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf:21:  LIBRARY_CLASS
>         = BaseMemoryLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
> UEFI_APPLICATION UEFI_DRIVER
>
> /Volumes/Case/edk2(master)*>*git grep SerialPortLib  -- \*.inf | grep
> LIBRARY_CLASS
>
> ArmPkg/Library/SemiHostingSerialPortLib/SemiHostingSerialPortLib.inf:17:
> LIBRARY_CLASS                  = SerialPortLib
>
> ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf:17:
> LIBRARY_CLASS                  = SerialPortLib
>
> ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf:17:
> LIBRARY_CLASS                  = SerialPortLib|SEC PEI_CORE PEIM
>
> ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf:17:
> LIBRARY_CLASS                  = SerialPortLib|DXE_CORE DXE_DRIVER
> UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
>
> EmulatorPkg/Library/DxeEmuSerialPortLib/DxeEmuSerialPortLib.inf:18:
> LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE
>
> EmulatorPkg/Library/DxeEmuStdErrSerialPortLib/DxeEmuStdErrSerialPortLib.inf:18:
> LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE
>
> EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf:18:
> LIBRARY_CLASS                  = SerialPortLib| PEI_CORE PEIM SEC
>
> MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf:18:
> LIBRARY_CLASS                  = SerialPortLib
>
> MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf:18:
> LIBRARY_CLASS                  = SerialPortLib
>
> OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf:17:
> LIBRARY_CLASS                  = SerialPortLib
>
> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf:16:  LIBRARY_CLASS
>             = SerialPortLib
>
> UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf:16:
> LIBRARY_CLASS                  = SerialPortLib
>
> UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf:15:
> LIBRARY_CLASS                  = SerialPortLib
>
>
> [1]
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/PeiMemoryAllocationLib/MemoryAllocationLib.c#L373
> [2]
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c#L376
>
> [3]
> https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptPei
> [4]
> https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptDxe
>
> [5] https://uefi.org/specifications
>
> Thanks,
>
> Andrew Fish
>
> On Feb 19, 2023, at 5:36 PM, Benjamin Mordaunt <
> crawford.benjamin15@gmail.com> wrote:
>
> Hi,
>
> I am working on implementing a new Arm platform in edk2-platforms, and
> have reached the stage of tackling writing a DSC to describe it, along with
> a "dsc.inc" which contains defines etc. common to platforms sharing the
> same SoC. I'm using, as reference the platforms currently present in the
> repository, such as RPi, Beagleboard and HiKey.
>
> However, it's a horrible process and I can't wrap my head around why I
> need to list hundreds of libraries (in the LibraryClasses.X) sections,
> seemingly by hand. By the looks of it, most of them pull from ArmPkg,
> ArmPlatformPkg, MdePkg etc., being either the default implementations of
> these libraries, or "Null" versions which stub out the functionality. Only
> a few are platform specific, such as some drivers I've implemented
> (display, serial etc.).
>
> Not only that, I then need to list them all over again for each EFI boot
> stage, SEC, PEI_CORE, PEIM and so on... why?! Why can't I just say "Hey,
> this is an aarch64 platform with the following drivers/libraries/quirks for
> my platform. It's a huge pain, not to mention a maintenance nightmare...
>
> Am I looking at this wrong? This seems like an obvious problem and greatly
> increases the labour and maintenance effort required to implement a new
> platform.
>
> ---
>
> Ben
> 
>
>
>

[-- Attachment #2: Type: text/html, Size: 20392 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] DSC nightmare
  2023-02-24 11:25     ` Benjamin Mordaunt
@ 2023-03-01 22:49       ` Nate DeSimone
  0 siblings, 0 replies; 4+ messages in thread
From: Nate DeSimone @ 2023-03-01 22:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, crawford.benjamin15@gmail.com

[-- Attachment #1: Type: text/plain, Size: 11125 bytes --]

Hi Ben,

On the x86 side we developed the MinPlatformPkg to help mitigate this problem. As Andrew points out the edk2 build system is extremely flexible… to the point that it can become unwieldy and difficult to get started. For that reason, our Minimum Platform Architecture<https://tianocore-docs.github.io/edk2-MinimumPlatformSpecification/draft/> intentionally constrains the solution space down to sensible default answers, which in turn reduces the amount of code each individual platform port needs to carry. A lot of these default answers are architecturally codified in MinPlatformPkg/Include<https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Include>.

For example, you can see that TigerlakeOpenBoardPkg<https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc> has a 350 line DSC file, compared to 800 lines for the Raspberry Pi<https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/RPi4/RPi4.dsc>.

Thanks,
Nate

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Benjamin Mordaunt
Sent: Friday, February 24, 2023 3:25 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] DSC nightmare

Hi Andrew,

(I appreciate this is the wrong group - I subbed to the discussion thread on groups.io<http://groups.io>, but it decided to post here instead - oh well).

Thank you for the comprehensive response, I understand there is lots of X86 legacy embedded in all parts of the EFI architecture.
Perhaps you could shed some light on the following questions/points:

  *   You state that the flexibility in specification of library instances is a handy feature and has been designed that way. From a safety certification perspective, I certainly can see how explicit specification helps. Folks in the automotive space still stick to Device Tree specification and typically stay away from UEFI/ACPI with a 10 foot barge pole for exactly this reason, but I digress. I still take issue on the practical point though - looking through most of the platforms in edk2-platforms, you can clearly see that most of the DSCs are copy-pastes of the same common base. Most of the comments contain the same spelling errors and a good 80% or so of the lines are common between implementations. This really irks me. There is no way this isn't causing a maintenance headache. There (IMHO) needs to be a significant increase in the use of includes and dsc.inc files, for example. So there seems to be two conflicting things here - the narrative and the reality.
  *   How am I supposed to discover all of the libraries I need for my implementation, if dependencies are not pulled in automatically and I need to go out and figure it all out myself? There are some really obscure libraries/library classes and, unless I look at an existing platform specification, I have no chance. It feels like this is reinventing the wheel a little. Toolchain designers have had this sort of system figured out since forever with linkers and related infrastructure.
I would be interested to hear your (or others') comments on this.

Kind regards,

---
Ben

On Thu, Feb 23, 2023 at 1:19 AM Andrew (EFI) Fish <afish@apple.com<mailto:afish@apple.com>> wrote:
Ben,

I’d say the tools are optimized to be general purpose. So they are not really designed to be the easy button.

In terms of things like SEC/PEI or DXE it is very common for this code on x86 to be compiled for different architectures. SEC/PEI is usually i386 and DXE is x86_64. Crazy I know but historically on x86 you can’t be in 64-bit mode without paging being enabled, and you can’t (legally) put page tables in ROM. So the early code at the reset vector (SEC) and the code to turn on memory (PEI) is 32-bit i386. PEI got a lot bigger than we envisioned mainly due to it taking a PhD in antenna design to turn on memory on an Intel chipset. Then all the power on engineers got good at writing PEI code and moved more code to PEI, but I digress….

OK let us leave behind binary compatibility and talk about the different contracts that exist in the different phases. For example how to allocate memory. There is no standard way in SEC (People use PHIT HOB), there is a PEI specific API[1], and a DXE/UEFI specific API[2]. So you can end up with different instances of the same library API coded with the same API, but implemented differently. This was a conscious choice on our part to make it much easier to port code between the different worlds. We also support the concept of multiple instances of a give library that use different hardware, or are optimized for different things. For example we have some libs that produce our version of memcpy etc that have different instances for PEI[3] and DXE[4]. Seems when you are running from ROM vs. running from memory the optimal optimization schemes are different. Thus we give the user a lot of power over which instance of a library they can pick. When you need it, it is very handy. You also don’t have other people randomly changing the rules on you, which can be important for things like security reviews etc.

So a little lingo sometimes helps. A library class is the contract, or the header file that defines the public interface for the library. The library instance is the implementation, as I mentioned above there can be more than one for many reasons. Base means the code does not depend on any of the boot phases. SEC/PEI/DXE etc are phases defined by the UEFI PI Spec [1].

I guess we could try to combine libraries of different phases together to simplify things, but at this point we have lots of customers with (as you point out) complex DSC files that would all break if we refactored the libs. So kind of makes fixing things a little harder.

In terms of tips I find it I useful to use a little git foo to find instances of libraries. The library INF file defines which library class the library implements so you can filter by LIBRARY_CLASS to get the instances (implementations) of a given library class.


/Volumes/Case/edk2(master)>git grep BaseMemoryLib -- \*.inf | grep LIBRARY_CLASS

MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf:20:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf:79:  LIBRARY_CLASS = BaseMemoryLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_DRIVER UEFI_APPLICATION

MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf:20:  LIBRARY_CLASS                  = BaseMemoryLib

MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib|PEIM

MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf:21:  LIBRARY_CLASS                  = BaseMemoryLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER


/Volumes/Case/edk2(master)>git grep SerialPortLib  -- \*.inf | grep LIBRARY_CLASS

ArmPkg/Library/SemiHostingSerialPortLib/SemiHostingSerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib

ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib

ArmVirtPkg/Library/FdtPL011SerialPortLib/EarlyFdtPL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib|SEC PEI_CORE PEIM

ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib|DXE_CORE DXE_DRIVER UEFI_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION

EmulatorPkg/Library/DxeEmuSerialPortLib/DxeEmuSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE

EmulatorPkg/Library/DxeEmuStdErrSerialPortLib/DxeEmuStdErrSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVE

EmulatorPkg/Library/PeiEmuSerialPortLib/PeiEmuSerialPortLib.inf:18:  LIBRARY_CLASS                  = SerialPortLib| PEI_CORE PEIM SEC

MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf:18:  LIBRARY_CLASS                  = SerialPortLib

MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf:18:  LIBRARY_CLASS                  = SerialPortLib

OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf:17:  LIBRARY_CLASS                  = SerialPortLib

PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf:16:  LIBRARY_CLASS                  = SerialPortLib

UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf:16:  LIBRARY_CLASS                  = SerialPortLib

UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf:15:  LIBRARY_CLASS                  = SerialPortLib


[1] https://github.com/tianocore/edk2/blob/master/MdePkg/Library/PeiMemoryAllocationLib/MemoryAllocationLib.c#L373
[2] https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c#L376

[3] https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptPei
[4] https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseMemoryLibOptDxe

[5] https://uefi.org/specifications

Thanks,

Andrew Fish


On Feb 19, 2023, at 5:36 PM, Benjamin Mordaunt <crawford.benjamin15@gmail.com<mailto:crawford.benjamin15@gmail.com>> wrote:

Hi,

I am working on implementing a new Arm platform in edk2-platforms, and have reached the stage of tackling writing a DSC to describe it, along with a "dsc.inc" which contains defines etc. common to platforms sharing the same SoC. I'm using, as reference the platforms currently present in the repository, such as RPi, Beagleboard and HiKey.

However, it's a horrible process and I can't wrap my head around why I need to list hundreds of libraries (in the LibraryClasses.X) sections, seemingly by hand. By the looks of it, most of them pull from ArmPkg, ArmPlatformPkg, MdePkg etc., being either the default implementations of these libraries, or "Null" versions which stub out the functionality. Only a few are platform specific, such as some drivers I've implemented (display, serial etc.).

Not only that, I then need to list them all over again for each EFI boot stage, SEC, PEI_CORE, PEIM and so on... why?! Why can't I just say "Hey, this is an aarch64 platform with the following drivers/libraries/quirks for my platform. It's a huge pain, not to mention a maintenance nightmare...

Am I looking at this wrong? This seems like an obvious problem and greatly increases the labour and maintenance effort required to implement a new platform.

---

Ben



[-- Attachment #2: Type: text/html, Size: 28445 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-03-01 22:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-20  1:36 DSC nightmare Benjamin Mordaunt
2023-02-23  1:19 ` [edk2-devel] " Andrew Fish
     [not found]   ` <CANtEDzSL1hxwhACgHNMRd_c8_4YVF0sbX64ROTnLNPYFaqeiHA@mail.gmail.com>
2023-02-24 11:25     ` Benjamin Mordaunt
2023-03-01 22:49       ` Nate DeSimone

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox