Re: [edk2-devel] [PATCH RFC v3 01/22] UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs

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

Hi Brijesh,

On 05/27/21 01:10, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> Define the PCDs used by the MpLib while creating the AP when SEV-SNP is
> active in the guest VMs.
> 
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  UefiCpuPkg/UefiCpuPkg.dec | 11 +++++++++++
>  1 file changed, 11 insertions(+)

I'm really sorry that this is my first "actual" comment (on the 7th day
of your posting this series), but the CC list is incorrect on this
patch, and (at least) on patch#11 as well.

I'm a stickler myself for being CC'd on any patches per
"Maintainers.txt", so I'm going to request a resending of this series,
with the CC lists extended.

Please run "$EDK_TOOLS_PATH/Scripts/GetMaintainer.py" on every patch in
the series, and add the missing CC's. Manually copying all of the
"confidential computing" reviewers on all of the patches is great, but
we need to keep the usual package-level reviewers/maintainers as well.

In particular, the UefiCpuPkg patches need to be copied to Eric, Ray and
Rahul -- like you actually did in patch#21. Patch#22 also misses the
MdePkg reviewers (Michael, Liming, Zhiguang).

I'm truly sorry that this must feel like a cop-out on my part. I won't
deny that I appreciate a breather after the last two, hellishly busy,
two weeks, but I *did* resolve last night to start reviewing this series
today. (I should note that we expect reviewers to start reviewing a
patch set within a calendar week, so my feedback *is* just in time by my
count -- except, I expected to make actual patch review comments.)

If you can resend today with the CC's correct, I'll make an attempt to
start reviewing tomorrow.

Also, let me point this out: we have a nice long CC list now, by virtue
of the "confidential computing" section in "Maintainers.txt", and a
calendar week has passed -- I'd have appreciated seeing comments from
others as well!

Thanks,
Laszlo

> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 62acb291f309..0ec25871a50f 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -396,5 +396,16 @@ [PcdsDynamic, PcdsDynamicEx]
>    # @Prompt SEV-ES Status
>    gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled|FALSE|BOOLEAN|0x60000016
>  
> +  ## This dynamic PCD indicates whether SEV-SNP is enabled
> +  #   TRUE  - SEV-SNP is enabled
> +  #   FALSE - SEV-SNP is not enabled
> +  # @Prompt SEV-SNP Status
> +  gUefiCpuPkgTokenSpaceGuid.PcdSevSnpIsEnabled|FALSE|BOOLEAN|0x60000017
> +
> +  ## This dynamic PCD contains the hypervisor features value obtained through the GHCB HYPERVISOR
> +  #  features VMGEXIT defined in the version 2 of GHCB spec.
> +  # @Prompt GHCB Hypervisor Features
> +  gUefiCpuPkgTokenSpaceGuid.PcdGhcbHypervisorFeatures|0x0|UINT64|0x60000018
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    UefiCpuPkgExtra.uni
>