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.web08.31474.1620036622250882020 for ; Mon, 03 May 2021 03:10:22 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=caXuGmO0; 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=1620036621; 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=vfCcCVYwh2JwsI9t6kkVcWYjgZITzyxKy/zAyiE7CIQ=; b=caXuGmO0n8+SFIdC8nVIeLz3jNruCAyl+bzQEu3P4L9qVEqHECtlkQmZPsL2bGuT+07f8f jMpuJK+Dn/zb0XueTfpj1dBynWD+FjSdXItecoFMyVqo/p8w8Rk51IL+b0UDfTuYmU4/0a HaFJSnbIcfSuTHNsXFGEHM58GsxR0xA= 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-332-Bz0ngdjHNDWSlWGG6jwCeg-1; Mon, 03 May 2021 06:10:17 -0400 X-MC-Unique: Bz0ngdjHNDWSlWGG6jwCeg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 412321B18BC0; Mon, 3 May 2021 10:10:16 +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 14B895D9D5; Mon, 3 May 2021 10:10:13 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH RFC v2 02/28] MdePkg: Define the GHCB Hypervisor features To: brijesh.singh@amd.com, Tom Lendacky Cc: devel@edk2.groups.io, James Bottomley , Min Xu , Jiewen Yao , Jordan Justen , Ard Biesheuvel , Erdem Aktas References: <20210430115148.22267-1-brijesh.singh@amd.com> <20210430115148.22267-3-brijesh.singh@amd.com> From: "Laszlo Ersek" Message-ID: <3eaa0aa6-9ebc-7e7f-fa69-564f049ce68d@redhat.com> Date: Mon, 3 May 2021 12:10:13 +0200 MIME-Version: 1.0 In-Reply-To: <20210430115148.22267-3-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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 Hi Brijesh, Tom, On 04/30/21 13:51, Brijesh Singh wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275 > > Version 2 of GHCB introduces advertisement of features that are supported > by the hypervisor. See the GHCB spec section 2.2 for an additional 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 +++++++ > MdePkg/Include/Register/Amd/Ghcb.h | 6 ++++++ > 2 files changed, 13 insertions(+) > > diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h > index 4d33bef220..a65d51ab12 100644 > --- a/MdePkg/Include/Register/Amd/Fam17Msr.h > +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h > @@ -48,6 +48,11 @@ typedef union { > UINT32 Reserved2:32; > } GhcbTerminate; > > + struct { > + UINT64 Function:12; > + UINT64 Features:52; > + } GhcbHypervisorFeatures; > + > VOID *Ghcb; > > UINT64 GhcbPhysicalAddress; > @@ -57,6 +62,8 @@ typedef union { > #define GHCB_INFO_SEV_INFO_GET 2 > #define GHCB_INFO_CPUID_REQUEST 4 > #define GHCB_INFO_CPUID_RESPONSE 5 > +#define GHCB_HYPERVISOR_FEATURES_REQUEST 128 > +#define GHCB_HYPERVISOR_FEATURES_RESPONSE 129 > #define GHCB_INFO_TERMINATE_REQUEST 256 > > #define GHCB_TERMINATE_GHCB 0 > diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h > index ccdb662af7..2d64a4c28f 100644 > --- a/MdePkg/Include/Register/Amd/Ghcb.h > +++ b/MdePkg/Include/Register/Amd/Ghcb.h > @@ -54,6 +54,7 @@ > #define SVM_EXIT_NMI_COMPLETE 0x80000003ULL > #define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL > #define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL > +#define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL > #define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL > > // > @@ -154,4 +155,9 @@ typedef union { > #define GHCB_EVENT_INJECTION_TYPE_EXCEPTION 3 > #define GHCB_EVENT_INJECTION_TYPE_SOFT_INT 4 > > +// Hypervisor features (1) Comment style -- leading and trailing // lines missing. > +#define GHCB_HV_FEATURES_SNP BIT0 > +#define GHCB_HV_FEATURES_SNP_AP_CREATE (GHCB_HV_FEATURES_SNP | BIT1) > +#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION (GHCB_HV_FEATURES_SNP_AP_CREATE | BIT2) > +#define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER (GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION | BIT3) > #endif > I'm going to take this series slow, because I need to rebuild whatever understanding I've ever had of SEV-ES from the bottom up. The patch looks good to me (I checked the GHCB spec 2.0, and the values seem to match). But I need some confirmation. The GHCB spec defines the "GHCB MSR" protocol, where MSR_SEV_ES_GHCB can be used for a direct request/response protocol when the least significant 12 bits are nonzero (i.e., they stand for a "function"). The sequence in this case (from the guest side is): wrmsr, vmgexit, rdmsr. On the host side, upon vmgexit, the MSR's twelve least significant bits are checked, and if they are nonzero, the function is handled, and the response is provided in the high-order bits of the MSR. Otherwise, if the "function" is zero, the MSR's contents are taken as a GPA, and then the pointed-to page (the GHCB) is consulted for the actual request. This means that some functions are possible for the guest to call in two ways -- with and without a (decrypted) GHCB existing. (The spec writes in 2.3.1, "The GHCB MSR protocol is valid at any time but is most useful when the GHCB page cannot be written by the guest in an unencrypted fashion"). One of the new things the GHCB 2.0 spec introduces is the "hypervisor feature advertisement", which is (apparently) one of those functions that are available to the guest via both the GHCB *MSR protocol* (function = GHCB_HYPERVISOR_FEATURES_REQUEST) and the GHCB *page* (SwExitCode = SVM_EXIT_HYPERVISOR_FEATURES, response in SwExitInfo2). My question is: when is it useful to fetch the hv features through the GHCB *page* (i.e., not through the MSR protocol)? At the end of the series, I don't see any use for SVM_EXIT_HYPERVISOR_FEATURES. A similarly unused macro (from before this series) is SVM_EXIT_NMI_COMPLETE. So I guess the approach in the edk2 SEV* work has been to incorporate all spec-defined constants in MdePkg. That's a valid approach per se; what I'd like to understand is what use case for SVM_EXIT_HYPERVISOR_FEATURES the GHCB *spec* foresees. (2) Does the spec define SVM_EXIT_HYPERVISOR_FEATURES for completeness' sake -- so that no function be restricted to the MSR protocol? (IOW, should the MSR protocol be a subset, by principle, of the functions available through the GHCB *page*?) I prefer to define only such macros in edk2 that are actually used -- but I admit that may be different from the general MdePkg rules. So I don't mind SVM_EXIT_HYPERVISOR_FEATURES, it's just a bit more difficult to review / understand without actual use. (3) I suggest the following subject: MdePkg/Register/Amd: define GHCB macros for hypervisor feature detection (72 chars) With (1) and (3) fixed: Reviewed-by: Laszlo Ersek Thanks Laszlo