From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.27872.1679435999899236310 for ; Tue, 21 Mar 2023 15:00:01 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=mnzGK0/s; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id EC01924050F for ; Tue, 21 Mar 2023 22:59:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1679435998; bh=8l+nTqdyDtiRVqPl5d1QkmUK1ARhliNd26YNwRTWXFE=; h=From:Subject:Date:Cc:To:From; b=mnzGK0/shyMWkc8syDnBbxQar4Y9G7PtFnuGWmzkA7UyCU/TCusWkSVe/qOE46rUT AGcgl9RKaYkfgZ+PCiFLVf2B3XSTPh4iA7kzNsmsSn2UKkF48p1K+5sSP1PHDC7Ac/ 3625vVlDH0lgoawZz9dX/cI54sXRRIeTHvLIsvJOk0dbZt/Wp05QrCfz0yd+4d8Cr4 vR3qVS3XdfOPwjsK5UvCHn34PG+iuAk44tfD8vHPKYJ8VGB4D7SXpRZ7VvBFpmjnMw MLzw9QCWMvTGfU9oLZ/BahgbXTBBUILr+nQHA57XPvrJN4cZSkAHCjmB16104BAMPW KsVNSmsaTZ3Mg== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Ph5B114Qmz9rxT; Tue, 21 Mar 2023 22:59:52 +0100 (CET) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\)) Subject: Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros Date: Tue, 21 Mar 2023 21:59:42 +0000 In-Reply-To: Cc: Gerd Hoffmann , "devel@edk2.groups.io" , Ard Biesheuvel , "Wang, Jian J" , "Yao, Jiewen" , James Bottomley , Michael Roth , "Wu, Hao A" , Oliver Steffen , "Xu, Min M" , "Gao, Liming" , "Ni, Ray" , Tom Lendacky , "Aktas, Erdem" , "Liu, Zhiguang" , Pawel Polawski , "Justen, Jordan L" , Vitaly Cheptsov To: "Kinney, Michael D" References: <20230303065115.406020-1-kraxel@redhat.com> <20230303065115.406020-4-kraxel@redhat.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_18666D28-A613-4EB4-8E4D-FFA57BB90E53" --Apple-Mail=_18666D28-A613-4EB4-8E4D-FFA57BB90E53 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi Mike, > On 21. Mar 2023, at 22:37, Kinney, Michael D = wrote: >=20 > Hi Gerd, >=20 > A few comments included below. >=20 > Thanks, >=20 > Mike >=20 >> -----Original Message----- >> From: Gerd Hoffmann > >> Sent: Thursday, March 2, 2023 10:51 PM >> To: devel@edk2.groups.io >> Cc: Ard Biesheuvel >; Gerd Hoffmann >; Wang, Jian J >; Yao, >> Jiewen >; Marvin = H=C3=A4user >; James = Bottomley >; Michael Roth >> >; Wu, Hao A = >; Kinney, Michael D = >; Oliver = Steffen >> >; Xu, Min M = >; Gao, Liming = >; Ni, Ray = >; Tom >> Lendacky >; = Aktas, Erdem >; = Liu, Zhiguang >; = Pawel Polawski >> >; Justen, Jordan L = >; Vitaly = Cheptsov > >> Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various = alignment-related macros >>=20 >> From: Marvin H=C3=A4user >>=20 >> ALIGNOF: Determining the alignment requirement of data types is >> crucial to ensure safe memory accesses when parsing untrusted data. >>=20 >> IS_POW2: Determining whether a value is a power of two is important >> to verify whether untrusted values are valid alignment values. >>=20 >> IS_ALIGNED: In combination with ALIGNOF data offsets can be verified. >> A more general version of the IS_ALIGNED macro previously defined by = several modules. >>=20 >> ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses. >> Replaces module-specific definitions throughout the codebase. >>=20 >> ALIGN_VALUE_ADDEND: The addend to align up can be used to directly >> determine the required offset for data alignment. >>=20 >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Cc: Vitaly Cheptsov >> Signed-off-by: Marvin H=C3=A4user >> --- >> MdePkg/Include/Base.h | 95 = ++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 94 insertions(+), 1 deletion(-) >>=20 >> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h >> index d209e6de280a..2053314b50d1 100644 >> --- a/MdePkg/Include/Base.h >> +++ b/MdePkg/Include/Base.h >> @@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST; >> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field)) >> #endif >>=20 >> +/** >> + Returns the alignment requirement of a type. >> + >> + @param TYPE The name of the type to retrieve the alignment = requirement of. >> + >> + @return Alignment requirement, in Bytes, of TYPE. >> +**/ >> +#if defined (__cplusplus) >> +// >> +// Standard C++ operator. >> +// >> +#define ALIGNOF(TYPE) alignof (TYPE) >> +#elif defined (__GNUC__) || defined (__clang__) || (defined = (_MSC_VER) && _MSC_VER >=3D 1900) >> +// >> +// All supported versions of GCC and Clang, as well as MSVC 2015 and = later, >> +// support the standard operator _Alignof. >> +// >> +#define ALIGNOF(TYPE) _Alignof (TYPE) >> +#elif defined (_MSC_EXTENSIONS) >> +// >> +// Earlier versions of MSVC, at least MSVC 2008 and later, support = the vendor >> +// extension __alignof. >> +// >> +#define ALIGNOF(TYPE) __alignof (TYPE) >> +#else >> +// >> +// For compilers that do not support inbuilt alignof operators, use = OFFSET_OF. >> +// CHAR8 is known to have both a size and an alignment requirement = of 1 Byte. >> +// As such, A must be located exactly at the offset equal to its = alignment >> +// requirement. >> +// >> +#define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A) >> +#endif >> + >> /** >> Portable definition for compile time assertions. >> Equivalent to C11 static_assert macro from assert.h. >> @@ -793,6 +827,21 @@ STATIC_ASSERT (sizeof (CHAR16) =3D=3D 2, = "sizeof (CHAR16) does not meet UEFI Specif >> STATIC_ASSERT (sizeof (L'A') =3D=3D 2, "sizeof (L'A') does not = meet UEFI Specification Data Type requirements"); >> STATIC_ASSERT (sizeof (L"A") =3D=3D 4, "sizeof (L\"A\") does not = meet UEFI Specification Data Type requirements"); >>=20 >> +STATIC_ASSERT (ALIGNOF (BOOLEAN) =3D=3D sizeof (BOOLEAN), "Alignment = of BOOLEAN does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (INT8) =3D=3D sizeof (INT8), "Alignment of = INT8 does not meet UEFI Specification Data Type requirements"); >> +STATIC_ASSERT (ALIGNOF (UINT8) =3D=3D sizeof (UINT8), "Alignment = of INT16 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (INT16) =3D=3D sizeof (INT16), "Alignment = of INT16 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (UINT16) =3D=3D sizeof (UINT16), "Alignment = of UINT16 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (INT32) =3D=3D sizeof (INT32), "Alignment = of INT32 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (UINT32) =3D=3D sizeof (UINT32), "Alignment = of UINT32 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (INT64) =3D=3D sizeof (INT64), "Alignment = of INT64 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (UINT64) =3D=3D sizeof (UINT64), "Alignment = of UINT64 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (CHAR8) =3D=3D sizeof (CHAR8), "Alignment = of CHAR8 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (CHAR16) =3D=3D sizeof (CHAR16), "Alignment = of CHAR16 does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (INTN) =3D=3D sizeof (INTN), "Alignment of = INTN does not meet UEFI Specification Data Type requirements"); >> +STATIC_ASSERT (ALIGNOF (UINTN) =3D=3D sizeof (UINTN), "Alignment = of UINTN does not meet UEFI Specification Data Type >> requirements"); >> +STATIC_ASSERT (ALIGNOF (VOID *) =3D=3D sizeof (VOID *), "Alignment = of VOID * does not meet UEFI Specification Data Type >> requirements"); >> + >> // >> // The following three enum types are used to verify that the = compiler >> // configuration for enum types is compliant with Section 2.3.1 of = the >> @@ -816,6 +865,10 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) = =3D=3D 4, "Size of enum does not me >> STATIC_ASSERT (sizeof (__VERIFY_UINT16_ENUM_SIZE) =3D=3D 4, "Size of = enum does not meet UEFI Specification Data Type requirements"); >> STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) =3D=3D 4, "Size of = enum does not meet UEFI Specification Data Type requirements"); >>=20 >> +STATIC_ASSERT (ALIGNOF (__VERIFY_UINT8_ENUM_SIZE) =3D=3D sizeof = (__VERIFY_UINT8_ENUM_SIZE), "Alignment of enum does not meet UEFI >> Specification Data Type requirements"); >> +STATIC_ASSERT (ALIGNOF (__VERIFY_UINT16_ENUM_SIZE) =3D=3D sizeof = (__VERIFY_UINT16_ENUM_SIZE), "Alignment of enum does not meet UEFI >> Specification Data Type requirements"); >> +STATIC_ASSERT (ALIGNOF (__VERIFY_UINT32_ENUM_SIZE) =3D=3D sizeof = (__VERIFY_UINT32_ENUM_SIZE), "Alignment of enum does not meet UEFI >> Specification Data Type requirements"); >=20 > This will need to be merged with latest edk2 because of change from = UINT32 to INT32 for the 32-bit size checks >=20 >> + >> /** >> Macro that returns a pointer to the data structure that contains a = specified field of >> that data structure. This is a lightweight method to hide = information by placing a >> @@ -837,6 +890,46 @@ STATIC_ASSERT (sizeof = (__VERIFY_UINT32_ENUM_SIZE) =3D=3D 4, "Size of enum does not m >> **/ >> #define BASE_CR(Record, TYPE, Field) ((TYPE *) ((CHAR8 *) (Record) - = OFFSET_OF (TYPE, Field))) >>=20 >> +/** >> + Checks whether a value is a power of two. >> + >> + @param Value The value to check. >> + >> + @return Whether Value is a power of two. >=20 > Change to @retval TRUE and @retval FALSE descriptions >=20 >=20 >=20 >> +**/ >> +#define IS_POW2(Value) ((Value) !=3D 0U && ((Value) & ((Value) - = 1U)) =3D=3D 0U) >> + >> +/** >> + Checks whether a value is aligned by a specified alignment. >> + >> + @param Value The value to check. >> + @param Alignment The alignment boundary used to check against. >> + >> + @return Whether Value is aligned by Alignment. >=20 > Change to @retval TRUE and @retval FALSE descriptions >=20 >> +**/ >> +#define IS_ALIGNED(Value, Alignment) (((Value) & ((Alignment) - = 1U)) =3D=3D 0U) >> + >> +/** >> + Checks whether a pointer or address is aligned by a specified = alignment. >> + >> + @param Address The pointer or address to check. >> + @param Alignment The alignment boundary used to check against. >> + >> + @return Whether Address is aligned by Alignment. >=20 >=20 > Change to @retval TRUE and @retval FALSE descriptions I wouldn't object, but this adds verbosity with no additional = information. >=20 >> +**/ >> +#define ADDRESS_IS_ALIGNED(Address, Alignment) IS_ALIGNED ((UINTN) = (Address), Alignment) >> + >> +/** >> + Determines the addend to add to a value to round it up to the next = boundary of >> + a specified alignment. >=20 > Determines the minimum number of bytes to add to a value to round it = up to the next boundary of a specified alignment. >=20 >> + >> + @param Value The value to round up. >> + @param Alignment The alignment boundary used to return the = addend. >> + >> + @return Addend to round Value up to alignment boundary Alignment. >=20 > Minimum number of bytes to add to Value to reach the next alignment = boundary specified by Alignment. Hmm. I would not object against explicitly mentioning "bytes", but there = is no reason why this would be limited to this unit, so I don't quite = see the point. I would object against "minimum", as the value is = unambiguous (i.e., the result of the function spec is well-defined) - = there is no "non-minimum". Best regards, Marvin >=20 >> +**/ >> +#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - = (Value)) & ((Alignment) - 1U)) >> + >> /** >> Rounds a value up to the next boundary using a specified alignment. >>=20 >> @@ -849,7 +942,7 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) = =3D=3D 4, "Size of enum does not m >> @return A value up to the next boundary. >>=20 >> **/ >> -#define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - = (Value)) & ((Alignment) - 1))) >> +#define ALIGN_VALUE(Value, Alignment) ((Value) + ALIGN_VALUE_ADDEND = (Value, Alignment)) >>=20 >> /** >> Adjust a pointer by adding the minimum offset required for it to be = aligned on >> -- >> 2.39.2 --Apple-Mail=_18666D28-A613-4EB4-8E4D-FFA57BB90E53 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hi = Mike,

On 21. Mar 2023, at = 22:37, Kinney, Michael D <michael.d.kinney@intel.com> = wrote:

Hi Gerd,

A few comments included below.

Thanks,

Mike

-----Original = Message-----
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: = Thursday, March 2, 2023 10:51 PM
To: devel@edk2.groups.io
Cc: Ard = Biesheuvel <ardb+tianocore@kernel.org>= ;; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian = J <jian.j.wang@intel.com>; = Yao,
Jiewen <jiewen.yao@intel.com>; = Marvin H=C3=A4user <mhaeuser@posteo.de>; James = Bottomley <jejb@linux.ibm.com>; Michael = Roth
<michael.roth@amd.com>; Wu, = Hao A <hao.a.wu@intel.com>; Kinney, = Michael D <michael.d.kinney@intel.com&= gt;; Oliver Steffen
<osteffen@redhat.com>; Xu, Min = M <min.m.xu@intel.com>; = Gao, Liming <gaoliming@byosoft.com.cn>;= Ni, Ray <ray.ni@intel.com>;= Tom
Lendacky <thomas.lendacky@amd.com>; = Aktas, Erdem <erdemaktas@google.com>; = Liu, Zhiguang <zhiguang.liu@intel.com>; = Pawel Polawski
<ppolawsk@redhat.com>; Justen, = Jordan L <jordan.l.justen@intel.com>= ;; Vitaly Cheptsov <vit9696@protonmail.com>
S= ubject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related = macros

From: Marvin H=C3=A4user = <mhaeuser@posteo.de>

ALIGNOF: Determining the alignment = requirement of data types is
crucial to ensure safe memory accesses = when parsing untrusted data.

IS_POW2: Determining whether a value = is a power of two is important
to verify whether untrusted values are = valid alignment values.

IS_ALIGNED: In combination with ALIGNOF = data offsets can be verified.
A more general version of the = IS_ALIGNED macro previously defined by several = modules.

ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers = and addresses.
Replaces module-specific definitions throughout the = codebase.

ALIGN_VALUE_ADDEND: The addend to align up can be used = to directly
determine the required offset for data = alignment.

Cc: Michael D Kinney = <michael.d.kinney@intel.com>
Cc: Liming Gao = <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu = <zhiguang.liu@intel.com>
Cc: Vitaly Cheptsov = <vit9696@protonmail.com>
Signed-off-by: Marvin H=C3=A4user = <mhaeuser@posteo.de>
---
MdePkg/Include/Base.h | 95 = ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 = insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Base.h = b/MdePkg/Include/Base.h
index d209e6de280a..2053314b50d1 = 100644
--- a/MdePkg/Include/Base.h
+++ = b/MdePkg/Include/Base.h
@@ -758,6 +758,40 @@ typedef UINTN = *BASE_LIST;
#define OFFSET_OF(TYPE, Field)  ((UINTN) = &(((TYPE *)0)->Field))
#endif

+/**
+  Returns = the alignment requirement of a type.
+
+  @param =   TYPE  The name of the type to retrieve the alignment = requirement of.
+
+  @return  Alignment requirement, in = Bytes, of TYPE.
+**/
+#if defined (__cplusplus)
+//
+// = Standard C++ operator.
+//
+#define ALIGNOF(TYPE)  alignof = (TYPE)
+#elif defined (__GNUC__) || defined (__clang__) || (defined = (_MSC_VER) && _MSC_VER >=3D 1900)
+//
+// All supported = versions of GCC and Clang, as well as MSVC 2015 and later,
+// = support the standard operator _Alignof.
+//
+#define ALIGNOF(TYPE) =  _Alignof (TYPE)
+#elif defined (_MSC_EXTENSIONS)
+//
+// = Earlier versions of MSVC, at least MSVC 2008 and later, support the = vendor
+// extension __alignof.
+//
+#define ALIGNOF(TYPE) =  __alignof (TYPE)
+#else
+//
+// For compilers that do not = support inbuilt alignof operators, use OFFSET_OF.
+// CHAR8 is known = to have both a size and an alignment requirement of 1 Byte.
+// As = such, A must be located exactly at the offset equal to its = alignment
+// requirement.
+//
+#define ALIGNOF(TYPE) =  OFFSET_OF (struct { CHAR8 C; TYPE A; }, = A)
+#endif
+
/**
  Portable definition for compile = time assertions.
  Equivalent to C11 static_assert macro = from assert.h.
@@ -793,6 +827,21 @@ STATIC_ASSERT (sizeof (CHAR16) =  =3D=3D 2, "sizeof (CHAR16) does not meet UEFI = Specif
STATIC_ASSERT (sizeof (L'A')    =3D=3D 2, = "sizeof (L'A') does not meet UEFI Specification Data Type = requirements");
STATIC_ASSERT (sizeof (L"A")    =3D=3D = 4, "sizeof (L\"A\") does not meet UEFI Specification Data Type = requirements");

+STATIC_ASSERT (ALIGNOF (BOOLEAN) =3D=3D sizeof = (BOOLEAN), "Alignment of BOOLEAN does not meet UEFI Specification Data = Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT8) =    =3D=3D sizeof (INT8), "Alignment of INT8 does not meet = UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF = (UINT8)   =3D=3D sizeof (UINT8), "Alignment of INT16 does not = meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT = (ALIGNOF (INT16)   =3D=3D sizeof (INT16), "Alignment of INT16 = does not meet UEFI Specification Data = Type
requirements");
+STATIC_ASSERT (ALIGNOF (UINT16)  =3D=3D = sizeof (UINT16), "Alignment of UINT16 does not meet UEFI Specification = Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT32) =   =3D=3D sizeof (INT32), "Alignment of INT32 does not meet = UEFI Specification Data Type
requirements");
+STATIC_ASSERT = (ALIGNOF (UINT32)  =3D=3D sizeof (UINT32), "Alignment of UINT32 = does not meet UEFI Specification Data = Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT64) =   =3D=3D sizeof (INT64), "Alignment of INT64 does not meet = UEFI Specification Data Type
requirements");
+STATIC_ASSERT = (ALIGNOF (UINT64)  =3D=3D sizeof (UINT64), "Alignment of UINT64 = does not meet UEFI Specification Data = Type
requirements");
+STATIC_ASSERT (ALIGNOF (CHAR8) =   =3D=3D sizeof (CHAR8), "Alignment of CHAR8 does not meet = UEFI Specification Data Type
requirements");
+STATIC_ASSERT = (ALIGNOF (CHAR16)  =3D=3D sizeof (CHAR16), "Alignment of CHAR16 = does not meet UEFI Specification Data = Type
requirements");
+STATIC_ASSERT (ALIGNOF (INTN) =    =3D=3D sizeof (INTN), "Alignment of INTN does not meet = UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF = (UINTN)   =3D=3D sizeof (UINTN), "Alignment of UINTN does not = meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT = (ALIGNOF (VOID *)  =3D=3D sizeof (VOID *), "Alignment of VOID * = does not meet UEFI Specification Data = Type
requirements");
+
//
// The following three enum types = are used to verify that the compiler
// configuration for enum types = is compliant with Section 2.3.1 of the
@@ -816,6 +865,10 @@ = STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) =3D=3D 4, "Size of enum = does not me
STATIC_ASSERT (sizeof (__VERIFY_UINT16_ENUM_SIZE) =3D=3D = 4, "Size of enum does not meet UEFI Specification Data Type = requirements");
STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) =3D=3D= 4, "Size of enum does not meet UEFI Specification Data Type = requirements");

+STATIC_ASSERT (ALIGNOF = (__VERIFY_UINT8_ENUM_SIZE)  =3D=3D sizeof = (__VERIFY_UINT8_ENUM_SIZE), "Alignment of enum does not meet = UEFI
Specification Data Type requirements");
+STATIC_ASSERT = (ALIGNOF (__VERIFY_UINT16_ENUM_SIZE) =3D=3D sizeof = (__VERIFY_UINT16_ENUM_SIZE), "Alignment of enum does not meet = UEFI
Specification Data Type requirements");
+STATIC_ASSERT = (ALIGNOF (__VERIFY_UINT32_ENUM_SIZE) =3D=3D sizeof = (__VERIFY_UINT32_ENUM_SIZE), "Alignment of enum does not meet = UEFI
Specification Data Type requirements");

This will need to be merged with latest = edk2 because of change from UINT32 to INT32 for the 32-bit size = checks

+
/**
  Macro that returns a pointer to the data = structure that contains a specified field of
  that data = structure.  This is a lightweight method to hide information by = placing a
@@ -837,6 +890,46 @@ STATIC_ASSERT (sizeof = (__VERIFY_UINT32_ENUM_SIZE) =3D=3D 4, "Size of enum does not = m
**/
#define BASE_CR(Record, TYPE, Field)  ((TYPE *) ((CHAR8 = *) (Record) - OFFSET_OF (TYPE, Field)))

+/**
+  Checks = whether a value is a power of two.
+
+  @param =   Value  The value to check.
+
+  @return =  Whether Value is a power of two.

Change to @retval TRUE and @retval FALSE = descriptions



+**/
+#define = IS_POW2(Value)  ((Value) !=3D 0U && ((Value) & ((Value) = - 1U)) =3D=3D 0U)
+
+/**
+  Checks whether a value is = aligned by a specified alignment.
+
+  @param =   Value      The value to check.
+ =  @param   Alignment  The alignment boundary used to = check against.
+
+  @return  Whether Value is aligned by = Alignment.

Change = to @retval TRUE and @retval FALSE descriptions

+**/
+#define = IS_ALIGNED(Value, Alignment)  (((Value) & ((Alignment) - 1U)) = =3D=3D 0U)
+
+/**
+  Checks whether a pointer or address = is aligned by a specified alignment.
+
+  @param =   Address    The pointer or address to = check.
+  @param   Alignment  The alignment = boundary used to check against.
+
+  @return  Whether = Address is aligned by Alignment.


Change = to @retval TRUE and @retval FALSE descriptions

I wouldn't object, but = this adds verbosity with no additional information.


+**/
+#define = ADDRESS_IS_ALIGNED(Address, Alignment)  IS_ALIGNED ((UINTN) = (Address), Alignment)
+
+/**
+  Determines the addend to = add to a value to round it up to the next boundary of
+  a = specified alignment.

Determines the minimum number of bytes to add to a value to = round it up to the next boundary of a specified alignment.

+
+ =  @param   Value      The value = to round up.
+  @param   Alignment  The alignment = boundary used to return the addend.
+
+  @return  Addend = to round Value up to alignment boundary Alignment.

Minimum number of bytes to add to Value to = reach the next alignment boundary specified by Alignment.

Hmm. I would not object = against explicitly mentioning "bytes", but there is no reason why this = would be limited to this unit, so I don't quite see the point. I would = object against "minimum", as the value is unambiguous (i.e., the result = of the function spec is well-defined) - there is no = "non-minimum".

Best = regards,
Marvin


+**/
+#define = ALIGN_VALUE_ADDEND(Value, Alignment)  (((Alignment) - (Value)) = & ((Alignment) - 1U))
+
/**
  Rounds a value up = to the next boundary using a specified alignment.

@@ -849,7 = +942,7 @@ STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) =3D=3D 4, = "Size of enum does not m
  @return  A value up to the = next boundary.

**/
-#define ALIGN_VALUE(Value, Alignment) =  ((Value) + (((Alignment) - (Value)) & ((Alignment) - = 1)))
+#define ALIGN_VALUE(Value, Alignment)  ((Value) + = ALIGN_VALUE_ADDEND (Value, Alignment))

/**
  Adjust = a pointer by adding the minimum offset required for it to be aligned = on
--
2.39.2

= --Apple-Mail=_18666D28-A613-4EB4-8E4D-FFA57BB90E53--