Re: [edk2] [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.

2017-04-25 Thread Yao, Jiewen 
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@intel.com>; edk2-devel@lists.01.org
Cc: Fan, Jeff <jeff@intel.com>; Gao, Liming <liming@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-boun...@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@intel.com<mailto:jeff@intel.com>>; Gao, Liming 
> <liming....@intel.com<mailto:liming....@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@intel.com<mailto:jeff@intel.com>>
> Cc: Liming Gao <liming@intel.com<mailto:liming@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen@intel.com<mailto:jiewen@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 000..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.
> +  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 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +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;
> +  UINT8MemPhysicalAddressBits;
> +
> +  //
> +  // Get physical address bits supported.
> +  //
> +  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> +  if (Hob != NULL) {
> +MemPhysicalAddressBits = ((EFI_HOB_CPU *) Hob)-
> >SizeOfMemorySpace;
> +  } else {
> +AsmCpuid (0x8000, , NULL, NULL, NULL);
> +if (RegEax >= 0x8008) {
> +  AsmCpuid (0x8008, , 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_

Re: [edk2] [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.

2017-04-24 Thread Carsey, Jaben 
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-boun...@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Monday, April 24, 2017 7:15 AM
> To: edk2-devel@lists.01.org
> Cc: Fan, Jeff <jeff@intel.com>; Gao, Liming <liming....@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@intel.com>
> Cc: Liming Gao <liming@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen@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 000..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.
> +  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 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +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;
> +  UINT8MemPhysicalAddressBits;
> +
> +  //
> +  // Get physical address bits supported.
> +  //
> +  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> +  if (Hob != NULL) {
> +MemPhysicalAddressBits = ((EFI_HOB_CPU *) Hob)-
> >SizeOfMemorySpace;
> +  } else {
> +AsmCpuid (0x8000, , NULL, NULL, NULL);
> +if (RegEax >= 0x8008) {
> +  AsmCpuid (0x8008, , 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 OwnerA 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.
>

[edk2] [PATCH V2 2/4] MdePkg/SmmIoLib: Add sample instance.

2017-04-24 Thread Jiewen Yao 
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 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 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 000..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.
+  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 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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;
+  UINT8MemPhysicalAddressBits;
+
+  //
+  // Get physical address bits supported.
+  //
+  Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
+  if (Hob != NULL) {
+MemPhysicalAddressBits = ((EFI_HOB_CPU *) Hob)->SizeOfMemorySpace;
+  } else {
+AsmCpuid (0x8000, , NULL, NULL, NULL);
+if (RegEax >= 0x8008) {
+  AsmCpuid (0x8008, , 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 OwnerA 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 UINT64Length,
+  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 <