From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: "Carsey, Jaben" <jaben.carsey@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Fan, Jeff" <jeff.fan@intel.com>, "Gao, Liming" <liming.gao@intel.com>
Subject: Re: [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.
Date: Wed, 26 Apr 2017 01:02:41 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A930CF6@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CB6E33457884FA40993F35157061515C54C0D787@FMSMSX103.amr.corp.intel.com>
Thanks.
But this MemSpaceMap is allocated by DxeCore services. It is UEFI memory instead of SMRAM, I think using gBS->FreePool is more proper.
Thank you
Yao Jiewen
From: Carsey, Jaben
Sent: Monday, April 24, 2017 10:59 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Fan, Jeff <jeff.fan@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.
Slight cleanup suggestion.
Since you are already linking MemoryAllocationLib, you could just call FreePool in that library instead of using UefiBootServicesTableLib and using gBS->FreePool (since you have no other use of gBS that I see).
-Jaben
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Monday, April 24, 2017 7:15 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Subject: [edk2] [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.
>
> The sample instance check if IO resource is valid
> one defined in GCD.
> A platform may choose add more check to exclude some
> other IO resource.
>
> Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
> MdePkg/Library/SmmIoLib/SmmIoLib.c | 330 ++++++++++++++++++++
> MdePkg/Library/SmmIoLib/SmmIoLib.inf | 53 ++++
> MdePkg/Library/SmmIoLib/SmmIoLib.uni | 23 ++
> 3 files changed, 406 insertions(+)
>
> diff --git a/MdePkg/Library/SmmIoLib/SmmIoLib.c
> b/MdePkg/Library/SmmIoLib/SmmIoLib.c
> new file mode 100644
> index 0000000..181abb8
> --- /dev/null
> +++ b/MdePkg/Library/SmmIoLib/SmmIoLib.c
> @@ -0,0 +1,330 @@
> +/** @file
> + Instance of SMM IO check library.
> +
> + SMM IO check library library implementation. This library consumes GCD to
> collect all valid
> + IO space defined by a platform.
> + A platform may have its own SmmIoLib instance to exclude more IO space.
> +
> + Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD
> License
> + which accompanies this distribution. The full text of the license may be
> found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +#include <PiSmm.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/SmmServicesTableLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Protocol/SmmReadyToLock.h>
> +#include <Protocol/SmmEndOfDxe.h>
> +
> +EFI_GCD_MEMORY_SPACE_DESCRIPTOR *mSmmIoLibGcdMemSpace =
> NULL;
> +UINTN mSmmIoLibGcdMemNumberOfDesc = 0;
> +
> +EFI_PHYSICAL_ADDRESS
> mSmmIoLibInternalMaximumSupportMemAddress = 0;
> +
> +VOID *mSmmIoLibRegistrationEndOfDxe;
> +VOID *mSmmIoLibRegistrationReadyToLock;
> +
> +BOOLEAN mSmmIoLibReadyToLock = FALSE;
> +
> +/**
> + Calculate and save the maximum support address.
> +
> +**/
> +VOID
> +SmmIoLibInternalCalculateMaximumSupportAddress (
> + VOID
> + )
> +{
> + VOID *Hob;
> + UINT32 RegEax;
> + UINT8 MemPhysicalAddressBits;
> +
> + //
> + // Get physical address bits supported.
> + //
> + Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> + if (Hob != NULL) {
> + MemPhysicalAddressBits = ((EFI_HOB_CPU *) Hob)-
> >SizeOfMemorySpace;
> + } else {
> + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> + if (RegEax >= 0x80000008) {
> + AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> + MemPhysicalAddressBits = (UINT8) RegEax;
> + } else {
> + MemPhysicalAddressBits = 36;
> + }
> + }
> + //
> + // IA-32e paging translates 48-bit linear addresses to 52-bit physical
> addresses.
> + //
> + ASSERT (MemPhysicalAddressBits <= 52);
> + if (MemPhysicalAddressBits > 48) {
> + MemPhysicalAddressBits = 48;
> + }
> +
> + //
> + // Save the maximum support address in one global variable
> + //
> + mSmmIoLibInternalMaximumSupportMemAddress =
> (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, MemPhysicalAddressBits) -
> 1);
> + DEBUG ((DEBUG_INFO,
> "mSmmIoLibInternalMaximumSupportMemAddress = 0x%lx\n",
> mSmmIoLibInternalMaximumSupportMemAddress));
> +}
> +
> +/**
> + This function check if the MMIO resource is valid per processor
> architecture and
> + valid per platform design.
> +
> + @param BaseAddress The MMIO start address to be checked.
> + @param Length The MMIO length to be checked.
> + @param Owner A GUID representing the owner of the resource.
> + This GUID may be used by producer to correlate the device
> ownership of the resource.
> + NULL means no specific owner.
> +
> + @retval TRUE This MMIO resource is valid per processor architecture and
> valid per platform design.
> + @retval FALSE This MMIO resource is not valid per processor architecture
> or valid per platform design.
> +**/
> +BOOLEAN
> +EFIAPI
> +SmmIsMmioValid (
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN EFI_GUID *Owner OPTIONAL
> + )
> +{
> + UINTN Index;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc;
> + BOOLEAN InValidRegion;
> +
> + //
> + // Check override.
> + // NOTE: (B:0->L:4G) is invalid for IA32, but (B:1->L:4G-1)/(B:4G-1->L:1) is
> valid.
> + //
> + if ((Length > mSmmIoLibInternalMaximumSupportMemAddress) ||
> + (BaseAddress > mSmmIoLibInternalMaximumSupportMemAddress) ||
> + ((Length != 0) && (BaseAddress >
> (mSmmIoLibInternalMaximumSupportMemAddress - (Length - 1)))) ) {
> + //
> + // Overflow happen
> + //
> + DEBUG ((
> + DEBUG_ERROR,
> + "SmmIsMmioValid: Overflow: BaseAddress (0x%lx) - Length (0x%lx),
> MaximumSupportMemAddress (0x%lx)\n",
> + BaseAddress,
> + Length,
> + mSmmIoLibInternalMaximumSupportMemAddress
> + ));
> + return FALSE;
> + }
> +
> + //
> + // Check override for valid MMIO region
> + //
> + if (mSmmIoLibReadyToLock) {
> + InValidRegion = FALSE;
> + for (Index = 0; Index < mSmmIoLibGcdMemNumberOfDesc; Index ++) {
> + Desc = &mSmmIoLibGcdMemSpace[Index];
> + if ((Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo)
> &&
> + (BaseAddress >= Desc->BaseAddress) &&
> + ((BaseAddress + Length) <= (Desc->BaseAddress + Desc->Length))) {
> + InValidRegion = TRUE;
> + }
> + }
> +
> + if (!InValidRegion) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "SmmIsMmioValid: Not in valid MMIO region: BaseAddress (0x%lx) -
> Length (0x%lx)\n",
> + BaseAddress,
> + Length
> + ));
> + return FALSE;
> + }
> + }
> + return TRUE;
> +}
> +
> +/**
> + Merge continous entries whose type is
> EfiGcdMemoryTypeMemoryMappedIo.
> +
> + @param[in, out] GcdMemoryMap A pointer to the buffer in which
> firmware places
> + the current GCD memory map.
> + @param[in, out] NumberOfDescriptors A pointer to the number of the
> + GcdMemoryMap buffer. On input, this is the number
> of
> + the current GCD memory map. On output,
> + it is the number of new GCD memory map after
> merge.
> +**/
> +STATIC
> +VOID
> +MergeGcdMmioEntry (
> + IN OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR *GcdMemoryMap,
> + IN OUT UINTN *NumberOfDescriptors
> + )
> +{
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *GcdMemoryMapEntry;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *GcdMemoryMapEnd;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *NewGcdMemoryMapEntry;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *NextGcdMemoryMapEntry;
> +
> + GcdMemoryMapEntry = GcdMemoryMap;
> + NewGcdMemoryMapEntry = GcdMemoryMap;
> + GcdMemoryMapEnd = (EFI_GCD_MEMORY_SPACE_DESCRIPTOR *)
> ((UINT8 *) GcdMemoryMap + (*NumberOfDescriptors) *
> sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
> + while ((UINTN)GcdMemoryMapEntry < (UINTN)GcdMemoryMapEnd) {
> + CopyMem (NewGcdMemoryMapEntry, GcdMemoryMapEntry,
> sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
> + NextGcdMemoryMapEntry = GcdMemoryMapEntry + 1;
> +
> + do {
> + if (((UINTN)NextGcdMemoryMapEntry < (UINTN)GcdMemoryMapEnd)
> &&
> + (GcdMemoryMapEntry->GcdMemoryType ==
> EfiGcdMemoryTypeMemoryMappedIo) && (NextGcdMemoryMapEntry-
> >GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo) &&
> + ((GcdMemoryMapEntry->BaseAddress + GcdMemoryMapEntry-
> >Length) == NextGcdMemoryMapEntry->BaseAddress)) {
> + GcdMemoryMapEntry->Length += NextGcdMemoryMapEntry-
> >Length;
> + if (NewGcdMemoryMapEntry != GcdMemoryMapEntry) {
> + NewGcdMemoryMapEntry->Length += NextGcdMemoryMapEntry-
> >Length;
> + }
> +
> + NextGcdMemoryMapEntry = NextGcdMemoryMapEntry + 1;
> + continue;
> + } else {
> + GcdMemoryMapEntry = NextGcdMemoryMapEntry - 1;
> + break;
> + }
> + } while (TRUE);
> +
> + GcdMemoryMapEntry = GcdMemoryMapEntry + 1;
> + NewGcdMemoryMapEntry = NewGcdMemoryMapEntry + 1;
> + }
> +
> + *NumberOfDescriptors = ((UINTN)NewGcdMemoryMapEntry -
> (UINTN)GcdMemoryMap) /
> sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR);
> +
> + return ;
> +}
> +
> +/**
> + Notification for SMM EndOfDxe protocol.
> +
> + @param[in] Protocol Points to the protocol's unique identifier.
> + @param[in] Interface Points to the interface instance.
> + @param[in] Handle The handle on which the interface was installed.
> +
> + @retval EFI_SUCCESS Notification runs successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmIoLibInternalEndOfDxeNotify (
> + IN CONST EFI_GUID *Protocol,
> + IN VOID *Interface,
> + IN EFI_HANDLE Handle
> + )
> +{
> + UINTN NumberOfDescriptors;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *MemSpaceMap;
> + EFI_STATUS Status;
> +
> + Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors,
> &MemSpaceMap);
> + if (!EFI_ERROR (Status)) {
> +
> + MergeGcdMmioEntry (MemSpaceMap, &NumberOfDescriptors);
> +
> + mSmmIoLibGcdMemSpace = AllocateCopyPool (NumberOfDescriptors *
> sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR), MemSpaceMap);
> + ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + gBS->FreePool (MemSpaceMap);
> + return Status;
> + }
> +
> + mSmmIoLibGcdMemNumberOfDesc = NumberOfDescriptors;
> + gBS->FreePool (MemSpaceMap);
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Notification for SMM ReadyToLock protocol.
> +
> + @param[in] Protocol Points to the protocol's unique identifier.
> + @param[in] Interface Points to the interface instance.
> + @param[in] Handle The handle on which the interface was installed.
> +
> + @retval EFI_SUCCESS Notification runs successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmIoLibInternalReadyToLockNotify (
> + IN CONST EFI_GUID *Protocol,
> + IN VOID *Interface,
> + IN EFI_HANDLE Handle
> + )
> +{
> + mSmmIoLibReadyToLock = TRUE;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + The constructor function initializes the Smm IO library
> +
> + @param ImageHandle The firmware allocated handle for the EFI image.
> + @param SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The constructor always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmIoLibConstructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // Calculate and save maximum support address
> + //
> + SmmIoLibInternalCalculateMaximumSupportAddress ();
> +
> + //
> + // Register EndOfDxe to get GCD resource map
> + //
> + Status = gSmst->SmmRegisterProtocolNotify
> (&gEfiSmmEndOfDxeProtocolGuid, SmmIoLibInternalEndOfDxeNotify,
> &mSmmIoLibRegistrationEndOfDxe);
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> + // Register ready to lock so that we can know when to check valid resource
> region
> + //
> + Status = gSmst->SmmRegisterProtocolNotify
> (&gEfiSmmReadyToLockProtocolGuid, SmmIoLibInternalReadyToLockNotify,
> &mSmmIoLibRegistrationReadyToLock);
> + ASSERT_EFI_ERROR (Status);
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + The destructor function frees resource used in the Smm IO library
> +
> + @param[in] ImageHandle The firmware allocated handle for the EFI
> image.
> + @param[in] SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The deconstructor always returns EFI_SUCCESS.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmIoLibDestructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + gSmst->SmmRegisterProtocolNotify (&gEfiSmmEndOfDxeProtocolGuid,
> NULL, &mSmmIoLibRegistrationEndOfDxe);
> + gSmst->SmmRegisterProtocolNotify (&gEfiSmmReadyToLockProtocolGuid,
> NULL, &mSmmIoLibRegistrationReadyToLock);
> + return EFI_SUCCESS;
> +}
> diff --git a/MdePkg/Library/SmmIoLib/SmmIoLib.inf
> b/MdePkg/Library/SmmIoLib/SmmIoLib.inf
> new file mode 100644
> index 0000000..d508408
> --- /dev/null
> +++ b/MdePkg/Library/SmmIoLib/SmmIoLib.inf
> @@ -0,0 +1,53 @@
> +## @file
> +# Instance of SMM IO check library.
> +#
> +# SMM IO check library library implementation. This library consumes GCD
> to collect all valid
> +# IO space defined by a platform.
> +# A platform may have its own SmmIoLib instance to exclude more IO
> space.
> +#
> +# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions of the
> BSD License
> +# which accompanies this distribution. The full text of the license may be
> found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = SmmIoLib
> + MODULE_UNI_FILE = SmmIoLib.uni
> + FILE_GUID = F0F5A845-E3ED-4C6E-82D6-4ECE85DAC00F
> + MODULE_TYPE = DXE_SMM_DRIVER
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = SmmIoLib|DXE_SMM_DRIVER SMM_CORE
> + CONSTRUCTOR = SmmIoLibConstructor
> + DESTRUCTOR = SmmIoLibDestructor
> +
> +#
> +# The following information is for reference only and not required by the
> build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[Sources]
> + SmmIoLib.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + SmmServicesTableLib
> + UefiBootServicesTableLib
> + DxeServicesTableLib
> + DebugLib
> + BaseMemoryLib
> + MemoryAllocationLib
> + HobLib
> +
> +[Protocols]
> + gEfiSmmReadyToLockProtocolGuid ## CONSUMES
> + gEfiSmmEndOfDxeProtocolGuid ## CONSUMES
> diff --git a/MdePkg/Library/SmmIoLib/SmmIoLib.uni
> b/MdePkg/Library/SmmIoLib/SmmIoLib.uni
> new file mode 100644
> index 0000000..5d20b4d
> --- /dev/null
> +++ b/MdePkg/Library/SmmIoLib/SmmIoLib.uni
> @@ -0,0 +1,23 @@
> +// /** @file
> +// Instance of SMM IO check library.
> +//
> +// SMM IO check library library implementation. This library consumes GCD
> to collect all valid
> +// IO space defined by a platform.
> +// A platform may have its own SmmIoLib instance to exclude more IO
> space.
> +//
> +// Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +//
> +// This program and the accompanying materials
> +// are licensed and made available under the terms and conditions of the
> BSD License
> +// which accompanies this distribution. The full text of the license may be
> found at
> +// http://opensource.org/licenses/bsd-license.php.
> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT #language en-US "Instance of SMM
> IO check library."
> +
> +#string STR_MODULE_DESCRIPTION #language en-US "SMM IO check
> library library implementation. This library consumes GCD to collect all valid IO
> space defined by a platform. A platform may have its own SmmIoLib instance
> to exclude more IO space."
> +
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-04-26 1:02 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-24 14:14 [PATCH V2 0/4] Add SmmIoLib Jiewen Yao
2017-04-24 14:14 ` [PATCH V2 1/4] MdePkg/SmmIoLib: Add header file Jiewen Yao
2017-04-24 14:14 ` [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance Jiewen Yao
2017-04-24 14:58 ` Carsey, Jaben
2017-04-26 1:02 ` Yao, Jiewen [this message]
2017-04-24 14:14 ` [PATCH V2 3/4] MdePkg/dec: Add SmmIoLib Jiewen Yao
2017-04-24 14:14 ` [PATCH V2 4/4] MdePkg/dsc: add SmmIoLib Jiewen Yao
2017-04-25 1:42 ` [PATCH V2 0/4] Add SmmIoLib Gao, Liming
2017-04-26 1:18 ` Fan, Jeff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503A930CF6@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox