RE: [Edk2 Dev] Pcds in AutoGen.h vs AutoGen.c

Subject: RE: [Edk2 Dev] Pcds in AutoGen.h vs AutoGen.c

From: Lu ken <ken.lu@intel.com>

To: dev@edk2.tianocore.org

Date: 2009-06-16 17:51:52

Hi, Carl:
	Thanks for you thinking the maintenance problem. How about change the comments from detail value enumeration to header file path? So it can help your tools the figure out where the mask PCD value come from
	I notice there is another similar cases that there are many PcdStatusCodeValueXXX exist in MdePkg.dec file, the value is computed from IntelFrameworkPkg\Include\Framework\StatusCode.h file, but the definitions in status code do not exists in PI/UEFI specification, so PcdStatusCodeValueXXX is used computed value in MdePkg.dec file. Does your tool could handle this case?

	I know the current implementation of inheriting for PCD/ overriding for Library instance maybe opaque. 
	For PCD inheriting, I think the big reason is to let different roles focus the things they need, for example:
	1) Package/module developer would focus on defining whole definitions including PCD, they should know how to make release package more flexible for different architecture and different platform: mobile/desktop/server.
	2) Platform developer would focus on integration, he need not has knowledge to configure everything, but select the configuration he or his platform cared to configure.
	
	For override library instance/PCD, I give the following example to express how it make flexible. For example, platform developer could choice following ways for debug output
	a) Turn on the debug output for whole platform. So he needs setting following PCD in DSC common [PcdsFixedAtBuild] section:
      gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
      gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
      gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000042
	And he need select debug version library instances for DebugLib, ReportStatusCodeLib, SerialPortLib in DSC's common library instance section, for example:
	DebugLib|IntelFrameworkModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
	ReportStatusCodeLib|DuetPkg/Library/DxeCoreReportStatusCodeLibFromHob/DxeCoreReportStatusCodeLibFromHob.inf
    SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
	b) Turn off all above setting in common PCD/library section in DSC file and just only turn on above setting in special modules, please ref to DuetPkg\DuetPkg.dsc, I intent gives a example in this platform DSC, you could see all of debug output is turned off in duet platform DSC except MdeModulePkg/Core/Dxe/DxeMain.inf module in line 103.

	I think great tool will help to reduce the complexity and make flexible explicitly. 
	
Best Regard
K
---------------------------
hear and you forget; see and you remember; do and you understand


-----Original Message-----
From: Carl Norum [mailto:carl.norum@apple.com] 
Sent: 2009616 0:21
To: dev@edk2.tianocore.org
Subject: Re: [Edk2 Dev] Pcds in AutoGen.h vs AutoGen.c

That's a good interim solution, but it does have a maintenance problem  
keeping the code in sync with the comments in the dec.  Not to mention  
any overrides in a dsc file will still be opaque.  I am contemplating  
syntax now and I hope to have a patch ready sometime this week (not to  
say that the lawyers will let it out, but we can hope).

-- Carl


On Jun 14, 2009, at 6:51 PM, Lu ken wrote:

> Hi, Carl:
> 	I think it will be cool if some tools could help to improve and  
> checking building meta files, currently, build tools does limited  
> checking for performance consideration.
>
> 	Yes I agree with you it is hard to write and read the PCD value  
> which is mask value or enumerated value. I am thinking to add more  
> comment in *.dec file of PCD's definition. For example:
> 	## the value of this PCD comes from MdePkg\Include\Library\DebugLib.h
>      # 	DEBUG_INIT      0x00000001  // Initialization
>  	# 	DEBUG_WARN      0x00000002  // Warnings
> 	# 	DEBUG_LOAD      0x00000004  // Load events
> 	# 	DEBUG_FS        0x00000008  // EFI File system
> 	# 	DEBUG_POOL      0x00000010  // Alloc & Free's
> 	# 	DEBUG_PAGE      0x00000020  // Alloc & Free's
> 	# 	DEBUG_INFO      0x00000040  // Verbose
> 	# 	DEBUG_DISPATCH  0x00000080  // PEI/DXE Dispatchers
> 	# 	DEBUG_VARIABLE  0x00000100  // Variable
> 	# 	DEBUG_BM        0x00000400  // Boot Manager
> 	# 	DEBUG_BLKIO     0x00001000  // BlkIo Driver
> 	# 	DEBUG_NET       0x00004000  // SNI Driver
> 	# 	DEBUG_UNDI      0x00010000  // UNDI Driver
> 	# 	DEBUG_LOADFILE  0x00020000  // UNDI Driver
> 	# 	DEBUG_EVENT     0x00080000  // Event messages
> 	# 	DEBUG_ERROR     0x80000000  // Error	 
> gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000|UINT32| 
> 0x00000006
>
> 	
> Best Regard
>
> K
> Tiano Core Team
> ------------------------------------
> Hear and you forget; see and you remember; do and you understand
>
>
> -----Original Message-----
> From: Carl Norum [mailto:carl.norum@apple.com]
> Sent: Saturday, June 13, 2009 1:28 AM
> To: dev@edk2.tianocore.org
> Subject: Re: [Edk2 Dev] Pcds in AutoGen.h vs AutoGen.c
>
> Thanks guys,
>
> I got it all figured out - the answer was "DependentLibraryList" in
> ModuleAutoGen.  The goal was to add support for enumerated types to
> the build system for PCD settings.  That will let us use symbolic
> names for things like the Debug flag mask, among other uses.
> Particularly for embedded system development it will come in handy.
> I'm sure we'll be able to contribute some patches back soon - have to
> wait for Andrew to get back from vacation to work out the details with
> the lawyers.
>
> I have it working now - have to work on some syntax and type checking
> improvements and it should be pretty cool.
>
> -- Carl
>
>
> On Jun 11, 2009, at 11:54 PM, Lu ken wrote:
>
>> Hi Carl Norum:
>> 	In edk2, I think the PCD's usage model is very flexible:
>> 	1) Platform developer could choice different PCD type for a PCD
>> entry according to platform policy in platform scope, for example,
>> PcdPciExpressBaseAddress is fixed at build in some platform, but is
>> dynamic in other platform;
>> 	2) Platform developer could choice different PCD value for
>> different platform even for different module in one platform.
>>
>> 	The flexibility of usage model brings the little complexity of
>> implementation. When doing a platform building, build tools will
>> determine each PCD's type and value for each module at build time.
>> So I think this is the reason that why edk2 need autogen.h/autogen.c
>> to give the definitions of PCD and PCD's value. If you interesting
>> the whole build work flow in python code, I could explain it in
>> detail.
>>
>> 	I am *not* sure your first question, do you mean you want to write
>> tool to analysis autogen.h/autogen.c to test the feature is
>> correctly adopted by driver? In another words, are you going to
>> write build testing tool?
>>
>> 	For you second question, you can ref the build tools source from https://buildtools.tianocore.org/source/browse/buildtools/
>> .
>>
>> 	Thanks.
>>
>> Best Regard
>>
>> K
>> Tiano Core Team
>> ------------------------------------
>> Hear and you forget; see and you remember; do and you understand
>>
>> -----Original Message-----
>> From: Carl Norum [mailto:carl.norum@apple.com]
>> Sent: 2009612 2:43
>> To: dev@edk2.tianocore.org; dev@buildtools.tianocore.org
>> Subject: [Edk2 Dev] Pcds in AutoGen.h vs AutoGen.c
>>
>> Hi everybody,
>>
>> I'm doing some build system experimentation with Edk2.  What's the
>> reasoning behind some PCDs ending up in AutoGen.h vs. AutoGen.c, and
>> where does that decision get made?  Right now my changes are getting
>> output through to the AutoGen.h files for any driver/library that
>> directly includes a .dec file where my feature has been added, but  
>> any
>> implicit ones don't come through.  Where in the build system does  
>> that
>> hierarchical step take place?
>>
>> Thanks for any help!
>>
>> -- 
>> Carl Norum
>> Firmware Engineer
>> Apple, Inc.
>> Cupertino, CA
>>
>> ------------------------------------------------------
>> https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44405
>>
>> To unsubscribe from this discussion, please e-mail [unsubscribeURL]
>>
>> ------------------------------------------------------
>> https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44465
>>
>> To unsubscribe from this discussion, please e-mail [unsubscribeURL]
>
> ------------------------------------------------------
> https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44561
>
> To unsubscribe from this discussion, please e-mail [unsubscribeURL]
>
> ------------------------------------------------------
> https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44734
>
> To unsubscribe from this discussion, please e-mail [unsubscribeURL]

------------------------------------------------------
https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44773

To unsubscribe from this discussion, please e-mail [unsubscribeURL]

------------------------------------------------------
https://edk2.tianocore.org/ds/viewMessage.do?dsForumId=135&dsMessageId=44817

To unsubscribe from this discussion, please e-mail [unsubscribeURL]