[edk2-devel][edk2-platforms][PATCH V1 19/27] Usb3DebugFeaturePkg: Align with feature design guidelines
Nate DeSimone
nathaniel.l.desimone at intel.com
Thu Jan 13 02:47:27 UTC 2022
Hi Isaac,
Comments inline.
Thanks,
Nate
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Oram,
> Isaac W
> Sent: Tuesday, January 11, 2022 6:20 PM
> To: devel at edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram at intel.com>; Dong, Eric
> <eric.dong at intel.com>; Gao, Liming <gaoliming at 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.dong at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
>
> Signed-off-by: Isaac Oram <isaac.w.oram at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list