Just notice there will be an IS_POW2 macro.
Gerd, can you help to clean MtrrLib to use the macro? Today it’s using an
internal local function as below:
BOOLEAN
MtrrLibIsPowerOfTwo (
IN UINT64 Operand
)
{
ASSERT (Operand != 0);
return (BOOLEAN)((Operand & (Operand - 1)) == 0);
}
From: Kinney, Michael D <michael.d.kin...@intel.com>
Sent: Wednesday, March 22, 2023 6:51 AM
To: Marvin Häuser <mhaeu...@posteo.de>
Cc: Gerd Hoffmann <kra...@redhat.com>; devel@edk2.groups.io; Ard Biesheuvel
<ardb+tianoc...@kernel.org>; Wang, Jian J <jian.j.w...@intel.com>; Yao, Jiewen
<jiewen....@intel.com>; James Bottomley <j...@linux.ibm.com>; Michael Roth
<michael.r...@amd.com>; Wu, Hao A <hao.a...@intel.com>; Oliver Steffen
<ostef...@redhat.com>; Xu, Min M <min.m...@intel.com>; Gao, Liming
<gaolim...@byosoft.com.cn>; Ni, Ray <ray...@intel.com>; Tom Lendacky
<thomas.lenda...@amd.com>; Aktas, Erdem <erdemak...@google.com>; Liu, Zhiguang
<zhiguang....@intel.com>; Pawel Polawski <ppola...@redhat.com>; Justen, Jordan
L <jordan.l.jus...@intel.com>; Vitaly Cheptsov <vit9...@protonmail.com>;
Kinney, Michael D <michael.d.kin...@intel.com>
Subject: RE: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related
macros
Ok. No objections to current description.
Mike
From: Marvin Häuser <mhaeu...@posteo.de<mailto:mhaeu...@posteo.de>>
Sent: Tuesday, March 21, 2023 3:44 PM
To: Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Gerd Hoffmann <kra...@redhat.com<mailto:kra...@redhat.com>>;
devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel
<ardb+tianoc...@kernel.org<mailto:ardb+tianoc...@kernel.org>>; Wang, Jian J
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Yao, Jiewen
<jiewen....@intel.com<mailto:jiewen....@intel.com>>; James Bottomley
<j...@linux.ibm.com<mailto:j...@linux.ibm.com>>; Michael Roth
<michael.r...@amd.com<mailto:michael.r...@amd.com>>; Wu, Hao A
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Oliver Steffen
<ostef...@redhat.com<mailto:ostef...@redhat.com>>; Xu, Min M
<min.m...@intel.com<mailto:min.m...@intel.com>>; Gao, Liming
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>; Ni, Ray
<ray...@intel.com<mailto:ray...@intel.com>>; Tom Lendacky
<thomas.lenda...@amd.com<mailto:thomas.lenda...@amd.com>>; Aktas, Erdem
<erdemak...@google.com<mailto:erdemak...@google.com>>; Liu, Zhiguang
<zhiguang....@intel.com<mailto:zhiguang....@intel.com>>; Pawel Polawski
<ppola...@redhat.com<mailto:ppola...@redhat.com>>; Justen, Jordan L
<jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; Vitaly Cheptsov
<vit9...@protonmail.com<mailto:vit9...@protonmail.com>>
Subject: Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related
macros
On 21. Mar 2023, at 23:29, Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> wrote:
I am suggesting adding “minimum” because aligning to a boundary could be to the
next one or any multiples past that.
For example, offset is 1, and alignment request is 4. The obvious result is 3,
but 7, 11, 15, 19, … also satisfy the current description.
They do not, the current description explicitly says “*next* alignment
boundary”.
Best regards,
Marvin
Mike
From: Marvin Häuser <mhaeu...@posteo.de<mailto:mhaeu...@posteo.de>>
Sent: Tuesday, March 21, 2023 3:00 PM
To: Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Gerd Hoffmann <kra...@redhat.com<mailto:kra...@redhat.com>>;
devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel
<ardb+tianoc...@kernel.org<mailto:ardb+tianoc...@kernel.org>>; Wang, Jian J
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Yao, Jiewen
<jiewen....@intel.com<mailto:jiewen....@intel.com>>; James Bottomley
<j...@linux.ibm.com<mailto:j...@linux.ibm.com>>; Michael Roth
<michael.r...@amd.com<mailto:michael.r...@amd.com>>; Wu, Hao A
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Oliver Steffen
<ostef...@redhat.com<mailto:ostef...@redhat.com>>; Xu, Min M
<min.m...@intel.com<mailto:min.m...@intel.com>>; Gao, Liming
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>; Ni, Ray
<ray...@intel.com<mailto:ray...@intel.com>>; Tom Lendacky
<thomas.lenda...@amd.com<mailto:thomas.lenda...@amd.com>>; Aktas, Erdem
<erdemak...@google.com<mailto:erdemak...@google.com>>; Liu, Zhiguang
<zhiguang....@intel.com<mailto:zhiguang....@intel.com>>; Pawel Polawski
<ppola...@redhat.com<mailto:ppola...@redhat.com>>; Justen, Jordan L
<jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; Vitaly Cheptsov
<vit9...@protonmail.com<mailto:vit9...@protonmail.com>>
Subject: Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related
macros
Hi Mike,
On 21. Mar 2023, at 22:37, Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>> wrote:
Hi Gerd,
A few comments included below.
Thanks,
Mike
-----Original Message-----
From: Gerd Hoffmann <kra...@redhat.com<mailto:kra...@redhat.com>>
Sent: Thursday, March 2, 2023 10:51 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel
<ardb+tianoc...@kernel.org<mailto:ardb+tianoc...@kernel.org>>; Gerd Hoffmann
<kra...@redhat.com<mailto:kra...@redhat.com>>; Wang, Jian J
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Yao,
Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>>; Marvin Häuser
<mhaeu...@posteo.de<mailto:mhaeu...@posteo.de>>; James Bottomley
<j...@linux.ibm.com<mailto:j...@linux.ibm.com>>; Michael Roth
<michael.r...@amd.com<mailto:michael.r...@amd.com>>; Wu, Hao A
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Kinney, Michael D
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>; Oliver Steffen
<ostef...@redhat.com<mailto:ostef...@redhat.com>>; Xu, Min M
<min.m...@intel.com<mailto:min.m...@intel.com>>; Gao, Liming
<gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>; Ni, Ray
<ray...@intel.com<mailto:ray...@intel.com>>; Tom
Lendacky <thomas.lenda...@amd.com<mailto:thomas.lenda...@amd.com>>; Aktas,
Erdem <erdemak...@google.com<mailto:erdemak...@google.com>>; Liu, Zhiguang
<zhiguang....@intel.com<mailto:zhiguang....@intel.com>>; Pawel Polawski
<ppola...@redhat.com<mailto:ppola...@redhat.com>>; Justen, Jordan L
<jordan.l.jus...@intel.com<mailto:jordan.l.jus...@intel.com>>; Vitaly Cheptsov
<vit9...@protonmail.com<mailto:vit9...@protonmail.com>>
Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
From: Marvin Häuser <mhaeu...@posteo.de<mailto:mhaeu...@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.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Liming Gao <gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>>
Cc: Zhiguang Liu <zhiguang....@intel.com<mailto:zhiguang....@intel.com>>
Cc: Vitaly Cheptsov <vit9...@protonmail.com<mailto:vit9...@protonmail.com>>
Signed-off-by: Marvin Häuser <mhaeu...@posteo.de<mailto:mhaeu...@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 >= 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) == 2, "sizeof (CHAR16)
does not meet UEFI Specif
STATIC_ASSERT (sizeof (L'A') == 2, "sizeof (L'A') does not meet UEFI
Specification Data Type requirements");
STATIC_ASSERT (sizeof (L"A") == 4, "sizeof (L\"A\") does not meet UEFI
Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (BOOLEAN) == sizeof (BOOLEAN), "Alignment of BOOLEAN
does not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT8) == sizeof (INT8), "Alignment of INT8 does not
meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINT8) == sizeof (UINT8), "Alignment of INT16 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT16) == sizeof (INT16), "Alignment of INT16 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (UINT16) == sizeof (UINT16), "Alignment of UINT16 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT32) == sizeof (INT32), "Alignment of INT32 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (UINT32) == sizeof (UINT32), "Alignment of UINT32 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INT64) == sizeof (INT64), "Alignment of INT64 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (UINT64) == sizeof (UINT64), "Alignment of UINT64 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (CHAR8) == sizeof (CHAR8), "Alignment of CHAR8 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (CHAR16) == sizeof (CHAR16), "Alignment of CHAR16 does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (INTN) == sizeof (INTN), "Alignment of INTN does not
meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (UINTN) == sizeof (UINTN), "Alignment of UINTN does
not meet UEFI Specification Data Type
requirements");
+STATIC_ASSERT (ALIGNOF (VOID *) == 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) == 4,
"Size of enum does not me
STATIC_ASSERT (sizeof (__VERIFY_UINT16_ENUM_SIZE) == 4, "Size of enum does not
meet UEFI Specification Data Type requirements");
STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not
meet UEFI Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT8_ENUM_SIZE) == sizeof
(__VERIFY_UINT8_ENUM_SIZE), "Alignment of enum does not meet UEFI
Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT16_ENUM_SIZE) == sizeof
(__VERIFY_UINT16_ENUM_SIZE), "Alignment of enum does not meet UEFI
Specification Data Type requirements");
+STATIC_ASSERT (ALIGNOF (__VERIFY_UINT32_ENUM_SIZE) == 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) == 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) != 0U && ((Value) & ((Value) - 1U)) == 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)) == 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) == 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101525): https://edk2.groups.io/g/devel/message/101525
Mute This Topic: https://groups.io/mt/97357268/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
