* [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL
@ 2016-10-24 4:56 Star Zeng
2016-10-24 9:15 ` Gao, Liming
0 siblings, 1 reply; 3+ messages in thread
From: Star Zeng @ 2016-10-24 4:56 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Liming Gao, Sean Brogan
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=160
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
MdePkg/Library/PeiServicesLib/PeiServicesLib.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
index 3428addcc63b..2373251bc3e5 100644
--- a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
+++ b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
@@ -1,7 +1,7 @@
/** @file
Implementation for PEI Services Library.
- Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2006 - 2016, 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
@@ -574,6 +574,7 @@ PeiServicesFfsGetVolumeInfo (
the parameters passed in to initialize the fields of the EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI instance.
If the resources can not be allocated for EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI, then ASSERT().
If the EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI can not be installed, then ASSERT().
+ If NULL is specified for FvFormat, but FvInfo does not have the firmware file system 2 format, then ASSERT.
@param InstallFvInfoPpi Install FvInfo Ppi if it is TRUE. Otherwise, install FvInfo2 Ppi.
@param FvFormat Unique identifier of the format of the memory-mapped
@@ -640,6 +641,16 @@ InternalPeiServicesInstallFvInfoPpi (
CopyGuid (&FvInfoPpi->FvFormat, FvFormat);
} else {
CopyGuid (&FvInfoPpi->FvFormat, &gEfiFirmwareFileSystem2Guid);
+ //
+ // Since the EFI_FIRMWARE_FILE_SYSTEM2_GUID format is assumed if NULL is specified for FvFormat,
+ // check the FileSystemGuid pointed by FvInfo against EFI_FIRMWARE_FILE_SYSTEM2_GUID to make sure
+ // FvInfo has the firmware file system 2 format.
+ // If the ASSERT really appears, FvFormat needs to be specified correctly, for example,
+ // EFI_FIRMWARE_FILE_SYSTEM3_GUID can be used for firmware file system 3 format, or
+ // &(((EFI_FIRMWARE_VOLUME_HEADER *) FvInfo)->FileSystemGuid can be just used for both
+ // firmware file system 2 and 3 format.
+ //
+ ASSERT (CompareGuid (&(((EFI_FIRMWARE_VOLUME_HEADER *) FvInfo)->FileSystemGuid), &gEfiFirmwareFileSystem2Guid));
}
FvInfoPpi->FvInfo = (VOID *) FvInfo;
FvInfoPpi->FvInfoSize = FvInfoSize;
@@ -672,6 +683,7 @@ InternalPeiServicesInstallFvInfoPpi (
the parameters passed in to initialize the fields of the EFI_PEI_FIRMWARE_VOLUME_INFO_PPI instance.
If the resources can not be allocated for EFI_PEI_FIRMWARE_VOLUME_INFO_PPI, then ASSERT().
If the EFI_PEI_FIRMWARE_VOLUME_INFO_PPI can not be installed, then ASSERT().
+ If NULL is specified for FvFormat, but FvInfo does not have the firmware file system 2 format, then ASSERT.
@param FvFormat Unique identifier of the format of the memory-mapped
firmware volume. This parameter is optional and
@@ -714,6 +726,7 @@ PeiServicesInstallFvInfoPpi (
the parameters passed in to initialize the fields of the EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI instance.
If the resources can not be allocated for EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI, then ASSERT().
If the EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI can not be installed, then ASSERT().
+ If NULL is specified for FvFormat, but FvInfo does not have the firmware file system 2 format, then ASSERT.
@param FvFormat Unique identifier of the format of the memory-mapped
firmware volume. This parameter is optional and
--
2.7.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL
2016-10-24 4:56 [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL Star Zeng
@ 2016-10-24 9:15 ` Gao, Liming
2016-10-24 9:21 ` Zeng, Star
0 siblings, 1 reply; 3+ messages in thread
From: Gao, Liming @ 2016-10-24 9:15 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Zeng, Star
Star:
Besides this patch, I think PeiCore also requires to add this check to make sure FvInfo and FvImage match each other, because PPI may directly be installed from PEI module, not from this library API.
Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Monday, October 24, 2016 12:56 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2
> format if FvFormat == NULL
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=160
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> MdePkg/Library/PeiServicesLib/PeiServicesLib.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> index 3428addcc63b..2373251bc3e5 100644
> --- a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> +++ b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> @@ -1,7 +1,7 @@
> /** @file
> Implementation for PEI Services Library.
>
> - Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2006 - 2016, 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
> @@ -574,6 +574,7 @@ PeiServicesFfsGetVolumeInfo (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI can not be installed,
> then ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the firmware
> file system 2 format, then ASSERT.
>
> @param InstallFvInfoPpi Install FvInfo Ppi if it is TRUE. Otherwise, install
> FvInfo2 Ppi.
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> @@ -640,6 +641,16 @@ InternalPeiServicesInstallFvInfoPpi (
> CopyGuid (&FvInfoPpi->FvFormat, FvFormat);
> } else {
> CopyGuid (&FvInfoPpi->FvFormat, &gEfiFirmwareFileSystem2Guid);
> + //
> + // Since the EFI_FIRMWARE_FILE_SYSTEM2_GUID format is assumed if
> NULL is specified for FvFormat,
> + // check the FileSystemGuid pointed by FvInfo against
> EFI_FIRMWARE_FILE_SYSTEM2_GUID to make sure
> + // FvInfo has the firmware file system 2 format.
> + // If the ASSERT really appears, FvFormat needs to be specified correctly,
> for example,
> + // EFI_FIRMWARE_FILE_SYSTEM3_GUID can be used for firmware file
> system 3 format, or
> + // &(((EFI_FIRMWARE_VOLUME_HEADER *) FvInfo)->FileSystemGuid can
> be just used for both
> + // firmware file system 2 and 3 format.
> + //
> + ASSERT (CompareGuid (&(((EFI_FIRMWARE_VOLUME_HEADER *)
> FvInfo)->FileSystemGuid), &gEfiFirmwareFileSystem2Guid));
> }
> FvInfoPpi->FvInfo = (VOID *) FvInfo;
> FvInfoPpi->FvInfoSize = FvInfoSize;
> @@ -672,6 +683,7 @@ InternalPeiServicesInstallFvInfoPpi (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO_PPI can not be installed, then
> ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the firmware
> file system 2 format, then ASSERT.
>
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> firmware volume. This parameter is optional and
> @@ -714,6 +726,7 @@ PeiServicesInstallFvInfoPpi (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI can not be installed, then
> ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the firmware
> file system 2 format, then ASSERT.
>
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> firmware volume. This parameter is optional and
> --
> 2.7.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL
2016-10-24 9:15 ` Gao, Liming
@ 2016-10-24 9:21 ` Zeng, Star
0 siblings, 0 replies; 3+ messages in thread
From: Zeng, Star @ 2016-10-24 9:21 UTC (permalink / raw)
To: Gao, Liming, edk2-devel@lists.01.org, Sean Brogan; +Cc: Zeng, Star
You are right, platform may install the FvInfo(2) ppi directly without calling PeiServicesInstallFvInfo(2)Ppi, for example, edk2\Vlv2TbltDevicePkg\FvInfoPei\FvInfoPei.c.
Just sent out another patch to cover it, please help review that.
Thanks,
Star
-----Original Message-----
From: Gao, Liming
Sent: Monday, October 24, 2016 5:16 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL
Star:
Besides this patch, I think PeiCore also requires to add this check to make sure FvInfo and FvImage match each other, because PPI may directly be installed from PEI module, not from this library API.
Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Monday, October 24, 2016 12:56 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [edk2] [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has
> FFS2 format if FvFormat == NULL
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=160
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> MdePkg/Library/PeiServicesLib/PeiServicesLib.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> index 3428addcc63b..2373251bc3e5 100644
> --- a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> +++ b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
> @@ -1,7 +1,7 @@
> /** @file
> Implementation for PEI Services Library.
>
> - Copyright (c) 2006 - 2013, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2006 - 2016, 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 @@ -574,6 +574,7 @@ PeiServicesFfsGetVolumeInfo (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO(2)_PPI can not be installed,
> then ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the
> + firmware
> file system 2 format, then ASSERT.
>
> @param InstallFvInfoPpi Install FvInfo Ppi if it is TRUE. Otherwise, install
> FvInfo2 Ppi.
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> @@ -640,6 +641,16 @@ InternalPeiServicesInstallFvInfoPpi (
> CopyGuid (&FvInfoPpi->FvFormat, FvFormat);
> } else {
> CopyGuid (&FvInfoPpi->FvFormat, &gEfiFirmwareFileSystem2Guid);
> + //
> + // Since the EFI_FIRMWARE_FILE_SYSTEM2_GUID format is assumed if
> NULL is specified for FvFormat,
> + // check the FileSystemGuid pointed by FvInfo against
> EFI_FIRMWARE_FILE_SYSTEM2_GUID to make sure
> + // FvInfo has the firmware file system 2 format.
> + // If the ASSERT really appears, FvFormat needs to be specified
> + correctly,
> for example,
> + // EFI_FIRMWARE_FILE_SYSTEM3_GUID can be used for firmware file
> system 3 format, or
> + // &(((EFI_FIRMWARE_VOLUME_HEADER *) FvInfo)->FileSystemGuid can
> be just used for both
> + // firmware file system 2 and 3 format.
> + //
> + ASSERT (CompareGuid (&(((EFI_FIRMWARE_VOLUME_HEADER *)
> FvInfo)->FileSystemGuid), &gEfiFirmwareFileSystem2Guid));
> }
> FvInfoPpi->FvInfo = (VOID *) FvInfo;
> FvInfoPpi->FvInfoSize = FvInfoSize; @@ -672,6 +683,7 @@
> InternalPeiServicesInstallFvInfoPpi (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO_PPI can not be installed, then
> ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the
> + firmware
> file system 2 format, then ASSERT.
>
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> firmware volume. This parameter is
> optional and @@ -714,6 +726,7 @@ PeiServicesInstallFvInfoPpi (
> the parameters passed in to initialize the fields of the
> EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI instance.
> If the resources can not be allocated for
> EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI, then ASSERT().
> If the EFI_PEI_FIRMWARE_VOLUME_INFO2_PPI can not be installed, then
> ASSERT().
> + If NULL is specified for FvFormat, but FvInfo does not have the
> + firmware
> file system 2 format, then ASSERT.
>
> @param FvFormat Unique identifier of the format of the memory-
> mapped
> firmware volume. This parameter is
> optional and
> --
> 2.7.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-24 9:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-24 4:56 [PATCH] MdePkg PeiServicesLib: Make sure FvInfo has FFS2 format if FvFormat == NULL Star Zeng
2016-10-24 9:15 ` Gao, Liming
2016-10-24 9:21 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox