Mike,
I did a quick prototype that just outputs the current PCD DSC syntax with an
extra | C typedef name at the end when you build. The example includes a
structure with bit-fields and a device path.
build -p MdeModulePkg/MdeModulePkg.dsc -a X64 -t XCODE5
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
[PcdsFixedAtBuild.common]
gMdeModulePkgTokenGuid.PcdFakePcdExample| { 0x53, 0x47, 0xc1, 0xe0, 0xbe,
0xf9, 0xd2, 0x11, 0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d, 0x21, 0x30,
0x04, 0x00, 0x03, 0x00, 0x00, 0x00,} | FAKE_PCD_EXAMPLE
gMdeModulePkgTokenGuid.PcdFakeDevicePathExample| { 0x03, 0x0f, 0x0b, 0x00,
0xff, 0xff, 0xff, 0xff, 0x03, 0x01, 0x01, 0x7f, 0xff, 0x04, 0x00,} |
EFI_DEVICE_PATH_PROTOCOL
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
There are a few hacks:
1) The parser is not very robust from an error handling point of view.
2) Some of the compiler flags are missing. The reason is I could not undo the
-E from the flags I was trying to reuse. So this may take a little flags
cleanup to work.
3) I only had enough time to do the XCODE version, and I set the flags needed
to make sure you get the EFI Spec structure packing.
Thanks,
Andrew Fish
diff --git a/BaseTools/BinWrappers/PosixLike/StructuredPcdAutoGen.py
b/BaseTools/BinWrappers/PosixLike/StructuredPcdAutoGen.py
new file mode 100755
index 0000000..82a9b3b
--- /dev/null
+++ b/BaseTools/BinWrappers/PosixLike/StructuredPcdAutoGen.py
@@ -0,0 +1,85 @@
+#!/usr/bin/python -u
+## @file
+# Prototype .pcd parser
+#
+# Parse the meta data
+#// @PCDTYPE [PcdsFixedAtBuild.common]
+#// @PCD
gMdeModulePkgTokenGuid.PcdFakePcdExample|FAKE_PCD_EXAMPLE|gFakePcdExample
+#
+# Create a C application to generate the PCD data.
+#
+# Copyright (c) 2016, Apple Inc. 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
+# 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.
+#
+
+import sys
+
+programStart = '''
+// AutoGen file do not hand edit.
+#include <stdio.h>
+
+#ifdef NULL
+#undef NULL
+#endif
+
+#include "%s"
+
+int
+main (int argv, char **argc)
+{
+ unsigned char *c;
+ int i;
+
+'''
+
+programWork = '''
+ printf (" @1| {");
+ for (i=0, c = (unsigned char *)(unsigned long long) &@2; i < sizeof (@2);
i++) {
+ printf (" 0x%02x,", c[i]);
+ }
+ printf ("} | @3\\n");
+'''
+
+""
+
+programEnd = '''
+ printf ("\\n");
+ return 0;
+}
+
+'''
+
+def main (filename):
+ ''' strip out lines that do not start with @, and the @'''
+ print programStart % filename
+
+ pcd = {}
+ pcdType = ''
+ with open(filename) as openfile:
+ for line_of_c in openfile.readlines():
+ line = line_of_c.split()
+ if len(line) >= 3 and line[0] == '//':
+ if '@PCDTYPE' in line[1]:
+ #// @PCDTYPE [PcdsFixedAtBuild.common]
+ print ' printf ("%s\\n");' % line[2]
+ if '@PCD' in line[1]:
+ #// @PCD
gMdeModulePkgTokenGuid.PcdFakePcdExample|FAKE_PCD_EXAMPLE|gFakePcdExample
+ PcdEntry = line[2].split('|')
+ if len (PcdEntry) >= 3:
+ print programWork.replace('@1',
PcdEntry[0]).replace('@2', PcdEntry[2]).replace('@3', PcdEntry[1])
+
+ print programEnd
+
+if __name__ == "__main__":
+ if len (sys.argv) < 2:
+ sys.exit()
+
+ main (sys.argv[1])
+
+
diff --git a/BaseTools/Conf/build_rule.template
b/BaseTools/Conf/build_rule.template
index 91bcc18..3d2c47f 100644
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -604,3 +604,21 @@
GenFw -o $(OUTPUT_DIR)(+)$(MODULE_NAME)hii.rc -g $(MODULE_GUID)
--hiibinpackage $(HII_BINARY_PACKAGES)
+[Pcd-Code-File]
+ <InputFile>
+ ?.pcd
+
+ <OutputFile>
+ $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}
+
+ <ExtraDependency>
+ $(MAKE_FILE)
+
+ <Command.XCODE>
+ StructuredPcdAutoGen.py ${src} >
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.AutoGen.c
+ "$(CC)" -mms-bitfields -fshort-wchar -funsigned-char $(INC) -o
$(OUTPUT_DIR)(+)${s_dir}(+)AutoGenTool
$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.AutoGen.c
+ @echo
"\n+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++"
+ @$(OUTPUT_DIR)(+)${s_dir}(+)AutoGenTool
+ @echo
"+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n"
+
+
diff --git a/MdeModulePkg/Include/Pcd/FakeExample.h
b/MdeModulePkg/Include/Pcd/FakeExample.h
new file mode 100644
index 0000000..06d5526
--- /dev/null
+++ b/MdeModulePkg/Include/Pcd/FakeExample.h
@@ -0,0 +1,33 @@
+/** @file
+ Example of Structured PCDs include file.
+
+ Copyright (c) 2016, Apple Inc. 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
+ 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.
+
+**/
+
+#ifndef __FAKE_EXAMPLE_H__
+#define __FAKE_EXAMPLE_H__
+
+typedef struct {
+ UINT32 BitsA:4;
+ UINT32 BitsB:8;
+ UINT32 BitsC:4;
+ UINT32 BitsD:8;
+} EXAMPLE_BITFIELD;
+
+typedef struct {
+ EFI_GUID Compression;
+ EXAMPLE_BITFIELD BitField;
+ UINT8 ClassCode[3];
+} FAKE_PCD_EXAMPLE;
+
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 936f46d..4dbedd6 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -187,6 +187,9 @@
[PcdsFixedAtBuild.IPF]
gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000
+[Components]
+ MdeModulePkg/Pcd/Example.inf
+
###################################################################################################
#
# Components Section - list of the modules and components that will be
processed by compilation
diff --git a/MdeModulePkg/Pcd/Example.inf b/MdeModulePkg/Pcd/Example.inf
new file mode 100644
index 0000000..b12c383
--- /dev/null
+++ b/MdeModulePkg/Pcd/Example.inf
@@ -0,0 +1,31 @@
+## @file
+# Example of Structured PCDs set via C include infrastructure
+#
+# Portions copyright (c) 2016, Apple Inc. 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
+# 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]
+ INF_VERSION = 0x00010005
+ BASE_NAME = ExamplePcd
+ FILE_GUID = C8BE4EB8-2035-11E6-9969-B8E8562CBAFA
+ MODULE_TYPE = USER_DEFINED
+ VERSION_STRING = 1.0
+
+[Sources]
+ Example.pcd
+
+[Sources.IA32]
+ YouMustCompileInX64Mode.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
diff --git a/MdeModulePkg/Pcd/Example.pcd b/MdeModulePkg/Pcd/Example.pcd
new file mode 100644
index 0000000..f01dcd8
--- /dev/null
+++ b/MdeModulePkg/Pcd/Example.pcd
@@ -0,0 +1,74 @@
+/** @file
+ Example of Structured PCDs set via C include infrastructure
+
+ Copyright (c) 2016, Apple Inc. 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
+ 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.
+
+**/
+
+#include <Uefi.h>
+#include <IndustryStandard/Pci.h>
+#include <Guid/PcAnsi.h>
+#include <Protocol/DevicePath.h>
+
+#include <Pcd/FakeExample.h>
+
+#define A 1
+#define B 2
+#define C 3
+#define D 4
+
+typedef struct {
+ USB_CLASS_DEVICE_PATH UsbClass;
+ EFI_DEVICE_PATH_PROTOCOL End;
+} USB_CLASS_FORMAT_DEVICE_PATH;
+
+#define CLASS_HID 3
+#define SUBCLASS_BOOT 1
+#define PROTOCOL_KEYBOARD 1
+
+// @PCDTYPE [PcdsFixedAtBuild.common]
+
+
+// @PCD
gMdeModulePkgTokenGuid.PcdFakePcdExample|FAKE_PCD_EXAMPLE|gFakePcdExample
+FAKE_PCD_EXAMPLE gFakePcdExample = {
+ EFI_PC_ANSI_GUID,
+ { A, B, C, D },
+ { PCI_CLASS_DISPLAY, PCI_CLASS_DISPLAY_VGA, PCI_IF_VGA_VGA }
+};
+
+
+// @PCD
gMdeModulePkgTokenGuid.PcdFakeDevicePathExample|EFI_DEVICE_PATH_PROTOCOL|gFakePcdDevicePathExample
+USB_CLASS_FORMAT_DEVICE_PATH gFakePcdDevicePathExample = {
+ {
+ {
+ MESSAGING_DEVICE_PATH,
+ MSG_USB_CLASS_DP,
+ {
+ (UINT8) (sizeof (USB_CLASS_DEVICE_PATH)),
+ (UINT8) ((sizeof (USB_CLASS_DEVICE_PATH)) >> 8)
+ }
+ },
+ 0xffff, // VendorId
+ 0xffff, // ProductId
+ CLASS_HID, // DeviceClass
+ SUBCLASS_BOOT, // DeviceSubClass
+ PROTOCOL_KEYBOARD // DeviceProtocol
+ },
+
+ {
+ END_DEVICE_PATH_TYPE,
+ END_ENTIRE_DEVICE_PATH_SUBTYPE,
+ {
+ sizeof (EFI_DEVICE_PATH_PROTOCOL),
+ 0
+ }
+ }
+};
+
> On May 20, 2016, at 2:48 PM, Andrew Fish <[email protected]> wrote:
>
>>
>> On May 20, 2016, at 12:34 PM, Kinney, Michael D <[email protected]>
>> wrote:
>>
>> Andrew,
>>
>> I hope there is not a misunderstanding in the include file used by a DEC
>> file for a PCD.
>>
>> It is not a parallel include file. The same include file is used in DEC
>> parsing and
>> by modules/libs that access the same Structured PCD. This is especially
>> important for
>> sharing a .h file between a PCD and VFR NV storage sources to eliminate
>> manual maintenance
>> of config value offsets in HII type PCD.
>>
>
> Mike,
>
> Sorry for the confusion I assume .h files are the same in both cases, and
> that is an absolute requirement. My point is the current DSC, INF, Conf/*.txt
> syntax supports figuring out the include pathing and passing the build
> generated compiler flags to the compiler, getting code auto generated, and
> built. Adding the C syntax info the .DEC and .DSC files means you have
> duplicate this mechanism. So I guess what I'm saying is if it is C like
> syntax use an INF, just like the C and ACPI code does today since you are
> really all sharing the C include infrastructure. If you try and duplicate
> build infrastructure with different code it is going to break (not get the
> same result as the INF path) in some subtle way at some point.Ssince it is a
> different syntax we could hit and end case where things are not symmetric
> (for example the INF files can have [BuildOptions] settings and new
> extensions to DEC/DSC don't support that). Also extending the DEC and DSC
> syntax is more new things for a developer to learn.
>
> Basically if it is a C like syntax (like C and ACPI) then we should use an
> INF file like today is what I'm advocating.
>
>> I only added syntax for some optional tags in comments for fields in the C
>> structure.
>> I want developer to only enter information (C struct in this case) one time.
>> We also
>> want to focus platform configuration information in the one DSC file for
>> simpler platform
>> configuration and generate PCD reports that show PCD configuration settings
>> (including
>> all field values in a Structured PCD).
>>
>> At this time, bit fields are a must have requirement for Structured PCDs.
>>
>> Main use case is register values with bit fields with no translation from
>> PCD to
>> register access. As a result SIZE_OF and OFFSET_OF are not sufficient. I
>> have ideas
>> on how to get bit offset/bit length from C compiler without using CLANG/AST.
>> It was
>> just very easy to get this info from CLANG/AST.
>>
>> Additional reason is size reduction of configuration data. Single bit or
>> BOOLEAN
>> like configuration setting takes up minimum of 8-bits today. If there are
>> many
>> configuration settings like these, then they can add up.
>>
>
> It is not so much as they are not supported, but you don't get an atomic
> write. You have to read modify write them, so 2 calls and a local variable.
> Actually we "solved that problem" in the IoLib by adding IoBitField*
> functions.... So that would imply what you need is an AND mask and you can
> hide the read modify write behind the library....
>
> Thus if you could generate a local, global or #define with the bit filed
> value all 1's and all the other fields 0's you can automate the read modify
> write into a single call. So maybe it is possible?
>
> Do all the compilers support C99? Named structure initialization makes this
> is easy if you know the type of the bitfield and the member name. Well you
> have to suppress the warning.
>
> ~/work/Compiler>cat bitfield.c
> typedef struct {
> unsigned int a:2;
> unsigned int b:4;
> unsigned int c:2;
> } TEST;
>
> TEST Test0 = { 0 };
> TEST Testa = { .a = 0xFFFFFFFF };
> TEST Testb = { .b = 0xFFFFFFFF };
> TEST Testc = { .c = 0xFFFFFFFF };
> ~/work/Compiler>clang -S -Wno-bitfield-constant-conversion bitfield.c
> ~/work/Compiler>cat bitfield.S
> .section __TEXT,__text,regular,pure_instructions
> .macosx_version_min 10, 11
> .section __DATA,__data
> .globl _Test0 ## @Test0
> .align 2
> _Test0:
> .byte 0 ## 0x0
> .space 3
>
> .globl _Testa ## @Testa
> .align 2
> _Testa:
> .byte 3 ## 0x3
> .space 3
>
> .globl _Testb ## @Testb
> .align 2
> _Testb:
> .byte 60 ## 0x3c
> .space 3
>
> .globl _Testc ## @Testc
> .align 2
> _Testc:
> .byte 192 ## 0xc0
> .space 3
>
> OK I think I'm convinced now it is likely we will be able to support
> bitfields.
>
> Thanks,
>
> Andrew Fish
>
> PS Mike if you look into this bitfield thing I can look at prototyping the
> INF based parsing with an example syntax from my previous mail. It will be
> easier to discuss with a prototype to look at.
>
> Well assuming I can track down the PACKAGES_PATH bug with ACPI tables.
> FfsInf.EfiOutputPath is missing the PACKAGES_PATH part and so I don't get any
> ACPI tables as the OUTPUT directory path is wrong so the search for the files
> fail :(.
>
>> Mike
>>
>>> -----Original Message-----
>>> From: [email protected] [mailto:[email protected]]
>>> Sent: Friday, May 20, 2016 12:15 PM
>>> To: Kinney, Michael D <[email protected]>
>>> Cc: [email protected] <[email protected]>
>>> Subject: Re: [edk2] [RFC] Structured PCD Proposal
>>>
>>>
>>>> On May 20, 2016, at 10:48 AM, Kinney, Michael D
>>>> <[email protected]> wrote:
>>>>
>>>> Andrew,
>>>>
>>>> 1) I think the strongest concern is on use of CLANG/AST. I will look at
>>>> your ideas
>>> and
>>>> some other ideas I have thought of as well to use standard C compiler that
>>>> is
>>> already
>>>> installed to build FW to support Structured PCD features.
>>>>
>>>> 2) The other concern I see here is extending PcdLib to access fields. You
>>>> prefer
>>> to
>>>> not add those APIs and always read/write entire structure. I have
>>>> requests to
>>> support
>>>> field access, so that is a bit of a requirement conflict.
>>>>
>>>
>>> Mike,
>>>
>>> Given the complexity we ended up with the basic edk2 design, that was also
>>> driven by
>>> complex requirements, I wanted to start the conversation of how some of the
>>> requirements maybe driving a disproportionate amount of the complexity. I
>>> would hate
>>> for a "nice to have" feature drive 80% of the complexity. I'm still feeling
>>> guilty
>>> about those PCD section names....
>>>
>>> I think there are two vectors of complexity:
>>> 1) Implementation - Requiring clang/AST
>>> I think stepping away with the assumption of clang/AST being required helps
>>> this
>>> vector out a lot.
>>>
>>> 2) Workflow - how a developer uses the feature day to day.
>>> I think my comments about the default values in the .DEC relate to the
>>> complexities a
>>> developer will face. As you point out there is going to be a .DEC grammar
>>> extension
>>> required just to support this feature. Also if I understand correctly
>>> changing a
>>> #define or a structure definition in another package could change the
>>> default. We
>>> currently don't have this behavior and I think that will lead to more
>>> confusion and
>>> hard to track down bugs. Thus I think we should consider sticking with the
>>> current
>>> .DEC syntax for the default variable if it is required. We could advocate
>>> comments
>>> that explain the fields and maybe even output information like that in
>>> build log PCD
>>> section so you can copy paste into the .DEC.
>>>
>>> Using an INF file and new file type for the structured PCD is also trying
>>> to manage
>>> developer complexity by making it symmetric with C and ACPI code. You can
>>> make the
>>> argument that PCDs always worked a certain way, but I will make the a
>>> counter
>>> argument that managing the C include file infrastructure has always worked
>>> a certain
>>> way too, and that is the reason to consider not building a parallel include
>>> infrastructure definition for the .DSC (and .DEC) files and use an INF
>>> file, not to
>>> mention it is probably easier to implement since it is a solved problem.
>>>
>>>
>>> I was also thinking about the PcdLib field accessors. If you don't support
>>> bit fields
>>> and don't need pedantic type safety I think this idea may get you 80% of
>>> what you
>>> want (I'm guessing you might have already thought of this). If the build
>>> system knows
>>> the C type of the structure and places it in the AutoGen.h and you know the
>>> structure
>>> member name in the calling code the Code calling the PcdLib.h can derive the
>>> structure offset and size of the field. It may also be possible to verify
>>> the size
>>> of the argument via a sizeof() in a macro.
>>>
>>> I just double checked that nested structures/unions would work, and clang
>>> looks
>>> happy.
>>>
>>> #include <stdio.h>
>>>
>>> #define OFFSET_OF(TYPE, Field) __builtin_offsetof(TYPE, Field)
>>> #define SIZE_OF(TYPE, Field) (sizeof(((TYPE *)0)->Field))
>>>
>>> typedef struct {
>>> int a;
>>> int b;
>>> int c;
>>> int leaf;
>>> } STRUCT_LEAF;
>>>
>>> typedef struct {
>>> STRUCT_LEAF Leaf;
>>> int i;
>>> } STRUCT;
>>>
>>> int
>>> main ()
>>> {
>>> STRUCT S;
>>>
>>> printf ("Offset 0x%lx size 0x%lx\n", OFFSET_OF(STRUCT, Leaf.leaf),
>>> SIZE_OF(STRUCT,
>>> Leaf.leaf));
>>> return 0;
>>> }
>>>
>>> So maybe the complexity tradeoff is how you support bitfields (union with a
>>> real type
>>> and granularity of access is that type) and how pedantic the type checking
>>> (C is not
>>> type safe, so this is no different than a cast in the C code) is going to
>>> be for the
>>> PcdLib structure field accessor.
>>>
>>> Thanks,
>>>
>>> Andrew Fish
>>>
>>>> More responses below.
>>>>
>>>> Mike
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: [email protected] [mailto:[email protected]]
>>>>> Sent: Thursday, May 19, 2016 3:25 PM
>>>>> To: Kinney, Michael D <[email protected]>
>>>>> Cc: [email protected] <[email protected]>
>>>>> Subject: Re: [edk2] [RFC] Structured PCD Proposal
>>>>>
>>>>>
>>>>>> On May 19, 2016, at 12:16 PM, Kinney, Michael D
>>>>>> <[email protected]>
>>> wrote:
>>>>>>
>>>>>> Andrew,
>>>>>>
>>>>>> My responses embedded below.
>>>>>>
>>>>>
>>>>> Mike,
>>>>>
>>>>> Likewise.
>>>>>
>>>>>> Mike
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: edk2-devel [mailto:[email protected]] On Behalf Of
>>>>>>> Andrew
>>> Fish
>>>>>>> Sent: Thursday, May 19, 2016 10:53 AM
>>>>>>> To: Kinney, Michael D <[email protected]>;
>>>>>>> [email protected]
>>> <edk2-
>>>>>>> [email protected]>
>>>>>>> Subject: Re: [edk2] [RFC] Structured PCD Proposal
>>>>>>>
>>>>>>>
>>>>>>>> On May 19, 2016, at 2:26 AM, Laszlo Ersek <[email protected]> wrote:
>>>>>>>>
>>>>>>>> On 05/19/16 01:28, Kinney, Michael D wrote:
>>>>>>>>
>>>>>>>> [snip]
>>>>>>>>
>>>>>>>>> ## C Structure Syntax Proposal
>>>>>>>>> * Use C typedef syntax to describe complex C structures
>>>>>>>>> * Use a C language parser with support for struct/union/array/bit
>>>>>>>>> fields/pack
>>>>>>>>> * Recommend use of libclang C language parser into Abstract Syntax
>>>>>>>>> Tree (AST)
>>>>>>>>> - Included with LLVM release
>>>>>>>>> - http://llvm.org/releases/download.html
>>>>>>>>> - http://clang.llvm.org/doxygen/group__CINDEX.html
>>>>>>>>> * Recommend use of Python bindings for CLANG for EDK II Python based
>>>>>>>>> BaseTools
>>>>>>>>> - pip install clang
>>>>>>>>
>>>>>>>> What versions are necessary? On RHEL-7, the following versions are
>>>>>>>> available (from EPEL only; RHEL does not provide clang):
>>>>>>>>
>>>>>>>> clang-devel: 3.4.2-8.el7
>>>>>>>> python-pip: 7.1.0-1.el7
>>>>>>>>
>>>>>>>
>>>>>>> Will it be possible to check everything into the edk2 git tree?
>>>>>>> Currently we
>>> only
>>>>>>> require the developer to have Xcode installed and everything else is in
>>>>>>> the git
>>>>> tree.
>>>>>>> I'm not sure it is possible to run pip on our production build servers?
>>>>>>
>>>>>> Is clang and clang libs included in your environment?
>>>>>
>>>>> clang is the compiler for Xcode so we have that for sure. I'm not sure
>>>>> about the
>>> libs
>>>>> but they are likely accessible.
>>>>>
>>>>>> That is not edk2 git today.
>>>>>> The only other component is the Python wrapper on top of the clang lib
>>>>>> and that
>>>>>> is available from another github project if you want to pull python
>>>>>> sources to
>>> it.
>>>>>>
>>>>>
>>>>> If is just Python code it should not be too complex?
>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>>
>>>>>>>>> ## DEC File Extensions
>>>>>>>>> * Declare link between PCD and C data structure and token range for
>>>>>>>>> fields
>>>>>>>>> * @Include path to include file with C typedef [Required]
>>>>>>>>> * Replace |VOID*| with name of C typedef
>>>>>>>>> * @TokenMap path to file with token number assignments [Optional]
>>>>>>>>> * Range of token numbers to assign to fields in C typedef [Required]
>>>>>>>>> * Example:
>>>>>>>>>
>>>>>>>>> ```
>>>>>>>>> # @Include Include/Pcd/AcpiLowerPowerIdleTable.h
>>>>>>>>> # @TokenMap Include/Pcd/AcpiLowerPowerIdleTable.map
>>>>>>>>>
>>>>>>>
>>>>>
>>> gPlatformTokenSpaceGuid.AcpiLowPowerIdleTable|{}|ACPI_LOW_POWER_IDLE_TABLE|0x00010080
>>>>>>> |0x0E000200-0x0E0002FF
>>>>>>>>> ```
>>>>>>>>
>>>>>>>> What does 0x00010080 mean here?
>>>>>>>>
>>>>>>>>>
>>>>>>>>> * Recommended File Paths
>>>>>>>>> - <PackageName>/Include/Pcd/<StructuredPcdName>.h
>>>>>>>>> - <PackageName>/Include/Pcd/<StructuredPcdName>.map [Optional]
>>>>>>>>> - <PackageName>/Include/Pcd/<StructuredPcdName>.uni [Optional]
>>>>>>>>> * C Pre-Processor Support
>>>>>>>>> - Use of #include, #define, #if supported
>>>>>>>>> - #include limited to files in same package
>>>>>>>>> - Including files from other packages being evaluated
>>>>>>>>>
>>>>>>>
>>>>>>> I think you need to share the C headers with the edk2 C and ASL code,
>>>>>>> so you
>>>>> should
>>>>>>> have full access including all the things set in the [BuildOptions]
>>>>>>> flags in the
>>>>> DSC.
>>>>>>> So use something like PP_FLAGS or ASLPP_FLAGS and allow customization
>>>>>>> via the
>>>>>>> Conf/build_rule.txt. I'd rather we not makeup a new thing and just use
>>>>>>> one of
>>> the
>>>>>>> existing pre processor schemes.
>>>>>>
>>>>>> I agree. Any flags set up in [BuildOptions] or -D flags on command line
>>>>>> would be
>>>>>> passed in, so the .h file associated with the Structured PCD is treated
>>>>>> the same
>>>>>> as any other .h file used to build modules/libs. There is no intention
>>>>>> to create
>>>>>> any new build rules to support Structured PCDs.
>>>>>
>>>>> OK I was confused by this statement. "Including files from other packages
>>>>> being
>>>>> evaluated". The more I think about it it is probably more related to this
>>>>> not
>>> being
>>>>> symmetric to how C and ASL works (you have an INF file to declare your
>>> dependencies).
>>>>> I'm worried that could cause issue. What if I needed a include file from
>>>>> a package
>>>>> that was not included any other way? If I was writing C or ASL I'd just
>>>>> update the
>>>>> INF file.
>>>>
>>>> Today, all of the content in a DEC file of a package does not have any
>>>> dependencies
>>>> outside that package. Modules/library implementations within a package
>>>> may have
>>>> dependencies on content from other packages as defined by the [Packages]
>>>> section
>>>> in an INF file. A PCD, declared in a package DEC files, that is
>>>> associated with
>>>> a typedef struct in an include file must follow the DEC file constraint
>>>> where there
>>>> are no dependencies on other packages.
>>>>
>>>> What is being evaluated is how to support a use case where an include file
>>>> used for
>>>> a Structured PCD needs definitions from include files in another package
>>>> (e.g. MdePkg for some industry standard defines/structs)
>>>>
>>>> We support multiple package dependencies in modules/library INF files
>>>> today with a
>>>> [Packages] section in the INF. One idea (not part of this proposal at
>>>> this time),
>>>> is to add a [Packages] section to a DEC file. The include paths used to
>>>> process a .h file associated with a Structured PCD can be appended with
>>>> all the
>>>> dependent package include paths. The [Packages] section in a DEC would
>>>> only be
>>>> used for DEC file dependencies. It would never be used to package
>>>> dependencies of
>>>> modules/libraries within that package.
>>>>
>>>> Adding a [Packages] section to a DEC file has wider impacts to other
>>>> BaseTools,
>>>> which is why I did not provide any more details than "under evaluation" at
>>>> this
>>>> time. My suggestion with the current proposal is to make sure all the .h
>>>> file
>>>> content required for a Structured PCD be within the package itself.
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> IMHO it will be easy enough to follow the proposed restrictive rules
>>>>>>> when you
>>> are
>>>>>>> starting from scratch, but I'm more worried about the case when you
>>>>>>> have an
>>>>> existing
>>>>>>> (very complex) include infrastructure that supports feature X in C and
>>>>>>> ASL and
>>> you
>>>>>>> decide to add a PCD for X. You don't want it to not work due to more
>>>>>>> restrictive
>>>>>>> rules around the PCD.
>>>>>>
>>>>>> A .h file can have lots of content in it. Only the typedef structure
>>>>>> name
>>>>> referenced
>>>>>> in the DEC file declaration has to follow any of the restrictions
>>>>>> associated with
>>>>>> a Structured PCD. The C Preprocessor statements above are there to show
>>>>>> that
>>> there
>>>>>> .h file for a Structured PCD can use the same C Preprocessor directives
>>>>>> that the
>>>>> rest
>>>>>> of the C code in modules/libs can use. The biggest restriction is in
>>>>>> the data
>>>>> types
>>>>>> of the leaf fields of the structures. Those have to be compatible with
>>>>>> the PCD
>>>>>> get/set APIs.
>>>>>>
>>>>>
>>>>> I think this makes my point about the complexity driving restrictions. If
>>>>> you did
>>> not
>>>>> have leaf field accessors you would not have that restriction.
>>>>
>>>> I agree that removing the requirement for leaf accessors reduces
>>>> complexity. I
>>> have
>>>> a request to support leaf accessors, so that is why they are part of this
>>>> proposal.
>>>>
>>>>>
>>>>>>>
>>>>>>>>> ## C Structure Syntax
>>>>>>>>> * Support struct/union/array with nesting
>>>>>>>>> * Support bit fields
>>>>>>>>
>>>>>>>> Bit fields will require new PcdLib interaces, right?
>>>>>>>>
>>>>>>>
>>>>>>> Yikes bit fileds. My over all feedback is this feature seems very
>>>>>>> complex and
>>>>>>> designed by committee (too many features). The complexity already seems
>>>>>>> to be
>>>>>>> introducing restrictions and will possible make it harder for
>>>>>>> developers to
>>>>>>> understand.
>>>>>>
>>>>>> The goal was to allow use of C syntax for a typedef struct with as few
>>> restrictions
>>>>>> as possible, so exiting C structs associated with VOID* PCDs could be
>>>>>> used "as
>>> is".
>>>>>> Yes. Supporting full C syntax can be complex, but by using CLANG front
>>>>>> end for
>>>>>> parsing into an AST, the complexity is handled by a well supported
>>>>>> parser.
>>>>>>
>>>>>>>
>>>>>>> I'll take a quick stab at simplification.
>>>>>>> 1) Add the new syntax to a new file type: *.pcd. This file would
>>>>>>> contain [Pcd*]
>>>>>>> sections and the C like syntax.
>>>>>>
>>>>>> C like syntax. That is what concerns me. Now we have to tell
>>>>>> developers that
>>>>>> they can use full C syntax for modules/libs, but there are different C
>>>>>> rules for
>>>>>> PCDs. I think it is easier is developers can use came C syntax for
>>>>>> modules/libs
>>>>>> and PCDs.
>>>>>
>>>>> I would assume the structure initialization is pure C, but there is meta
>>>>> data
>>>>> associated with the PCD that needs to get collected and associated
>>>>> initialized C
>>>>> structure. This is going to be true for any scheme.
>>>>
>>>> Yes. There is optional meta data in a comment block for a field of a
>>>> structure.
>>>> I have used the same meta-data tags and syntax that is defined in DEC file
>>>> comment blocks, to take advantage of the source style a developer already
>>>> knowns
>>>> when declaring PCDs in a DEC file. Only @TokenNumber (likely very rarely
>>>> used) and
>>>> @DefaultValue are new tags. But even the value syntax for these reuses
>>>> the value
>>>> syntax for a token number and default value in a PCD declaration in a DEC
>>>> file.
>>>>
>>>>>
>>>>>>
>>>>>>> 2) Don't support default values in the .dec with the C like syntax.
>>>>>>> Recommend no
>>>>>>> defaults and a build break if a PCD is not set by the platform build
>>>>>>> for
>>> complex
>>>>>>> structures.
>>>>>>
>>>>>> Why not support defaults in scope of DEC file or associated .h file? The
>>>>>> @DefaultValue tag in .h file is optional, so what you suggest here is
>>>>>> supported.
>>>>>>
>>>>>
>>>>> To simplify the .DEC parser so it does not require AST.
>>>>
>>>>
>>>> This is pushing more work onto platform porting in DSC/FDF file. The PCD
>>>> concept
>>>> from the beginning included the idea that the developer that designs a new
>>>> PCD
>>>> and adds it to a DEC file gets to support a "preferred" default value that
>>>> is
>>>> applicable to most platforms, and the DSC/FDF files only need to provide
>>>> override
>>>> values if the "preferred" default does not work for that specific
>>>> platform. Why
>>>> should we change this fundamental PCD concept just to simplify the tools.
>>>>
>>>> I agree that dependency on CLANG/AST/Python bindings may not be the best
>>>> Implementation choice based on feedback received so far, but should not
>>>> mean
>>>> burden the platform port and don't implement in better tools.
>>>>
>>>>>
>>>>>>> 3) Use the C pre processor with PP_FLAGS or ASLPP_FLAGS to handle the
>>>>>>> include
>>>>>>> infrastructure for maximum compatibility. Just like other rules in
>>>>>>> Conf/build_rule.txt. ?.pcd could be added to build_rule.txt.
>>>>>>
>>>>>> That is part of the current proposal.
>>>>>>
>>>>>>> 4) Don't add new PcdLib APIs. Return a pointer to the structure and use
>>>>>>> C code
>>> to
>>>>>>> access the data.
>>>>>>
>>>>>> That is supported today without this proposal.
>>>>>>
>>>>>> By adding new APIs, we can support access to individual fields within a
>>> Structured
>>>>>> PCD without having to require a local variable of the full C typedef.
>>>>>
>>>>> I don't see that as solving a real problem. From a walk up an use
>>>>> standpoint I
>>>>> initialize a C structure in a file, there is an API to return that
>>>>> structure and I
>>>>> dereference it with a pointer. That is how you write C code in any
>>>>> environment.
>>> Thus
>>>>> why do we need to invent another way to do this?
>>>>>
>>>>>> For Dynamic
>>>>>> or DynamicEx PCDs, individual fields can be accessed without having to
>>>>>> read/write
>>>>>> the entire structure.
>>>>>
>>>>> That could be an implementation optimization.
>>>>>
>>>>>>
>>>>>>> 5) OK someone is going to complain about type checking so add a new
>>>>>>> version of
>>>>>>> PcdGetPtr() that casts the pointer to the C pointer type returned. The
>>>>>>> type is
>>> in
>>>>> the
>>>>>>> AutoGen.h file (_PCD_GET_PTR_TYPE_##TokenName). If you don't use the
>>>>>>> new .pcd
>>> file
>>>>>>> syntax then you get a build break.
>>>>>>> 6) Don't be too restrictive on type checking. Support something like a
>>>>>>> device
>>>>> path.
>>>>>>> For example the globals in PlatformData.c of PlatformBdsLib could be PCD
>>> entries.
>>>>>>> This could be phase 2, and require a modified syntax if needed.
>>>>>>
>>>>>> This could be supported today using a VOID* PCD for each of this
>>>>>> structures.
>>>>>> Configuring through PCDs in a DSC file today as list of hex bytes is not
>>>>>> very
>>>>>> developer friendly. I agree that a phase 2 could help improve the
>>>>>> developer
>>>>>> experience configuring a VOID* buffer that is a sequence of Device Path
>>>>>> nodes.
>>>>>> Maybe even support the Device Path To String syntax a value syntax
>>>>>> extension.
>>>>>>
>>>>>>>
>>>>>>> This simpler scheme only requires initializing a global variable and
>>>>>>> being able
>>> to
>>>>>>> extract that information vs AST. The real value in this scheme is
>>>>>>> unifying the
>>>>>>> include infrastructure for C, ASL, and PCD. C can already access C
>>>>>>> structure
>>>>> members,
>>>>>>> seems like reinventing the wheel to come up with a complex scheme to
>>>>>>> abstract
>>> how
>>>>> to
>>>>>>> access C structure elements when a C pointer solves that problem.
>>>>>>
>>>>>> How would you initialize the global for a nested structure of many types
>>>>>> without
>>>>>> a parser?
>>>>>> How would you handle unions for a corner case when multiple union
>>>>>> members from the same union need to be initialized?
>>>>>
>>>>> Use the compiler we already have installed on the system as it is already
>>> configured
>>>>> to do the correct thing, and the produce and consumer are using the same
>>>>> tools.
>>>>>
>>>>> How are you going to deal with something like long (it is not the same
>>>>> width on
>>> all
>>>>> the IA32 compilers supported by edk2)? Or even structure packing rules?
>>>>> The
>>> default
>>>>> output of clang in not compatible with the EFI ABI and we have compiler
>>>>> flags and
>>>>> triples that fix that.
>>>>
>>>> I believe I have resolved all of the issues you raise here using
>>>> CLANG/AST. Type
>>>> 'long' is not supported as a leaf field type and generated an error in the
>>> prototype.
>>>> The supported leaf field types are in this proposal. I use the same
>>> ProcessorBind.h
>>>> mappings for CLANG for the supported leaf field types and it all works for
>>>> me.
>>>> CLANG/AST is only used to parse a C structure in a .h file. It is not
>>>> used for
>>>> compiling code, so there are never any EFI ABI issues with this use. CLANG
>>> supports
>>>> byte packing using the same #pragma pack syntax that is used in FW sources
>>>> and the
>>>> byte/bit offsets computed by CLANG match the byte/bit offsets from the
>>>> compiler
>>>> used to compile FW.
>>>>
>>>> As I mentioned at the top of email, I will evaluate using the current tool
>>>> chain
>>> tag
>>>> instead of CLANG/AST. I am confident the results will be the same for the
>>>> concerns
>>>> you raise here.
>>>>
>>>>>
>>>>> So the syntax probably needs some work as I'm making this up as I go
>>>>> along but you
>>>>> could have a *.pcd file that looks like:
>>>>>
>>>>> #include <BdsPlatform.h>
>>>>>
>>>>>
>>>>> // @ PCDTYPE [PcdsFixedAtBuild.common]
>>>>>
>>>>>
>>>>> // @PCD
>>>>>
>>> gMdeModulePkgTokenGuid|PcdUsbClassKeyboardDevicePath|EFI_DEVICE_PATH|gUsbClassDeviceP
>>>>> ath
>>>>> USB_CLASS_FORMAT_DEVICE_PATH gUsbClassDevicePath = {
>>>>> {
>>>>> {
>>>>> MESSAGING_DEVICE_PATH,
>>>>> MSG_USB_CLASS_DP,
>>>>> {
>>>>> (UINT8) (sizeof (USB_CLASS_DEVICE_PATH)),
>>>>> (UINT8) ((sizeof (USB_CLASS_DEVICE_PATH)) >> 8)
>>>>> }
>>>>> },
>>>>> 0xffff, // VendorId
>>>>> 0xffff, // ProductId
>>>>> CLASS_HID, // DeviceClass
>>>>> SUBCLASS_BOOT, // DeviceSubClass
>>>>> PROTOCOL_KEYBOARD // DeviceProtocol
>>>>> },
>>>>>
>>>>> {
>>>>> END_DEVICE_PATH_TYPE,
>>>>> END_ENTIRE_DEVICE_PATH_SUBTYPE,
>>>>> {
>>>>> END_DEVICE_PATH_LENGTH,
>>>>> 0
>>>>> }
>>>>> }
>>>>> };
>>>>>
>>>>> You do a quick parse to extract the meta data from the file: @ PCDTYPE
>>>>> [PcdsFixedAtBuild.common] and @PCD
>>>>>
>>> gMdeModulePkgTokenGuid|PcdUsbClassKeyboardDevicePath|EFI_DEVICE_PATH|gUsbClassDeviceP
>>>>> ath
>>>>> AutoGen a C program to do what you want and append it to the .pcd file.
>>>>> Compile and run the AutoGen code to create the output that could be as
>>>>> simple as
>>> the
>>>>> current PCD Pointer syntax.
>>>>>
>>>>> autogen code output looks like:
>>>>> // cat *.pcd file contents.
>>>>> // AutoGen Start.
>>>>> #include <stdio.h>
>>>>>
>>>>> int main() {
>>>>> char *c;
>>>>> int i;
>>>>>
>>>>> // @ PCDTYPE [PcdsFixedAtBuild.common]
>>>>> printf (" [PcdsFixedAtBuild.common]\n");
>>>>> // @PCD
>>>>>
>>> gMdeModulePkgTokenGuid|PcdUsbClassKeyboardDevicePath|EFI_DEVICE_PATH|gUsbClassDeviceP
>>>>> ath
>>>>> printf
>>> ("gMdeModulePkgTokenGuid|PcdUsbClassKeyboardDevicePath|EFI_DEVICE_PATH|{");
>>>>> for (i=0, c = (char *)(unsigned long long) &gUsbClassDevicePath; i <
>>>>> sizeof
>>>>> (gUsbClassDevicePath); i++) {
>>>>> printf (" 0x%02x,", c[i]);
>>>>> }
>>>>> printf (" }\n");
>>>>>
>>>>> return 0;
>>>>> }
>>>>>
>>>>> Thus basically use the same compiler on both sides, and some slightly
>>>>> modified
>>> form
>>>>> of the current PCD syntax (add the type). You could actually do this
>>>>> today with an
>>>>> INF file and a custom rule in the Conf/build_rule.txt. The only
>>>>> modification to
>>> the
>>>>> build system would adding the output files to the PCD database creation.
>>>>> There are
>>>>> other ways to do it but I'm a fan of making it work like C and ASL, as I
>>>>> think the
>>>>> symmetry is intuitive for the developer, and you don't need to learn a
>>>>> new way to
>>>>> specify packages, BuildOptions etc. as you are using the existing INF
>>>>> syntax to
>>> build
>>>>> and process for all file types.
>>>>>
>>>>> Another option is to abstract all the meta data in a C macro and follow
>>>>> the macro
>>>>> with the = {}; data. That is what we did, basically 1st pass the macro
>>>>> expands to
>>> the
>>>>> global. 2nd pass the macro expands to the code to write out the date. You
>>>>> still
>>> have
>>>>> to strip to bits you don't want before you compile the program (you don't
>>>>> want 2
>>>>> copies of all the preprocessed include stuff etc.
>>>>>
>>>>> So my point is you can get an 80% solution that is rock solid with zero
>>> constraints
>>>>> without going down the AST route, have it be a lot simpler, and not
>>>>> require a
>>> bunch
>>>>> of extra tools to be installed. I'm also a fan of using a new file type
>>>>> and an INF
>>>>> file vs. coming up with yet another edk2 unique syntax to do something
>>>>> the edk2
>>>>> already has a syntax to support.
>>>>>
>>>>> Ironically we asked the clang developers (that wrote the AST stuff) what
>>>>> was the
>>>>> safest way to do something like this and they recommended using the
>>>>> compiler in
>>> this
>>>>> fashion of my example. Our example was slightly more complex as we
>>>>> constructed an
>>>>> entire database, but close enough (no issue with the packing between
>>>>> structures in
>>>>> the PCD case).
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Andrew Fish
>>>>>
>>>>>>
>>>>>>>
>>>>>>> "The real art is knowing what to leave out, not what to put in" quote
>>>>>>> from Steve
>>>>>>> Jobs.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Andrew Fish
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> edk2-devel mailing list
>>>>>>> [email protected]
>>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>>>>> _______________________________________________
>>>>>> edk2-devel mailing list
>>>>>> [email protected]
>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
