Re: [PATCH V4 06/12] SignedCapsulePkg/CapsulePkg.dec: Add capsule related definition.

["Yao, Jiewen" <jiewen.yao@intel.com>]

I do think {0} is supported. I do not see any error reported if I use {0}.

If we do not want to support {0}, I suggest we report an error to the end user.

Thank you
Yao Jiewen

From: Zhu, Yonghong
Sent: Wednesday, October 26, 2016 8:56 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
Subject: RE: [PATCH V4 06/12] SignedCapsulePkg/CapsulePkg.dec: Add capsule related definition.

Per DEC spec, we only support {0x0},  not support {0} and {}.  The value in the { } must be a Hex value (start with '0x' or '0X').

I can enhance the error handling to report reasonable error message but not this crash. There have a similar bug in Bugzilla https://bugzilla.tianocore.org/show_bug.cgi?id=159

Best Regards,
Zhu Yonghong

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Wednesday, October 26, 2016 7:51 AM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: Re: [edk2] [PATCH V4 06/12] SignedCapsulePkg/CapsulePkg.dec: Add capsule related definition.

Hi Mike
I fixed all according to your suggestion, exception "Is the intended default value {0x0}, or is the intended default value empty {}?"

I can only use {0} or {0x0}, but not {}. Build.py crash if I use {}.

===============================
build...
: error C0DE: Unknown fatal error when processing [c:\home\edkiigit\edk2\Signed CapsulePkg\SignedCapsulePkg.dec]

(Please send email to edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> for help, attaching following call stack trace!)

(Python 2.7.3 on win32) Traceback (most recent call last):
  File "build\build.py", line 2274, in Main
  File "build\build.py", line 2026, in Launch
  File "build\build.py", line 1821, in _MultiThreadBuildPlatform
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGe
n\AutoGen.py", line 176, in __new__
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGe
n\AutoGen.py", line 547, in _Init
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceDatabase.py", line 767, in _GetPcds
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceDatabase.py", line 846, in _GetPcd
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceDatabase.py", line 708, in _ValidatePcd
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceCommon.py", line 50, in GetDeclaredPcd
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceDatabase.py", line 1556, in _GetPcds
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\WorkspaceDatabase.py", line 1574, in _GetPcd
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\MetaFileParser.py", line 239, in __getitem__
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\MetaFileParser.py", line 1671, in Start
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\MetaFileParser.py", line 43, in MacroParser
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Worksp
ace\MetaFileParser.py", line 1886, in _PcdParser
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 275, in __call__
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 320, in _OrExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 324, in _AndExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 328, in _BitOr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 332, in _BitXor
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 336, in _BitAnd
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 340, in _EqExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 356, in _RelExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 360, in _AddExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 308, in _ExprFuncTemplate
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 371, in _UnaryExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 375, in _IdenExpr
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 536, in _GetToken
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 495, in __GetNList
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 567, in _GetSingleToken
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 411, in __GetArray
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 495, in __GetNList
  File "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\Common
\Expression.py", line 573, in _GetSingleToken
BadExpression: No more valid token found from rest of string: [}].

From: Kinney, Michael D
Sent: Wednesday, October 26, 2016 6:46 AM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Subject: RE: [PATCH V4 06/12] SignedCapsulePkg/CapsulePkg.dec: Add capsule related definition.

Jiewen,

Comments included inline.

Mike

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Saturday, October 22, 2016 7:22 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>;
> Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>>; Kinney,
> Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; Gao,
> Liming <liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>>>; Zhang,
> Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> Subject: [PATCH V4 06/12] SignedCapsulePkg/CapsulePkg.dec: Add capsule
> related definition.
>
> 1) Add capsule related GUID.
>    EdkiiSystemFmpCapsule
> 2) Add capsule related library.
>    EdkiiSystemCapsuleLib
>    IniParsingLib
>    PlatformFlashAccessLib
> 5) Add EDKII system capsule related DynamicEx PCD
>    PcdEdkiiSystemFirmwareImageDescriptor
>    PcdEdkiiSystemFirmwareFileGuid
>    NOTE: We use DynamicEx here because the update driver may be in
>    the capsule FMP, instead of system firmware.
>    The update driver MUST use the PCD info produced system firmware.
>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>>
> Cc: Michael D Kinney
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>
> Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>>>
> Cc: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>
> Reviewed-by: Liming Gao
> <liming.gao@intel.com<mailto:liming.gao@intel.com<mailto:liming.gao@intel.com%3cmailto:liming.gao@intel.com>>>
> ---
>  SignedCapsulePkg/SignedCapsulePkg.dec | 65 ++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/SignedCapsulePkg/SignedCapsulePkg.dec
> b/SignedCapsulePkg/SignedCapsulePkg.dec
> new file mode 100644
> index 0000000..c03f70c
> --- /dev/null
> +++ b/SignedCapsulePkg/SignedCapsulePkg.dec
> @@ -0,0 +1,65 @@
> +## @file
> +# This package provides EDKII capsule related support.
> +#
> +# Copyright (c) 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 that 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]
> +  DEC_SPECIFICATION              = 0x00010005
> +  PACKAGE_NAME                   = SignedCapsulePkg
> +  PACKAGE_GUID                   = 75AA5D82-7BC4-44A9-82FB-0820EBC79BED
> +  PACKAGE_VERSION                = 0.1
> +
> +[Includes]
> +  Include
> +
> +[LibraryClasses]
> +  ## @libraryclass  Provides services for EDKII system FMP capsule.
> +  #
> +  EdkiiSystemCapsuleLib|Include/Library/EdkiiSystemCapsuleLib.h
> +
> +  ## @libraryclass  Provides services to parse the INI configuration file.
> +  #
> +  IniParsingLib|Include/Library/IniParsingLib.h
> +
> +  ## @libraryclass  Provides services to access flash device.
> +  #
> +  PlatformFlashAccessLib|Include/Library/PlatformFlashAccessLib.h
> +
> +[Guids]
> +  gEfiSignedCapsulePkgTokenSpaceGuid      = { 0xe1eb612f, 0x1c6c, 0x485d, { 0x9d, 0x6,
> 0x65, 0x8, 0x44, 0x88, 0x15, 0x69 }}
> +
> +  ## Include/Guid/EdkiiSystemFmpCapsule.h
> +  gEdkiiSystemFirmwareImageDescriptorFileGuid = {0x90b2b846, 0xca6d,
> + 0x4d6e, {0xa8,
> 0xd3, 0xc1, 0x40, 0xa8, 0xe1, 0x10, 0xac}}
> +  gEdkiiSystemFmpCapsuleConfigFileGuid        = {0x812136d3, 0x4d3a, 0x433a, {0x94,
> 0x18, 0x29, 0xbb, 0x9b, 0xf7, 0x8f, 0x6e}}
> +  gEdkiiSystemFmpCapsuleDriverFvFileGuid      = {0xce57b167, 0xb0e4, 0x41e8, {0xa8,
> 0x97, 0x5f, 0x4f, 0xeb, 0x78, 0x1d, 0x40}}
> +
> +[PcdsDynamicEx]
> +  ## This dynamic PCD hold the EDKII system firmware image descriptor.

Typo.  Should be "holds"

> +  #  This information can be used for version check in EDKII system FMP capsule.
> +  #  Only if the new EdkiiSystemFrimwareImageDescriptor.Version is no
> + less than

Typo.  Should be "not less than".

> +  #  the current
> + PcdEdkiiSystemFirmwareImageDescriptor.LowestSupportedVersion,
> +  #  the EDKII system FmpCapsule will be processed.
> +  #  It must be dynamicEx, because the EDKII system firmware update
> + module may

Should be "Is must be in [PcdsDynamicEx], because"

> +  #  consume the PCD produced in current system firmware.
> +  # @Prompt EDKII system firwmare image descriptor.

Typo.  Should be "system firmware image"


The name of C structure type associated with this VOID* PCD should be included in the description so a developer knows how to fill in all the byte values.

> +
> gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescript
> or|{0}|VOID*|0x00
> 000037

Build does not work with this PCD.  The value 0 is not a hex value.  It must be 0x0.
Is the intended default value {0x0}, or is the intended default value empty {}?

> +
> +  ## This dynamic PCD hold the GUID of a firmware FFS which includes
> + EDKII  #  system firmware image.
> +  #  An EDKII system firmware udpate module need consume this PCD to
> + extract

Typos.  Should be "system firmware update module consumes this PCD".

> +  #  the EDKII system firmware from the capsule image.
> +  #  It must be dynamicEx, because the EDKII system firmware update
> + module may

Should be "It must be in [PcdsDynamicEx], because"

> +  #  consume the PCD produced in current system firmware.
> +  # @Prompt EDKII system firmware image FFS GUID.
> +
> + gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid|{0
> + xb2, 0x9e, 0x9c,
> 0xaf, 0xad, 0x12, 0x3e, 0x4d, 0xa4, 0xd4, 0x96, 0xf6, 0xc9, 0x96,
> 0x62,
> 0x15}|VOID*|0x00001010
> +
> --
> 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