* [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
@ 2023-03-03 6:51 ` Gerd Hoffmann
2023-03-21 2:01 ` Wu, Hao A
2023-03-21 21:40 ` [edk2-devel] " Michael D Kinney
2023-03-03 6:51 ` [PATCH 2/5] OvmfPkg: " Gerd Hoffmann
` (5 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-03 6:51 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jian J Wang, Jiewen Yao,
Marvin Häuser, James Bottomley, Michael Roth, Hao A Wu,
Michael D Kinney, Oliver Steffen, Min Xu, Liming Gao, Ray Ni,
Tom Lendacky, Erdem Aktas, Zhiguang Liu, Pawel Polawski,
Jordan Justen
From: Marvin Häuser <mhaeuser@posteo.de>
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.
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/AhciPei.h | 4 +--
.../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 +-
MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 2 +-
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 +-
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 +-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +-
MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 4 +--
MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
.../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
.../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
.../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
.../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
14 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
index 4aed1cb7ad8a..71d34c962ad1 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
@@ -146,8 +146,8 @@ typedef union {
#define AHCI_PORT_SERR 0x0030
#define AHCI_PORT_CI 0x0038
-#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
-#define TIMER_PERIOD_SECONDS(Seconds) MultU64x32((UINT64)(Seconds), 10000000)
+#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 62ba6d6680dd..7937886614e1 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -148,7 +148,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 4428c484fd6c..47346e911d47 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 controller
diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
index 5b4047e1dbdd..ed384ad52182 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
@@ -38,7 +38,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 a0b615b7eab3..1adb382aa8c3 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 2b4f5d32d901..0ec37e56652b 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 32b8670c5b2a..6dc6730ab095 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
@@ -14,8 +14,8 @@ 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 ASSERT_ALIGNED(addr, size) ASSERT (!((UINT32) (addr) & (size - 1)))
+#define ADDRESS_IS_ALIGNED_(addr, size) !((UINT32) (addr) & (size - 1))
//
// Debug macro
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
index 7b97887c5dbd..d93fa78c81f3 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
@@ -2126,7 +2126,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),
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
index d5ed93dc4f67..0c49059a00d5 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
@@ -194,15 +194,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 1070516b358a..324abadd02dd 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -1299,15 +1299,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;
}
@@ -2039,15 +2039,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 4334169d256a..18aa4f9bb666 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
@@ -1040,7 +1040,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 fbc236cb465e..faf4ae332e46 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -2029,7 +2029,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;
@@ -2249,7 +2249,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 ae593ff03a0d..392a295caf04 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -171,15 +171,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 82a7782fb99a..28f108c44873 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
@@ -2015,7 +2015,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,
@@ -2074,7 +2074,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,
@@ -2097,7 +2097,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,
@@ -3158,7 +3158,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,
@@ -4733,7 +4733,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 {
//
@@ -4795,7 +4795,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 {
//
@@ -4857,7 +4857,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 {
//
@@ -4922,7 +4922,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++) {
@@ -4985,7 +4985,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 {
//
@@ -5029,7 +5029,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);
}
@@ -5068,7 +5068,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);
}
@@ -5105,7 +5105,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 {
//
@@ -5147,7 +5147,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);
}
@@ -5184,7 +5184,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);
}
@@ -5247,7 +5247,7 @@ VmReadMem16 (
//
// Read direct if aligned
//
- if (IS_ALIGNED (Addr, sizeof (UINT16))) {
+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
return *(UINT16 *)Addr;
}
@@ -5281,7 +5281,7 @@ VmReadMem32 (
//
// Read direct if aligned
//
- if (IS_ALIGNED (Addr, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
return *(UINT32 *)Addr;
}
@@ -5319,7 +5319,7 @@ VmReadMem64 (
//
// Read direct if aligned
//
- if (IS_ALIGNED (Addr, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
return *(UINT64 *)Addr;
}
@@ -5388,7 +5388,7 @@ VmReadMemN (
//
// Read direct if aligned
//
- if (IS_ALIGNED (Addr, sizeof (UINTN))) {
+ if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
return *(UINTN *)Addr;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-03 6:51 ` [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Gerd Hoffmann
@ 2023-03-21 2:01 ` Wu, Hao A
2023-03-21 21:40 ` [edk2-devel] " Michael D Kinney
1 sibling, 0 replies; 22+ messages in thread
From: Wu, Hao A @ 2023-03-21 2:01 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Kinney, Michael D, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, March 3, 2023 2:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James
> Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min
> M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> <ray.ni@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Aktas,
> Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Pawel Polawski <ppolawsk@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>
> Subject: [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid
> name collisions
>
> From: Marvin Häuser <mhaeuser@posteo.de>
>
> 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.
>
> 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/AhciPei.h | 4 +--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 +-
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 +-
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 +-
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +-
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 4 +--
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> .../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
> 14 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 4aed1cb7ad8a..71d34c962ad1 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -146,8 +146,8 @@ typedef union {
> #define AHCI_PORT_SERR 0x0030
> #define AHCI_PORT_CI 0x0038
>
> -#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
> -#define TIMER_PERIOD_SECONDS(Seconds) MultU64x32((UINT64)(Seconds),
> 10000000)
> +#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 62ba6d6680dd..7937886614e1 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -148,7 +148,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 4428c484fd6c..47346e911d47 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 controller
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> index 5b4047e1dbdd..ed384ad52182 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -38,7 +38,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 a0b615b7eab3..1adb382aa8c3 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 2b4f5d32d901..0ec37e56652b 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 32b8670c5b2a..6dc6730ab095 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> @@ -14,8 +14,8 @@ 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 ASSERT_ALIGNED(addr, size) ASSERT (!((UINT32) (addr) & (size -
> 1)))
> +#define ADDRESS_IS_ALIGNED_(addr, size) !((UINT32) (addr) & (size - 1))
>
> //
> // Debug macro
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index 7b97887c5dbd..d93fa78c81f3 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2126,7 +2126,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),
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index d5ed93dc4f67..0c49059a00d5 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -194,15 +194,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 1070516b358a..324abadd02dd 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1299,15 +1299,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;
> }
>
> @@ -2039,15 +2039,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 4334169d256a..18aa4f9bb666 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1040,7 +1040,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 fbc236cb465e..faf4ae332e46 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2029,7 +2029,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;
> @@ -2249,7 +2249,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 ae593ff03a0d..392a295caf04 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -171,15 +171,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 82a7782fb99a..28f108c44873 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2015,7 +2015,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,
> @@ -2074,7 +2074,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,
> @@ -2097,7 +2097,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,
> @@ -3158,7 +3158,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,
> @@ -4733,7 +4733,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 {
> //
> @@ -4795,7 +4795,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 {
> //
> @@ -4857,7 +4857,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 {
> //
> @@ -4922,7 +4922,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++) {
> @@ -4985,7 +4985,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 {
> //
> @@ -5029,7 +5029,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);
> }
>
> @@ -5068,7 +5068,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);
> }
>
> @@ -5105,7 +5105,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 {
> //
> @@ -5147,7 +5147,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);
> }
>
> @@ -5184,7 +5184,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);
> }
>
> @@ -5247,7 +5247,7 @@ VmReadMem16 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT16))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
> return *(UINT16 *)Addr;
> }
>
> @@ -5281,7 +5281,7 @@ VmReadMem32 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT32))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
> return *(UINT32 *)Addr;
> }
>
> @@ -5319,7 +5319,7 @@ VmReadMem64 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT64))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
> return *(UINT64 *)Addr;
> }
>
> @@ -5388,7 +5388,7 @@ VmReadMemN (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINTN))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
> return *(UINTN *)Addr;
> }
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [edk2-devel] [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-03 6:51 ` [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Gerd Hoffmann
2023-03-21 2:01 ` Wu, Hao A
@ 2023-03-21 21:40 ` Michael D Kinney
1 sibling, 0 replies; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 21:40 UTC (permalink / raw)
To: devel@edk2.groups.io, kraxel@redhat.com
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Kinney, Michael D
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann
> Sent: Thursday, March 2, 2023 10:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>; Michael Roth
> <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski
> <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
>
> From: Marvin Häuser <mhaeuser@posteo.de>
>
> 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.
>
> 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/AhciPei.h | 4 +--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 +-
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 +-
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 +-
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 +-
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 4 +--
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> .../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
> 14 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 4aed1cb7ad8a..71d34c962ad1 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -146,8 +146,8 @@ typedef union {
> #define AHCI_PORT_SERR 0x0030
> #define AHCI_PORT_CI 0x0038
>
> -#define IS_ALIGNED(addr, size) (((UINTN) (addr) & (size - 1)) == 0)
> -#define TIMER_PERIOD_SECONDS(Seconds) MultU64x32((UINT64)(Seconds), 10000000)
> +#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 62ba6d6680dd..7937886614e1 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -148,7 +148,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 4428c484fd6c..47346e911d47 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 controller
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> index 5b4047e1dbdd..ed384ad52182 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -38,7 +38,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 a0b615b7eab3..1adb382aa8c3 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 2b4f5d32d901..0ec37e56652b 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 32b8670c5b2a..6dc6730ab095 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.h
> @@ -14,8 +14,8 @@ 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 ASSERT_ALIGNED(addr, size) ASSERT (!((UINT32) (addr) & (size - 1)))
> +#define ADDRESS_IS_ALIGNED_(addr, size) !((UINT32) (addr) & (size - 1))
>
> //
> // Debug macro
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index 7b97887c5dbd..d93fa78c81f3 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2126,7 +2126,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),
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index d5ed93dc4f67..0c49059a00d5 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -194,15 +194,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 1070516b358a..324abadd02dd 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1299,15 +1299,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;
> }
>
> @@ -2039,15 +2039,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 4334169d256a..18aa4f9bb666 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1040,7 +1040,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 fbc236cb465e..faf4ae332e46 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2029,7 +2029,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;
> @@ -2249,7 +2249,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 ae593ff03a0d..392a295caf04 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -171,15 +171,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 82a7782fb99a..28f108c44873 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2015,7 +2015,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,
> @@ -2074,7 +2074,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,
> @@ -2097,7 +2097,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,
> @@ -3158,7 +3158,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,
> @@ -4733,7 +4733,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 {
> //
> @@ -4795,7 +4795,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 {
> //
> @@ -4857,7 +4857,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 {
> //
> @@ -4922,7 +4922,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++) {
> @@ -4985,7 +4985,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 {
> //
> @@ -5029,7 +5029,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);
> }
>
> @@ -5068,7 +5068,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);
> }
>
> @@ -5105,7 +5105,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 {
> //
> @@ -5147,7 +5147,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);
> }
>
> @@ -5184,7 +5184,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);
> }
>
> @@ -5247,7 +5247,7 @@ VmReadMem16 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT16))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
> return *(UINT16 *)Addr;
> }
>
> @@ -5281,7 +5281,7 @@ VmReadMem32 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT32))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
> return *(UINT32 *)Addr;
> }
>
> @@ -5319,7 +5319,7 @@ VmReadMem64 (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINT64))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
> return *(UINT64 *)Addr;
> }
>
> @@ -5388,7 +5388,7 @@ VmReadMemN (
> //
> // Read direct if aligned
> //
> - if (IS_ALIGNED (Addr, sizeof (UINTN))) {
> + if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
> return *(UINTN *)Addr;
> }
>
> --
> 2.39.2
>
>
>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] OvmfPkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
2023-03-03 6:51 ` [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Gerd Hoffmann
@ 2023-03-03 6:51 ` Gerd Hoffmann
2023-03-21 21:40 ` [edk2-devel] " Michael D Kinney
2023-03-03 6:51 ` [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
` (4 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-03 6:51 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jian J Wang, Jiewen Yao,
Marvin Häuser, James Bottomley, Michael Roth, Hao A Wu,
Michael D Kinney, Oliver Steffen, Min Xu, Liming Gao, Ray Ni,
Tom Lendacky, Erdem Aktas, Zhiguang Liu, Pawel Polawski,
Jordan Justen
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 OvmfPkg.
Temporarily rename them to avoid build failure, till they can be
substituted with the new, shared definitions.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 +++---
.../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index a726498e2792..71a1eaaf0a1d 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -44,7 +44,7 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
-#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
+#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
STATIC
EFI_STATUS
@@ -60,8 +60,8 @@ AmdSevMemoryAccept (
// multiple of SIZE_4KB. Use an assert instead of returning an erros since
// this is an EDK2-internal protocol.
//
- ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
- ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+ ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
ASSERT (Size != 0);
MemEncryptSevSnpPreValidateSystemRam (
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
index 4d684964d838..f35bba5deb46 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -20,7 +20,7 @@
#include "SnpPageStateChange.h"
-#define IS_ALIGNED(x, y) ((((x) & (y - 1)) == 0))
+#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
#define PAGES_PER_LARGE_ENTRY 512
STATIC
@@ -150,7 +150,7 @@ BuildPageStateBuffer (
//
// Is this a 2MB aligned page? Check if we can use the Large RMP entry.
//
- if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
+ if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
((EndAddress - BaseAddress) >= SIZE_2MB))
{
RmpPageSize = PvalidatePageSize2MB;
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [edk2-devel] [PATCH 2/5] OvmfPkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-03 6:51 ` [PATCH 2/5] OvmfPkg: " Gerd Hoffmann
@ 2023-03-21 21:40 ` Michael D Kinney
2023-03-21 23:30 ` Yao, Jiewen
0 siblings, 1 reply; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 21:40 UTC (permalink / raw)
To: devel@edk2.groups.io, kraxel@redhat.com
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Kinney, Michael D
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann
> Sent: Thursday, March 2, 2023 10:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>; Michael Roth
> <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski
> <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [edk2-devel] [PATCH 2/5] OvmfPkg: 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 OvmfPkg.
> Temporarily rename them to avoid build failure, till they can be
> substituted with the new, shared definitions.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 +++---
> .../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index a726498e2792..71a1eaaf0a1d 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -44,7 +44,7 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
>
> STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
>
> -#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
> +#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
>
> STATIC
> EFI_STATUS
> @@ -60,8 +60,8 @@ AmdSevMemoryAccept (
> // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> // this is an EDK2-internal protocol.
> //
> - ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> - ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> + ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
> + ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
> ASSERT (Size != 0);
>
> MemEncryptSevSnpPreValidateSystemRam (
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> index 4d684964d838..f35bba5deb46 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> @@ -20,7 +20,7 @@
>
> #include "SnpPageStateChange.h"
>
> -#define IS_ALIGNED(x, y) ((((x) & (y - 1)) == 0))
> +#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
> #define PAGES_PER_LARGE_ENTRY 512
>
> STATIC
> @@ -150,7 +150,7 @@ BuildPageStateBuffer (
> //
> // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
> //
> - if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
> + if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
> ((EndAddress - BaseAddress) >= SIZE_2MB))
> {
> RmpPageSize = PvalidatePageSize2MB;
> --
> 2.39.2
>
>
>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [edk2-devel] [PATCH 2/5] OvmfPkg: Rename IS_ALIGNED macros to avoid name collisions
2023-03-21 21:40 ` [edk2-devel] " Michael D Kinney
@ 2023-03-21 23:30 ` Yao, Jiewen
0 siblings, 0 replies; 22+ messages in thread
From: Yao, Jiewen @ 2023-03-21 23:30 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io, kraxel@redhat.com
Cc: Ard Biesheuvel, Wang, Jian J, Marvin Häuser, James Bottomley,
Michael Roth, Wu, Hao A, Oliver Steffen, Xu, Min M, Gao, Liming,
Ni, Ray, Tom Lendacky, Aktas, Erdem, Liu, Zhiguang,
Pawel Polawski, Justen, Jordan L
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, March 22, 2023 5:41 AM
> To: devel@edk2.groups.io; kraxel@redhat.com
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Marvin
> Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>;
> Michael Roth <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Oliver Steffen <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel
> Polawski <ppolawsk@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH 2/5] OvmfPkg: Rename IS_ALIGNED
> macros to avoid name collisions
>
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> > Sent: Thursday, March 2, 2023 10:51 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>;
> James Bottomley <jejb@linux.ibm.com>; Michael Roth
> > <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> > <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> > Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel
> Polawski
> > <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> > Subject: [edk2-devel] [PATCH 2/5] OvmfPkg: 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 OvmfPkg.
> > Temporarily rename them to avoid build failure, till they can be
> > substituted with the new, shared definitions.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 +++---
> > .../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 4 ++--
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index a726498e2792..71a1eaaf0a1d 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -44,7 +44,7 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
> >
> > STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
> >
> > -#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
> > +#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
> >
> > STATIC
> > EFI_STATUS
> > @@ -60,8 +60,8 @@ AmdSevMemoryAccept (
> > // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> > // this is an EDK2-internal protocol.
> > //
> > - ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> > - ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> > + ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
> > + ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
> > ASSERT (Size != 0);
> >
> > MemEncryptSevSnpPreValidateSystemRam (
> > diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> >
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > index 4d684964d838..f35bba5deb46 100644
> > ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > @@ -20,7 +20,7 @@
> >
> > #include "SnpPageStateChange.h"
> >
> > -#define IS_ALIGNED(x, y) ((((x) & (y - 1)) == 0))
> > +#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
> > #define PAGES_PER_LARGE_ENTRY 512
> >
> > STATIC
> > @@ -150,7 +150,7 @@ BuildPageStateBuffer (
> > //
> > // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
> > //
> > - if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
> > + if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
> > ((EndAddress - BaseAddress) >= SIZE_2MB))
> > {
> > RmpPageSize = PvalidatePageSize2MB;
> > --
> > 2.39.2
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
2023-03-03 6:51 ` [PATCH 1/5] MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions Gerd Hoffmann
2023-03-03 6:51 ` [PATCH 2/5] OvmfPkg: " Gerd Hoffmann
@ 2023-03-03 6:51 ` Gerd Hoffmann
2023-03-21 21:37 ` Michael D Kinney
2023-03-03 6:51 ` [PATCH 4/5] MdeModulePkg: Consume new " Gerd Hoffmann
` (3 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-03 6:51 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jian J Wang, Jiewen Yao,
Marvin Häuser, James Bottomley, Michael Roth, Hao A Wu,
Michael D Kinney, Oliver Steffen, Min Xu, Liming Gao, Ray Ni,
Tom Lendacky, Erdem Aktas, Zhiguang Liu, Pawel Polawski,
Jordan Justen, Vitaly Cheptsov
From: Marvin Häuser <mhaeuser@posteo.de>
ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.
IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.
IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by several modules.
ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the codebase.
ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index d209e6de280a..2053314b50d1 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
#define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif
+/**
+ Returns the alignment requirement of a type.
+
+ @param TYPE The name of the type to retrieve the alignment requirement of.
+
+ @return Alignment requirement, in Bytes, of TYPE.
+**/
+#if defined (__cplusplus)
+//
+// Standard C++ operator.
+//
+#define ALIGNOF(TYPE) alignof (TYPE)
+#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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");
+
/**
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.
+**/
+#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.
@@ -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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-03 6:51 ` [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
@ 2023-03-21 21:37 ` Michael D Kinney
2023-03-21 21:59 ` Marvin Häuser
0 siblings, 1 reply; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 21:37 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L, Vitaly Cheptsov,
Kinney, Michael D
Hi Gerd,
A few comments included below.
Thanks,
Mike
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Thursday, March 2, 2023 10:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>; Michael Roth
> <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski
> <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
>
> From: Marvin Häuser <mhaeuser@posteo.de>
>
> ALIGNOF: Determining the alignment requirement of data types is
> crucial to ensure safe memory accesses when parsing untrusted data.
>
> IS_POW2: Determining whether a value is a power of two is important
> to verify whether untrusted values are valid alignment values.
>
> IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
> A more general version of the IS_ALIGNED macro previously defined by several modules.
>
> ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
> Replaces module-specific definitions throughout the codebase.
>
> ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
> determine the required offset for data alignment.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
> ---
> MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index d209e6de280a..2053314b50d1 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
> #endif
>
> +/**
> + Returns the alignment requirement of a type.
> +
> + @param TYPE The name of the type to retrieve the alignment requirement of.
> +
> + @return Alignment requirement, in Bytes, of TYPE.
> +**/
> +#if defined (__cplusplus)
> +//
> +// Standard C++ operator.
> +//
> +#define ALIGNOF(TYPE) alignof (TYPE)
> +#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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
> +**/
> +#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.
> +**/
> +#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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-21 21:37 ` Michael D Kinney
@ 2023-03-21 21:59 ` Marvin Häuser
2023-03-21 22:28 ` Michael D Kinney
0 siblings, 1 reply; 22+ messages in thread
From: Marvin Häuser @ 2023-03-21 21:59 UTC (permalink / raw)
To: Kinney, Michael D
Cc: Gerd Hoffmann, devel@edk2.groups.io, Ard Biesheuvel, Wang, Jian J,
Yao, Jiewen, James Bottomley, Michael Roth, Wu, Hao A,
Oliver Steffen, Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky,
Aktas, Erdem, Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Vitaly Cheptsov
[-- Attachment #1: Type: text/plain, Size: 11452 bytes --]
Hi Mike,
> On 21. Mar 2023, at 22:37, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
>
> Hi Gerd,
>
> A few comments included below.
>
> Thanks,
>
> Mike
>
>> -----Original Message-----
>> From: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>
>> Sent: Thursday, March 2, 2023 10:51 PM
>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>; Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Yao,
>> Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>; Marvin Häuser <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de>>; James Bottomley <jejb@linux.ibm.com <mailto:jejb@linux.ibm.com>>; Michael Roth
>> <michael.roth@amd.com <mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com <mailto:hao.a.wu@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Oliver Steffen
>> <osteffen@redhat.com <mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com <mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>; Tom
>> Lendacky <thomas.lendacky@amd.com <mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com <mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>; Pawel Polawski
>> <ppolawsk@redhat.com <mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com <mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com <mailto:vit9696@protonmail.com>>
>> Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
>>
>> From: Marvin Häuser <mhaeuser@posteo.de>
>>
>> ALIGNOF: Determining the alignment requirement of data types is
>> crucial to ensure safe memory accesses when parsing untrusted data.
>>
>> IS_POW2: Determining whether a value is a power of two is important
>> to verify whether untrusted values are valid alignment values.
>>
>> IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
>> A more general version of the IS_ALIGNED macro previously defined by several modules.
>>
>> ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
>> Replaces module-specific definitions throughout the codebase.
>>
>> ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
>> determine the required offset for data alignment.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Cc: Vitaly Cheptsov <vit9696@protonmail.com>
>> Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
>> ---
>> MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 94 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
>> index d209e6de280a..2053314b50d1 100644
>> --- a/MdePkg/Include/Base.h
>> +++ b/MdePkg/Include/Base.h
>> @@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
>> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>> #endif
>>
>> +/**
>> + Returns the alignment requirement of a type.
>> +
>> + @param TYPE The name of the type to retrieve the alignment requirement of.
>> +
>> + @return Alignment requirement, in Bytes, of TYPE.
>> +**/
>> +#if defined (__cplusplus)
>> +//
>> +// Standard C++ operator.
>> +//
>> +#define ALIGNOF(TYPE) alignof (TYPE)
>> +#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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
[-- Attachment #2: Type: text/html, Size: 28560 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-21 21:59 ` Marvin Häuser
@ 2023-03-21 22:28 ` Michael D Kinney
2023-03-21 22:43 ` Marvin Häuser
0 siblings, 1 reply; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 22:28 UTC (permalink / raw)
To: Marvin Häuser
Cc: Gerd Hoffmann, devel@edk2.groups.io, Ard Biesheuvel, Wang, Jian J,
Yao, Jiewen, James Bottomley, Michael Roth, Wu, Hao A,
Oliver Steffen, Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky,
Aktas, Erdem, Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Vitaly Cheptsov, Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 12206 bytes --]
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.
Mike
From: Marvin Häuser <mhaeuser@posteo.de>
Sent: Tuesday, March 21, 2023 3:00 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; James Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
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.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Gerd,
A few comments included below.
Thanks,
Mike
-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Sent: Thursday, March 2, 2023 10:51 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao,
Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth
<michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Oliver Steffen
<osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom
Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski
<ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
From: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.
IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.
IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by several modules.
ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the codebase.
ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Cc: Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
---
MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index d209e6de280a..2053314b50d1 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
#define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif
+/**
+ Returns the alignment requirement of a type.
+
+ @param TYPE The name of the type to retrieve the alignment requirement of.
+
+ @return Alignment requirement, in Bytes, of TYPE.
+**/
+#if defined (__cplusplus)
+//
+// Standard C++ operator.
+//
+#define ALIGNOF(TYPE) alignof (TYPE)
+#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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
[-- Attachment #2: Type: text/html, Size: 61495 bytes --]
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-21 22:28 ` Michael D Kinney
@ 2023-03-21 22:43 ` Marvin Häuser
2023-03-21 22:50 ` Michael D Kinney
0 siblings, 1 reply; 22+ messages in thread
From: Marvin Häuser @ 2023-03-21 22:43 UTC (permalink / raw)
To: Kinney, Michael D
Cc: Gerd Hoffmann, devel, Ard Biesheuvel, Wang, Jian J, Yao, Jiewen,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L, Vitaly Cheptsov
[-- Attachment #1: Type: text/html, Size: 60181 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-21 22:43 ` Marvin Häuser
@ 2023-03-21 22:50 ` Michael D Kinney
2023-03-22 0:09 ` Ni, Ray
0 siblings, 1 reply; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 22:50 UTC (permalink / raw)
To: Marvin Häuser
Cc: Gerd Hoffmann, devel@edk2.groups.io, Ard Biesheuvel, Wang, Jian J,
Yao, Jiewen, James Bottomley, Michael Roth, Wu, Hao A,
Oliver Steffen, Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky,
Aktas, Erdem, Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Vitaly Cheptsov, Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 13986 bytes --]
Ok. No objections to current description.
Mike
From: Marvin Häuser <mhaeuser@posteo.de>
Sent: Tuesday, March 21, 2023 3:44 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; James Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
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.kinney@intel.com<mailto:michael.d.kinney@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 <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
Sent: Tuesday, March 21, 2023 3:00 PM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth <michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Oliver Steffen <osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@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.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Gerd,
A few comments included below.
Thanks,
Mike
-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Sent: Thursday, March 2, 2023 10:51 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao,
Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth
<michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Oliver Steffen
<osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom
Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski
<ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
From: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.
IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.
IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by several modules.
ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the codebase.
ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Cc: Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
---
MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index d209e6de280a..2053314b50d1 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
#define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif
+/**
+ Returns the alignment requirement of a type.
+
+ @param TYPE The name of the type to retrieve the alignment requirement of.
+
+ @return Alignment requirement, in Bytes, of TYPE.
+**/
+#if defined (__cplusplus)
+//
+// Standard C++ operator.
+//
+#define ALIGNOF(TYPE) alignof (TYPE)
+#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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
[-- Attachment #2: Type: text/html, Size: 66477 bytes --]
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-21 22:50 ` Michael D Kinney
@ 2023-03-22 0:09 ` Ni, Ray
0 siblings, 0 replies; 22+ messages in thread
From: Ni, Ray @ 2023-03-22 0:09 UTC (permalink / raw)
To: Kinney, Michael D, Marvin Häuser
Cc: Gerd Hoffmann, devel@edk2.groups.io, Ard Biesheuvel, Wang, Jian J,
Yao, Jiewen, James Bottomley, Michael Roth, Wu, Hao A,
Oliver Steffen, Xu, Min M, Gao, Liming, Tom Lendacky,
Aktas, Erdem, Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Vitaly Cheptsov
[-- Attachment #1: Type: text/plain, Size: 15826 bytes --]
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.kinney@intel.com>
Sent: Wednesday, March 22, 2023 6:51 AM
To: Marvin Häuser <mhaeuser@posteo.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; James Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>; Kinney, Michael D <michael.d.kinney@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 <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
Sent: Tuesday, March 21, 2023 3:44 PM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth <michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Oliver Steffen <osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@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.kinney@intel.com<mailto:michael.d.kinney@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 <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
Sent: Tuesday, March 21, 2023 3:00 PM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth <michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Oliver Steffen <osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski <ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@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.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
Hi Gerd,
A few comments included below.
Thanks,
Mike
-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Sent: Thursday, March 2, 2023 10:51 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Yao,
Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>; James Bottomley <jejb@linux.ibm.com<mailto:jejb@linux.ibm.com>>; Michael Roth
<michael.roth@amd.com<mailto:michael.roth@amd.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Oliver Steffen
<osteffen@redhat.com<mailto:osteffen@redhat.com>>; Xu, Min M <min.m.xu@intel.com<mailto:min.m.xu@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Tom
Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Aktas, Erdem <erdemaktas@google.com<mailto:erdemaktas@google.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Pawel Polawski
<ppolawsk@redhat.com<mailto:ppolawsk@redhat.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Subject: [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros
From: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
ALIGNOF: Determining the alignment requirement of data types is
crucial to ensure safe memory accesses when parsing untrusted data.
IS_POW2: Determining whether a value is a power of two is important
to verify whether untrusted values are valid alignment values.
IS_ALIGNED: In combination with ALIGNOF data offsets can be verified.
A more general version of the IS_ALIGNED macro previously defined by several modules.
ADDRESS_IS_ALIGNED: Variant of IS_ALIGNED for pointers and addresses.
Replaces module-specific definitions throughout the codebase.
ALIGN_VALUE_ADDEND: The addend to align up can be used to directly
determine the required offset for data alignment.
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
Cc: Zhiguang Liu <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Cc: Vitaly Cheptsov <vit9696@protonmail.com<mailto:vit9696@protonmail.com>>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
---
MdePkg/Include/Base.h | 95 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 94 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index d209e6de280a..2053314b50d1 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -758,6 +758,40 @@ typedef UINTN *BASE_LIST;
#define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif
+/**
+ Returns the alignment requirement of a type.
+
+ @param TYPE The name of the type to retrieve the alignment requirement of.
+
+ @return Alignment requirement, in Bytes, of TYPE.
+**/
+#if defined (__cplusplus)
+//
+// Standard C++ operator.
+//
+#define ALIGNOF(TYPE) alignof (TYPE)
+#elif defined (__GNUC__) || defined (__clang__) || (defined (_MSC_VER) && _MSC_VER >= 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
[-- Attachment #2: Type: text/html, Size: 27818 bytes --]
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/5] MdeModulePkg: Consume new alignment-related macros
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
` (2 preceding siblings ...)
2023-03-03 6:51 ` [PATCH 3/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
@ 2023-03-03 6:51 ` Gerd Hoffmann
2023-03-21 2:01 ` Wu, Hao A
2023-03-21 21:39 ` Michael D Kinney
2023-03-03 6:51 ` [PATCH 5/5] OvmfPkg: " Gerd Hoffmann
` (2 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-03 6:51 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jian J Wang, Jiewen Yao,
Marvin Häuser, James Bottomley, Michael Roth, Hao A Wu,
Michael D Kinney, Oliver Steffen, Min Xu, Liming Gao, Ray Ni,
Tom Lendacky, Erdem Aktas, Zhiguang Liu, Pawel Polawski,
Jordan Justen, Vitaly Cheptsov
From: Marvin Häuser <mhaeuser@posteo.de>
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/AhciPei.h | 3 +-
.../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 --
MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 --
.../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 --
.../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 --
MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
.../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
.../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
.../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
.../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
14 files changed, 36 insertions(+), 47 deletions(-)
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
index 71d34c962ad1..e2e4ba43e7c4 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
@@ -146,8 +146,7 @@ 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)
+#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 7937886614e1..016fc6890ae9 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -148,8 +148,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 47346e911d47..6bc345f7e777 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 ed384ad52182..5a25b55c4952 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
@@ -38,8 +38,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 1adb382aa8c3..ed4776f548e0 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 0ec37e56652b..bc1139da6e3b 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 6dc6730ab095..f3768e79528e 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
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
index d93fa78c81f3..0f0198d3085b 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
@@ -2126,7 +2126,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),
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
index 0c49059a00d5..cd55272c96cd 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
@@ -194,15 +194,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 324abadd02dd..50406fe0270d 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -1299,15 +1299,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;
}
@@ -2039,15 +2039,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 18aa4f9bb666..a77852bae054 100644
--- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
+++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
@@ -1040,7 +1040,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 faf4ae332e46..873581d817ce 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -2029,7 +2029,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;
@@ -2249,7 +2249,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 392a295caf04..880e7d85114e 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -171,15 +171,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 28f108c44873..3221f95a739f 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
@@ -2015,7 +2015,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,
@@ -2074,7 +2074,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,
@@ -2097,7 +2097,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,
@@ -3158,7 +3158,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,
@@ -4733,7 +4733,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 {
//
@@ -4795,7 +4795,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 {
//
@@ -4857,7 +4857,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 {
//
@@ -4922,7 +4922,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++) {
@@ -4985,7 +4985,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 {
//
@@ -5029,7 +5029,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);
}
@@ -5068,7 +5068,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);
}
@@ -5105,7 +5105,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 {
//
@@ -5147,7 +5147,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);
}
@@ -5184,7 +5184,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);
}
@@ -5247,7 +5247,7 @@ VmReadMem16 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
return *(UINT16 *)Addr;
}
@@ -5281,7 +5281,7 @@ VmReadMem32 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
return *(UINT32 *)Addr;
}
@@ -5319,7 +5319,7 @@ VmReadMem64 (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
return *(UINT64 *)Addr;
}
@@ -5388,7 +5388,7 @@ VmReadMemN (
//
// Read direct if aligned
//
- if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
+ if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
return *(UINTN *)Addr;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/5] MdeModulePkg: Consume new alignment-related macros
2023-03-03 6:51 ` [PATCH 4/5] MdeModulePkg: Consume new " Gerd Hoffmann
@ 2023-03-21 2:01 ` Wu, Hao A
2023-03-21 21:39 ` Michael D Kinney
1 sibling, 0 replies; 22+ messages in thread
From: Wu, Hao A @ 2023-03-21 2:01 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Kinney, Michael D, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L, Vitaly Cheptsov
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, March 3, 2023 2:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James
> Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Oliver Steffen <osteffen@redhat.com>; Xu, Min
> M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray
> <ray.ni@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Aktas,
> Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>;
> Pawel Polawski <ppolawsk@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [PATCH 4/5] MdeModulePkg: Consume new alignment-related macros
>
> From: Marvin Häuser <mhaeuser@posteo.de>
>
> 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/AhciPei.h | 3 +-
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 --
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 --
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 --
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 --
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> .../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
> 14 files changed, 36 insertions(+), 47 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 71d34c962ad1..e2e4ba43e7c4 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -146,8 +146,7 @@ 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)
> +#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 7937886614e1..016fc6890ae9 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -148,8 +148,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 47346e911d47..6bc345f7e777 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 ed384ad52182..5a25b55c4952 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -38,8 +38,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 1adb382aa8c3..ed4776f548e0 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 0ec37e56652b..bc1139da6e3b 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 6dc6730ab095..f3768e79528e 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
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index d93fa78c81f3..0f0198d3085b 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2126,7 +2126,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),
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index 0c49059a00d5..cd55272c96cd 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -194,15 +194,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 324abadd02dd..50406fe0270d 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1299,15 +1299,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;
> }
>
> @@ -2039,15 +2039,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 18aa4f9bb666..a77852bae054 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1040,7 +1040,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 faf4ae332e46..873581d817ce 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2029,7 +2029,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;
> @@ -2249,7 +2249,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 392a295caf04..880e7d85114e 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -171,15 +171,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 28f108c44873..3221f95a739f 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2015,7 +2015,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,
> @@ -2074,7 +2074,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,
> @@ -2097,7 +2097,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,
> @@ -3158,7 +3158,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,
> @@ -4733,7 +4733,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 {
> //
> @@ -4795,7 +4795,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 {
> //
> @@ -4857,7 +4857,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 {
> //
> @@ -4922,7 +4922,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++) {
> @@ -4985,7 +4985,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 {
> //
> @@ -5029,7 +5029,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);
> }
>
> @@ -5068,7 +5068,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);
> }
>
> @@ -5105,7 +5105,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 {
> //
> @@ -5147,7 +5147,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);
> }
>
> @@ -5184,7 +5184,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);
> }
>
> @@ -5247,7 +5247,7 @@ VmReadMem16 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
> return *(UINT16 *)Addr;
> }
>
> @@ -5281,7 +5281,7 @@ VmReadMem32 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
> return *(UINT32 *)Addr;
> }
>
> @@ -5319,7 +5319,7 @@ VmReadMem64 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
> return *(UINT64 *)Addr;
> }
>
> @@ -5388,7 +5388,7 @@ VmReadMemN (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
> return *(UINTN *)Addr;
> }
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/5] MdeModulePkg: Consume new alignment-related macros
2023-03-03 6:51 ` [PATCH 4/5] MdeModulePkg: Consume new " Gerd Hoffmann
2023-03-21 2:01 ` Wu, Hao A
@ 2023-03-21 21:39 ` Michael D Kinney
1 sibling, 0 replies; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 21:39 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L, Vitaly Cheptsov,
Kinney, Michael D
Reviewed-by: Michael D Kinney >michael.d.kinney@intel.com>
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Thursday, March 2, 2023 10:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>; Michael Roth
> <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski
> <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Vitaly Cheptsov <vit9696@protonmail.com>
> Subject: [PATCH 4/5] MdeModulePkg: Consume new alignment-related macros
>
> From: Marvin Häuser <mhaeuser@posteo.de>
>
> 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/AhciPei.h | 3 +-
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 --
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 --
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 --
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 --
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> .../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 ++--
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +++----
> .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +--
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 ++--
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++++++++----------
> 14 files changed, 36 insertions(+), 47 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 71d34c962ad1..e2e4ba43e7c4 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -146,8 +146,7 @@ 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)
> +#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 7937886614e1..016fc6890ae9 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
> @@ -148,8 +148,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 47346e911d47..6bc345f7e777 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 ed384ad52182..5a25b55c4952 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h
> @@ -38,8 +38,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 1adb382aa8c3..ed4776f548e0 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 0ec37e56652b..bc1139da6e3b 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 6dc6730ab095..f3768e79528e 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
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> index d93fa78c81f3..0f0198d3085b 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c
> @@ -2126,7 +2126,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),
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> index 0c49059a00d5..cd55272c96cd 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPeiPassThru.c
> @@ -194,15 +194,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 324abadd02dd..50406fe0270d 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1299,15 +1299,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;
> }
>
> @@ -2039,15 +2039,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 18aa4f9bb666..a77852bae054 100644
> --- a/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> +++ b/MdeModulePkg/Bus/Ata/AtaBusDxe/AtaPassThruExecute.c
> @@ -1040,7 +1040,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 faf4ae332e46..873581d817ce 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -2029,7 +2029,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;
> @@ -2249,7 +2249,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 392a295caf04..880e7d85114e 100644
> --- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> +++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
> @@ -171,15 +171,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 28f108c44873..3221f95a739f 100644
> --- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> +++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
> @@ -2015,7 +2015,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,
> @@ -2074,7 +2074,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,
> @@ -2097,7 +2097,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,
> @@ -3158,7 +3158,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,
> @@ -4733,7 +4733,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 {
> //
> @@ -4795,7 +4795,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 {
> //
> @@ -4857,7 +4857,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 {
> //
> @@ -4922,7 +4922,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++) {
> @@ -4985,7 +4985,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 {
> //
> @@ -5029,7 +5029,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);
> }
>
> @@ -5068,7 +5068,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);
> }
>
> @@ -5105,7 +5105,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 {
> //
> @@ -5147,7 +5147,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);
> }
>
> @@ -5184,7 +5184,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);
> }
>
> @@ -5247,7 +5247,7 @@ VmReadMem16 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT16))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT16))) {
> return *(UINT16 *)Addr;
> }
>
> @@ -5281,7 +5281,7 @@ VmReadMem32 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT32))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT32))) {
> return *(UINT32 *)Addr;
> }
>
> @@ -5319,7 +5319,7 @@ VmReadMem64 (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINT64))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINT64))) {
> return *(UINT64 *)Addr;
> }
>
> @@ -5388,7 +5388,7 @@ VmReadMemN (
> //
> // Read direct if aligned
> //
> - if (ADDRESS_IS_ALIGNED_ (Addr, sizeof (UINTN))) {
> + if (ADDRESS_IS_ALIGNED (Addr, sizeof (UINTN))) {
> return *(UINTN *)Addr;
> }
>
> --
> 2.39.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
` (3 preceding siblings ...)
2023-03-03 6:51 ` [PATCH 4/5] MdeModulePkg: Consume new " Gerd Hoffmann
@ 2023-03-03 6:51 ` Gerd Hoffmann
2023-03-21 21:40 ` Michael D Kinney
2023-03-03 15:12 ` [PATCH 0/5] MdePkg/Base.h: Introduce various " Lendacky, Thomas
2023-03-20 10:04 ` Gerd Hoffmann
6 siblings, 1 reply; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-03 6:51 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Gerd Hoffmann, Jian J Wang, Jiewen Yao,
Marvin Häuser, James Bottomley, Michael Roth, Hao A Wu,
Michael D Kinney, Oliver Steffen, Min Xu, Liming Gao, Ray Ni,
Tom Lendacky, Erdem Aktas, Zhiguang Liu, Pawel Polawski,
Jordan Justen
This patch substitutes the macros that were renamed in the second
patch with the new, shared alignment macros.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 ++----
.../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 3 +--
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 71a1eaaf0a1d..9b0d0e92b6f7 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -44,8 +44,6 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
-#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
-
STATIC
EFI_STATUS
EFIAPI
@@ -60,8 +58,8 @@ AmdSevMemoryAccept (
// multiple of SIZE_4KB. Use an assert instead of returning an erros since
// this is an EDK2-internal protocol.
//
- ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
- ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
+ ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED (Size, SIZE_4KB));
ASSERT (Size != 0);
MemEncryptSevSnpPreValidateSystemRam (
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
index f35bba5deb46..7a8878b1a9c2 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -20,7 +20,6 @@
#include "SnpPageStateChange.h"
-#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
#define PAGES_PER_LARGE_ENTRY 512
STATIC
@@ -150,7 +149,7 @@ BuildPageStateBuffer (
//
// Is this a 2MB aligned page? Check if we can use the Large RMP entry.
//
- if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
+ if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
((EndAddress - BaseAddress) >= SIZE_2MB))
{
RmpPageSize = PvalidatePageSize2MB;
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
2023-03-03 6:51 ` [PATCH 5/5] OvmfPkg: " Gerd Hoffmann
@ 2023-03-21 21:40 ` Michael D Kinney
2023-03-21 23:30 ` Yao, Jiewen
0 siblings, 1 reply; 22+ messages in thread
From: Michael D Kinney @ 2023-03-21 21:40 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Yao, Jiewen, Marvin Häuser,
James Bottomley, Michael Roth, Wu, Hao A, Oliver Steffen,
Xu, Min M, Gao, Liming, Ni, Ray, Tom Lendacky, Aktas, Erdem,
Liu, Zhiguang, Pawel Polawski, Justen, Jordan L,
Kinney, Michael D
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Thursday, March 2, 2023 10:51 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>; Michael Roth
> <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel Polawski
> <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
>
> This patch substitutes the macros that were renamed in the second
> patch with the new, shared alignment macros.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 ++----
> .../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 3 +--
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 71a1eaaf0a1d..9b0d0e92b6f7 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -44,8 +44,6 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
>
> STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
>
> -#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
> -
> STATIC
> EFI_STATUS
> EFIAPI
> @@ -60,8 +58,8 @@ AmdSevMemoryAccept (
> // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> // this is an EDK2-internal protocol.
> //
> - ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
> - ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
> + ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> + ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> ASSERT (Size != 0);
>
> MemEncryptSevSnpPreValidateSystemRam (
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> index f35bba5deb46..7a8878b1a9c2 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
> @@ -20,7 +20,6 @@
>
> #include "SnpPageStateChange.h"
>
> -#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
> #define PAGES_PER_LARGE_ENTRY 512
>
> STATIC
> @@ -150,7 +149,7 @@ BuildPageStateBuffer (
> //
> // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
> //
> - if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
> + if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
> ((EndAddress - BaseAddress) >= SIZE_2MB))
> {
> RmpPageSize = PvalidatePageSize2MB;
> --
> 2.39.2
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
2023-03-21 21:40 ` Michael D Kinney
@ 2023-03-21 23:30 ` Yao, Jiewen
0 siblings, 0 replies; 22+ messages in thread
From: Yao, Jiewen @ 2023-03-21 23:30 UTC (permalink / raw)
To: Kinney, Michael D, Gerd Hoffmann, devel@edk2.groups.io
Cc: Ard Biesheuvel, Wang, Jian J, Marvin Häuser, James Bottomley,
Michael Roth, Wu, Hao A, Oliver Steffen, Xu, Min M, Gao, Liming,
Ni, Ray, Tom Lendacky, Aktas, Erdem, Liu, Zhiguang,
Pawel Polawski, Justen, Jordan L
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, March 22, 2023 5:41 AM
> To: Gerd Hoffmann <kraxel@redhat.com>; devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Wang, Jian J
> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Marvin
> Häuser <mhaeuser@posteo.de>; James Bottomley <jejb@linux.ibm.com>;
> Michael Roth <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Oliver Steffen <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel
> Polawski <ppolawsk@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
>
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
>
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Thursday, March 2, 2023 10:51 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Gerd Hoffmann
> <kraxel@redhat.com>; Wang, Jian J <jian.j.wang@intel.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Marvin Häuser <mhaeuser@posteo.de>;
> James Bottomley <jejb@linux.ibm.com>; Michael Roth
> > <michael.roth@amd.com>; Wu, Hao A <hao.a.wu@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Oliver Steffen
> > <osteffen@redhat.com>; Xu, Min M <min.m.xu@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Tom
> > Lendacky <thomas.lendacky@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>; Liu, Zhiguang <zhiguang.liu@intel.com>; Pawel
> Polawski
> > <ppolawsk@redhat.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> > Subject: [PATCH 5/5] OvmfPkg: Consume new alignment-related macros
> >
> > This patch substitutes the macros that were renamed in the second
> > patch with the new, shared alignment macros.
> >
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 6 ++----
> > .../BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 3 +--
> > 2 files changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index 71a1eaaf0a1d..9b0d0e92b6f7 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -44,8 +44,6 @@ STATIC BOOLEAN mAcceptAllMemoryAtEBS = TRUE;
> >
> > STATIC EFI_EVENT mAcceptAllMemoryEvent = NULL;
> >
> > -#define IS_ALIGNED_(x, y) ((((x) & ((y) - 1)) == 0))
> > -
> > STATIC
> > EFI_STATUS
> > EFIAPI
> > @@ -60,8 +58,8 @@ AmdSevMemoryAccept (
> > // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> > // this is an EDK2-internal protocol.
> > //
> > - ASSERT (IS_ALIGNED_ (StartAddress, SIZE_4KB));
> > - ASSERT (IS_ALIGNED_ (Size, SIZE_4KB));
> > + ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> > + ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> > ASSERT (Size != 0);
> >
> > MemEncryptSevSnpPreValidateSystemRam (
> > diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> >
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > index f35bba5deb46..7a8878b1a9c2 100644
> > ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeIntern
> al.c
> > @@ -20,7 +20,6 @@
> >
> > #include "SnpPageStateChange.h"
> >
> > -#define IS_ALIGNED_(x, y) ((((x) & (y - 1)) == 0))
> > #define PAGES_PER_LARGE_ENTRY 512
> >
> > STATIC
> > @@ -150,7 +149,7 @@ BuildPageStateBuffer (
> > //
> > // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
> > //
> > - if (UseLargeEntry && IS_ALIGNED_ (BaseAddress, SIZE_2MB) &&
> > + if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
> > ((EndAddress - BaseAddress) >= SIZE_2MB))
> > {
> > RmpPageSize = PvalidatePageSize2MB;
> > --
> > 2.39.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
` (4 preceding siblings ...)
2023-03-03 6:51 ` [PATCH 5/5] OvmfPkg: " Gerd Hoffmann
@ 2023-03-03 15:12 ` Lendacky, Thomas
2023-03-20 10:04 ` Gerd Hoffmann
6 siblings, 0 replies; 22+ messages in thread
From: Lendacky, Thomas @ 2023-03-03 15:12 UTC (permalink / raw)
To: Gerd Hoffmann, devel
Cc: Ard Biesheuvel, Jian J Wang, Jiewen Yao, Marvin Häuser,
James Bottomley, Michael Roth, Hao A Wu, Michael D Kinney,
Oliver Steffen, Min Xu, Liming Gao, Ray Ni, Erdem Aktas,
Zhiguang Liu, Pawel Polawski, Jordan Justen
On 3/3/23 00:51, Gerd Hoffmann wrote:
>
>
> Gerd Hoffmann (2):
> OvmfPkg: Rename IS_ALIGNED macros to avoid name collisions
> OvmfPkg: Consume new alignment-related macros
>
> Marvin Häuser (3):
> MdeModulePkg: Rename IS_ALIGNED macros to avoid name collisions
> MdePkg/Base.h: Introduce various alignment-related macros
> MdeModulePkg: Consume new alignment-related macros
For the OVMF related pieces:
Acked-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h | 1 -
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.h | 2 -
> MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBus.h | 1 -
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 2 -
> .../Bus/Ufs/UfsBlockIoPei/UfsBlockIoPei.h | 2 -
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 2 -
> MdeModulePkg/Universal/EbcDxe/EbcExecute.h | 3 +-
> MdePkg/Include/Base.h | 95 ++++++++++++++++++-
> MdeModulePkg/Bus/Ata/AhciPei/AhciMode.c | 2 +-
> .../Bus/Ata/AhciPei/AhciPeiPassThru.c | 6 +-
> .../Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 12 +--
> .../Bus/Ata/AtaBusDxe/AtaPassThruExecute.c | 2 +-
> MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 4 +-
> .../Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 6 +-
> MdeModulePkg/Universal/EbcDxe/EbcExecute.c | 36 +++----
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 2 -
> .../X64/SnpPageStateChangeInternal.c | 1 -
> 17 files changed, 129 insertions(+), 50 deletions(-)
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros
2023-03-03 6:51 [PATCH 0/5] MdePkg/Base.h: Introduce various alignment-related macros Gerd Hoffmann
` (5 preceding siblings ...)
2023-03-03 15:12 ` [PATCH 0/5] MdePkg/Base.h: Introduce various " Lendacky, Thomas
@ 2023-03-20 10:04 ` Gerd Hoffmann
6 siblings, 0 replies; 22+ messages in thread
From: Gerd Hoffmann @ 2023-03-20 10:04 UTC (permalink / raw)
To: devel
Cc: Ard Biesheuvel, Jian J Wang, Jiewen Yao, Marvin Häuser,
James Bottomley, Michael Roth, Hao A Wu, Michael D Kinney,
Oliver Steffen, Min Xu, Liming Gao, Ray Ni, Tom Lendacky,
Erdem Aktas, Zhiguang Liu, Pawel Polawski, Jordan Justen
On Fri, Mar 03, 2023 at 07:51:10AM +0100, Gerd Hoffmann wrote:
Ping. Any comments on this?
The interesting update actually adding the macros is in patch #3.
thanks & take care,
Gerd
^ permalink raw reply [flat|nested] 22+ messages in thread