[edk2-devel] [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

Yao, Jiewen jiewen.yao at intel.com
Mon Dec 28 06:37:50 UTC 2020


Sounds good. Thank you!

From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Kun Qin
Sent: Monday, December 28, 2020 2:36 PM
To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io
Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>; Sami Mujawar <sami.mujawar at arm.com>; Supreeth Venkatesh <supreeth.venkatesh at arm.com>
Subject: Re: [edk2-devel] [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

Understood, not including this library for x86 will fail CI build on some new drivers in this patch series, so we will need to give it a full fix. I will add the fix at least for x86 in patch v2.

Thanks,
Kun

From: Yao, Jiewen<mailto:jiewen.yao at intel.com>
Sent: Sunday, December 27, 2020 22:24
To: Kun Qin<mailto:kun.q at outlook.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Ard Biesheuvel<mailto:ard.biesheuvel at arm.com>; Sami Mujawar<mailto:sami.mujawar at arm.com>; Supreeth Venkatesh<mailto:supreeth.venkatesh at arm.com>
Subject: RE: [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

I am not familiar with AArch64. So I will let ARM people comment it.

For X86, I think it is security hole, if we do not fill mMmMemLibInternalMmramRanges.

A partial fix with known security vulnerability is not the best idea.

I prefer to we give a full fix, or no fix it in V2.

Thank you
Yao Jiewen


From: Kun Qin <kun.q at outlook.com<mailto:kun.q at outlook.com>>
Sent: Monday, December 28, 2020 12:15 PM
To: Yao, Jiewen <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>; Sami Mujawar <sami.mujawar at arm.com<mailto:sami.mujawar at arm.com>>; Supreeth Venkatesh <supreeth.venkatesh at arm.com<mailto:supreeth.venkatesh at arm.com>>
Subject: RE: [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

Hi Jiewen,

I did not fill these 2 variables for this patch because I thought to follow up fix for this issue separately because the AARCH64 instance does not fill it either and I meant for this patch to solely extends the coverage to x64 (and IA32). But please let me know if you think otherwise, I can add the x64 fix in this patch as well. But I do not have fix for AARCH64 since I am not familiar with how it should work.

Thanks,
Kun

From: Yao, Jiewen<mailto:jiewen.yao at intel.com>
Sent: Sunday, December 27, 2020 16:18
To: Kun Qin<mailto:kun.q at outlook.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Ard Biesheuvel<mailto:ard.biesheuvel at arm.com>; Sami Mujawar<mailto:sami.mujawar at arm.com>; Supreeth Venkatesh<mailto:supreeth.venkatesh at arm.com>
Subject: RE: [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib: Extends support for X64 architecture

May I know where is the code to fill below?
EFI_MMRAM_DESCRIPTOR *mMmMemLibInternalMmramRanges;
UINTN                mMmMemLibInternalMmramCount;



> -----Original Message-----
> From: Kun Qin <kun.q at outlook.com<mailto:kun.q at outlook.com>>
> Sent: Saturday, December 19, 2020 2:50 AM
> To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>; Sami Mujawar
> <sami.mujawar at arm.com<mailto:sami.mujawar at arm.com>>; Yao, Jiewen <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>; Supreeth
> Venkatesh <supreeth.venkatesh at arm.com<mailto:supreeth.venkatesh at arm.com>>
> Subject: [PATCH v1 04/15] StandaloneMmPkg: StandaloneMmMemLib:
> Extends support for X64 architecture
>
> This change extends StandaloneMmMemLib library to support X64
> architecture. The implementation is ported from
> MdePkg/Library/SmmMemLib.
>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>
> Cc: Sami Mujawar <sami.mujawar at arm.com<mailto:sami.mujawar at arm.com>>
> Cc: Jiewen Yao <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>
> Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com<mailto:supreeth.venkatesh at arm.com>>
>
> Signed-off-by: Kun Qin <kun.q at outlook.com<mailto:kun.q at outlook.com>>
> ---
>
> StandaloneMmPkg/Library/StandaloneMmMemLib/X64/StandaloneMmMe
> mLibInternal.c | 67 ++++++++++++++++++++
>
> StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib
> .inf           |  6 +-
>  2 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmMemLib/X64/StandaloneMm
> MemLibInternal.c
> b/StandaloneMmPkg/Library/StandaloneMmMemLib/X64/StandaloneMm
> MemLibInternal.c
> new file mode 100644
> index 000000000000..c5e21c583f44
> --- /dev/null
> +++
> b/StandaloneMmPkg/Library/StandaloneMmMemLib/X64/StandaloneMm
> MemLibInternal.c
> @@ -0,0 +1,67 @@
> +/** @file
> +  Internal ARCH Specific file of MM memory check library.
> +
> +  MM memory check library implementation. This library consumes
> MM_ACCESS_PROTOCOL
> +  to get MMRAM information. In order to use this library instance, the
> platform should produce
> +  all MMRAM range via MM_ACCESS_PROTOCOL, including the range for
> firmware (like MM Core
> +  and MM driver) and/or specific dedicated hardware.
> +
> +  Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +//
> +// Maximum support address used to check input buffer
> +//
> +extern EFI_PHYSICAL_ADDRESS
> mMmMemLibInternalMaximumSupportAddress;
> +
> +/**
> +  Calculate and save the maximum support address.
> +
> +**/
> +VOID
> +MmMemLibInternalCalculateMaximumSupportAddress (
> +  VOID
> +  )
> +{
> +  VOID         *Hob;
> +  UINT32       RegEax;
> +  UINT8        PhysicalAddressBits;
> +
> +  //
> +  // Get physical address bits supported.
> +  //
> +  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> +  if (Hob != NULL) {
> +    PhysicalAddressBits = ((EFI_HOB_CPU *) Hob)->SizeOfMemorySpace;
> +  } else {
> +    AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> +    if (RegEax >= 0x80000008) {
> +      AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> +      PhysicalAddressBits = (UINT8) RegEax;
> +    } else {
> +      PhysicalAddressBits = 36;
> +    }
> +  }
> +  //
> +  // IA-32e paging translates 48-bit linear addresses to 52-bit physical
> addresses.
> +  //
> +  ASSERT (PhysicalAddressBits <= 52);
> +  if (PhysicalAddressBits > 48) {
> +    PhysicalAddressBits = 48;
> +  }
> +
> +  //
> +  // Save the maximum support address in one global variable
> +  //
> +  mMmMemLibInternalMaximumSupportAddress =
> (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, PhysicalAddressBits) - 1);
> +  DEBUG ((DEBUG_INFO, "mMmMemLibInternalMaximumSupportAddress
> = 0x%lx\n", mMmMemLibInternalMaximumSupportAddress));
> +}
> +
> +
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem
> Lib.inf
> b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem
> Lib.inf
> index 49da02e54e6d..65ad0a48905c 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem
> Lib.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMem
> Lib.inf
> @@ -26,12 +26,15 @@ [Defines]
>  #
>  # The following information is for reference only and not required by the
> build tools.
>  #
> -#  VALID_ARCHITECTURES           = AARCH64
> +#  VALID_ARCHITECTURES           = X64 AARCH64
>  #
>
>  [Sources.Common]
>    StandaloneMmMemLib.c
>
> +[Sources.X64]
> +  X64/StandaloneMmMemLibInternal.c
> +
>  [Sources.AARCH64]
>    AArch64/StandaloneMmMemLibInternal.c
>
> @@ -42,3 +45,4 @@ [Packages]
>  [LibraryClasses]
>    BaseMemoryLib
>    DebugLib
> +  HobLib
> --
> 2.28.0.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69476): https://edk2.groups.io/g/devel/message/69476
Mute This Topic: https://groups.io/mt/79068287/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20201228/59e32ab6/attachment.htm>


More information about the edk2-devel-archive mailing list