From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.32790.1620044385437815312 for ; Mon, 03 May 2021 05:19:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bIFDLty/; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620044384; 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=Kdk2qghk0ZIRcI06zQk/kWQQoVJjJrSZPEOtLY3jL6s=; b=bIFDLty/FpaToD/3IahMtNsliig85CPPLOphQfxEKaRF9NHe9VR6yXcK/qT+ZJ28MIBi5q Y6OW2bVNkYkxxQx1PNY2qOXZ4t5SVR8F7QdNoCKcfAg3hjZzl4J/uRxPgHzqjziZTDpjHw 5zsWLaJUf+Naof81huCCxZxhduxLvBU= 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-462-3rq8u4pPMyubDGC7NcNMoA-1; Mon, 03 May 2021 08:19:40 -0400 X-MC-Unique: 3rq8u4pPMyubDGC7NcNMoA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 74B50801F98; Mon, 3 May 2021 12:19:39 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-8.ams2.redhat.com [10.36.114.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1F74679904; Mon, 3 May 2021 12:19:36 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH RFC v2 03/28] MdePkg: Define the GHCB GPA structure From: "Laszlo Ersek" To: devel@edk2.groups.io, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Tom Lendacky , Jordan Justen , Ard Biesheuvel , Erdem Aktas References: <20210430115148.22267-1-brijesh.singh@amd.com> <20210430115148.22267-4-brijesh.singh@amd.com> Message-ID: Date: Mon, 3 May 2021 14:19:36 +0200 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 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 05/03/21 12:24, Laszlo Ersek wrote: > On 04/30/21 13:51, Brijesh Singh wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 >> >> An SEV-SNP guest is required to perform the GHCB GPA registration. See >> the GHCB specification for further details. >> >> 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 >> --- >> MdePkg/Include/Register/Amd/Fam17Msr.h | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h >> index a65d51ab12..e19bd04b6c 100644 >> --- a/MdePkg/Include/Register/Amd/Fam17Msr.h >> +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h >> @@ -53,6 +53,11 @@ typedef union { >> UINT64 Features:52; >> } GhcbHypervisorFeatures; >> >> + struct { >> + UINT64 Function:12; >> + UINT64 GuestFrameNumber:52; >> + } GhcbGpaRegister; >> + >> VOID *Ghcb; >> >> UINT64 GhcbPhysicalAddress; >> @@ -62,6 +67,8 @@ typedef union { >> #define GHCB_INFO_SEV_INFO_GET 2 >> #define GHCB_INFO_CPUID_REQUEST 4 >> #define GHCB_INFO_CPUID_RESPONSE 5 >> +#define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST 18 >> +#define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE 19 >> #define GHCB_HYPERVISOR_FEATURES_REQUEST 128 >> #define GHCB_HYPERVISOR_FEATURES_RESPONSE 129 >> #define GHCB_INFO_TERMINATE_REQUEST 256 >> > > The number match the spec (2.0), but I have some remarks / questions. > > (1) Patch #2 (SVM_EXIT_HYPERVISOR_FEATURES) and this patch > (GHCB_INFO_GHCB_GPA_REGISTER_REQUEST) break the nice alignments of the > macro values (replacement texts) in both header files. Can you prepend a > whitespace-only patch that simply moves the affected "columns" to the > right far enough? > > (2) I've checked section 2.3.2 "GHCB GPA Registration" in the spec > (2.0). What is the specific risk of allowing a guest to switch from one > GHCB address to another? > > (3) It seems strange to expect that a guest stick with a particular GHCB > address for its entire lifetime (including firmware and OS) -- in fact > OVMF already uses multiple GHCB addresses. The spec does not explain how > the guest can "unlock" (de-register) a registered GHCB address. > Furthermore, if a guest can do that *at all* (which I think it must -- > we're already using different GHCB addresses between SEC and DXE, for > example), then what protection does the *temporary* locking of the GHCB > address provide? > > I'll stop reviewing here, because I think I need to understand your > answers. I'd like to have a rudimentary mental basis for reviewing the rest. ... interestingly, with reference to my question (2) under patch "RFC v2 02/28", the GHCB GPA registration function is one that can *only* be performed with the GHCB MSR protocol, and not through the GHCB page. So that shows that the MSR protocol's functions cannot be considered a pure subset of the GHCB page's functions. If SVM_EXIT_HYPERVISOR_FEATURES didn't exist (and the same function would only be accessible via GHCB_HYPERVISOR_FEATURES_REQUEST), then no "larger principle" would be damaged. Thanks Laszlo