[edk2-devel] [PATCH 03/10] EmbeddedPkg/MemoryAllocationLib: Add null stub for AllocateCopyPool

Min Xu posted 10 patches 4 years, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH 03/10] EmbeddedPkg/MemoryAllocationLib: Add null stub for AllocateCopyPool
Posted by Min Xu 4 years, 1 month ago
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429

This function is a null stub to make the build success.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 .../MemoryAllocationLib.c                     | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
index 78f8da5e9527..ddc27150c680 100644
--- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
+++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
@@ -220,6 +220,34 @@ AllocateZeroPool (
   return Buffer;
 }
 
+/**
+  Copies a buffer to an allocated buffer of type EfiBootServicesData.
+
+  Allocates the number bytes specified by AllocationSize of type EfiBootServicesData, copies
+  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
+  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
+  is not enough memory remaining to satisfy the request, then NULL is returned.
+
+  If Buffer is NULL, then ASSERT().
+  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  AllocationSize        The number of bytes to allocate and zero.
+  @param  Buffer                The buffer to copy to the allocated buffer.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocateCopyPool (
+  IN UINTN       AllocationSize,
+  IN CONST VOID  *Buffer
+  )
+{
+  ASSERT (FALSE);
+  return NULL;
+}
+
 /**
   Frees a buffer that was previously allocated with one of the pool allocation functions in the
   Memory Allocation Library.
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84806): https://edk2.groups.io/g/devel/message/84806
Mute This Topic: https://groups.io/mt/87720795/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 03/10] EmbeddedPkg/MemoryAllocationLib: Add null stub for AllocateCopyPool
Posted by Ard Biesheuvel 4 years, 1 month ago
On Tue, 14 Dec 2021 at 14:42, Min Xu <min.m.xu@intel.com> wrote:
>
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>
> This function is a null stub to make the build success.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  .../MemoryAllocationLib.c                     | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>

Why is it justified to implement a broken version of this routine?
This is not a NULL library class that only exists for build test
purposes, it is actually used in production builds.

If the TDVF code needs the symbol but does not actually call it,
perhaps there is another place where this should get fixed?


> diff --git a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
> index 78f8da5e9527..ddc27150c680 100644
> --- a/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
> +++ b/EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c
> @@ -220,6 +220,34 @@ AllocateZeroPool (
>    return Buffer;
>  }
>
> +/**
> +  Copies a buffer to an allocated buffer of type EfiBootServicesData.
> +
> +  Allocates the number bytes specified by AllocationSize of type EfiBootServicesData, copies
> +  AllocationSize bytes from Buffer to the newly allocated buffer, and returns a pointer to the
> +  allocated buffer.  If AllocationSize is 0, then a valid buffer of 0 size is returned.  If there
> +  is not enough memory remaining to satisfy the request, then NULL is returned.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If AllocationSize is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
> +
> +  @param  AllocationSize        The number of bytes to allocate and zero.
> +  @param  Buffer                The buffer to copy to the allocated buffer.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocateCopyPool (
> +  IN UINTN       AllocationSize,
> +  IN CONST VOID  *Buffer
> +  )
> +{
> +  ASSERT (FALSE);
> +  return NULL;
> +}
> +
>  /**
>    Frees a buffer that was previously allocated with one of the pool allocation functions in the
>    Memory Allocation Library.
> --
> 2.29.2.windows.2
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84816): https://edk2.groups.io/g/devel/message/84816
Mute This Topic: https://groups.io/mt/87720795/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 03/10] EmbeddedPkg/MemoryAllocationLib: Add null stub for AllocateCopyPool
Posted by Min Xu 4 years, 1 month ago
On December 14, 2021 9:59 PM, Ard Biesheuvel wrote:
> On Tue, 14 Dec 2021 at 14:42, Min Xu <min.m.xu@intel.com> wrote:
> >
> > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >
> > This function is a null stub to make the build success.
> >
> > ---
> >  .../MemoryAllocationLib.c                     | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> 
> Why is it justified to implement a broken version of this routine?
> This is not a NULL library class that only exists for build test purposes, it is
> actually used in production builds.
> 
> If the TDVF code needs the symbol but does not actually call it, perhaps there is
> another place where this should get fixed?
> 
PeiServicesLib (MdePkg/Library/PeiServicesLib/PeiServicesLib.inf) is imported in In OvmfPkg/Sec/SecMain.inf. In the implementation of this lib, AllocateCopyPool is used. AllocateCopyPool is implemented in MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf. 
With the introduction of TDVF Config-B, MemoryAllocationLib is changed to EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf. In this MemoryAllocationLib, AllocateCopyPool is not implemented.
So I have to add a null stub of AllocateCopyPool in EmbeddedPkg/Library/PrePiMemoryAllocationLib/MemoryAllocationLib.c

I carefully re-check the OvmfPkg/Sec and find PeiServicesLib is not needed. So PeiServiceLib is removed from SecMain.inf. This time AllocateCopyPool is not needed either. So this commit will be dropped in the next version. Thanks much for your reminder.

Min





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84948): https://edk2.groups.io/g/devel/message/84948
Mute This Topic: https://groups.io/mt/87720795/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-