From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.7435.1622725664481712817 for ; Thu, 03 Jun 2021 06:07:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=AQyeYheW; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1622725663; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZaZ1KmqTvrNs4CmS8CZBcB1gVhVa2PUuFUqOjD3HCyg=; b=AQyeYheWh6MAqnZphsXAiC/vHr9Y4LFjDTaqKkvD6Kt9RRRWBEi2r8UBFfkIU54DixkWCO 5mnuRn2oD9ceUA1mWIu99vpA50Ca2ZLY++SOr63UnxndjMH9fBfIJaNBGx5kft7AYblMkd wCTILh4s9A3Zk0bQiapgP8n01Mzxo9A= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-3-zmL7yBVYPxGhTJoe6oL1EQ-1; Thu, 03 Jun 2021 09:07:40 -0400 X-MC-Unique: zmL7yBVYPxGhTJoe6oL1EQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 41D1B80293C; Thu, 3 Jun 2021 13:07:38 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-70.ams2.redhat.com [10.36.112.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7C0A55C290; Thu, 3 Jun 2021 13:07:35 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH RFC v3 01/22] UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs To: Brijesh Singh , devel@edk2.groups.io, James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Erdem Aktas , Eric Dong , Ray Ni , Rahul Kumar Cc: Ard Biesheuvel References: <20210526231118.12946-1-brijesh.singh@amd.com> <20210526231118.12946-2-brijesh.singh@amd.com> <5789af86-593f-46b3-0502-c0afb2223786@redhat.com> From: "Laszlo Ersek" Message-ID: <11f16307-45b7-786e-3f18-2ee99b83fcfc@redhat.com> Date: Thu, 3 Jun 2021 15:07:34 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 06/03/21 14:16, Brijesh Singh wrote: > > On 6/3/21 3:15 AM, Laszlo Ersek wrote: >> Hi Brijesh, >> >> On 05/27/21 01:10, Brijesh Singh wrote: >>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb4d27ba148e544f984ee08d92667d860%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637583049740462496%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=DaecxXoG%2BK932q9T70ISC%2BAIX4%2BUrAf6is6lEJmi9co%3D&reserved=0 >>> >>> Define the PCDs used by the MpLib while creating the AP when SEV-SNP is >>> active in the guest VMs. >>> >>> Cc: James Bottomley >>> Cc: Min Xu >>> Cc: Jiewen Yao >>> Cc: Tom Lendacky >>> Cc: Jordan Justen >>> Cc: Ard Biesheuvel >>> Cc: Laszlo Ersek >>> Cc: Erdem Aktas >>> Signed-off-by: Brijesh Singh >>> --- >>> 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). > > Actually all the UefiCpuPkg maintainers/reviewers are CC'ed in the whole > series. Its just that they are not listed in the git commit Cc tag. I'm sorry for missing that -- I never use git's command line CC switch(es), because those are easy to forget upon rebases / multiple postings. I exclusively use the commit messages for CC's, and so I didn't even check the actual address list on your patch email. My apologies. > > Patch#22 was a single one liner which bumps the version number after the > SNP support is added. I was not sure if MdePkg maintainer will have much > comment so I tried to avoid flooding the inbox -- I guess I should have > copied them. I will resend with Cc tags updated. Assuming it's going to be me to (potentially) merge this series, I'd never merge an MdePkg patch without MdePkg owner/reviewer approval, regardless of content (deciding about the content's impact is up to the MdePkg owners/reviewers too). On the other hand, if it's really just the last patch missing some CC's, then I'll bring in the MdePkg reviewers by replying to that patch and keeping full context for them. Based on the above, please don't bother with another resend, and I'm sorry about missing the actual CC's. Thanks, Laszlo > > >> 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 >>> >