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

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Etienne,

Thank you for this patch.

A space should not be there between a unary operator add its operand.
See
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/52_spacing#5-2-2-3-do-not-put-space-between-unary-operators-and-their-object

However, the existing code does not follow this anyways.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 04/05/2021 04:20 PM, Etienne Carriere wrote:
> 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
>                );

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.