[edk2-devel] [PATCH RFC v2 03/28] MdePkg: Define the GHCB GPA structure
Laszlo Ersek
lersek at redhat.com
Mon May 3 12:19:36 UTC 2021
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 <jejb at linux.ibm.com>
>> Cc: Min Xu <min.m.xu at intel.com>
>> Cc: Jiewen Yao <jiewen.yao at intel.com>
>> Cc: Tom Lendacky <thomas.lendacky at amd.com>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
>> Cc: Laszlo Ersek <lersek at redhat.com>
>> Cc: Erdem Aktas <erdemaktas at google.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
>> ---
>> 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74699): https://edk2.groups.io/g/devel/message/74699
Mute This Topic: https://groups.io/mt/82479050/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list