Re: [PATCH 1/2] Maintainers.txt: Add reviewers for the OvmfPkg TDX-related files

["Laszlo Ersek" <lersek@redhat.com>]

On 03/10/21 14:12, Laszlo Ersek wrote:
> On 03/10/21 03:55, Min Xu wrote:
>> Register reviewers for the TDX-related files in OvmfPkg.
>>
>> Cc: Andrew Fish <afish@apple.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Signed-off-by: Min Xu <min.m.xu@intel.com>
>> ---
>>  Maintainers.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Maintainers.txt b/Maintainers.txt
>> index e38497123857..7d9fe89d6d28 100644
>> --- a/Maintainers.txt
>> +++ b/Maintainers.txt
>> @@ -465,6 +465,10 @@ F: OvmfPkg/PlatformPei/AmdSev.c
>>  R: Tom Lendacky <thomas.lendacky@amd.com>
>>  R: Brijesh Singh <brijesh.singh@amd.com>
>>  
>> +OvmfPkg: TDX-related modules
>> +R: Jiewen Yao <jiewen.yao@intel.com>
>> +R: Min Xu <min.m.xu@intel.com>
>> +
>>  OvmfPkg: TCG- and TPM2-related modules
>>  F: OvmfPkg/Include/IndustryStandard/QemuTpm.h
>>  F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>
> 
> I'm OK with the patch content, however the placement of the new section
> is not right. We strive to keep the section headers alphabetically
> ordered. Here's an excerpt (pre-patch):
> 
> OvmfPkg: SEV-related modules
> OvmfPkg: TCG- and TPM2-related modules
> OvmfPkg: Xen-related modules
> 
> I think that the new section belongs between TCG and Xen.

Also -- I actually forgot the more important issue -- this patch does
not add any pathname patterns that the section is supposed to apply to!

Please refer to the "F:" patterns in the "Maintainers.txt" file, both
the explanation near the top of the file, and the various OvmfPkg examples.

And then, given that there are no TDX-related files in the tree at the
moment (so no F: pattern makes sense at the moment), I think adding this
section is premature. This patch should become part of your initial
TdxLib contribution -- the patch could be the last one in your series,
covering the pathnames introduced by the previous patches in your
series. So (just an example) we might have something like:

OvmfPkg: TDX-related modules
F: OvmfPkg/Library/TdxLib/
R: Jiewen Yao <jiewen.yao@intel.com>
R: Min Xu <min.m.xu@intel.com>

Subsequently to that, every time you introduced a new TDX-related module
(library instance, lib class header, driver, ...), you'd add matching
"F:" patterns to the above subsystem.

Thanks
Laszlo