[edk2-devel] [RFC] Fine-grained review ownership for MdeModulePkg
Leif Lindholm
leif.lindholm at linaro.org
Wed Jun 19 09:35:33 UTC 2019
On Wed, Jun 19, 2019 at 05:09:40AM +0000, Wu, Hao A wrote:
> Hello all,
>
> As suggested by Ray and Leif, modules (with wildcard) in MdeModulePkg are
> classified to a list of features.
>
> Please note that:
> * The below list is a draft at this moment, please help to provide
> feedbacks/comments;
> * Modules with no clear classification are listed under the 'Misc' section
> at the bottom of the list.
Thank you for doing the heavy lifting.
A few comments below on how this could be more easily described in the
Maintainer.txt syntax.
> ACPI:
> MdeModulePkg/Include/*/*Acpi*.h
> MdeModulePkg/Universal/Acpi/
>
> BDS:
> MdeModulePkg/Include/Library/PlatformBootManagerLib.h
> MdeModulePkg/Include/Library/UefiBootManagerLib.h
> MdeModulePkg/Library/PlatformBootManagerLibNull/
> MdeModulePkg/Library/UefiBootManagerLib/
MdeModulePkg/*BootManagerLib*/
> MdeModulePkg/Universal/BdsDxe/
> MdeModulePkg/Universal/BootManagerPolicyDxe/
Or maybe even
MdeModulePkg/*BootManager*/, which would also match the line above.
> MdeModulePkg/Universal/LoadFileOnFv2/
> MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.*
>
> Console:
> MdeModulePkg/Include/Guid/ConnectConInEvent.h
> MdeModulePkg/Include/Guid/ConsoleInDevice.h
> MdeModulePkg/Include/Guid/ConsoleOutDevice.h
> MdeModulePkg/Include/Guid/StandardErrorDevice.h
> MdeModulePkg/Include/Guid/TtyTerm.h
> MdeModulePkg/Universal/Console/ConPlatformDxe/
> MdeModulePkg/Universal/Console/ConSplitterDxe/
> MdeModulePkg/Universal/Console/GraphicsConsoleDxe/
> MdeModulePkg/Universal/Console/TerminalDxe/
I was intrigued as to why this did not specify
MdeModulePkg/Universal/Console/
See [1] below.
However, even if suggestions included below were not implemented, the
situation could be described as:
F: MdeModulePkg/Universal/Console/
X: MdeModulePkg/Universal/Console/GraphicsOutputDxe/
> Core (PEI, DXE and Runtime):
> MdeModulePkg/Core/Dxe/*
> MdeModulePkg/Core/Dxe/Dispatcher/
> MdeModulePkg/Core/Dxe/DxeMain/
> MdeModulePkg/Core/Dxe/Event/
> MdeModulePkg/Core/Dxe/FwVol*/
> MdeModulePkg/Core/Dxe/Hand/
> MdeModulePkg/Core/Dxe/Image/
> MdeModulePkg/Core/Dxe/Library/
> MdeModulePkg/Core/Dxe/Misc/
> MdeModulePkg/Core/Dxe/SectionExtraction/
F: MdeModulePkg/Core/Dxe/
X: MdeModulePkg/Core/Dxe/Mem/
> MdeModulePkg/Core/DxeIplPeim/
> MdeModulePkg/Core/Pei/*
> MdeModulePkg/Core/Pei/BootMode/
> MdeModulePkg/Core/Pei/CpuIo/
> MdeModulePkg/Core/Pei/Dependency/
> MdeModulePkg/Core/Pei/Dispatcher/
> MdeModulePkg/Core/Pei/FwVol/
> MdeModulePkg/Core/Pei/Hob/
> MdeModulePkg/Core/Pei/Image/
> MdeModulePkg/Core/Pei/PeiMain/
> MdeModulePkg/Core/Pei/Ppi/
> MdeModulePkg/Core/Pei/Security/
F: MdeModulePkg/Core/Pei/
X: MdeModulePkg/Core/Pei/Memory/
X: MdeModulePkg/Core/Pei/PciCfg2/
X: MdeModulePkg/Core/Pei/Reset/
X: MdeModulePkg/Core/Pei/StatusCode/
I'm going to stop there, because I'm lazy, and I realise this is about
the responsibility areas rather than an actual patch to
Maintainers.txt - and I think my point is made.
Further comments below.
> MdeModulePkg/Core/RuntimeDxe/
> MdeModulePkg/Include/Guid/Crc32GuidedSectionExtraction.h
> MdeModulePkg/Include/Guid/EventExitBootServiceFailed.h
> MdeModulePkg/Include/Guid/IdleLoopEvent.h
> MdeModulePkg/Include/Guid/LoadModuleAtFixedAddress.h
> MdeModulePkg/Include/Library/SecurityManagementLib.h
> MdeModulePkg/Library/*SectionExtract*/
> MdeModulePkg/Library/DxeSecurityManagementLib/
> MdeModulePkg/Universal/PlatformDriOverrideDxe/
> MdeModulePkg/Universal/SectionExtraction*/
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c
>
> Debug:
> MdeModulePkg/Include/Guid/DebugMask.h
> MdeModulePkg/Include/Library/DebugAgentLib.h
> MdeModulePkg/Include/Ppi/Debug.h
> MdeModulePkg/Library/*Debug*/
> MdeModulePkg/Universal/Debug*/
>
> Decompress:
> MdeModulePkg/Include/Guid/LzmaDecompress.h
> MdeModulePkg/Library/*Decompress*/
>
> Device:
> MdeModulePkg/Bus/Ata/
> MdeModulePkg/Bus/I2c/
> MdeModulePkg/Bus/Isa/
> MdeModulePkg/Bus/Pci/Ehci*/
> MdeModulePkg/Bus/Pci/IdeBusPei/
> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/
> MdeModulePkg/Bus/Pci/NvmExpress*/
> MdeModulePkg/Bus/Pci/PciSioSerialDxe/
> MdeModulePkg/Bus/Pci/SataControllerDxe/
> MdeModulePkg/Bus/Pci/SdMmc*/
> MdeModulePkg/Bus/Pci/Ufs*/
> MdeModulePkg/Bus/Pci/Uhci*/
> MdeModulePkg/Bus/Pci/Xhci*/
> MdeModulePkg/Bus/Scsi/
> MdeModulePkg/Bus/Sd/
> MdeModulePkg/Bus/Ufs/
> MdeModulePkg/Bus/Usb/
> MdeModulePkg/Include/*/*Ata*.h
> MdeModulePkg/Include/*/*NonDiscoverableDevice*.h
> MdeModulePkg/Include/*/*NvmExpress*.h
> MdeModulePkg/Include/*/*SerialPort*.h
> MdeModulePkg/Include/*/*SdMmc*.h
> MdeModulePkg/Include/*/*Ufs*.h
> MdeModulePkg/Include/*/*Usb*.h
> MdeModulePkg/Include/Guid/S3StorageDeviceInitList.h
> MdeModulePkg/Include/Guid/RecoveryDevice.h
> MdeModulePkg/Include/Guid/UsbKeyBoardLayout.h
> MdeModulePkg/Include/Ppi/StorageSecurityCommand.h
> MdeModulePkg/Include/Protocol/Ps2Policy.h
> MdeModulePkg/Library/BaseSerialPortLib16550/
> MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/
> MdeModulePkg/Universal/SerialDxe/
>
> Disk:
> MdeModulePkg/Universal/Disk/
>
> EBC:
> MdeModulePkg/Include/*/*Ebc*.h
> MdeModulePkg/Include/Protocol/DebuggerConfiguration.h
> MdeModulePkg/Universal/EbcDxe/
>
> Firmware Update:
> MdeModulePkg/Application/CapsuleApp/
> MdeModulePkg/Include/*/*Capsule*.h
> MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
> MdeModulePkg/Include/Library/FmpAuthenticationLib.h
> MdeModulePkg/Include/Protocol/EsrtManagement.h
> MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
> MdeModulePkg/Library/DisplayUpdateProgressLib*/
> MdeModulePkg/Library/DxeCapsuleLib*/
> MdeModulePkg/Library/FmpAuthenticationLibNull/
> MdeModulePkg/Universal/Capsule*/
> MdeModulePkg/Universal/Esrt*/
>
> Graphic:
> MdeModulePkg/Include/*/*Logo*.h
> MdeModulePkg/Include/Library/BmpSupportLib.h
> MdeModulePkg/Include/Library/FrameBufferBltLib.h
> MdeModulePkg/Library/BaseBmpSupportLib/
> MdeModulePkg/Library/BootLogoLib/
> MdeModulePkg/Library/FrameBufferBltLib/
> MdeModulePkg/Logo/
> MdeModulePkg/Universal/Console/GraphicsOutputDxe/
[1] I would say this suggests GraphicsOutputDxe could be moved to,
say, MdeModulePkg/Universal/Graphics - and Logo be moved there
too.
>
> HII/UI:
> MdeModulePkg/Application/BootManagerMenuApp/
> MdeModulePkg/Application/UiApp/
> MdeModulePkg/Include/*/*FileExplorer*.h
> MdeModulePkg/Include/*/*FormBrowser*.h
> MdeModulePkg/Include/*/*Hii*.h
> MdeModulePkg/Include/Library/CustomizedDisplayLib.h
> MdeModulePkg/Include/Protocol/DisplayProtocol.h
> MdeModulePkg/Library/*FileExplorer*/
> MdeModulePkg/Library/*Hii*/
> MdeModulePkg/Library/*UiLib/
> MdeModulePkg/Library/CustomizedDisplayLib/
> MdeModulePkg/Universal/DisplayEngineDxe/
> MdeModulePkg/Universal/FileExplorerDxe/
> MdeModulePkg/Universal/Hii*/
> MdeModulePkg/Universal/SetupBrowserDxe/
>
> IPMI:
> MdeModulePkg/Include/*/*Ipmi*.h
> MdeModulePkg/Library/*Ipmi*/
>
> Memory Management:
> MdeModulePkg/Application/MemoryProfileInfo/
> MdeModulePkg/Core/Dxe/Gcd/
> MdeModulePkg/Core/Dxe/Mem/
> MdeModulePkg/Core/Pei/Memory/
> MdeModulePkg/Include/*/*Mem*.h
> MdeModulePkg/Include/*/*IoMmu*.h
> MdeModulePkg/Library/*MemoryAllocation*/
> MdeModulePkg/Universal/MemoryTest/
>
> PCD:
> MdeModulePkg/Application/DumpDynPcd/
> MdeModulePkg/Include/*/*Pcd*.h
> MdeModulePkg/Universal/PCD/
>
> PCI Bus:
> MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/
> MdeModulePkg/Bus/Pci/PciBusDxe/
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/
> MdeModulePkg/Core/Pei/PciCfg2/
> MdeModulePkg/Include/Library/PciHostBridgeLib.h
> MdeModulePkg/Library/PciHostBridgeLibNull/
> MdeModulePkg/Universal/PcatSingleSegmentPciCfg2Pei/
>
> Performance:
> MdeModulePkg/Include/*/*Perf*.h
> MdeModulePkg/Library/*Perf*/
>
> Reset:
> MdeModulePkg/Core/Pei/Reset/
> MdeModulePkg/Include/*/*Reset*.h
> MdeModulePkg/Library/*Reset*/
> MdeModulePkg/Universal/ResetSystem*/
>
> S3:
> MdeModulePkg/Include/*/*BootScript*.h
> MdeModulePkg/Include/*/*LockBox*.h
> MdeModulePkg/Include/*/*S3*.h
> MdeModulePkg/Library/*LockBox*/
> MdeModulePkg/Library/*S3*/
> MdeModulePkg/Universal/LockBox/
>
> SMBIOS:
> MdeModulePkg/Universal/Smbios*/
>
> SMM:
> MdeModulePkg/Application/SmiHandlerProfileInfo/
> MdeModulePkg/Core/PiSmmCore/
> MdeModulePkg/Include/*/*Smi*.h
> MdeModulePkg/Include/*/*Smm*.h
> MdeModulePkg/Library/*Smi*/
> MdeModulePkg/Library/*Smm*/
> MdeModulePkg/Universal/SmmCommunicationBufferDxe/
>
> Status Code:
> MdeModulePkg/Core/Pei/StatusCode/
> MdeModulePkg/Include/*/*StatusCode*.h
> MdeModulePkg/Library/*StatusCode*/
> MdeModulePkg/Universal/*StatusCode*/
>
> Variable:
> MdeModulePkg/Application/VariableInfo/
> MdeModulePkg/Include/*/*FaultTolerantWrite*.h
> MdeModulePkg/Include/*/*Var*.h
> MdeModulePkg/Include/Guid/SystemNvDataGuid.h
> MdeModulePkg/Include/Protocol/SwapAddressRange.h
> MdeModulePkg/Library/*Var*/
> MdeModulePkg/Universal/FaultTolerantWrite*/
> MdeModulePkg/Universal/Variable/
>
> Misc:
I have no issue with the below, but just wanted to point out that if
there was an overarching MdeModulePkg/
section, these would automatically match against that anyway - we
don't need to explicitly tag everything.
> MdeModulePkg/Application/HelloWorld/
> MdeModulePkg/Include/Guid/MdeModulePkgTokenSpace.h
> MdeModulePkg/Include/Guid/MtcVendor.h
> MdeModulePkg/Include/Guid/ZeroGuid.h
> MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> MdeModulePkg/Include/Library/PlatformHookLib.h
> MdeModulePkg/Include/Library/RecoveryLib.h
> MdeModulePkg/Include/Library/SortLib.h
> MdeModulePkg/Include/Library/TpmMeasurementLib.h
> MdeModulePkg/Include/Protocol/Dpc.h
> MdeModulePkg/Include/Protocol/LoadPe32Image.h
> MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
> MdeModulePkg/Include/Protocol/Print2.h
> MdeModulePkg/Library/BaseHobLibNull/
> MdeModulePkg/Library/BasePlatformHookLibNull/
> MdeModulePkg/Library/BaseSortLib/
> MdeModulePkg/Library/CpuExceptionHandlerLibNull/
> MdeModulePkg/Library/DxePrintLibPrint2Protocol/
> MdeModulePkg/Library/PeiRecoveryLibNull/
> MdeModulePkg/Library/PlatformHookLibSerialPortPpi/
> MdeModulePkg/Library/TpmMeasurementLibNull/
> MdeModulePkg/Library/UefiSortLib/
> MdeModulePkg/Universal/DevicePathDxe/
> MdeModulePkg/Universal/DriverHealthManagerDxe/
> MdeModulePkg/Universal/DriverSampleDxe/
> MdeModulePkg/Universal/FvSimpleFileSystemDxe/
> MdeModulePkg/Universal/LegacyRegion2Dxe/
> MdeModulePkg/Universal/Metronome/
> MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/
> MdeModulePkg/Universal/PrintDxe/
> MdeModulePkg/Universal/RegularExpressionDxe/
> MdeModulePkg/Universal/TimestampDxe/
> MdeModulePkg/Universal/WatchdogTimerDxe/
Looking through this made me realise it would be useful to have a way
of quickly looking up section matches for a given path. So I added a
command line option -l/--lookup to GetMaintainer.py - inline below:
/
Leif
>From 11a04f4690e7b8bc348483a26bb3840b74b1fd9d Mon Sep 17 00:00:00 2001
From: Leif Lindholm <leif.lindholm at linaro.org>
Date: Wed, 19 Jun 2019 10:07:26 +0100
Subject: [RFC PATCH 1/1] BaseTools: add lookup function to GetMaintainer.py
Add command line option -l/--lookup, which lets you specify a path
(existent or not in the current tree) to look up maintainers for.
Signed-off-by: Leif Lindholm <leif.lindholm at linaro.org>
---
BaseTools/Scripts/GetMaintainer.py | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Scripts/GetMaintainer.py b/BaseTools/Scripts/GetMaintainer.py
index 616342cdb434..444e601c55d1 100644
--- a/BaseTools/Scripts/GetMaintainer.py
+++ b/BaseTools/Scripts/GetMaintainer.py
@@ -160,15 +160,22 @@ if __name__ == '__main__':
help='git revision to examine (default: HEAD)',
nargs='?',
default='HEAD')
+ PARSER.add_argument('-l', '--lookup',
+ help='Find section matches for path LOOKUP',
+ required=False)
ARGS = PARSER.parse_args()
REPO = SetupGit.locate_repo()
- FILES = get_modified_files(REPO, ARGS)
CONFIG_FILE = os.path.join(REPO.working_dir, 'Maintainers.txt')
SECTIONS = parse_maintainers_file(CONFIG_FILE)
+ if ARGS.lookup:
+ FILES = [ARGS.lookup]
+ else:
+ FILES = get_modified_files(REPO, ARGS)
+
ADDRESSES = []
for file in FILES:
--
2.11.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#42587): https://edk2.groups.io/g/devel/message/42587
Mute This Topic: https://groups.io/mt/32001926/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