Hi Isaac, Comments inline.
Thanks, Nate > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oram, > Isaac W > Sent: Tuesday, January 11, 2022 6:20 PM > To: devel@edk2.groups.io > Cc: Oram, Isaac W <isaac.w.o...@intel.com>; Dong, Eric > <eric.d...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn> > Subject: [edk2-devel][edk2-platforms][PATCH V1 19/27] > Usb3DebugFeaturePkg: Align with feature design guidelines > > Remove build of common libraries. Boards will already have those. > > Modified Usb3DebugFeature.dsc to treat libraries like libraries. > Usb3DebugFeaturePkg.dsc uses the component trick for standalone build > testing of the libraries. > > Added a PCD to allow board to select between NULL, regular, and IO MMU > library instances. Prior implementation of Usb3DebugFeature.dsc was not > useful as an includable file because it didn't specify LibaryClass instance to > use. > > Removed unused CMOS PCD. > > Updated some of the readme sections. > > Cc: Eric Dong <eric.d...@intel.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Signed-off-by: Isaac Oram <isaac.w.o...@intel.com> > --- > > Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFeatu > re.dsc | 131 ++++---------------- > Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md | > 50 +++++--- > > Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.d > ec | 14 +-- > > Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.d > sc | 18 +++ > 4 files changed, 81 insertions(+), 132 deletions(-) > > diff --git > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFea > ture.dsc > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFea > ture.dsc > index 1e3aaecd5d..a87a5b428b 100644 > --- > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFea > ture.dsc > +++ > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Include/Usb3DebugFea > t > +++ ure.dsc > @@ -31,122 +31,35 @@ > # > > ########################################################## > ###################### > > -!include MdePkg/MdeLibs.dsc.inc > +[LibraryClasses.Common] > + > +Usb3DebugPortParamLibo|Usb3DebugFeaturePkg/Library/Usb3DebugPort > ParamLi Typo here. Usb3DebugPortParamLibo should be Usb3DebugPortParamLib. > +bPcd/Usb3DebugPortParamLibPcd.inf > > -[LibraryClasses] > - ####################################### > - # Edk2 Packages > - ####################################### > - BaseLib|MdePkg/Library/BaseLib/BaseLib.inf > - BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf > - DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf > - DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf > - DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf > - IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > - PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > - PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf > - PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf > - > TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTem > plate.inf > - > UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBo > otServicesTableLib.inf > - > UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntry > Point.inf > - UefiLib|MdePkg/Library/UefiLib/UefiLib.inf > - > UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib > /UefiRuntimeServicesTableLib.inf > - > -[LibraryClasses.common.PEIM] > - ####################################### > - # Edk2 Packages > - ####################################### > - HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > - > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemory > AllocationLib.inf > - > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/P > eiServicesTablePointerLibIdt.inf > - > -[LibraryClasses.common.DXE_DRIVER] > - ####################################### > - # Edk2 Packages > - ####################################### > - HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > - > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo > ryAllocationLib.inf > - > -[LibraryClasses.common.DXE_RUNTIME_DRIVER] > - ####################################### > - # Edk2 Packages > - ####################################### > - HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > - > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo > ryAllocationLib.inf > - UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf > - > -[LibraryClasses.common.UEFI_DRIVER] > - ####################################### > - # Edk2 Packages > - ####################################### > - HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > - > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemo > ryAllocationLib.inf > - > - > ########################################################## > ###################### > -# > -# Component section - list of all components that need built for this > feature. > -# > -# Note: The EDK II DSC file is not used to specify how compiled binary > images get placed > -# into firmware volume images. This section is just a list of modules > to > compile from > -# source into UEFI-compliant binaries. > -# It is the FDF file that contains information on combining binary > files into > firmware > -# volume images, whose concept is beyond UEFI and is described in PI > specification. > -# There may also be modules listed in this section that are not > required in > the FDF file, > -# When a module listed here is excluded from FDF file, then UEFI- > compliant binary will be > -# generated for it, but the binary will not be put into any firmware > volume. > -# > - > ########################################################## > ###################### > # > -# Feature PEI Components > +# If NULL Usb3DebugPortLib library instance desired > # > - > -# @todo: Change below line to [Components.$(PEI_ARCH)] after > https://bugzilla.tianocore.org/show_bug.cgi?id=2308 > -# is completed. > -[Components.IA32] > - ##################################### > - # USB3 Debug Feature Package > - ##################################### > - > - # Add library instances here that are not included in package components > and should be tested > - # in the package build. > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibNull.in > f > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortParamLibPcd/Usb3DebugPor > tParamLibPcd.inf > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibPei.inf > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibPeiIo > Mmu.inf > - > - # Add components here that should be included in the package build. > +!if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance > == 0 > + [LibraryClasses.Common] > + > +Usb3DebugPortLib|Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb > 3Debug > +PortLibNull.inf > +!endif > > # > -# Feature DXE Components > +# If regular Usb3DebugPortLib library instance desired > # > +!if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance > == 1 > + [LibraryClasses.PEI_CORE, LibraryClasses.PEIM] > + > +Usb3DebugPortLib|Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb > 3Debug > +PortLibPei.inf > > -# @todo: Change below line to [Components.$(DXE_ARCH)] after > https://bugzilla.tianocore.org/show_bug.cgi?id=2308 > -# is completed. > -[Components.X64] > - ##################################### > - # USB3 Debug Feature Package > - ##################################### > - > - # Add library instances here that are not included in package components > and should be tested > - # in the package build. > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibNull.in > f > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortParamLibPcd/Usb3DebugPor > tParamLibPcd.inf > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibDxe.in > f > - > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibDxeIo > Mmu.inf > + [LibraryClasses.DXE_CORE, LibraryClasses.DXE_DRIVER, > LibraryClasses.DXE_RUNTIME_DRIVER, LibraryClasses.SMM_CORE, > LibraryClasses.DXE_SMM_DRIVER, LibraryClasses.UEFI_DRIVER] > + > +Usb3DebugPortLib|Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb > 3Debug > +PortLibDxe.inf > +!endif > > - # Add components here that should be included in the package build. > - > - > ########################################################## > ######################################### > -# > -# BuildOptions Section - Define the module specific tool chain flags that > should be used as > -# the default flags for a module. These flags are > appended to any > -# standard flags that are defined by the build > process. They can > be > -# applied for any modules or only those modules with > the > specific > -# module style (EDK or EDKII) specified in > [Components] section. > # > -# For advanced features, it is recommended to enable > [BuildOptions] in > -# the applicable INF file so it does not affect the > whole board > package > -# build when this DSC file is active. > +# If regular Usb3DebugPortLib library instance desired > # > - > ########################################################## > ######################################### > -[BuildOptions] > +!if gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance > == 2 > + [LibraryClasses.PEI_CORE, LibraryClasses.PEIM] > + > +Usb3DebugPortLib|Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb > 3Debug > +PortLibPeiIoMmu.inf > + > + [LibraryClasses.DXE_CORE, LibraryClasses.DXE_DRIVER, > LibraryClasses.DXE_RUNTIME_DRIVER, LibraryClasses.SMM_CORE, > LibraryClasses.DXE_SMM_DRIVER, LibraryClasses.UEFI_DRIVER] > + > +Usb3DebugPortLib|Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb > 3Debug > +PortLibDxeIoMmu.inf > +!endif > diff --git a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md > index dc92f108ff..c8afcf9fd2 100644 > --- a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md > +++ b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Readme.md > @@ -29,12 +29,15 @@ The description should not be constrained to > implementation details but provide feature is supposed to work. > > ## Firmware Volumes > -*_TODO_* > -A bulleted list of the firmware volumes that feature module(s) are placed in. > +Not applicable, the feature only produces libraries. > > ## Modules > -*_TODO_* > -A bulleted list of the modules that make up the feature. > +* Usb3DebugPortLibDxe > +* Usb3DebugPortLibDxeIoMmu > +* Usb3DebugPortLibNull > +* Usb3DebugPortLibPei > +* Usb3DebugPortLibPeiIoMmu > +* Usb3DebugPortParamLibPcd > > ## <Module Name> > *_TODO_* > @@ -76,11 +79,7 @@ This is particularly useful for features that use custom > build tools or require standard flow in the feature package template is used, > this section may be empty. > > ## Test Point Results > -*_TODO_* > -The test(s) that can verify porting is complete for the feature. > - > -Each feature must describe at least one test point to verify the feature is > successful. If the test point is not -implemented, this should be stated. > +No test points implemented > > ## Functional Exit Criteria > *_TODO_* > @@ -90,8 +89,28 @@ This section should provide an ordered list of criteria > that a board integrator functional on their board. > > ## Feature Enabling Checklist > -*_TODO_* > -An ordered list of required activities to achieve desired functionality for > the > feature. > +* In the board DSC file, enable the feature ``` [PcdsFeatureFlag] > + > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable|TRU > E > +``` > +* In the board DSC file, select the implementation desired ``` > +[PcdsFixedAtBuild] > + # 0 = Non-functional instance > + # 1 = Regular instance > + # 2 = IO MMU instance > + > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance|1 > +``` > +* In the board DSC file, configure the PCI device information ``` > +[PcdsFixedAtBuild] > + gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsbSerialXhciBus|0x00 > + gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsbSerialXhciDev|0x14 > + gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsbSerialXhciFunc|0x00 > + > +gUsb3DebugFeaturePkgTokenSpaceGuid.PcdXhciDefaultBaseAddress|0xFE > A10000 > +``` > + > > ## Performance Impact > A general expectation for the impact on overall boot performance due to > using this feature. > @@ -102,7 +121,8 @@ This section is expected to provide guidance on: > * How to manage performance impact of the feature > > ## Common Optimizations > -*_TODO_* > -Common size or performance tuning options for this feature. > - > -This section is recommended but not required. If not used, the contents > should be left empty. > +* In the board DSC file, tune the timeout value ``` [PcdsFixedAtBuild] > + > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdXhciHostWaitTimeout|2000000 > +``` > diff --git > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg. > dec > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg > .dec > index 2b19e48491..353001b0f8 100644 > --- > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg. > dec > +++ > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg > .d > +++ ec > @@ -36,6 +36,12 @@ > > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable|FAL > SE|BOOLEAN|0xA0000001 > > [PcdsFixedAtBuild] > + ## This PCD allows the board to select the Usb3DebugPortLib instance > + desired # 0 = NULL instance # 1 = Regular instance # 2 = IO MMU > + instance > + > + > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance|0| > UINT8 > + |0xF0000009 > + > ## These PCD specify XHCI controller Bus/Device/Function, which are used > to enable > # XHCI debug device. > > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsbSerialXhciBus|0x00|UINT8| > 0xF0000001 > @@ -47,11 +53,3 @@ > # Default timeout value is 2000000 microseconds. > # If user does not want system stall for long time, it can be set to small > value. > > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdXhciHostWaitTimeout|2000000 > |UINT64|0xF0000005 > - > - ## This PCD sepcifies the start index in CMOS, it will occupy 1 bytes > space. > - > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortBusIndex|0x59 > |UINT8|0xF0000006 > - ## This PCD sepcifies the start index in CMOS, it will occupy 1 bytes > space. > - > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortDeviceIndex|0 > x5A|UINT8|0xF0000007 > - ## This PCD sepcifies the start index in CMOS, it will occupy 1 bytes > space. > - > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortFunctionIndex| > 0x5B|UINT8|0xF0000008 > - > diff --git > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg. > dsc > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg > .dsc > index 02627d2ed4..6965d26721 100644 > --- > a/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg. > dsc > +++ > b/Features/Intel/Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg > .d > +++ sc > @@ -24,7 +24,25 @@ > PEI_ARCH = IA32 > DXE_ARCH = X64 > > +[PcdsFixedAtBuild] > + > gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugPortLibInstance|0 > + > # > # This package always builds the feature. > # > !include Include/Usb3DebugFeature.dsc > + > +# > +# This package currently only contains library classes. To force them > +to be built since there is no code to use them # we just tell the build > +that they are components and the build will compile the libraries # > + > +[Components] Did you test compilation for the Usb3DebugFeaturePkg? I've generally run into issues when a components sections does not specify a machine architecture through some sort of means. > + > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibNull.in > f > + > +Usb3DebugFeaturePkg/Library/Usb3DebugPortParamLibPcd/Usb3DebugPo > rtParam > +LibPcd.inf > + > + > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibPei.inf > + > + > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibPeiIo > Mmu. > + inf > + > + > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibDxe.in > f > + > + > Usb3DebugFeaturePkg/Library/Usb3DebugPortLib/Usb3DebugPortLibDxeIo > Mmu. > + inf > -- > 2.27.0.windows.1 > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85622): https://edk2.groups.io/g/devel/message/85622 Mute This Topic: https://groups.io/mt/88365351/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
