Re: [PATCH 2/2] Maintainers.txt: Add reviewers for Confidential Computing related modules

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

Adding James, Brijesh, Tom; comments below:

On 03/10/21 03:55, Min Xu wrote:
> Register reviewers for the Confidential Computing related modules 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 | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 7d9fe89d6d28..220af0ee9b80 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -469,6 +469,9 @@ OvmfPkg: TDX-related modules
>  R: Jiewen Yao <jiewen.yao@intel.com>
>  R: Min Xu <min.m.xu@intel.com>
>
> +OvmfPkg: Confidential Computing related modules
> +R: Jiewen Yao <jiewen.yao@intel.com>
> +
>  OvmfPkg: TCG- and TPM2-related modules
>  F: OvmfPkg/Include/IndustryStandard/QemuTpm.h
>  F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>

(1) Same comment as under patch#1 -- please honor the alphabetical
ordering of the subsystem titles.

(2) Reviewing this patch makes me realize we've missed some
"Maintainers.txt" updates in the past, in relation to SEV and/or
confidential computing.

Namely, we did not designated any reviewers for the following pathnames:

  OvmfPkg/AmdSev/
  OvmfPkg/Include/Guid/ConfidentialComputingSecret.h
  OvmfPkg/Library/PlatformBootManagerLibGrub/

(from <https://bugzilla.tianocore.org/show_bug.cgi?id=3077>), also

  OvmfPkg/ResetVector/

(from <https://bugzilla.tianocore.org/show_bug.cgi?id=2198>).

That should be fixed up before adding anything TDX related (I can submit
a patch series, but first, the next point needs to be cleared.)

(3) After racking my brain for half an hour, I can find no good way to
have TDX/SEV separation *plus* a Confidential Computing section in
"Maintainers.txt". Whatever I managed to think of requires us to either
duplicate email addresses, or duplicate pathnames ("F:" patterns) -- or
even both.

So... can we simply rename the current SEV subsystem to "Confidential
Computing", and keep both TDX and SEV modules under it? We could place a
unified email address list there, with Brijesh, James, Jiewen, Min, Tom.

I don't think this should cause any confusion, because:

- @intel.com emails are clearly closely associated with TDX, and
@amd.com emails are clearly closely associated with SEV,

- most filenames will (or do already) include "AmdSev" or "Tdx",

- future patches should clearly label themselves as "SEV only", "TDX
only", or "confidential computing in general" -- this should be clear
from the patch subjects.

IOW, there should be no confusion as to who's required to review what,
but at the same time we'd have a simple solution for cross-posting all
interested parties.

Thoughts?

Thanks,
Laszlo