[PATCH 4/5] StandaloneMmPkg: fix pointer/int casts against 32bit architectures


Etienne Carriere
 

Use intermediate (UINTN) cast when casting int from/to pointer. This
is needed as UINT64 values cast from/to 32bit pointer for 32bit
architectures.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c | 8 ++++----
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c | 14 +++++++-------
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 6884095c49..d4590bcd19 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -164,8 +164,8 @@ StandaloneMmCpuInitialize (

// Share the entry point of the CPU driver
DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
- (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
- (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+ (UINTN) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
+ (UINTN) PiMmStandaloneArmTfCpuDriverEntry));
*(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = PiMmStandaloneArmTfCpuDriverEntry;

// Find the descriptor that contains the whereabouts of the buffer for
@@ -180,8 +180,8 @@ StandaloneMmCpuInitialize (
return Status;
}

- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINT64) NsCommBufMmramRange->PhysicalStart));
- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINT64) NsCommBufMmramRange->PhysicalSize));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINTN) NsCommBufMmramRange->PhysicalStart));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINTN) NsCommBufMmramRange->PhysicalSize));

CopyMem (&mNsCommBuffer, NsCommBufMmramRange, sizeof(EFI_MMRAM_DESCRIPTOR));
DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n", mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize));
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
index e8fb96bd6e..4d4cf3d5ff 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
@@ -72,14 +72,14 @@ CreateHobListFromBootInfo (

// Create a hoblist with a PHIT and EOH
HobStart = HobConstructor (
- (VOID *) PayloadBootInfo->SpMemBase,
+ (VOID *) (UINTN) PayloadBootInfo->SpMemBase,
(UINTN) PayloadBootInfo->SpMemLimit - PayloadBootInfo->SpMemBase,
- (VOID *) PayloadBootInfo->SpHeapBase,
- (VOID *) (PayloadBootInfo->SpHeapBase + PayloadBootInfo->SpHeapSize)
+ (VOID *) (UINTN) PayloadBootInfo->SpHeapBase,
+ (VOID *) (UINTN) (PayloadBootInfo->SpHeapBase + PayloadBootInfo->SpHeapSize)
);

// Check that the Hoblist starts at the bottom of the Heap
- ASSERT (HobStart == (VOID *) PayloadBootInfo->SpHeapBase);
+ ASSERT (HobStart == (VOID *) (UINTN) PayloadBootInfo->SpHeapBase);

// Build a Boot Firmware Volume HOB
BuildFvHob (PayloadBootInfo->SpImageBase, PayloadBootInfo->SpImageSize);
@@ -190,9 +190,9 @@ CreateHobListFromBootInfo (
MmramRanges[3].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
- MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom - (EFI_PHYSICAL_ADDRESS) HobStart;
+ MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom - (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
MmramRanges[4].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 6c50f470aa..b445d6942e 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -328,7 +328,7 @@ _ModuleEntryPoint (

// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
- (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
+ (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PayloadBootInfo->SpImageBase,
&TeData,
&TeDataSize
);
--
2.17.1


Yao, Jiewen
 

Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

Need ARM expert to comment if it is OK to refer AArch64 for ARM?

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Etienne
Carriere
Sent: Tuesday, May 4, 2021 11:21 PM
To: devel@edk2.groups.io
Cc: Achin Gupta <achin.gupta@arm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Leif
Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>;
Sughosh Ganu <sughosh.ganu@linaro.org>; Etienne Carriere
<etienne.carriere@linaro.org>
Subject: [edk2-devel] [PATCH 4/5] StandaloneMmPkg: fix pointer/int casts
against 32bit architectures

Use intermediate (UINTN) cast when casting int from/to pointer. This
is needed as UINT64 values cast from/to 32bit pointer for 32bit
architectures.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
| 8 ++++----

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobL
ist.c | 14 +++++++-------

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalone
MmCoreEntryPoint.c | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 6884095c49..d4590bcd19 100644
---
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -164,8 +164,8 @@ StandaloneMmCpuInitialize (

// Share the entry point of the CPU driver
DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
- (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
- (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+ (UINTN) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
+ (UINTN) PiMmStandaloneArmTfCpuDriverEntry));
*(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) =
PiMmStandaloneArmTfCpuDriverEntry;

// Find the descriptor that contains the whereabouts of the buffer for
@@ -180,8 +180,8 @@ StandaloneMmCpuInitialize (
return Status;
}

- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINT64)
NsCommBufMmramRange->PhysicalStart));
- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINT64)
NsCommBufMmramRange->PhysicalSize));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINTN)
NsCommBufMmramRange->PhysicalStart));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINTN)
NsCommBufMmramRange->PhysicalSize));

CopyMem (&mNsCommBuffer, NsCommBufMmramRange,
sizeof(EFI_MMRAM_DESCRIPTOR));
DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n",
mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize));
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
index e8fb96bd6e..4d4cf3d5ff 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
@@ -72,14 +72,14 @@ CreateHobListFromBootInfo (

// Create a hoblist with a PHIT and EOH
HobStart = HobConstructor (
- (VOID *) PayloadBootInfo->SpMemBase,
+ (VOID *) (UINTN) PayloadBootInfo->SpMemBase,
(UINTN) PayloadBootInfo->SpMemLimit - PayloadBootInfo-
SpMemBase,
- (VOID *) PayloadBootInfo->SpHeapBase,
- (VOID *) (PayloadBootInfo->SpHeapBase + PayloadBootInfo-
SpHeapSize)
+ (VOID *) (UINTN) PayloadBootInfo->SpHeapBase,
+ (VOID *) (UINTN) (PayloadBootInfo->SpHeapBase + PayloadBootInfo-
SpHeapSize)
);

// Check that the Hoblist starts at the bottom of the Heap
- ASSERT (HobStart == (VOID *) PayloadBootInfo->SpHeapBase);
+ ASSERT (HobStart == (VOID *) (UINTN) PayloadBootInfo->SpHeapBase);

// Build a Boot Firmware Volume HOB
BuildFvHob (PayloadBootInfo->SpImageBase, PayloadBootInfo->SpImageSize);
@@ -190,9 +190,9 @@ CreateHobListFromBootInfo (
MmramRanges[3].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
- MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom -
(EFI_PHYSICAL_ADDRESS) HobStart;
+ MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom -
(EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
MmramRanges[4].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
index 6c50f470aa..b445d6942e 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
@@ -328,7 +328,7 @@ _ModuleEntryPoint (

// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
- (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
+ (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PayloadBootInfo-
SpImageBase,
&TeData,
&TeDataSize
);
--
2.17.1





Ard Biesheuvel
 

On Wed, 5 May 2021 at 04:10, Yao, Jiewen <jiewen.yao@intel.com> wrote:

Acked-by: Jiewen Yao <Jiewen.yao@intel.com>

Need ARM expert to comment if it is OK to refer AArch64 for ARM?
This looks fine to me.



-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Etienne
Carriere
Sent: Tuesday, May 4, 2021 11:21 PM
To: devel@edk2.groups.io
Cc: Achin Gupta <achin.gupta@arm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Leif
Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>;
Sughosh Ganu <sughosh.ganu@linaro.org>; Etienne Carriere
<etienne.carriere@linaro.org>
Subject: [edk2-devel] [PATCH 4/5] StandaloneMmPkg: fix pointer/int casts
against 32bit architectures

Use intermediate (UINTN) cast when casting int from/to pointer. This
is needed as UINT64 values cast from/to 32bit pointer for 32bit
architectures.

Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
| 8 ++++----

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobL
ist.c | 14 +++++++-------

StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalone
MmCoreEntryPoint.c | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)

diff --git
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 6884095c49..d4590bcd19 100644
---
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -164,8 +164,8 @@ StandaloneMmCpuInitialize (

// Share the entry point of the CPU driver
DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
- (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
- (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+ (UINTN) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
+ (UINTN) PiMmStandaloneArmTfCpuDriverEntry));
*(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) =
PiMmStandaloneArmTfCpuDriverEntry;

// Find the descriptor that contains the whereabouts of the buffer for
@@ -180,8 +180,8 @@ StandaloneMmCpuInitialize (
return Status;
}

- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINT64)
NsCommBufMmramRange->PhysicalStart));
- DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINT64)
NsCommBufMmramRange->PhysicalSize));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalStart - 0x%lx\n", (UINTN)
NsCommBufMmramRange->PhysicalStart));
+ DEBUG ((DEBUG_INFO, "mNsCommBuffer.PhysicalSize - 0x%lx\n", (UINTN)
NsCommBufMmramRange->PhysicalSize));

CopyMem (&mNsCommBuffer, NsCommBufMmramRange,
sizeof(EFI_MMRAM_DESCRIPTOR));
DEBUG ((DEBUG_INFO, "mNsCommBuffer: 0x%016lx - 0x%lx\n",
mNsCommBuffer.CpuStart, mNsCommBuffer.PhysicalSize));
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
index e8fb96bd6e..4d4cf3d5ff 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHo
bList.c
@@ -72,14 +72,14 @@ CreateHobListFromBootInfo (

// Create a hoblist with a PHIT and EOH
HobStart = HobConstructor (
- (VOID *) PayloadBootInfo->SpMemBase,
+ (VOID *) (UINTN) PayloadBootInfo->SpMemBase,
(UINTN) PayloadBootInfo->SpMemLimit - PayloadBootInfo-
SpMemBase,
- (VOID *) PayloadBootInfo->SpHeapBase,
- (VOID *) (PayloadBootInfo->SpHeapBase + PayloadBootInfo-
SpHeapSize)
+ (VOID *) (UINTN) PayloadBootInfo->SpHeapBase,
+ (VOID *) (UINTN) (PayloadBootInfo->SpHeapBase + PayloadBootInfo-
SpHeapSize)
);

// Check that the Hoblist starts at the bottom of the Heap
- ASSERT (HobStart == (VOID *) PayloadBootInfo->SpHeapBase);
+ ASSERT (HobStart == (VOID *) (UINTN) PayloadBootInfo->SpHeapBase);

// Build a Boot Firmware Volume HOB
BuildFvHob (PayloadBootInfo->SpImageBase, PayloadBootInfo->SpImageSize);
@@ -190,9 +190,9 @@ CreateHobListFromBootInfo (
MmramRanges[3].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
- MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) HobStart;
- MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom -
(EFI_PHYSICAL_ADDRESS) HobStart;
+ MmramRanges[4].PhysicalStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].CpuStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+ MmramRanges[4].PhysicalSize = HobStart->EfiFreeMemoryBottom -
(EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
MmramRanges[4].RegionState = EFI_CACHEABLE | EFI_ALLOCATED;

// Base and size of heap memory shared by all cpus
diff --git
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
index 6c50f470aa..b445d6942e 100644
---
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
+++
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
eMmCoreEntryPoint.c
@@ -328,7 +328,7 @@ _ModuleEntryPoint (

// Locate PE/COFF File information for the Standalone MM core module
Status = LocateStandaloneMmCorePeCoffData (
- (EFI_FIRMWARE_VOLUME_HEADER *) PayloadBootInfo->SpImageBase,
+ (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) PayloadBootInfo-
SpImageBase,
&TeData,
&TeDataSize
);
--
2.17.1