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

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

[-- Attachment #1: Type: text/plain, Size: 5748 bytes --]

Hi Etienne,

Thank you for this patch.

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

Regards,

Sami Mujawar


On 17/05/2021 08:40 AM, 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>
> ---
> No change since v2
> No change since v1
> ---
>   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
>                );


[-- Attachment #2: Type: text/html, Size: 6778 bytes --]