* [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-08-15 20:11 [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Marvin Häuser
@ 2021-08-15 20:11 ` Marvin Häuser
2021-08-16 9:42 ` [edk2-devel] " Ni, Ray
2021-08-15 20:11 ` [PATCH V2 3/3] MdeModulePkg: Consume new " Marvin Häuser
2021-08-20 5:21 ` [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Wu, Hao A
2 siblings, 1 reply; 12+ messages in thread
From: Marvin Häuser @ 2021-08-15 20:11 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Vitaly Cheptsov
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äuser <mhaeuser@posteo.de>
---
MdePkg/Include/Base.h | 90 +++++++++++++++++++-
1 file changed, 89 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 2da08b0c787f..32d0e512e05f 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -789,6 +789,35 @@ 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(__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_VER)
+ //
+ // 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.
@@ -824,6 +853,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
@@ -847,6 +891,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");
+
/**
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
@@ -868,6 +916,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.
+**/
+#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.
+**/
+#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.
+**/
+#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.
+
+ @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.
+**/
+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value)) & ((Alignment) - 1U))
+
/**
Rounds a value up to the next boundary using a specified alignment.
@@ -880,7 +968,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.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-08-15 20:11 ` [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros Marvin Häuser
@ 2021-08-16 9:42 ` Ni, Ray
2021-08-16 13:10 ` Marvin Häuser
0 siblings, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2021-08-16 9:42 UTC (permalink / raw)
To: devel@edk2.groups.io, mhaeuser@posteo.de
Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Vitaly Cheptsov
Marvin,
So lucky to have you in the edk2 project looking into these fundamentals!
+ #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
+#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
+0U)
2. Good to me. I learned this trick when implementing the MtrrLib.
+#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
+& ((Alignment) - 1U))
3. Is any other open source project using the same macro for the addend?
This is actually a general question to all new macros.
I would like the macros look familiar to developers from other open source projects.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-08-16 9:42 ` [edk2-devel] " Ni, Ray
@ 2021-08-16 13:10 ` Marvin Häuser
2021-08-17 1:17 ` Ni, Ray
2021-11-23 10:12 ` Marvin Häuser
0 siblings, 2 replies; 12+ messages in thread
From: Marvin Häuser @ 2021-08-16 13:10 UTC (permalink / raw)
To: devel, ray.ni
Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Vitaly Cheptsov
Hey Ray,
On 16/08/2021 11:42, Ni, Ray wrote:
> Marvin,
> So lucky to have you in the edk2 project looking into these fundamentals!
Thank you. :)
> + #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
>
> 1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
This should work, yes. The C standard defines offsetof as such:
"The macros are [...]
offsetof(type, member-designator)
which expands to an integer constant expression that has type size_t,
the value of
which is the offset in bytes, to the structure member (designated by
member-designator),
from the beginning of its structure (designated by type). The type and
member designator
shall be such that given
static type t;
then the expression &(t.member-designator) evaluates to an address
constant. [...]" [1]
If we plug in t:
static struct { CHAR8 C; TYPE A; } t;
we get a valid static storage duration variable declaration that
satisfies the the last condition because:
"An address constant is [...], a pointer to an lvalue designating an
object of static
storage duration, or [...]" [2]
It worked with all compilers I tinkered with at https://godbolt.org/
I sadly do not have access to any of the compilers where this may be
used effectively (RVCT, EBC).
> +#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
> +0U)
>
> 2. Good to me. I learned this trick when implementing the MtrrLib.
>
> +#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
> +& ((Alignment) - 1U))
>
> 3. Is any other open source project using the same macro for the addend?
> This is actually a general question to all new macros.
> I would like the macros look familiar to developers from other open source projects.
Good question, I never really saw it. I only came up with it because for
the new PE loader, we may align the PE memory within an underaligned
buffer, and for that we need the addend. I initially used to align up
and then subtract, but I saw this could be simplified with
ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have a
better name, I'll change it.
Best regards,
Marvin
[1] ISO/IEC 9899:2011, 7.19, 3.
[2] ISO/IEC 9899:2011, 6.6, 9.
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-08-16 13:10 ` Marvin Häuser
@ 2021-08-17 1:17 ` Ni, Ray
2021-11-23 10:12 ` Marvin Häuser
1 sibling, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2021-08-17 1:17 UTC (permalink / raw)
To: devel@edk2.groups.io, mhaeuser@posteo.de
Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Vitaly Cheptsov
I don't have better names.
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin Häuser
> Sent: Monday, August 16, 2021 9:10 PM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
>
> Hey Ray,
>
> On 16/08/2021 11:42, Ni, Ray wrote:
> > Marvin,
> > So lucky to have you in the edk2 project looking into these fundamentals!
>
> Thank you. :)
>
> > + #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
> >
> > 1. Does struct{} inside a macro conform to C standard? How is the compatibility with different compilers?
>
> This should work, yes. The C standard defines offsetof as such:
>
> "The macros are [...]
>
> offsetof(type, member-designator)
>
> which expands to an integer constant expression that has type size_t,
> the value of
> which is the offset in bytes, to the structure member (designated by
> member-designator),
> from the beginning of its structure (designated by type). The type and
> member designator
> shall be such that given
>
> static type t;
>
> then the expression &(t.member-designator) evaluates to an address
> constant. [...]" [1]
>
> If we plug in t:
>
> static struct { CHAR8 C; TYPE A; } t;
>
> we get a valid static storage duration variable declaration that
> satisfies the the last condition because:
>
> "An address constant is [...], a pointer to an lvalue designating an
> object of static
> storage duration, or [...]" [2]
>
> It worked with all compilers I tinkered with at https://godbolt.org/
> I sadly do not have access to any of the compilers where this may be
> used effectively (RVCT, EBC).
>
> > +#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
> > +0U)
> >
> > 2. Good to me. I learned this trick when implementing the MtrrLib.
> >
> > +#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
> > +& ((Alignment) - 1U))
> >
> > 3. Is any other open source project using the same macro for the addend?
> > This is actually a general question to all new macros.
> > I would like the macros look familiar to developers from other open source projects.
>
> Good question, I never really saw it. I only came up with it because for
> the new PE loader, we may align the PE memory within an underaligned
> buffer, and for that we need the addend. I initially used to align up
> and then subtract, but I saw this could be simplified with
> ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have a
> better name, I'll change it.
>
> Best regards,
> Marvin
>
>
> [1] ISO/IEC 9899:2011, 7.19, 3.
>
> [2] ISO/IEC 9899:2011, 6.6, 9.
>
> >
> >
> >
> >
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-08-16 13:10 ` Marvin Häuser
2021-08-17 1:17 ` Ni, Ray
@ 2021-11-23 10:12 ` Marvin Häuser
2021-12-08 9:10 ` mjsbeaton
2022-03-22 19:06 ` Marvin Häuser
1 sibling, 2 replies; 12+ messages in thread
From: Marvin Häuser @ 2021-11-23 10:12 UTC (permalink / raw)
To: devel, ray.ni
Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Vitaly Cheptsov
Ping? :)
On 16.08.21 15:10, Marvin Häuser wrote:
> Hey Ray,
>
> On 16/08/2021 11:42, Ni, Ray wrote:
>> Marvin,
>> So lucky to have you in the edk2 project looking into these
>> fundamentals!
>
> Thank you. :)
>
>> + #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
>>
>> 1. Does struct{} inside a macro conform to C standard? How is the
>> compatibility with different compilers?
>
> This should work, yes. The C standard defines offsetof as such:
>
> "The macros are [...]
>
> offsetof(type, member-designator)
>
> which expands to an integer constant expression that has type size_t,
> the value of
> which is the offset in bytes, to the structure member (designated by
> member-designator),
> from the beginning of its structure (designated by type). The type and
> member designator
> shall be such that given
>
> static type t;
>
> then the expression &(t.member-designator) evaluates to an address
> constant. [...]" [1]
>
> If we plug in t:
>
> static struct { CHAR8 C; TYPE A; } t;
>
> we get a valid static storage duration variable declaration that
> satisfies the the last condition because:
>
> "An address constant is [...], a pointer to an lvalue designating an
> object of static
> storage duration, or [...]" [2]
>
> It worked with all compilers I tinkered with at https://godbolt.org/
> I sadly do not have access to any of the compilers where this may be
> used effectively (RVCT, EBC).
>
>> +#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) - 1U)) ==
>> +0U)
>>
>> 2. Good to me. I learned this trick when implementing the MtrrLib.
>>
>> +#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
>> +& ((Alignment) - 1U))
>>
>> 3. Is any other open source project using the same macro for the addend?
>> This is actually a general question to all new macros.
>> I would like the macros look familiar to developers from other open
>> source projects.
>
> Good question, I never really saw it. I only came up with it because
> for the new PE loader, we may align the PE memory within an
> underaligned buffer, and for that we need the addend. I initially used
> to align up and then subtract, but I saw this could be simplified with
> ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE anyway. If you have
> a better name, I'll change it.
>
> Best regards,
> Marvin
>
>
> [1] ISO/IEC 9899:2011, 7.19, 3.
>
> [2] ISO/IEC 9899:2011, 6.6, 9.
>
>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-11-23 10:12 ` Marvin Häuser
@ 2021-12-08 9:10 ` mjsbeaton
2021-12-08 10:36 ` Marvin Häuser
2022-03-22 19:06 ` Marvin Häuser
1 sibling, 1 reply; 12+ messages in thread
From: mjsbeaton @ 2021-12-08 9:10 UTC (permalink / raw)
To: Marvin Häuser, devel
[-- Attachment #1: Type: text/plain, Size: 283 bytes --]
Can I tentatively suggest ALIGN_VALUE_OFFSET as a better name? (Speaking as a native English speaker, and with a relatively high level of command including technical language, addend is not at all a common word!)
While I'm here additional ping and +1 for this to be merged, pls!
[-- Attachment #2: Type: text/html, Size: 340 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-12-08 9:10 ` mjsbeaton
@ 2021-12-08 10:36 ` Marvin Häuser
0 siblings, 0 replies; 12+ messages in thread
From: Marvin Häuser @ 2021-12-08 10:36 UTC (permalink / raw)
To: mjsbeaton, devel
Hey Mike,
Thanks! I agree using "offset" may make it more readable, but I haven't
seen it being used much outside of memory terminology (the macro also
applies to plain integers). Any feedback from the maintainers for
preferences? Thanks!
Best regards,
Marvin
On 08.12.21 10:10, mjsbeaton@gmail.com wrote:
> Can I tentatively suggest ALIGN_VALUE_OFFSET as a better name?
> (Speaking as a native English speaker, and with a relatively high
> level of command including technical language, addend is not at all a
> common word!) While I'm here additional ping and +1 for this to be
> merged, pls!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros
2021-11-23 10:12 ` Marvin Häuser
2021-12-08 9:10 ` mjsbeaton
@ 2022-03-22 19:06 ` Marvin Häuser
1 sibling, 0 replies; 12+ messages in thread
From: Marvin Häuser @ 2022-03-22 19:06 UTC (permalink / raw)
To: devel
Cc: Kinney, Michael D, Liming Gao, Liu, Zhiguang, Vitaly Cheptsov,
Bret Barkelew, Leif Lindholm, Ray Ni
Considering all my branches are badly broken because of the Uncrustify
changes, there have been many changes to the existing PE/COFF libraries
in the mean time (and more external dependencies incoming with PMR), and
even this non-invasive patch has been mostly ignored (thanks for the
review, Ray!), can I now safely assume there is no (and frankly never
was) any actual interest in the reworked PE/COFF library? Things are at
a point where I cannot update all my changes to latest master on my own,
and if nobody provides any active support, you may best mark all related
BZs as WONTFIX to reflect the roadmap.
Best regards,
Marvin
On 23.11.21 11:12, Marvin Häuser wrote:
> Ping? :)
>
> On 16.08.21 15:10, Marvin Häuser wrote:
>> Hey Ray,
>>
>> On 16/08/2021 11:42, Ni, Ray wrote:
>>> Marvin,
>>> So lucky to have you in the edk2 project looking into these
>>> fundamentals!
>>
>> Thank you. :)
>>
>>> + #define ALIGNOF(TYPE) OFFSET_OF (struct { CHAR8 C; TYPE A; }, A)
>>>
>>> 1. Does struct{} inside a macro conform to C standard? How is the
>>> compatibility with different compilers?
>>
>> This should work, yes. The C standard defines offsetof as such:
>>
>> "The macros are [...]
>>
>> offsetof(type, member-designator)
>>
>> which expands to an integer constant expression that has type size_t,
>> the value of
>> which is the offset in bytes, to the structure member (designated by
>> member-designator),
>> from the beginning of its structure (designated by type). The type
>> and member designator
>> shall be such that given
>>
>> static type t;
>>
>> then the expression &(t.member-designator) evaluates to an address
>> constant. [...]" [1]
>>
>> If we plug in t:
>>
>> static struct { CHAR8 C; TYPE A; } t;
>>
>> we get a valid static storage duration variable declaration that
>> satisfies the the last condition because:
>>
>> "An address constant is [...], a pointer to an lvalue designating an
>> object of static
>> storage duration, or [...]" [2]
>>
>> It worked with all compilers I tinkered with at https://godbolt.org/
>> I sadly do not have access to any of the compilers where this may be
>> used effectively (RVCT, EBC).
>>
>>> +#define IS_POW2(Value) ((Value) != 0U && ((Value) & ((Value) -
>>> 1U)) ==
>>> +0U)
>>>
>>> 2. Good to me. I learned this trick when implementing the MtrrLib.
>>>
>>> +#define ALIGN_VALUE_ADDEND(Value, Alignment) (((Alignment) - (Value))
>>> +& ((Alignment) - 1U))
>>>
>>> 3. Is any other open source project using the same macro for the
>>> addend?
>>> This is actually a general question to all new macros.
>>> I would like the macros look familiar to developers from other open
>>> source projects.
>>
>> Good question, I never really saw it. I only came up with it because
>> for the new PE loader, we may align the PE memory within an
>> underaligned buffer, and for that we need the addend. I initially
>> used to align up and then subtract, but I saw this could be
>> simplified with ALIGN_VALUE_ADDEND, which was used in ALIGN_VALUE
>> anyway. If you have a better name, I'll change it.
>>
>> Best regards,
>> Marvin
>>
>>
>> [1] ISO/IEC 9899:2011, 7.19, 3.
>>
>> [2] ISO/IEC 9899:2011, 6.6, 9.
>>
>>>
>>>
>>>
>>>
>>>
>>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 3/3] MdeModulePkg: Consume new alignment-related macros
2021-08-15 20:11 [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Marvin Häuser
2021-08-15 20:11 ` [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros Marvin Häuser
@ 2021-08-15 20:11 ` Marvin Häuser
2021-08-20 5:21 ` [edk2-devel] " Wu, Hao A
2021-08-20 5:21 ` [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Wu, Hao A
2 siblings, 1 reply; 12+ messages in thread
From: Marvin Häuser @ 2021-08-15 20:11 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Ray Ni, Vitaly Cheptsov
This patch substitutes the macros that were renamed in the first
patch with the new, shared alignment macros.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 ++++++++++----------
MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 1 -
MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 --
MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 --
MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 --
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 --
MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
14 files changed, 35 insertions(+), 46 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
index cc32b5de4f98..520197aee752 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
@@ -2099,7 +2099,7 @@ TrustTransferAtaDevice (
// ATA PassThru PPI.
//
if ((AtaPassThru->Mode->IoAlign > 1) &&
- !ADDRESS_IS_ALIGNED_ (Buffer, AtaPassThru->Mode->IoAlign)) {
+ !ADDRESS_IS_ALIGNED (Buffer, AtaPassThru->Mode->IoAlign)) {
NewBuffer = AllocateAlignedPages (
EFI_SIZE_TO_PAGES (TransferLength),
AtaPassThru->Mode->IoAlign
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
index 31ded8a31048..057ad42d596b 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
@@ -193,15 +193,15 @@ AhciAtaPassThruPassThru (
}
IoAlign = This->Mode->IoAlign;
- if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->InDataBuffer, IoAlign)) {
+ if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->InDataBuffer, IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->OutDataBuffer, IoAlign)) {
+ if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->OutDataBuffer, IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->Asb, IoAlign)) {
+ if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->Asb, IoAlign)) {
return EFI_INVALID_PARAMETER;
}
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index cf98fcdaf344..c7b3cfce1340 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -1281,15 +1281,15 @@ AtaPassThruPassThru (
Instance = ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS (This);
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->Asb, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->Asb, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
@@ -2012,15 +2012,15 @@ ExtScsiPassThruPassThru (
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->SenseData, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
index 471fb6a7f440..eabab8ac5bc5 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
@@ -1036,7 +1036,7 @@ TrustTransferAtaDevice (
// Check the alignment of the incoming buffer prior to invoking underlying ATA PassThru
//
AtaPassThru = AtaDevice->AtaBusDriverData->AtaPassThru;
- if ((AtaPassThru->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Buffer, AtaPassThru->Mode->IoAlign)) {
+ if ((AtaPassThru->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED (Buffer, AtaPassThru->Mode->IoAlign)) {
NewBuffer = AllocateAlignedBuffer (AtaDevice, TransferLength);
if (NewBuffer == NULL) {
return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index 81c0fa217a0b..81db2efd0599 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -1956,7 +1956,7 @@ ScsiDiskReceiveData (
goto Done;
}
- if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
+ if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
AlignedBuffer = AllocateAlignedBuffer (ScsiDiskDevice, PayloadBufferSize);
if (AlignedBuffer == NULL) {
Status = EFI_OUT_OF_RESOURCES;
@@ -2171,7 +2171,7 @@ ScsiDiskSendData (
goto Done;
}
- if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
+ if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
AlignedBuffer = AllocateAlignedBuffer (ScsiDiskDevice, PayloadBufferSize);
if (AlignedBuffer == NULL) {
Status = EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index abfb422d1ea3..c4d01a20fcbe 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -170,15 +170,15 @@ UfsPassThruPassThru (
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
- if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->SenseData, This->Mode->IoAlign)) {
+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {
return EFI_INVALID_PARAMETER;
}
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
index 35f60cabdeb4..ba66f441bcea 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
@@ -2004,7 +2004,7 @@ ExecuteJMP (
// check for alignment, and jump absolute.
//
Data64 = (UINT64) VmReadImmed64 (VmPtr, 2);
- if (!ADDRESS_IS_ALIGNED_ ((UINTN) Data64, sizeof (UINT16))) {
+ if (!ADDRESS_IS_ALIGNED ((UINTN) Data64, sizeof (UINT16))) {
EbcDebugSignalException (
EXCEPT_EBC_ALIGNMENT_CHECK,
EXCEPTION_FLAG_FATAL,
@@ -2059,7 +2059,7 @@ ExecuteJMP (
// Form: JMP32 @Rx {Index32}
//
Addr = VmReadMemN (VmPtr, (UINTN) Data64 + Index32);
- if (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16))) {
+ if (!ADDRESS_IS_ALIGNED ((UINTN) Addr, sizeof (UINT16))) {
EbcDebugSignalException (
EXCEPT_EBC_ALIGNMENT_CHECK,
EXCEPTION_FLAG_FATAL,
@@ -2082,7 +2082,7 @@ ExecuteJMP (
// Form: JMP32 Rx {Immed32}
//
Addr = (UINTN) (Data64 + Index32);
- if (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16))) {
+ if (!ADDRESS_IS_ALIGNED ((UINTN) Addr, sizeof (UINT16))) {
EbcDebugSignalException (
EXCEPT_EBC_ALIGNMENT_CHECK,
EXCEPTION_FLAG_FATAL,
@@ -3128,7 +3128,7 @@ ExecuteRET (
// Pull the return address off the VM app's stack and set the IP
// to it
//
- if (!ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Gpr[0], sizeof (UINT16))) {
+ if (!ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Gpr[0], sizeof (UINT16))) {
EbcDebugSignalException (
EXCEPT_EBC_ALIGNMENT_CHECK,
EXCEPTION_FLAG_FATAL,
@@ -4693,7 +4693,7 @@ VmWriteMem16 (
//
// Do a simple write if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
*(UINT16 *) Addr = Data;
} else {
//
@@ -4756,7 +4756,7 @@ VmWriteMem32 (
//
// Do a simple write if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
*(UINT32 *) Addr = Data;
} else {
//
@@ -4819,7 +4819,7 @@ VmWriteMem64 (
//
// Do a simple write if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
*(UINT64 *) Addr = Data;
} else {
//
@@ -4885,7 +4885,7 @@ VmWriteMemN (
//
// Do a simple write if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
*(UINTN *) Addr = Data;
} else {
for (Index = 0; Index < sizeof (UINTN) / sizeof (UINT32); Index++) {
@@ -4949,7 +4949,7 @@ VmReadImmed16 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (INT16))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (INT16))) {
return * (INT16 *) (VmPtr->Ip + Offset);
} else {
//
@@ -4993,7 +4993,7 @@ VmReadImmed32 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32))) {
return * (INT32 *) (VmPtr->Ip + Offset);
}
//
@@ -5032,7 +5032,7 @@ VmReadImmed64 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64))) {
return * (UINT64 *) (VmPtr->Ip + Offset);
}
//
@@ -5069,7 +5069,7 @@ VmReadCode16 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT16))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT16))) {
return * (UINT16 *) (VmPtr->Ip + Offset);
} else {
//
@@ -5110,7 +5110,7 @@ VmReadCode32 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32))) {
return * (UINT32 *) (VmPtr->Ip + Offset);
}
//
@@ -5147,7 +5147,7 @@ VmReadCode64 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64))) {
return * (UINT64 *) (VmPtr->Ip + Offset);
}
//
@@ -5210,7 +5210,7 @@ VmReadMem16 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
return * (UINT16 *) Addr;
}
//
@@ -5243,7 +5243,7 @@ VmReadMem32 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
return * (UINT32 *) Addr;
}
//
@@ -5280,7 +5280,7 @@ VmReadMem64 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
return * (UINT64 *) Addr;
}
//
@@ -5349,7 +5349,7 @@ VmReadMemN (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
return * (UINTN *) Addr;
}
//
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
index 2a74c9984791..59bb9e5d0bca 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
@@ -145,7 +145,6 @@ typedef union {
#define AHCI_PORT_SERR 0x0030
#define AHCI_PORT_CI 0x0038
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
#define TIMER_PERIOD_SECONDS(Seconds) MultU64x32((UINT64)(Seconds), 10000000)
#pragma pack(1)
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index aa3472a71677..99bbf7d14a17 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -147,8 +147,6 @@ struct _ATA_NONBLOCK_TASK {
#define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
#define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10)
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
-
#define ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \
CR (a, \
ATA_ATAPI_PASS_THRU_INSTANCE, \
diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
index 13767a8a13c8..172d2d61ea6c 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
@@ -76,7 +76,6 @@
#define ATA_TASK_SIGNATURE SIGNATURE_32 ('A', 'T', 'S', 'K')
#define ATA_DEVICE_SIGNATURE SIGNATURE_32 ('A', 'B', 'I', 'D')
#define ATA_SUB_TASK_SIGNATURE SIGNATURE_32 ('A', 'S', 'T', 'S')
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
//
// ATA bus data structure for ATA controller
diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
index abe1d06a98da..36fac258f3e6 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
@@ -39,8 +39,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define IS_DEVICE_FIXED(a) (a)->FixedDevice ? 1 : 0
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
-
#define UFS_WLUN_RPMB 0xC4
typedef struct {
diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
index 4802fdc0ab9b..7306106a4454 100644
--- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
+++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
@@ -133,8 +133,6 @@ typedef struct _UFS_PEIM_HC_PRIVATE_DATA {
#define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8)
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
-
#define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS(a) CR (a, UFS_PEIM_HC_PRIVATE_DATA, BlkIoPpi, UFS_PEIM_HC_SIG)
#define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS2(a) CR (a, UFS_PEIM_HC_PRIVATE_DATA, BlkIo2Ppi, UFS_PEIM_HC_SIG)
#define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS_NOTIFY(a) CR (a, UFS_PEIM_HC_PRIVATE_DATA, EndOfPeiNotifyList, UFS_PEIM_HC_SIG)
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index da3bf78bf9bc..11b5b197b67a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -105,8 +105,6 @@ typedef struct {
#define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8)
-#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
-
#define UFS_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \
CR (a, \
UFS_PASS_THRU_PRIVATE_DATA, \
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
index f21c757a75b4..dcbdc66637e9 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
@@ -14,8 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
//
// Macros to check and set alignment
//
-#define ASSERT_ALIGNED(addr, size) ASSERT (!((UINT32) (addr) & (size - 1)))
-#define ADDRESS_IS_ALIGNED_(addr, size) !((UINT32) (addr) & (size - 1))
+#define ASSERT_ALIGNED(addr, size) ASSERT (ADDRESS_IS_ALIGNED (addr, size))
//
// Debug macro
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH V2 3/3] MdeModulePkg: Consume new alignment-related macros
2021-08-15 20:11 ` [PATCH V2 3/3] MdeModulePkg: Consume new " Marvin Häuser
@ 2021-08-20 5:21 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2021-08-20 5:21 UTC (permalink / raw)
To: devel@edk2.groups.io, mhaeuser@posteo.de
Cc: Wang, Jian J, Ni, Ray, Vitaly Cheptsov
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin
> H?user
> Sent: Monday, August 16, 2021 4:12 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [edk2-devel] [PATCH V2 3/3] MdeModulePkg: Consume new
> alignment-related macros
>
> This patch substitutes the macros that were renamed in the first patch with
> the new, shared alignment macros.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> ---
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 ++++++++++---
> -------
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 1 -
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 --
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 --
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 --
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 --
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
> 14 files changed, 35 insertions(+), 46 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index cc32b5de4f98..520197aee752 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2099,7 +2099,7 @@ TrustTransferAtaDevice (
> // ATA PassThru PPI.
>
> //
>
> if ((AtaPassThru->Mode->IoAlign > 1) &&
>
> - !ADDRESS_IS_ALIGNED_ (Buffer, AtaPassThru->Mode->IoAlign)) {
>
> + !ADDRESS_IS_ALIGNED (Buffer, AtaPassThru->Mode->IoAlign)) {
>
> NewBuffer = AllocateAlignedPages (
>
> EFI_SIZE_TO_PAGES (TransferLength),
>
> AtaPassThru->Mode->IoAlign
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index 31ded8a31048..057ad42d596b 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -193,15 +193,15 @@ AhciAtaPassThruPassThru (
> }
>
>
>
> IoAlign = This->Mode->IoAlign;
>
> - if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->InDataBuffer,
> IoAlign)) {
>
> + if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->InDataBuffer,
> + IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->OutDataBuffer,
> IoAlign)) {
>
> + if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->OutDataBuffer,
> + IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Packet->Asb, IoAlign)) {
>
> + if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED (Packet->Asb, IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> index cf98fcdaf344..c7b3cfce1340 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1281,15 +1281,15 @@ AtaPassThruPassThru (
>
>
> Instance = ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS (This);
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >InDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >OutDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->Asb,
> This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED(Packet->Asb,
> + This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> @@ -2012,15 +2012,15 @@ ExtScsiPassThruPassThru (
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >InDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >OutDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >SenseData, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> index 471fb6a7f440..eabab8ac5bc5 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1036,7 +1036,7 @@ TrustTransferAtaDevice (
> // Check the alignment of the incoming buffer prior to invoking underlying
> ATA PassThru
>
> //
>
> AtaPassThru = AtaDevice->AtaBusDriverData->AtaPassThru;
>
> - if ((AtaPassThru->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Buffer,
> AtaPassThru->Mode->IoAlign)) {
>
> + if ((AtaPassThru->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED
> + (Buffer, AtaPassThru->Mode->IoAlign)) {
>
> NewBuffer = AllocateAlignedBuffer (AtaDevice, TransferLength);
>
> if (NewBuffer == NULL) {
>
> return EFI_OUT_OF_RESOURCES;
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index 81c0fa217a0b..81db2efd0599 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -1956,7 +1956,7 @@ ScsiDiskReceiveData (
> goto Done;
>
> }
>
>
>
> - if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_
> (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
>
> + if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED
> + (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
>
> AlignedBuffer = AllocateAlignedBuffer (ScsiDiskDevice,
> PayloadBufferSize);
>
> if (AlignedBuffer == NULL) {
>
> Status = EFI_OUT_OF_RESOURCES;
>
> @@ -2171,7 +2171,7 @@ ScsiDiskSendData (
> goto Done;
>
> }
>
>
>
> - if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_
> (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
>
> + if ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED
> + (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {
>
> AlignedBuffer = AllocateAlignedBuffer (ScsiDiskDevice,
> PayloadBufferSize);
>
> if (AlignedBuffer == NULL) {
>
> Status = EFI_OUT_OF_RESOURCES;
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index abfb422d1ea3..c4d01a20fcbe 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -170,15 +170,15 @@ UfsPassThruPassThru (
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >InDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >OutDataBuffer, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> - if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet-
> >SenseData, This->Mode->IoAlign)) {
>
> + if ((This->Mode->IoAlign > 1) &&
> + !ADDRESS_IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {
>
> return EFI_INVALID_PARAMETER;
>
> }
>
>
>
> diff --git a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> index 35f60cabdeb4..ba66f441bcea 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2004,7 +2004,7 @@ ExecuteJMP (
> // check for alignment, and jump absolute.
>
> //
>
> Data64 = (UINT64) VmReadImmed64 (VmPtr, 2);
>
> - if (!ADDRESS_IS_ALIGNED_ ((UINTN) Data64, sizeof (UINT16))) {
>
> + if (!ADDRESS_IS_ALIGNED ((UINTN) Data64, sizeof (UINT16))) {
>
> EbcDebugSignalException (
>
> EXCEPT_EBC_ALIGNMENT_CHECK,
>
> EXCEPTION_FLAG_FATAL,
>
> @@ -2059,7 +2059,7 @@ ExecuteJMP (
> // Form: JMP32 @Rx {Index32}
>
> //
>
> Addr = VmReadMemN (VmPtr, (UINTN) Data64 + Index32);
>
> - if (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16))) {
>
> + if (!ADDRESS_IS_ALIGNED ((UINTN) Addr, sizeof (UINT16))) {
>
> EbcDebugSignalException (
>
> EXCEPT_EBC_ALIGNMENT_CHECK,
>
> EXCEPTION_FLAG_FATAL,
>
> @@ -2082,7 +2082,7 @@ ExecuteJMP (
> // Form: JMP32 Rx {Immed32}
>
> //
>
> Addr = (UINTN) (Data64 + Index32);
>
> - if (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16))) {
>
> + if (!ADDRESS_IS_ALIGNED ((UINTN) Addr, sizeof (UINT16))) {
>
> EbcDebugSignalException (
>
> EXCEPT_EBC_ALIGNMENT_CHECK,
>
> EXCEPTION_FLAG_FATAL,
>
> @@ -3128,7 +3128,7 @@ ExecuteRET (
> // Pull the return address off the VM app's stack and set the IP
>
> // to it
>
> //
>
> - if (!ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Gpr[0], sizeof (UINT16))) {
>
> + if (!ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Gpr[0], sizeof (UINT16))) {
>
> EbcDebugSignalException (
>
> EXCEPT_EBC_ALIGNMENT_CHECK,
>
> EXCEPTION_FLAG_FATAL,
>
> @@ -4693,7 +4693,7 @@ VmWriteMem16 (
> //
>
> // Do a simple write if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
>
> *(UINT16 *) Addr = Data;
>
> } else {
>
> //
>
> @@ -4756,7 +4756,7 @@ VmWriteMem32 (
> //
>
> // Do a simple write if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
>
> *(UINT32 *) Addr = Data;
>
> } else {
>
> //
>
> @@ -4819,7 +4819,7 @@ VmWriteMem64 (
> //
>
> // Do a simple write if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
>
> *(UINT64 *) Addr = Data;
>
> } else {
>
> //
>
> @@ -4885,7 +4885,7 @@ VmWriteMemN (
> //
>
> // Do a simple write if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
>
> *(UINTN *) Addr = Data;
>
> } else {
>
> for (Index = 0; Index < sizeof (UINTN) / sizeof (UINT32); Index++) {
>
> @@ -4949,7 +4949,7 @@ VmReadImmed16 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (INT16))) {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (INT16)))
> + {
>
> return * (INT16 *) (VmPtr->Ip + Offset);
>
> } else {
>
> //
>
> @@ -4993,7 +4993,7 @@ VmReadImmed32 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32)))
> {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32)))
> + {
>
> return * (INT32 *) (VmPtr->Ip + Offset);
>
> }
>
> //
>
> @@ -5032,7 +5032,7 @@ VmReadImmed64 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64)))
> {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64)))
> + {
>
> return * (UINT64 *) (VmPtr->Ip + Offset);
>
> }
>
> //
>
> @@ -5069,7 +5069,7 @@ VmReadCode16 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT16)))
> {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT16)))
> + {
>
> return * (UINT16 *) (VmPtr->Ip + Offset);
>
> } else {
>
> //
>
> @@ -5110,7 +5110,7 @@ VmReadCode32 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32)))
> {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT32)))
> + {
>
> return * (UINT32 *) (VmPtr->Ip + Offset);
>
> }
>
> //
>
> @@ -5147,7 +5147,7 @@ VmReadCode64 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64)))
> {
>
> + if (ADDRESS_IS_ALIGNED ((UINTN) VmPtr->Ip + Offset, sizeof (UINT64)))
> + {
>
> return * (UINT64 *) (VmPtr->Ip + Offset);
>
> }
>
> //
>
> @@ -5210,7 +5210,7 @@ VmReadMem16 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
>
> return * (UINT16 *) Addr;
>
> }
>
> //
>
> @@ -5243,7 +5243,7 @@ VmReadMem32 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
>
> return * (UINT32 *) Addr;
>
> }
>
> //
>
> @@ -5280,7 +5280,7 @@ VmReadMem64 (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
>
> return * (UINT64 *) Addr;
>
> }
>
> //
>
> @@ -5349,7 +5349,7 @@ VmReadMemN (
> //
>
> // Read direct if aligned
>
> //
>
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
>
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
>
> return * (UINTN *) Addr;
>
> }
>
> //
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 2a74c9984791..59bb9e5d0bca 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -145,7 +145,6 @@ typedef union {
> #define AHCI_PORT_SERR 0x0030
>
> #define AHCI_PORT_CI 0x0038
>
>
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size
> - 1)) == 0)
>
> #define TIMER_PERIOD_SECONDS(Seconds)
> MultU64x32((UINT64)(Seconds), 10000000)
>
>
>
> #pragma pack(1)
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> index aa3472a71677..99bbf7d14a17 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -147,8 +147,6 @@ struct _ATA_NONBLOCK_TASK {
> #define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
>
> #define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10)
>
>
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1))
> == 0)
>
> -
>
> #define ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \
>
> CR (a, \
>
> ATA_ATAPI_PASS_THRU_INSTANCE, \
>
> diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> index 13767a8a13c8..172d2d61ea6c 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> @@ -76,7 +76,6 @@
> #define ATA_TASK_SIGNATURE SIGNATURE_32 ('A', 'T', 'S', 'K')
>
> #define ATA_DEVICE_SIGNATURE SIGNATURE_32 ('A', 'B', 'I', 'D')
>
> #define ATA_SUB_TASK_SIGNATURE SIGNATURE_32 ('A', 'S', 'T', 'S')
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size -
> 1)) == 0)
>
>
>
> //
>
> // ATA bus data structure for ATA controller
>
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> index abe1d06a98da..36fac258f3e6 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -39,8 +39,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
> #define IS_DEVICE_FIXED(a) (a)->FixedDevice ? 1 : 0
>
>
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1))
> == 0)
>
> -
>
> #define UFS_WLUN_RPMB 0xC4
>
>
>
> typedef struct {
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> index 4802fdc0ab9b..7306106a4454 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> @@ -133,8 +133,6 @@ typedef struct _UFS_PEIM_HC_PRIVATE_DATA {
>
>
> #define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8)
>
>
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1))
> == 0)
>
> -
>
> #define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS(a) CR (a,
> UFS_PEIM_HC_PRIVATE_DATA, BlkIoPpi, UFS_PEIM_HC_SIG)
>
> #define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS2(a) CR (a,
> UFS_PEIM_HC_PRIVATE_DATA, BlkIo2Ppi, UFS_PEIM_HC_SIG)
>
> #define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS_NOTIFY(a) CR (a,
> UFS_PEIM_HC_PRIVATE_DATA, EndOfPeiNotifyList, UFS_PEIM_HC_SIG)
>
> diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> index da3bf78bf9bc..11b5b197b67a 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> @@ -105,8 +105,6 @@ typedef struct {
>
>
> #define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8)
>
>
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1))
> == 0)
>
> -
>
> #define UFS_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \
>
> CR (a, \
>
> UFS_PASS_THRU_PRIVATE_DATA, \
>
> diff --git a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> index f21c757a75b4..dcbdc66637e9 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> @@ -14,8 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent //
>
> // Macros to check and set alignment
>
> //
>
> -#define ASSERT_ALIGNED(addr, size) ASSERT (!((UINT32) (addr) & (size - 1)))
>
> -#define ADDRESS_IS_ALIGNED_(addr, size) !((UINT32) (addr) & (size - 1))
>
> +#define ASSERT_ALIGNED(addr, size) ASSERT (ADDRESS_IS_ALIGNED (addr,
> +size))
>
>
>
> //
>
> // Debug macro
>
> --
> 2.31.1
>
>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
2021-08-15 20:11 [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Marvin Häuser
2021-08-15 20:11 ` [PATCH V2 2/3] MdePkg/Base.h: Introduce various alignment-related macros Marvin Häuser
2021-08-15 20:11 ` [PATCH V2 3/3] MdeModulePkg: Consume new " Marvin Häuser
@ 2021-08-20 5:21 ` Wu, Hao A
2 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2021-08-20 5:21 UTC (permalink / raw)
To: Marvin Häuser, devel@edk2.groups.io; +Cc: Wang, Jian J, Ni, Ray
> -----Original Message-----
> From: Marvin Häuser <mhaeuser@posteo.de>
> Sent: Monday, August 16, 2021 4:12 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH V2 1/3] MdeModulePkg: Rename IS_ALIGNED macros to
> avoid name collisions
>
> This patch is a preparation for the patches that follow. The subsequent
> patches will introduce and integrate new alignment-related macros, which
> collide with existing definitions in MdeModulePkg.
> Temporarily rename them to avoid build failure, till they can be substituted
> with the new, shared definitions.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> ---
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 ++++++++++---
> -------
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 2 +-
> MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 +-
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 +-
> MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 +-
> MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +-
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 2 +-
> 14 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index 7636ad27c86c..cc32b5de4f98 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2099,7 +2099,7 @@ TrustTransferAtaDevice (
> // ATA PassThru PPI. // if ((AtaPassThru->Mode->IoAlign > 1) &&-
> !IS_ALIGNED (Buffer, AtaPassThru->Mode->IoAlign))
> {+ !ADDRESS_IS_ALIGNED_ (Buffer, AtaPassThru->Mode->IoAlign))
> { NewBuffer = AllocateAlignedPages ( EFI_SIZE_TO_PAGES
> (TransferLength), AtaPassThru->Mode->IoAligndiff --git
> a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index 191b78c88541..31ded8a31048 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -193,15 +193,15 @@ AhciAtaPassThruPassThru (
> } IoAlign = This->Mode->IoAlign;- if ((IoAlign > 1) && !IS_ALIGNED
> (Packet->InDataBuffer, IoAlign)) {+ if ((IoAlign > 1)
> && !ADDRESS_IS_ALIGNED_ (Packet->InDataBuffer, IoAlign)) { return
> EFI_INVALID_PARAMETER; } - if ((IoAlign > 1) && !IS_ALIGNED (Packet-
> >OutDataBuffer, IoAlign)) {+ if ((IoAlign > 1) && !ADDRESS_IS_ALIGNED_
> (Packet->OutDataBuffer, IoAlign)) { return EFI_INVALID_PARAMETER; } -
> if ((IoAlign > 1) && !IS_ALIGNED (Packet->Asb, IoAlign)) {+ if ((IoAlign > 1)
> && !ADDRESS_IS_ALIGNED_ (Packet->Asb, IoAlign)) { return
> EFI_INVALID_PARAMETER; } diff --git
> a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> index 86fe9d954fdb..cf98fcdaf344 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1281,15 +1281,15 @@ AtaPassThruPassThru (
> Instance = ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS (This); - if
> ((This->Mode->IoAlign > 1) && !IS_ALIGNED(Packet->InDataBuffer, This-
> >Mode->IoAlign)) {+ if ((This->Mode->IoAlign > 1)
> && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer, This->Mode->IoAlign))
> { return EFI_INVALID_PARAMETER; } - if ((This->Mode->IoAlign > 1)
> && !IS_ALIGNED(Packet->OutDataBuffer, This->Mode->IoAlign)) {+ if ((This-
> >Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer,
> This->Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } - if ((This-
> >Mode->IoAlign > 1) && !IS_ALIGNED(Packet->Asb, This->Mode->IoAlign))
> {+ if ((This->Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->Asb,
> This->Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } @@ -2012,15
> +2012,15 @@ ExtScsiPassThruPassThru (
> return EFI_INVALID_PARAMETER; } - if ((This->Mode->IoAlign > 1)
> && !IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {+ if ((This-
> >Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer,
> This->Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } - if ((This-
> >Mode->IoAlign > 1) && !IS_ALIGNED(Packet->OutDataBuffer, This->Mode-
> >IoAlign)) {+ if ((This->Mode->IoAlign > 1)
> && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer, This->Mode->IoAlign))
> { return EFI_INVALID_PARAMETER; } - if ((This->Mode->IoAlign > 1)
> && !IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {+ if ((This-
> >Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->SenseData, This-
> >Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } diff --git
> a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> index 79026a4a957d..471fb6a7f440 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1036,7 +1036,7 @@ TrustTransferAtaDevice (
> // Check the alignment of the incoming buffer prior to invoking underlying
> ATA PassThru // AtaPassThru = AtaDevice->AtaBusDriverData-
> >AtaPassThru;- if ((AtaPassThru->Mode->IoAlign > 1) && !IS_ALIGNED
> (Buffer, AtaPassThru->Mode->IoAlign)) {+ if ((AtaPassThru->Mode-
> >IoAlign > 1) && !ADDRESS_IS_ALIGNED_ (Buffer, AtaPassThru->Mode-
> >IoAlign)) { NewBuffer = AllocateAlignedBuffer (AtaDevice,
> TransferLength); if (NewBuffer == NULL) { return
> EFI_OUT_OF_RESOURCES;diff --git
> a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index c80e78fa8a6b..81c0fa217a0b 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -1956,7 +1956,7 @@ ScsiDiskReceiveData (
> goto Done; } - if ((ScsiDiskDevice->ScsiIo->IoAlign > 1)
> && !IS_ALIGNED (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {+ if
> ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_
> (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) { AlignedBuffer =
> AllocateAlignedBuffer (ScsiDiskDevice, PayloadBufferSize); if
> (AlignedBuffer == NULL) { Status = EFI_OUT_OF_RESOURCES;@@ -
> 2171,7 +2171,7 @@ ScsiDiskSendData (
> goto Done; } - if ((ScsiDiskDevice->ScsiIo->IoAlign > 1)
> && !IS_ALIGNED (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) {+ if
> ((ScsiDiskDevice->ScsiIo->IoAlign > 1) && !ADDRESS_IS_ALIGNED_
> (PayloadBuffer, ScsiDiskDevice->ScsiIo->IoAlign)) { AlignedBuffer =
> AllocateAlignedBuffer (ScsiDiskDevice, PayloadBufferSize); if
> (AlignedBuffer == NULL) { Status = EFI_OUT_OF_RESOURCES;diff --git
> a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> index 92ff958f161e..abfb422d1ea3 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -170,15 +170,15 @@ UfsPassThruPassThru (
> return EFI_INVALID_PARAMETER; } - if ((This->Mode->IoAlign > 1)
> && !IS_ALIGNED(Packet->InDataBuffer, This->Mode->IoAlign)) {+ if ((This-
> >Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->InDataBuffer,
> This->Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } - if ((This-
> >Mode->IoAlign > 1) && !IS_ALIGNED(Packet->OutDataBuffer, This->Mode-
> >IoAlign)) {+ if ((This->Mode->IoAlign > 1)
> && !ADDRESS_IS_ALIGNED_(Packet->OutDataBuffer, This->Mode->IoAlign))
> { return EFI_INVALID_PARAMETER; } - if ((This->Mode->IoAlign > 1)
> && !IS_ALIGNED(Packet->SenseData, This->Mode->IoAlign)) {+ if ((This-
> >Mode->IoAlign > 1) && !ADDRESS_IS_ALIGNED_(Packet->SenseData, This-
> >Mode->IoAlign)) { return EFI_INVALID_PARAMETER; } diff --git
> a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> index 1c4a4f5155c9..35f60cabdeb4 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2004,7 +2004,7 @@ ExecuteJMP (
> // check for alignment, and jump absolute. // Data64 = (UINT64)
> VmReadImmed64 (VmPtr, 2);- if (!IS_ALIGNED ((UINTN) Data64, sizeof
> (UINT16))) {+ if (!ADDRESS_IS_ALIGNED_ ((UINTN) Data64, sizeof (UINT16)))
> { EbcDebugSignalException ( EXCEPT_EBC_ALIGNMENT_CHECK,
> EXCEPTION_FLAG_FATAL,@@ -2059,7 +2059,7 @@ ExecuteJMP (
> // Form: JMP32 @Rx {Index32} // Addr = VmReadMemN (VmPtr,
> (UINTN) Data64 + Index32);- if (!IS_ALIGNED ((UINTN) Addr, sizeof
> (UINT16))) {+ if (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16)))
> { EbcDebugSignalException ( EXCEPT_EBC_ALIGNMENT_CHECK,
> EXCEPTION_FLAG_FATAL,@@ -2082,7 +2082,7 @@ ExecuteJMP (
> // Form: JMP32 Rx {Immed32} // Addr = (UINTN) (Data64 + Index32);-
> if (!IS_ALIGNED ((UINTN) Addr, sizeof (UINT16))) {+ if
> (!ADDRESS_IS_ALIGNED_ ((UINTN) Addr, sizeof (UINT16)))
> { EbcDebugSignalException ( EXCEPT_EBC_ALIGNMENT_CHECK,
> EXCEPTION_FLAG_FATAL,@@ -3128,7 +3128,7 @@ ExecuteRET (
> // Pull the return address off the VM app's stack and set the IP // to it
> //- if (!IS_ALIGNED ((UINTN) VmPtr->Gpr[0], sizeof (UINT16))) {+ if
> (!ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Gpr[0], sizeof (UINT16)))
> { EbcDebugSignalException ( EXCEPT_EBC_ALIGNMENT_CHECK,
> EXCEPTION_FLAG_FATAL,@@ -4693,7 +4693,7 @@ VmWriteMem16 (
> // // Do a simple write if aligned //- if (IS_ALIGNED (Addr, sizeof
> (UINT16))) {+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16)))
> { *(UINT16 *) Addr = Data; } else { //@@ -4756,7 +4756,7 @@
> VmWriteMem32 (
> // // Do a simple write if aligned //- if (IS_ALIGNED (Addr, sizeof
> (UINT32))) {+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32)))
> { *(UINT32 *) Addr = Data; } else { //@@ -4819,7 +4819,7 @@
> VmWriteMem64 (
> // // Do a simple write if aligned //- if (IS_ALIGNED (Addr, sizeof
> (UINT64))) {+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64)))
> { *(UINT64 *) Addr = Data; } else { //@@ -4885,7 +4885,7 @@
> VmWriteMemN (
> // // Do a simple write if aligned //- if (IS_ALIGNED (Addr, sizeof (UINTN)))
> {+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) { *(UINTN *) Addr =
> Data; } else { for (Index = 0; Index < sizeof (UINTN) / sizeof (UINT32);
> Index++) {@@ -4949,7 +4949,7 @@ VmReadImmed16 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (INT16))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip +
> Offset, sizeof (INT16))) { return * (INT16 *) (VmPtr->Ip + Offset); } else
> { //@@ -4993,7 +4993,7 @@ VmReadImmed32 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (UINT32))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip
> + Offset, sizeof (UINT32))) { return * (INT32 *) (VmPtr->Ip + Offset); }
> //@@ -5032,7 +5032,7 @@ VmReadImmed64 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (UINT64))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip
> + Offset, sizeof (UINT64))) { return * (UINT64 *) (VmPtr->Ip + Offset); }
> //@@ -5069,7 +5069,7 @@ VmReadCode16 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (UINT16))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip
> + Offset, sizeof (UINT16))) { return * (UINT16 *) (VmPtr->Ip + Offset); }
> else { //@@ -5110,7 +5110,7 @@ VmReadCode32 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (UINT32))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip
> + Offset, sizeof (UINT32))) { return * (UINT32 *) (VmPtr->Ip + Offset); }
> //@@ -5147,7 +5147,7 @@ VmReadCode64 (
> // // Read direct if aligned //- if (IS_ALIGNED ((UINTN) VmPtr->Ip +
> Offset, sizeof (UINT64))) {+ if (ADDRESS_IS_ALIGNED_ ((UINTN) VmPtr->Ip
> + Offset, sizeof (UINT64))) { return * (UINT64 *) (VmPtr->Ip + Offset); }
> //@@ -5210,7 +5210,7 @@ VmReadMem16 (
> // // Read direct if aligned //- if (IS_ALIGNED (Addr, sizeof (UINT16))) {+
> if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) { return * (UINT16 *)
> Addr; } //@@ -5243,7 +5243,7 @@ VmReadMem32 (
> // // Read direct if aligned //- if (IS_ALIGNED (Addr, sizeof (UINT32))) {+
> if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) { return * (UINT32 *)
> Addr; } //@@ -5280,7 +5280,7 @@ VmReadMem64 (
> // // Read direct if aligned //- if (IS_ALIGNED (Addr, sizeof (UINT64))) {+
> if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) { return * (UINT64 *)
> Addr; } //@@ -5349,7 +5349,7 @@ VmReadMemN (
> // // Read direct if aligned //- if (IS_ALIGNED (Addr, sizeof (UINTN))) {+ if
> (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) { return * (UINTN *)
> Addr; } //diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 2be78076bee7..2a74c9984791 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -145,7 +145,7 @@ typedef union {
> #define AHCI_PORT_SERR 0x0030 #define AHCI_PORT_CI
> 0x0038 -#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1))
> == 0)+#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) &
> (size - 1)) == 0) #define TIMER_PERIOD_SECONDS(Seconds)
> MultU64x32((UINT64)(Seconds), 10000000) #pragma pack(1)diff --git
> a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> index 5f582b9b3e76..aa3472a71677 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -147,7 +147,7 @@ struct _ATA_NONBLOCK_TASK {
> #define ATA_ATAPI_TIMEOUT EFI_TIMER_PERIOD_SECONDS(3)
> #define ATA_SPINUP_TIMEOUT EFI_TIMER_PERIOD_SECONDS(10) -
> #define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) ==
> 0)+#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size -
> 1)) == 0) #define ATA_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \ CR (a,
> \diff --git a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> index a5a865209942..13767a8a13c8 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h
> @@ -76,7 +76,7 @@
> #define ATA_TASK_SIGNATURE SIGNATURE_32 ('A', 'T', 'S', 'K')
> #define ATA_DEVICE_SIGNATURE SIGNATURE_32 ('A', 'B', 'I', 'D')
> #define ATA_SUB_TASK_SIGNATURE SIGNATURE_32 ('A', 'S', 'T', 'S')-
> #define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) ==
> 0)+#define ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size
> - 1)) == 0) // // ATA bus data structure for ATA controllerdiff --git
> a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> index ed9bbd6f8ba8..abe1d06a98da 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -39,7 +39,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define IS_DEVICE_FIXED(a) (a)->FixedDevice ? 1 : 0 -#define
> IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)+#define
> ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
> #define UFS_WLUN_RPMB 0xC4 diff --git
> a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> index 6e2305aa2bc2..4802fdc0ab9b 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> +++ b/MdeModulePkg/Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h
> @@ -133,7 +133,7 @@ typedef struct _UFS_PEIM_HC_PRIVATE_DATA {
> #define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8) -#define
> IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)+#define
> ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
> #define GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS(a) CR (a,
> UFS_PEIM_HC_PRIVATE_DATA, BlkIoPpi, UFS_PEIM_HC_SIG) #define
> GET_UFS_PEIM_HC_PRIVATE_DATA_FROM_THIS2(a) CR (a,
> UFS_PEIM_HC_PRIVATE_DATA, BlkIo2Ppi, UFS_PEIM_HC_SIG)diff --git
> a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> index 79b86f7e6b3d..da3bf78bf9bc 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
> @@ -105,7 +105,7 @@ typedef struct {
> #define ROUNDUP8(x) (((x) % 8 == 0) ? (x) : ((x) / 8 + 1) * 8) -#define
> IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)+#define
> ADDRESS_IS_ALIGNED_(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
> #define UFS_PASS_THRU_PRIVATE_DATA_FROM_THIS(a) \ CR (a, \diff --git
> a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> index 1cb68bc5385a..f21c757a75b4 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> @@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> // Macros to check and set alignment // #define ASSERT_ALIGNED(addr, size)
> ASSERT (!((UINT32) (addr) & (size - 1)))-#define IS_ALIGNED(addr,
> size) !((UINT32) (addr) & (size - 1))+#define ADDRESS_IS_ALIGNED_(addr,
> size) !((UINT32) (addr) & (size - 1)) // // Debug macro--
> 2.31.1
^ permalink raw reply [flat|nested] 12+ messages in thread