Re: [edk2-devel] [PATCH v3 5/5] MdeModulePkg: Extend NestedInterruptTplLib to support Arm CPUs

["Michael Brown" <mcb30@ipxe.org>]

On 23/01/2024 17:10, Laszlo Ersek wrote:
> Other than that, the patch looks great to me; I'd only propose one
> (superficial) improvement:
> 
> rather than include <Library/ArmLib.h>, scavenge
> 
> #ifdef MDE_CPU_ARM
> #include <Chipset/ArmV7.h>
> #elif defined (MDE_CPU_AARCH64)
> #include <Chipset/AArch64.h>
> #endif
> 
> from it.
> 
> Reasons:
> 
> (a) Those are the headers that directly provide the macros we need, no
> need to include the rest of ArmLib.h. (Listing ArmPkg/ArmPkg.dec in the
> Packages section of the INF file will make these more direct #include
> directives work, too.)
> 
> (b) Including <Library/ArmLib.h> kind of de-synchronizes the #include
> directives in the C source from the [LibraryClasses] section in the INF
> file. Generally there should be a 1-to-1 match -- we should include the
> declarations of variables and functions for exactly those libraries that
> we link against. There are two exceptions (that I can think of at once):
> when we only want macros from a lib class header, or when we include a
> lib class header because we are implementing an instance for that lib
> class (i.e., we're providing, not consuming, the definitions for the
> header-declared variables and functions). In this case, neither seems to
> apply, this is not an ArmLib instance (= implementation), and the macros
> we need don't actually come from ArmLib.h.
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>

Will do, thanks!

Michael




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114225): https://edk2.groups.io/g/devel/message/114225
Mute This Topic: https://groups.io/mt/103911611/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-