[PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[fengyunhua]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3120
FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD TOKEN.
Their PCD TOKEN value can always be zero. If so, AutoGen.h will not be
changed when PCD is added or removed.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Yunhua Feng <fengyunhua@byosoft.com.cn>
---
 BaseTools/Source/Python/AutoGen/GenC.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py b/BaseTools/Source/Python/AutoGen/GenC.py
index a2053d5485..ac561ba82e 100755
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -913,6 +913,8 @@ def CreateModulePcdCode(Info, AutoGenC, AutoGenH, Pcd):
                                 ExtraData="[%s]" % str(Info))
         else:
             TokenNumber = PcdTokenNumber[Pcd.TokenCName, Pcd.TokenSpaceGuidCName]
+        if Pcd.Type not in PCD_DYNAMIC_TYPE_SET:
+            TokenNumber = 0
         AutoGenH.Append('\n#define %s  %dU\n' % (PcdTokenName, TokenNumber))
 
     EdkLogger.debug(EdkLogger.DEBUG_3, "Creating code for " + TokenCName + "." + Pcd.TokenSpaceGuidCName)
-- 
2.27.0.windows.1

Re: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[Bob Feng]

Yunhua, if FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD TOKEN, maybe it's better to remove "#define PcdTokenName TOKEN" statement for those static PCD from AutoGen.h.

Thanks,
Bob

-----Original Message-----
From: Yunhua Feng <fengyunhua@byosoft.com.cn> 
Sent: Wednesday, December 16, 2020 9:03 AM
To: devel@edk2.groups.io
Cc: Feng, Bob C <bob.c.feng@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
Subject: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3120
FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD TOKEN.
Their PCD TOKEN value can always be zero. If so, AutoGen.h will not be changed when PCD is added or removed.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
Signed-off-by: Yunhua Feng <fengyunhua@byosoft.com.cn>
---
 BaseTools/Source/Python/AutoGen/GenC.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py b/BaseTools/Source/Python/AutoGen/GenC.py
index a2053d5485..ac561ba82e 100755
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -913,6 +913,8 @@ def CreateModulePcdCode(Info, AutoGenC, AutoGenH, Pcd):
                                 ExtraData="[%s]" % str(Info))
         else:
             TokenNumber = PcdTokenNumber[Pcd.TokenCName, Pcd.TokenSpaceGuidCName]
+        if Pcd.Type not in PCD_DYNAMIC_TYPE_SET:
+            TokenNumber = 0
         AutoGenH.Append('\n#define %s  %dU\n' % (PcdTokenName, TokenNumber))
 
     EdkLogger.debug(EdkLogger.DEBUG_3, "Creating code for " + TokenCName + "." + Pcd.TokenSpaceGuidCName)
--
2.27.0.windows.1

回复: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[gaoliming]

Bob:
  Yes. Token value is designed for Dynamic and DynamicEx PCD. It is not used
for static PCD. I propose to use EFI_PCD_INVALID_TOKEN_NUMBER (0) for the
static PCD token value. If so, the consumer code can base on PcdToken value
to know whether this PCD is dynamic or not.

Thanks
Liming
> -----邮件原件-----
> 发件人: Feng, Bob C <bob.c.feng@intel.com>
> 发送时间: 2020年12月16日 9:26
> 收件人: Yunhua Feng <fengyunhua@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine
> <yuwei.chen@intel.com>
> 主题: RE: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero
> for static PCD
> 
> Yunhua, if FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use
> PCD TOKEN, maybe it's better to remove "#define PcdTokenName TOKEN"
> statement for those static PCD from AutoGen.h.
> 
> Thanks,
> Bob
> 
> -----Original Message-----
> From: Yunhua Feng <fengyunhua@byosoft.com.cn>
> Sent: Wednesday, December 16, 2020 9:03 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
> Subject: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero
> for static PCD
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3120
> FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD TOKEN.
> Their PCD TOKEN value can always be zero. If so, AutoGen.h will not be
> changed when PCD is added or removed.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Signed-off-by: Yunhua Feng <fengyunhua@byosoft.com.cn>
> ---
>  BaseTools/Source/Python/AutoGen/GenC.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py
> b/BaseTools/Source/Python/AutoGen/GenC.py
> index a2053d5485..ac561ba82e 100755
> --- a/BaseTools/Source/Python/AutoGen/GenC.py
> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> @@ -913,6 +913,8 @@ def CreateModulePcdCode(Info, AutoGenC,
> AutoGenH, Pcd):
>                                  ExtraData="[%s]" % str(Info))
>          else:
>              TokenNumber = PcdTokenNumber[Pcd.TokenCName,
> Pcd.TokenSpaceGuidCName]
> +        if Pcd.Type not in PCD_DYNAMIC_TYPE_SET:
> +            TokenNumber = 0
>          AutoGenH.Append('\n#define %s  %dU\n' % (PcdTokenName,
> TokenNumber))
> 
>      EdkLogger.debug(EdkLogger.DEBUG_3, "Creating code for " +
> TokenCName + "." + Pcd.TokenSpaceGuidCName)
> --
> 2.27.0.windows.1
> 

Re: [edk2-devel] 回复: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[Bob Feng]

Hi Liming,

So the PCD TOKEN is still useful for static PCD. 

For the sentence of "AutoGen.h will not be changed when PCD is added or removed.", 
I think if a user adds or deletes a dynamic PCD, the AutoGen.h will still change. This patch need update the commit message.

For the code, I think it would be better to write "#define PcdTokenName EFI_PCD_INVALID_TOKEN_NUMBER" to AutoGen.h and add some comments about the usage of EFI_PCD_INVALID_TOKEN_NUMBER.
Currently, PlatformAutoGen.PcdTokenNumber() still calculate the PCD TOKEN for static PCDs. I think that logic can also be deleted.

Thanks,
Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Wednesday, December 16, 2020 10:10 AM
To: Feng, Bob C <bob.c.feng@intel.com>; 'Yunhua Feng' <fengyunhua@byosoft.com.cn>; devel@edk2.groups.io
Cc: Chen, Christine <yuwei.chen@intel.com>
Subject: [edk2-devel] 回复: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

Bob:
  Yes. Token value is designed for Dynamic and DynamicEx PCD. It is not used for static PCD. I propose to use EFI_PCD_INVALID_TOKEN_NUMBER (0) for the static PCD token value. If so, the consumer code can base on PcdToken value to know whether this PCD is dynamic or not.

Thanks
Liming
> -----邮件原件-----
> 发件人: Feng, Bob C <bob.c.feng@intel.com>
> 发送时间: 2020年12月16日 9:26
> 收件人: Yunhua Feng <fengyunhua@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine 
> <yuwei.chen@intel.com>
> 主题: RE: [PATCH] BaseTools: Should always define PCD TOKEN value as 
> Zero for static PCD
> 
> Yunhua, if FixedAtBuild, PatchableInModule and FeatureFlag PCD don't 
> use PCD TOKEN, maybe it's better to remove "#define PcdTokenName TOKEN"
> statement for those static PCD from AutoGen.h.
> 
> Thanks,
> Bob
> 
> -----Original Message-----
> From: Yunhua Feng <fengyunhua@byosoft.com.cn>
> Sent: Wednesday, December 16, 2020 9:03 AM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
> Subject: [PATCH] BaseTools: Should always define PCD TOKEN value as 
> Zero for static PCD
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3120
> FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD TOKEN.
> Their PCD TOKEN value can always be zero. If so, AutoGen.h will not be 
> changed when PCD is added or removed.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Signed-off-by: Yunhua Feng <fengyunhua@byosoft.com.cn>
> ---
>  BaseTools/Source/Python/AutoGen/GenC.py | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py
> b/BaseTools/Source/Python/AutoGen/GenC.py
> index a2053d5485..ac561ba82e 100755
> --- a/BaseTools/Source/Python/AutoGen/GenC.py
> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> @@ -913,6 +913,8 @@ def CreateModulePcdCode(Info, AutoGenC, AutoGenH, 
> Pcd):
>                                  ExtraData="[%s]" % str(Info))
>          else:
>              TokenNumber = PcdTokenNumber[Pcd.TokenCName, 
> Pcd.TokenSpaceGuidCName]
> +        if Pcd.Type not in PCD_DYNAMIC_TYPE_SET:
> +            TokenNumber = 0
>          AutoGenH.Append('\n#define %s  %dU\n' % (PcdTokenName,
> TokenNumber))
> 
>      EdkLogger.debug(EdkLogger.DEBUG_3, "Creating code for " + 
> TokenCName + "." + Pcd.TokenSpaceGuidCName)
> --
> 2.27.0.windows.1
> 

回复: [edk2-devel] 回复: [PATCH] BaseTools: Should always define PCD TOKEN value as Zero for static PCD

[gaoliming]

Bob:
  You are right. This patch is to avoid autogen header be changed when static PCD is added or removed. I will update BZ for this detail. 

  And, EFI_PCD_INVALID_TOKEN_NUMBER is defined in Ppi\Pcd.h. It is not included by AutoGen.h. I don't expect to include more header files into AutoGen.h. 
  So, I suggest to generate 0 as the token value for static PCD with the comments // EFI_PCD_INVALID_TOKEN_NUMBER.

Thanks
Liming
> -----邮件原件-----
> 发件人: Feng, Bob C <bob.c.feng@intel.com>
> 发送时间: 2020年12月16日 14:16
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; 'Yunhua Feng'
> <fengyunhua@byosoft.com.cn>
> 抄送: Chen, Christine <yuwei.chen@intel.com>
> 主题: RE: [edk2-devel] 回复: [PATCH] BaseTools: Should always define PCD
> TOKEN value as Zero for static PCD
> 
> Hi Liming,
> 
> So the PCD TOKEN is still useful for static PCD.
> 
> For the sentence of "AutoGen.h will not be changed when PCD is added or
> removed.",
> I think if a user adds or deletes a dynamic PCD, the AutoGen.h will still change.
> This patch need update the commit message.
> 
> For the code, I think it would be better to write "#define PcdTokenName
> EFI_PCD_INVALID_TOKEN_NUMBER" to AutoGen.h and add some comments
> about the usage of EFI_PCD_INVALID_TOKEN_NUMBER.
> Currently, PlatformAutoGen.PcdTokenNumber() still calculate the PCD TOKEN
> for static PCDs. I think that logic can also be deleted.
> 
> Thanks,
> Bob
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
> Sent: Wednesday, December 16, 2020 10:10 AM
> To: Feng, Bob C <bob.c.feng@intel.com>; 'Yunhua Feng'
> <fengyunhua@byosoft.com.cn>; devel@edk2.groups.io
> Cc: Chen, Christine <yuwei.chen@intel.com>
> Subject: [edk2-devel] 回复: [PATCH] BaseTools: Should always define PCD
> TOKEN value as Zero for static PCD
> 
> Bob:
>   Yes. Token value is designed for Dynamic and DynamicEx PCD. It is not used
> for static PCD. I propose to use EFI_PCD_INVALID_TOKEN_NUMBER (0) for
> the static PCD token value. If so, the consumer code can base on PcdToken
> value to know whether this PCD is dynamic or not.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Feng, Bob C <bob.c.feng@intel.com>
> > 发送时间: 2020年12月16日 9:26
> > 收件人: Yunhua Feng <fengyunhua@byosoft.com.cn>;
> devel@edk2.groups.io
> > 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Chen, Christine
> > <yuwei.chen@intel.com>
> > 主题: RE: [PATCH] BaseTools: Should always define PCD TOKEN value as
> > Zero for static PCD
> >
> > Yunhua, if FixedAtBuild, PatchableInModule and FeatureFlag PCD don't
> > use PCD TOKEN, maybe it's better to remove "#define PcdTokenName
> TOKEN"
> > statement for those static PCD from AutoGen.h.
> >
> > Thanks,
> > Bob
> >
> > -----Original Message-----
> > From: Yunhua Feng <fengyunhua@byosoft.com.cn>
> > Sent: Wednesday, December 16, 2020 9:03 AM
> > To: devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Chen, Christine <yuwei.chen@intel.com>
> > Subject: [PATCH] BaseTools: Should always define PCD TOKEN value as
> > Zero for static PCD
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3120
> > FixedAtBuild, PatchableInModule and FeatureFlag PCD don't use PCD
> TOKEN.
> > Their PCD TOKEN value can always be zero. If so, AutoGen.h will not be
> > changed when PCD is added or removed.
> >
> > Cc: Bob Feng <bob.c.feng@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Yuwei Chen <yuwei.chen@intel.com>
> > Signed-off-by: Yunhua Feng <fengyunhua@byosoft.com.cn>
> > ---
> >  BaseTools/Source/Python/AutoGen/GenC.py | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/GenC.py
> > b/BaseTools/Source/Python/AutoGen/GenC.py
> > index a2053d5485..ac561ba82e 100755
> > --- a/BaseTools/Source/Python/AutoGen/GenC.py
> > +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> > @@ -913,6 +913,8 @@ def CreateModulePcdCode(Info, AutoGenC,
> AutoGenH,
> > Pcd):
> >                                  ExtraData="[%s]" % str(Info))
> >          else:
> >              TokenNumber = PcdTokenNumber[Pcd.TokenCName,
> > Pcd.TokenSpaceGuidCName]
> > +        if Pcd.Type not in PCD_DYNAMIC_TYPE_SET:
> > +            TokenNumber = 0
> >          AutoGenH.Append('\n#define %s  %dU\n' % (PcdTokenName,
> > TokenNumber))
> >
> >      EdkLogger.debug(EdkLogger.DEBUG_3, "Creating code for " +
> > TokenCName + "." + Pcd.TokenSpaceGuidCName)
> > --
> > 2.27.0.windows.1
> >
> 
> 
> 
> 
> 
> 
>