public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
@ 2018-01-31 23:33 Kinney, Michael D
  2018-01-31 23:33 ` [Patch 1/1] " Kinney, Michael D
  2018-02-01  1:19 ` [Patch 0/1] " Gao, Liming
  0 siblings, 2 replies; 7+ messages in thread
From: Kinney, Michael D @ 2018-01-31 23:33 UTC (permalink / raw)
  To: edk2-devel; +Cc: Sean Brogan, Jiewen Yao, Liming Gao, Michael D Kinney

https://bugzilla.tianocore.org/show_bug.cgi?id=838

Add new API to the UefiLib that locates and returns
an array of protocols instances that match a given
protocol.

Branch for review:

https://github.com/mdkinney/edk2/tree/Bug_838_EfiLocateProtocolBuffer_V3

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>

Michael D Kinney (1):
  MdePkg/UefiLib: Add EfiLocateProtocolBuffer()

 MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
 MdePkg/Library/UefiLib/UefiLib.c | 107 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 137 insertions(+), 2 deletions(-)

-- 
2.14.2.windows.3



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-01-31 23:33 [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer() Kinney, Michael D
@ 2018-01-31 23:33 ` Kinney, Michael D
  2018-02-01  1:31   ` Zeng, Star
  2018-02-01  1:19 ` [Patch 0/1] " Gao, Liming
  1 sibling, 1 reply; 7+ messages in thread
From: Kinney, Michael D @ 2018-01-31 23:33 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Sean Brogan, Jiewen Yao, Liming Gao

From: Michael D Kinney <michael.d.kinney@intel.com>

https://bugzilla.tianocore.org/show_bug.cgi?id=838

Add new API to the UefiLib that locates and returns
an array of protocols instances that match a given
protocol.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
 MdePkg/Library/UefiLib/UefiLib.c | 107 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 0b14792a0a..54bc2cc5a3 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -12,7 +12,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL implementations.
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, 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 that accompanies this distribution.  
 The full text of the license may be found at
@@ -1490,4 +1490,34 @@ CatSPrint (
   ...
   );
 
+/**
+  Returns an array of protocol instance that matches the given protocol.
+
+  @param[in]  Protocol      Provides the protocol to search for.
+  @param[out] NoProtocols   The number of protocols returned in Buffer.
+  @param[out] Buffer        A pointer to the buffer to return the requested
+                            array of protocol instances that match Protocol.
+                            The returned buffer is allocated using
+                            EFI_BOOT_SERVICES.AllocatePool().  The caller is
+                            responsible for freeing this buffer with
+                            EFI_BOOT_SERVICES.FreePool().
+
+  @retval EFI_SUCCESS            The array of protocols was returned in Buffer,
+                                 and the number of protocols in Buffer was
+                                 returned in NoProtocols.
+  @retval EFI_NOT_FOUND          No protocols found.
+  @retval EFI_OUT_OF_RESOURCES   There is not enough pool memory to store the
+                                 matching results.
+  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
+  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLocateProtocolBuffer (
+  IN  EFI_GUID  *Protocol,
+  OUT UINTN     *NoProtocols,
+  OUT VOID      ***Buffer
+  );
 #endif
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index a7eee01240..d86db4bd67 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -5,7 +5,7 @@
   EFI Driver Model related protocols, manage Unicode string tables for UEFI Drivers, 
   and print messages on the console output and standard error devices.
 
-  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2018, 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
@@ -1605,3 +1605,108 @@ GetBestLanguage (
   //
   return NULL;
 }
+
+/**
+  Returns an array of protocol instance that matches the given protocol.
+
+  @param[in]  Protocol      Provides the protocol to search for.
+  @param[out] NoProtocols   The number of protocols returned in Buffer.
+  @param[out] Buffer        A pointer to the buffer to return the requested
+                            array of protocol instances that match Protocol.
+                            The returned buffer is allocated using
+                            EFI_BOOT_SERVICES.AllocatePool().  The caller is
+                            responsible for freeing this buffer with
+                            EFI_BOOT_SERVICES.FreePool().
+
+  @retval EFI_SUCCESS            The array of protocols was returned in Buffer,
+                                 and the number of protocols in Buffer was
+                                 returned in NoProtocols.
+  @retval EFI_NOT_FOUND          No protocols found.
+  @retval EFI_OUT_OF_RESOURCES   There is not enough pool memory to store the
+                                 matching results.
+  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
+  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLocateProtocolBuffer (
+  IN  EFI_GUID  *Protocol,
+  OUT UINTN     *NoProtocols,
+  OUT VOID      ***Buffer
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       NoHandles;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN       Index;
+
+  //
+  // Check input parameters
+  //
+  if (Protocol == NULL || NoProtocols == NULL || Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Initialze output parameters
+  //
+  *NoProtocols = 0;
+  *Buffer = NULL;
+
+  //
+  // Retrieve the array of handles that support Protocol
+  //
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  Protocol,
+                  NULL,
+                  &NoHandles,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Allocate array of protocol instances
+  //
+  *Buffer = AllocateZeroPool (NoHandles * sizeof (VOID *));
+  if (*Buffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Lookup Protocol on each handle in HandleBuffer to fill in the array of
+  // protocol instances.  Handle case where protocol instance was present when
+  // LocateHandleBuffer() was called, but is not present when HandleProtocol()
+  // is called.
+  //
+  for (Index = 0, *NoProtocols = 0; Index < NoHandles; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    Protocol,
+                    &((*Buffer)[*NoProtocols])
+                    );
+    if (!EFI_ERROR (Status)) {
+      (*NoProtocols)++;
+    }
+  }
+
+  //
+  // Free the handle buffer
+  //
+  FreePool (HandleBuffer);
+
+  //
+  // Make sure at least one protocol instance was found
+  //
+  if (*NoProtocols == 0) {
+    FreePool (*Buffer);
+    *Buffer = NULL;
+    return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
-- 
2.14.2.windows.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-01-31 23:33 [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer() Kinney, Michael D
  2018-01-31 23:33 ` [Patch 1/1] " Kinney, Michael D
@ 2018-02-01  1:19 ` Gao, Liming
  2018-02-01  1:47   ` Kinney, Michael D
  1 sibling, 1 reply; 7+ messages in thread
From: Gao, Liming @ 2018-02-01  1:19 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org; +Cc: Sean Brogan, Yao, Jiewen

Mike:
  Just confirm. Do we still need to add new API in IntelFrameworkPkg FrameworkUefiLib? I think we don't need add new feature in IntelFrameworkPkg and IntelFrameworkModulePkg. 

>-----Original Message-----
>From: Kinney, Michael D
>Sent: Thursday, February 01, 2018 7:33 AM
>To: edk2-devel@lists.01.org
>Cc: Sean Brogan <sean.brogan@microsoft.com>; Yao, Jiewen
><jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Kinney,
>Michael D <michael.d.kinney@intel.com>
>Subject: [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=838
>
>Add new API to the UefiLib that locates and returns
>an array of protocols instances that match a given
>protocol.
>
>Branch for review:
>
>https://github.com/mdkinney/edk2/tree/Bug_838_EfiLocateProtocolBuffer_
>V3
>
>Cc: Sean Brogan <sean.brogan@microsoft.com>
>Cc: Jiewen Yao <jiewen.yao@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
>
>Michael D Kinney (1):
>  MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
>
> MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
> MdePkg/Library/UefiLib/UefiLib.c | 107
>++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 137 insertions(+), 2 deletions(-)
>
>--
>2.14.2.windows.3



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-01-31 23:33 ` [Patch 1/1] " Kinney, Michael D
@ 2018-02-01  1:31   ` Zeng, Star
  2018-02-01  2:06     ` Kinney, Michael D
  0 siblings, 1 reply; 7+ messages in thread
From: Zeng, Star @ 2018-02-01  1:31 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Yao, Jiewen, Gao, Liming, Zeng, Star

Hi Mike,

UefiLib is capable to be used by SMM Core or SMM driver before SmmReadyToLock.

And the new interface has the comment below.

+                            The returned buffer is allocated using
+                            EFI_BOOT_SERVICES.AllocatePool().  The caller is
+                            responsible for freeing this buffer with
+                            EFI_BOOT_SERVICES.FreePool().

But the implementation is using AllocateZeroPool() like below, that will direct to gSmst->AllocatePool for SMM Core or SMM driver.

+  //
+  // Allocate array of protocol instances
+  //
+  *Buffer = AllocateZeroPool (NoHandles * sizeof (VOID *));
+  if (*Buffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }

Should the implementation use gBS->AllocatePool() + ZeroMem() instead of AllocateZeroPool()?


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Kinney, Michael D
Sent: Thursday, February 1, 2018 7:33 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [edk2] [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()

From: Michael D Kinney <michael.d.kinney@intel.com>

https://bugzilla.tianocore.org/show_bug.cgi?id=838

Add new API to the UefiLib that locates and returns
an array of protocols instances that match a given
protocol.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
 MdePkg/Library/UefiLib/UefiLib.c | 107 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 137 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 0b14792a0a..54bc2cc5a3 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -12,7 +12,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL implementations.
 
-Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2018, 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 that accompanies this distribution.  
 The full text of the license may be found at
@@ -1490,4 +1490,34 @@ CatSPrint (
   ...
   );
 
+/**
+  Returns an array of protocol instance that matches the given protocol.
+
+  @param[in]  Protocol      Provides the protocol to search for.
+  @param[out] NoProtocols   The number of protocols returned in Buffer.
+  @param[out] Buffer        A pointer to the buffer to return the requested
+                            array of protocol instances that match Protocol.
+                            The returned buffer is allocated using
+                            EFI_BOOT_SERVICES.AllocatePool().  The caller is
+                            responsible for freeing this buffer with
+                            EFI_BOOT_SERVICES.FreePool().
+
+  @retval EFI_SUCCESS            The array of protocols was returned in Buffer,
+                                 and the number of protocols in Buffer was
+                                 returned in NoProtocols.
+  @retval EFI_NOT_FOUND          No protocols found.
+  @retval EFI_OUT_OF_RESOURCES   There is not enough pool memory to store the
+                                 matching results.
+  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
+  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLocateProtocolBuffer (
+  IN  EFI_GUID  *Protocol,
+  OUT UINTN     *NoProtocols,
+  OUT VOID      ***Buffer
+  );
 #endif
diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index a7eee01240..d86db4bd67 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -5,7 +5,7 @@
   EFI Driver Model related protocols, manage Unicode string tables for UEFI Drivers, 
   and print messages on the console output and standard error devices.
 
-  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2018, 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
@@ -1605,3 +1605,108 @@ GetBestLanguage (
   //
   return NULL;
 }
+
+/**
+  Returns an array of protocol instance that matches the given protocol.
+
+  @param[in]  Protocol      Provides the protocol to search for.
+  @param[out] NoProtocols   The number of protocols returned in Buffer.
+  @param[out] Buffer        A pointer to the buffer to return the requested
+                            array of protocol instances that match Protocol.
+                            The returned buffer is allocated using
+                            EFI_BOOT_SERVICES.AllocatePool().  The caller is
+                            responsible for freeing this buffer with
+                            EFI_BOOT_SERVICES.FreePool().
+
+  @retval EFI_SUCCESS            The array of protocols was returned in Buffer,
+                                 and the number of protocols in Buffer was
+                                 returned in NoProtocols.
+  @retval EFI_NOT_FOUND          No protocols found.
+  @retval EFI_OUT_OF_RESOURCES   There is not enough pool memory to store the
+                                 matching results.
+  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
+  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLocateProtocolBuffer (
+  IN  EFI_GUID  *Protocol,
+  OUT UINTN     *NoProtocols,
+  OUT VOID      ***Buffer
+  )
+{
+  EFI_STATUS  Status;
+  UINTN       NoHandles;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN       Index;
+
+  //
+  // Check input parameters
+  //
+  if (Protocol == NULL || NoProtocols == NULL || Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Initialze output parameters
+  //
+  *NoProtocols = 0;
+  *Buffer = NULL;
+
+  //
+  // Retrieve the array of handles that support Protocol
+  //
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  Protocol,
+                  NULL,
+                  &NoHandles,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Allocate array of protocol instances
+  //
+  *Buffer = AllocateZeroPool (NoHandles * sizeof (VOID *));
+  if (*Buffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Lookup Protocol on each handle in HandleBuffer to fill in the array of
+  // protocol instances.  Handle case where protocol instance was present when
+  // LocateHandleBuffer() was called, but is not present when HandleProtocol()
+  // is called.
+  //
+  for (Index = 0, *NoProtocols = 0; Index < NoHandles; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    Protocol,
+                    &((*Buffer)[*NoProtocols])
+                    );
+    if (!EFI_ERROR (Status)) {
+      (*NoProtocols)++;
+    }
+  }
+
+  //
+  // Free the handle buffer
+  //
+  FreePool (HandleBuffer);
+
+  //
+  // Make sure at least one protocol instance was found
+  //
+  if (*NoProtocols == 0) {
+    FreePool (*Buffer);
+    *Buffer = NULL;
+    return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
-- 
2.14.2.windows.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-02-01  1:19 ` [Patch 0/1] " Gao, Liming
@ 2018-02-01  1:47   ` Kinney, Michael D
  0 siblings, 0 replies; 7+ messages in thread
From: Kinney, Michael D @ 2018-02-01  1:47 UTC (permalink / raw)
  To: Gao, Liming, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: Sean Brogan, Yao, Jiewen

Liming,

I looks like we still do updates if a new API is added
to the UefiLib class.

https://github.com/tianocore/edk2/commit/6212b9481d822a6765f8cd264ceb8454291948bd#diff-b3a4a95eeed10a8932c1738829d8cd7f

I will update the patch.

Mike


> -----Original Message-----
> From: Gao, Liming
> Sent: Wednesday, January 31, 2018 5:20 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-
> devel@lists.01.org
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: RE: [Patch 0/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> 
> Mike:
>   Just confirm. Do we still need to add new API in
> IntelFrameworkPkg FrameworkUefiLib? I think we don't need
> add new feature in IntelFrameworkPkg and
> IntelFrameworkModulePkg.
> 
> >-----Original Message-----
> >From: Kinney, Michael D
> >Sent: Thursday, February 01, 2018 7:33 AM
> >To: edk2-devel@lists.01.org
> >Cc: Sean Brogan <sean.brogan@microsoft.com>; Yao, Jiewen
> ><jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Kinney,
> >Michael D <michael.d.kinney@intel.com>
> >Subject: [Patch 0/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> >
> >https://bugzilla.tianocore.org/show_bug.cgi?id=838
> >
> >Add new API to the UefiLib that locates and returns
> >an array of protocols instances that match a given
> >protocol.
> >
> >Branch for review:
> >
> >https://github.com/mdkinney/edk2/tree/Bug_838_EfiLocateP
> rotocolBuffer_
> >V3
> >
> >Cc: Sean Brogan <sean.brogan@microsoft.com>
> >Cc: Jiewen Yao <jiewen.yao@intel.com>
> >Cc: Liming Gao <liming.gao@intel.com>
> >Contributed-under: TianoCore Contribution Agreement 1.1
> >Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> >
> >Michael D Kinney (1):
> >  MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
> >
> > MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
> > MdePkg/Library/UefiLib/UefiLib.c | 107
> >++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 137 insertions(+), 2 deletions(-)
> >
> >--
> >2.14.2.windows.3



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-02-01  1:31   ` Zeng, Star
@ 2018-02-01  2:06     ` Kinney, Michael D
  2018-02-01  2:20       ` Zeng, Star
  0 siblings, 1 reply; 7+ messages in thread
From: Kinney, Michael D @ 2018-02-01  2:06 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: Yao, Jiewen, Gao, Liming

Star,

I agree with your observation.  And the API specifically says
that EFI_BOOT_SERVICES.AllocatePool() is used, so I agree with
your suggested change.

However, there are other APIs in the UefiLib implementation
that use the MemoryAllocationLib services.  Should those be 
updated too?

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, January 31, 2018 5:32 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-
> devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [Patch 1/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> 
> Hi Mike,
> 
> UefiLib is capable to be used by SMM Core or SMM driver
> before SmmReadyToLock.
> 
> And the new interface has the comment below.
> 
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> 
> But the implementation is using AllocateZeroPool() like
> below, that will direct to gSmst->AllocatePool for SMM
> Core or SMM driver.
> 
> +  //
> +  // Allocate array of protocol instances
> +  //
> +  *Buffer = AllocateZeroPool (NoHandles * sizeof (VOID
> *));
> +  if (*Buffer == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> 
> Should the implementation use gBS->AllocatePool() +
> ZeroMem() instead of AllocateZeroPool()?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> On Behalf Of Kinney, Michael D
> Sent: Thursday, February 1, 2018 7:33 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2] [Patch 1/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> 
> From: Michael D Kinney <michael.d.kinney@intel.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=838
> 
> Add new API to the UefiLib that locates and returns
> an array of protocols instances that match a given
> protocol.
> 
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> ---
>  MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-
>  MdePkg/Library/UefiLib/UefiLib.c | 107
> ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 137 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index 0b14792a0a..54bc2cc5a3 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -12,7 +12,7 @@
>    of size reduction when compiler optimization is
> disabled. If MDEPKG_NDEBUG is
>    defined, then debug and assert related macros wrapped
> by it are the NULL implementations.
> 
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights
> reserved.<BR>
> +Copyright (c) 2006 - 2018, 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 that
> accompanies this distribution.
>  The full text of the license may be found at
> @@ -1490,4 +1490,34 @@ CatSPrint (
>    ...
>    );
> 
> +/**
> +  Returns an array of protocol instance that matches the
> given protocol.
> +
> +  @param[in]  Protocol      Provides the protocol to
> search for.
> +  @param[out] NoProtocols   The number of protocols
> returned in Buffer.
> +  @param[out] Buffer        A pointer to the buffer to
> return the requested
> +                            array of protocol instances
> that match Protocol.
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> +
> +  @retval EFI_SUCCESS            The array of protocols
> was returned in Buffer,
> +                                 and the number of
> protocols in Buffer was
> +                                 returned in
> NoProtocols.
> +  @retval EFI_NOT_FOUND          No protocols found.
> +  @retval EFI_OUT_OF_RESOURCES   There is not enough
> pool memory to store the
> +                                 matching results.
> +  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
> +  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
> +  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiLocateProtocolBuffer (
> +  IN  EFI_GUID  *Protocol,
> +  OUT UINTN     *NoProtocols,
> +  OUT VOID      ***Buffer
> +  );
>  #endif
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c
> b/MdePkg/Library/UefiLib/UefiLib.c
> index a7eee01240..d86db4bd67 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -5,7 +5,7 @@
>    EFI Driver Model related protocols, manage Unicode
> string tables for UEFI Drivers,
>    and print messages on the console output and standard
> error devices.
> 
> -  Copyright (c) 2006 - 2017, Intel Corporation. All
> rights reserved.<BR>
> +  Copyright (c) 2006 - 2018, 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
> @@ -1605,3 +1605,108 @@ GetBestLanguage (
>    //
>    return NULL;
>  }
> +
> +/**
> +  Returns an array of protocol instance that matches the
> given protocol.
> +
> +  @param[in]  Protocol      Provides the protocol to
> search for.
> +  @param[out] NoProtocols   The number of protocols
> returned in Buffer.
> +  @param[out] Buffer        A pointer to the buffer to
> return the requested
> +                            array of protocol instances
> that match Protocol.
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> +
> +  @retval EFI_SUCCESS            The array of protocols
> was returned in Buffer,
> +                                 and the number of
> protocols in Buffer was
> +                                 returned in
> NoProtocols.
> +  @retval EFI_NOT_FOUND          No protocols found.
> +  @retval EFI_OUT_OF_RESOURCES   There is not enough
> pool memory to store the
> +                                 matching results.
> +  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
> +  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
> +  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiLocateProtocolBuffer (
> +  IN  EFI_GUID  *Protocol,
> +  OUT UINTN     *NoProtocols,
> +  OUT VOID      ***Buffer
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINTN       NoHandles;
> +  EFI_HANDLE  *HandleBuffer;
> +  UINTN       Index;
> +
> +  //
> +  // Check input parameters
> +  //
> +  if (Protocol == NULL || NoProtocols == NULL || Buffer
> == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Initialze output parameters
> +  //
> +  *NoProtocols = 0;
> +  *Buffer = NULL;
> +
> +  //
> +  // Retrieve the array of handles that support Protocol
> +  //
> +  Status = gBS->LocateHandleBuffer (
> +                  ByProtocol,
> +                  Protocol,
> +                  NULL,
> +                  &NoHandles,
> +                  &HandleBuffer
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Allocate array of protocol instances
> +  //
> +  *Buffer = AllocateZeroPool (NoHandles * sizeof (VOID
> *));
> +  if (*Buffer == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  //
> +  // Lookup Protocol on each handle in HandleBuffer to
> fill in the array of
> +  // protocol instances.  Handle case where protocol
> instance was present when
> +  // LocateHandleBuffer() was called, but is not present
> when HandleProtocol()
> +  // is called.
> +  //
> +  for (Index = 0, *NoProtocols = 0; Index < NoHandles;
> Index++) {
> +    Status = gBS->HandleProtocol (
> +                    HandleBuffer[Index],
> +                    Protocol,
> +                    &((*Buffer)[*NoProtocols])
> +                    );
> +    if (!EFI_ERROR (Status)) {
> +      (*NoProtocols)++;
> +    }
> +  }
> +
> +  //
> +  // Free the handle buffer
> +  //
> +  FreePool (HandleBuffer);
> +
> +  //
> +  // Make sure at least one protocol instance was found
> +  //
> +  if (*NoProtocols == 0) {
> +    FreePool (*Buffer);
> +    *Buffer = NULL;
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> --
> 2.14.2.windows.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()
  2018-02-01  2:06     ` Kinney, Michael D
@ 2018-02-01  2:20       ` Zeng, Star
  0 siblings, 0 replies; 7+ messages in thread
From: Zeng, Star @ 2018-02-01  2:20 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org
  Cc: Yao, Jiewen, Gao, Liming, Zeng, Star

Mike,

Depend on whether the allocated buffer will be returned to caller or not, and whether the implementation is matched with the function comments.
Some cases seem ok.
1. There are paired interfaces, for example AddUnicodeString and FreeUnicodeStringTable, etc.
2. There are clear comments, and implementation matches with comments, for example GetVariable2, etc.
" The returned buffer is allocated using AllocatePool().  The caller is responsible for freeing this buffer with FreePool(). "

More evaluation are needed for all the interfaces.


Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Thursday, February 1, 2018 10:07 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2] [Patch 1/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer()

Star,

I agree with your observation.  And the API specifically says that EFI_BOOT_SERVICES.AllocatePool() is used, so I agree with your suggested change.

However, there are other APIs in the UefiLib implementation that use the MemoryAllocationLib services.  Should those be updated too?

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, January 31, 2018 5:32 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; edk2- 
> devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star 
> <star.zeng@intel.com>
> Subject: RE: [edk2] [Patch 1/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> 
> Hi Mike,
> 
> UefiLib is capable to be used by SMM Core or SMM driver before 
> SmmReadyToLock.
> 
> And the new interface has the comment below.
> 
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> 
> But the implementation is using AllocateZeroPool() like below, that 
> will direct to gSmst->AllocatePool for SMM Core or SMM driver.
> 
> +  //
> +  // Allocate array of protocol instances  //  *Buffer = 
> + AllocateZeroPool (NoHandles * sizeof (VOID
> *));
> +  if (*Buffer == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> 
> Should the implementation use gBS->AllocatePool() +
> ZeroMem() instead of AllocateZeroPool()?
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> On Behalf Of Kinney, Michael D
> Sent: Thursday, February 1, 2018 7:33 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [edk2] [Patch 1/1] MdePkg/UefiLib: Add
> EfiLocateProtocolBuffer()
> 
> From: Michael D Kinney <michael.d.kinney@intel.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=838
> 
> Add new API to the UefiLib that locates and returns an array of 
> protocols instances that match a given protocol.
> 
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> ---
>  MdePkg/Include/Library/UefiLib.h |  32 +++++++++++-  
> MdePkg/Library/UefiLib/UefiLib.c | 107
> ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 137 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index 0b14792a0a..54bc2cc5a3 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -12,7 +12,7 @@
>    of size reduction when compiler optimization is disabled. If 
> MDEPKG_NDEBUG is
>    defined, then debug and assert related macros wrapped by it are the 
> NULL implementations.
> 
> -Copyright (c) 2006 - 2017, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2006 - 2018, 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 that 
> accompanies this distribution.
>  The full text of the license may be found at @@ -1490,4 +1490,34 @@ 
> CatSPrint (
>    ...
>    );
> 
> +/**
> +  Returns an array of protocol instance that matches the
> given protocol.
> +
> +  @param[in]  Protocol      Provides the protocol to
> search for.
> +  @param[out] NoProtocols   The number of protocols
> returned in Buffer.
> +  @param[out] Buffer        A pointer to the buffer to
> return the requested
> +                            array of protocol instances
> that match Protocol.
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> +
> +  @retval EFI_SUCCESS            The array of protocols
> was returned in Buffer,
> +                                 and the number of
> protocols in Buffer was
> +                                 returned in
> NoProtocols.
> +  @retval EFI_NOT_FOUND          No protocols found.
> +  @retval EFI_OUT_OF_RESOURCES   There is not enough
> pool memory to store the
> +                                 matching results.
> +  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
> +  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
> +  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiLocateProtocolBuffer (
> +  IN  EFI_GUID  *Protocol,
> +  OUT UINTN     *NoProtocols,
> +  OUT VOID      ***Buffer
> +  );
>  #endif
> diff --git a/MdePkg/Library/UefiLib/UefiLib.c
> b/MdePkg/Library/UefiLib/UefiLib.c
> index a7eee01240..d86db4bd67 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.c
> +++ b/MdePkg/Library/UefiLib/UefiLib.c
> @@ -5,7 +5,7 @@
>    EFI Driver Model related protocols, manage Unicode string tables 
> for UEFI Drivers,
>    and print messages on the console output and standard error 
> devices.
> 
> -  Copyright (c) 2006 - 2017, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2006 - 2018, 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 @@ -1605,3 +1605,108 @@ GetBestLanguage (
>    //
>    return NULL;
>  }
> +
> +/**
> +  Returns an array of protocol instance that matches the
> given protocol.
> +
> +  @param[in]  Protocol      Provides the protocol to
> search for.
> +  @param[out] NoProtocols   The number of protocols
> returned in Buffer.
> +  @param[out] Buffer        A pointer to the buffer to
> return the requested
> +                            array of protocol instances
> that match Protocol.
> +                            The returned buffer is
> allocated using
> +
> EFI_BOOT_SERVICES.AllocatePool().  The caller is
> +                            responsible for freeing this
> buffer with
> +
> EFI_BOOT_SERVICES.FreePool().
> +
> +  @retval EFI_SUCCESS            The array of protocols
> was returned in Buffer,
> +                                 and the number of
> protocols in Buffer was
> +                                 returned in
> NoProtocols.
> +  @retval EFI_NOT_FOUND          No protocols found.
> +  @retval EFI_OUT_OF_RESOURCES   There is not enough
> pool memory to store the
> +                                 matching results.
> +  @retval EFI_INVALID_PARAMETER  Protocol is NULL.
> +  @retval EFI_INVALID_PARAMETER  NoProtocols is NULL.
> +  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EfiLocateProtocolBuffer (
> +  IN  EFI_GUID  *Protocol,
> +  OUT UINTN     *NoProtocols,
> +  OUT VOID      ***Buffer
> +  )
> +{
> +  EFI_STATUS  Status;
> +  UINTN       NoHandles;
> +  EFI_HANDLE  *HandleBuffer;
> +  UINTN       Index;
> +
> +  //
> +  // Check input parameters
> +  //
> +  if (Protocol == NULL || NoProtocols == NULL || Buffer
> == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Initialze output parameters
> +  //
> +  *NoProtocols = 0;
> +  *Buffer = NULL;
> +
> +  //
> +  // Retrieve the array of handles that support Protocol  //  Status 
> + = gBS->LocateHandleBuffer (
> +                  ByProtocol,
> +                  Protocol,
> +                  NULL,
> +                  &NoHandles,
> +                  &HandleBuffer
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Allocate array of protocol instances  //  *Buffer = 
> + AllocateZeroPool (NoHandles * sizeof (VOID
> *));
> +  if (*Buffer == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  //
> +  // Lookup Protocol on each handle in HandleBuffer to
> fill in the array of
> +  // protocol instances.  Handle case where protocol
> instance was present when
> +  // LocateHandleBuffer() was called, but is not present
> when HandleProtocol()
> +  // is called.
> +  //
> +  for (Index = 0, *NoProtocols = 0; Index < NoHandles;
> Index++) {
> +    Status = gBS->HandleProtocol (
> +                    HandleBuffer[Index],
> +                    Protocol,
> +                    &((*Buffer)[*NoProtocols])
> +                    );
> +    if (!EFI_ERROR (Status)) {
> +      (*NoProtocols)++;
> +    }
> +  }
> +
> +  //
> +  // Free the handle buffer
> +  //
> +  FreePool (HandleBuffer);
> +
> +  //
> +  // Make sure at least one protocol instance was found  //  if 
> + (*NoProtocols == 0) {
> +    FreePool (*Buffer);
> +    *Buffer = NULL;
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> --
> 2.14.2.windows.3
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-02-01  2:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-31 23:33 [Patch 0/1] MdePkg/UefiLib: Add EfiLocateProtocolBuffer() Kinney, Michael D
2018-01-31 23:33 ` [Patch 1/1] " Kinney, Michael D
2018-02-01  1:31   ` Zeng, Star
2018-02-01  2:06     ` Kinney, Michael D
2018-02-01  2:20       ` Zeng, Star
2018-02-01  1:19 ` [Patch 0/1] " Gao, Liming
2018-02-01  1:47   ` Kinney, Michael D

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox