From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by mx.groups.io with SMTP id smtpd.web09.24947.1626682167545905412 for ; Mon, 19 Jul 2021 01:09:27 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@ibm.com header.s=pp1 header.b=DZ4hZx8q; spf=pass (domain: linux.ibm.com, ip: 148.163.156.1, mailfrom: dovmurik@linux.ibm.com) Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 16J89CHk189131; Mon, 19 Jul 2021 04:09:22 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=ORBALX9NkwHqlxBG6oBymbqsuOlsZkqcF7/nhzQCgQg=; b=DZ4hZx8qrrGkw0C05iZDg24KXPuh9rGdKqcvDb0ifK600UlLbnLWOYJoacaD6GN1292y MHMHqKzjzhH+bdrfZG2/W0spvXCGSg9J3a6HzKh+EDC/Wxk/8c/watVVA2aqsrBDcJvi nzTCFsas897xyOxKmDYmy/CdC5eYY09oou3nQHhaPv+z/PFtem1hucJgB1IJuBS87sRA Cw4STHKLton4Hnah2HHZF/HW+BpjioejgXrc+NyKKtuIhLzSDfVKCAZYD+l/S3D9JfvJ jYgCBbxwYpkAsIMJ/pDj89OwbGwYnEkNjFpOOTMYIUSLhexSEBZYfrqFN/qVJ5jM2f40 hw== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 39w5acgedb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jul 2021 04:09:22 -0400 Received: from m0098409.ppops.net (m0098409.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 16J89KBj190235; Mon, 19 Jul 2021 04:09:20 -0400 Received: from ppma02fra.de.ibm.com (47.49.7a9f.ip4.static.sl-reverse.com [159.122.73.71]) by mx0a-001b2d01.pphosted.com with ESMTP id 39w5acgdjh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jul 2021 04:09:20 -0400 Received: from pps.filterd (ppma02fra.de.ibm.com [127.0.0.1]) by ppma02fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 16J82rI7026417; Mon, 19 Jul 2021 08:04:30 GMT Received: from b06avi18626390.portsmouth.uk.ibm.com (b06avi18626390.portsmouth.uk.ibm.com [9.149.26.192]) by ppma02fra.de.ibm.com with ESMTP id 39upu88ajw-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 19 Jul 2021 08:04:30 +0000 Received: from b06wcsmtp001.portsmouth.uk.ibm.com (b06wcsmtp001.portsmouth.uk.ibm.com [9.149.105.160]) by b06avi18626390.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 16J828bx34865522 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 19 Jul 2021 08:02:08 GMT Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C6F50A4060; Mon, 19 Jul 2021 08:04:26 +0000 (GMT) Received: from b06wcsmtp001.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id DDBE8A4062; Mon, 19 Jul 2021 08:04:20 +0000 (GMT) Received: from [9.65.195.237] (unknown [9.65.195.237]) by b06wcsmtp001.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 19 Jul 2021 08:04:20 +0000 (GMT) Subject: Re: [PATCH v5 1/4] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall To: Ashish Kalra Cc: devel@edk2.groups.io, dovmurik@linux.vnet.ibm.com, brijesh.singh@amd.com, tobin@ibm.com, Thomas.Lendacky@amd.com, jejb@linux.ibm.com, lersek@redhat.com, jordan.l.justen@intel.com, ard.biesheuvel@arm.com, erdemaktas@google.com, jiewen.yao@intel.com, min.m.xu@intel.com References: <332172b262929880ef753a3bef36228115b7051a.1625687246.git.ashish.kalra@amd.com> <20210716122925.GA17576@ashkalra_ubuntu_server> From: "Dov Murik" Message-ID: <11cc351b-c1c6-f33c-5000-335125b098c8@linux.ibm.com> Date: Mon, 19 Jul 2021 11:04:17 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210716122925.GA17576@ashkalra_ubuntu_server> X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: dcJPmC7QfUNDWJVfhecrWEzoblaZIRcB X-Proofpoint-GUID: DIZ_C43xBv_w8NOvcKKCa88rg47I78-M X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-07-19_02:2021-07-16,2021-07-19 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 spamscore=0 mlxlogscore=999 phishscore=0 suspectscore=0 adultscore=0 clxscore=1015 bulkscore=0 lowpriorityscore=0 malwarescore=0 mlxscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000 definitions=main-2107190044 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 16/07/2021 15:29, Ashish Kalra wrote: > Hello Dov, > > On Thu, Jul 15, 2021 at 11:58:17PM +0300, Dov Murik wrote: >> Hi Ashish, >> >> On 08/07/2021 17:07, Ashish Kalra wrote: >>> From: Ashish Kalra >>> >>> By default all the SEV guest memory regions are considered encrypted, >>> if a guest changes the encryption attribute of the page (e.g mark a >>> page as decrypted) then notify hypervisor. Hypervisor will need to >>> track the unencrypted pages. The information will be used during >>> guest live migration, guest page migration and guest debugging. >>> >>> This hypercall is used to notify hypervisor when the page's >>> encryption state changes. >>> >>> Cc: Jordan Justen >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Signed-off-by: Brijesh Singh >>> Signed-off-by: Ashish Kalra >>> --- >>> OvmfPkg/Include/Library/MemEncryptSevLib.h | 69 ++++++++++++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 1 + >>> OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c | 39 +++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c | 27 ++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c | 51 +++++++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf | 1 + >>> OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c | 39 +++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c | 38 +++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm | 33 ++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c | 54 +++++++++++++++ >>> OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 22 ++++++- >>> 11 files changed, 373 insertions(+), 1 deletion(-) >>> >>> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h >>> index 76d06c206c..c2b2a99a08 100644 >>> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h >>> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h >>> @@ -90,6 +90,18 @@ MemEncryptSevIsEnabled ( >>> VOID >>> ); >>> >>> +/** >>> + Returns a boolean to indicate whether SEV live migration is enabled. >>> + >>> + @retval TRUE SEV live migration is enabled >>> + @retval FALSE SEV live migration is not enabled >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +MemEncryptSevLiveMigrationIsEnabled ( >>> + VOID >>> + ); >>> + >>> /** >>> This function clears memory encryption bit for the memory region specified by >>> BaseAddress and NumPages from the current page table context. >>> @@ -222,4 +234,61 @@ MemEncryptSevClearMmioPageEncMask ( >>> IN UINTN NumPages >>> ); >>> >>> +/** >>> + This hypercall is used to notify hypervisor when the page's encryption >>> + state changes. >>> + >>> + @param[in] PhysicalAddress The physical address that is the start address >>> + of a memory region. The PhysicalAddress is >>> + expected to be PAGE_SIZE aligned. >>> + @param[in] Pages Number of pages in memory region. >>> + @param[in] Status Encrypted(1) or Decrypted(0). >>> + >>> +@retval RETURN_SUCCESS Hypercall returned success. >>> +**/ >>> +RETURN_STATUS >>> +EFIAPI >>> +SetMemoryEncDecHypercall3 ( >>> + IN UINTN PhysicalAddress, >>> + IN UINTN Pages, >>> + IN UINTN Status >>> + ); >>> + >>> +#define KVM_HC_MAP_GPA_RANGE 12 >>> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_4K 0 >>> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_2M BIT0 >>> +#define KVM_MAP_GPA_RANGE_PAGE_SZ_1G BIT1 >>> +#define KVM_MAP_GPA_RANGE_ENC_STAT(n) ((n) << 4) >>> +#define KVM_MAP_GPA_RANGE_ENCRYPTED KVM_MAP_GPA_RANGE_ENC_STAT(1) >>> +#define KVM_MAP_GPA_RANGE_DECRYPTED KVM_MAP_GPA_RANGE_ENC_STAT(0) >>> + >>> +#define KVM_FEATURE_MIGRATION_CONTROL BIT17 >>> + >>> +/** >>> + Figures out if we are running inside KVM HVM and >>> + KVM HVM supports SEV Live Migration feature. >>> + >>> + @retval TRUE SEV live migration is supported. >>> + @retval FALSE SEV live migration is not supported. >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +KvmDetectSevLiveMigrationFeature( >>> + VOID >>> + ); >>> + >>> +/** >>> + Interface exposed by the ASM implementation of the core hypercall >>> + >>> + @retval Hypercall returned status. >>> +**/ >>> +UINTN >>> +EFIAPI >>> +SetMemoryEncDecHypercall3AsmStub ( >>> + IN UINTN HypercallNum, >>> + IN UINTN PhysicalAddress, >>> + IN UINTN Pages, >>> + IN UINTN Attributes >>> + ); >>> + >>> #endif // _MEM_ENCRYPT_SEV_LIB_H_ >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf >>> index f2e162d680..0c28afadee 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf >>> @@ -38,6 +38,7 @@ >>> X64/PeiDxeVirtualMemory.c >>> X64/VirtualMemory.c >>> X64/VirtualMemory.h >>> + X64/AsmHelperStub.nasm >>> >>> [Sources.IA32] >>> Ia32/MemEncryptSevLib.c >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c >>> index 2816f859a0..ead754cd7b 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLibInternal.c >>> @@ -20,6 +20,8 @@ >>> STATIC BOOLEAN mSevStatus = FALSE; >>> STATIC BOOLEAN mSevEsStatus = FALSE; >>> STATIC BOOLEAN mSevStatusChecked = FALSE; >>> +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE; >>> +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE; >>> >>> STATIC UINT64 mSevEncryptionMask = 0; >>> STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE; >>> @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus ( >>> mSevStatusChecked = TRUE; >>> } >>> >>> +/** >>> + Figures out if we are running inside KVM HVM and >>> + KVM HVM supports SEV Live Migration feature. >>> +**/ >>> +STATIC >>> +VOID >>> +EFIAPI >>> +InternalDetectSevLiveMigrationFeature( >>> + VOID >>> + ) >>> +{ >>> + if (KvmDetectSevLiveMigrationFeature()) { >>> + mSevLiveMigrationStatus = TRUE; >>> + } >>> + >>> + mSevLiveMigrationStatusChecked = TRUE; >>> +} >>> + >>> /** >>> Returns a boolean to indicate whether SEV-ES is enabled. >>> >>> @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled ( >>> return mSevStatus; >>> } >>> >>> +/** >>> + Returns a boolean to indicate whether SEV live migration is enabled. >>> + >>> + @retval TRUE SEV live migration is enabled >>> + @retval FALSE SEV live migration is not enabled >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +MemEncryptSevLiveMigrationIsEnabled ( >>> + VOID >>> + ) >>> +{ >>> + if (!mSevLiveMigrationStatusChecked) { >>> + InternalDetectSevLiveMigrationFeature (); >>> + } >>> + >>> + return mSevLiveMigrationStatus; >>> +} >>> + >>> /** >>> Returns the SEV encryption mask. >>> >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> index be260e0d10..62392309fe 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c >>> @@ -136,3 +136,30 @@ MemEncryptSevClearMmioPageEncMask ( >>> // >>> return RETURN_UNSUPPORTED; >>> } >>> + >>> +/** >>> + This hyercall is used to notify hypervisor when the page's encryption >>> + state changes. >>> + >>> + @param[in] PhysicalAddress The physical address that is the start address >>> + of a memory region. The physical address is >>> + expected to be PAGE_SIZE aligned. >>> + @param[in] Pages Number of Pages in the memory region. >>> + @param[in] Status Encrypted(1) or Decrypted(0). >>> + >>> +@retval RETURN_SUCCESS Hypercall returned success. >>> +**/ >>> +RETURN_STATUS >>> +EFIAPI >>> +SetMemoryEncDecHypercall3 ( >>> + IN UINTN PhysicalAddress, >>> + IN UINTN Pages, >>> + IN UINTN Status >>> + ) >>> +{ >>> + // >>> + // Memory encryption bit is not accessible in 32-bit mode >>> + // >>> + return RETURN_UNSUPPORTED; >>> +} >>> + >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c >>> index b4a9f464e2..0c9f7e17ba 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiDxeMemEncryptSevLibInternal.c >>> @@ -61,3 +61,54 @@ MemEncryptSevLocateInitialSmramSaveStateMapPages ( >>> >>> return RETURN_SUCCESS; >>> } >>> + >>> +/** >>> + Figures out if we are running inside KVM HVM and >>> + KVM HVM supports SEV Live Migration feature. >>> + >>> + @retval TRUE SEV live migration is supported. >>> + @retval FALSE SEV live migration is not supported. >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +KvmDetectSevLiveMigrationFeature( >>> + VOID >>> + ) >>> +{ >>> + CHAR8 Signature[13]; >>> + UINT32 mKvmLeaf; >>> + UINT32 RegEax, RegEbx, RegEcx, RegEdx; >>> + >>> + Signature[12] = '\0'; >>> + for (mKvmLeaf = 0x40000000; mKvmLeaf < 0x40010000; mKvmLeaf += 0x100) { >>> + AsmCpuid (mKvmLeaf, Put the first argument on its own line. >>> + NULL, >>> + (UINT32 *) &Signature[0], >>> + (UINT32 *) &Signature[4], >>> + (UINT32 *) &Signature[8]); >>> + >>> + if (AsciiStrCmp ((CHAR8 *) Signature, "KVMKVMKVM\0\0\0") == 0) { >> >> I assume this will also match if Signature is "KVMKVMKVM\0YZ". I don't >> know if that matters. >> > > I don't understand what do you mean by "KVMKVMKVM\0YZ", this is > comparing for "KVMKVMKVM\0\0\0" ? > AsciiStrCmp will stop at the first '\0' char. So adding those three '\0' at the end is pointless (the compiler will add one '\0' at the end of a literal string). Instead, you can use: if (CompareMem (Signature, "KVMKVMKVM\0\0\0", 12) == 0) and then you are sure to compare all 12 signature bytes. I'm not sure this matters at all, maybe a simple: if (AsciiStrCmp (Signature, "KVMKVMKVM") == 0) is good enough. I see similar code to detect Xen in OvmfPkg/XenPlatformPei/Xen.c . >>> + DEBUG (( >>> + DEBUG_INFO, >>> + "%a: KVM Detected, signature = %s\n", s/%s/%a/ (edk2 format strings are confusing.) >>> + __FUNCTION__, >>> + Signature >>> + )); >>> + >>> + RegEax = mKvmLeaf + 1; >>> + RegEcx = 0; >>> + AsmCpuid (mKvmLeaf + 1, &RegEax, &RegEbx, &RegEcx, &RegEdx); >>> + if ((RegEax & KVM_FEATURE_MIGRATION_CONTROL) != 0) { >>> + DEBUG (( >>> + DEBUG_INFO, >>> + "%a: Live Migration feature supported\n", >> >> I'd write: "%a: SEV Live Migration feature supported\n" >> > Ok. >> >>> + __FUNCTION__ >>> + )); >>> + >>> + return TRUE; >>> + } >>> + } >>> + } >>> + >>> + return FALSE; >>> +} >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf >>> index 03a78c32df..3233ca7979 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf >>> @@ -38,6 +38,7 @@ >>> X64/PeiDxeVirtualMemory.c >>> X64/VirtualMemory.c >>> X64/VirtualMemory.h >>> + X64/AsmHelperStub.nasm >>> >>> [Sources.IA32] >>> Ia32/MemEncryptSevLib.c >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c >>> index e2fd109d12..9db6c2ef71 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLibInternal.c >>> @@ -20,6 +20,8 @@ >>> STATIC BOOLEAN mSevStatus = FALSE; >>> STATIC BOOLEAN mSevEsStatus = FALSE; >>> STATIC BOOLEAN mSevStatusChecked = FALSE; >>> +STATIC BOOLEAN mSevLiveMigrationStatus = FALSE; >>> +STATIC BOOLEAN mSevLiveMigrationStatusChecked = FALSE; >>> >>> STATIC UINT64 mSevEncryptionMask = 0; >>> STATIC BOOLEAN mSevEncryptionMaskSaved = FALSE; >>> @@ -87,6 +89,24 @@ InternalMemEncryptSevStatus ( >>> mSevStatusChecked = TRUE; >>> } >>> >>> +/** >>> + Figures out if we are running inside KVM HVM and >>> + KVM HVM supports SEV Live Migration feature. >>> +**/ >>> +STATIC >>> +VOID >>> +EFIAPI >>> +InternalDetectSevLiveMigrationFeature( >>> + VOID >>> + ) >>> +{ >>> + if (KvmDetectSevLiveMigrationFeature()) { >>> + mSevLiveMigrationStatus = TRUE; >>> + } >>> + >>> + mSevLiveMigrationStatusChecked = TRUE; >>> +} >>> + >>> /** >>> Returns a boolean to indicate whether SEV-ES is enabled. >>> >>> @@ -125,6 +145,25 @@ MemEncryptSevIsEnabled ( >>> return mSevStatus; >>> } >>> >>> +/** >>> + Returns a boolean to indicate whether SEV live migration is enabled. >>> + >>> + @retval TRUE SEV live migration is enabled >>> + @retval FALSE SEV live migration is not enabled >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +MemEncryptSevLiveMigrationIsEnabled ( >>> + VOID >>> + ) >>> +{ >>> + if (!mSevLiveMigrationStatusChecked) { >>> + InternalDetectSevLiveMigrationFeature (); >>> + } >>> + >>> + return mSevLiveMigrationStatus; >>> +} >>> + >>> /** >>> Returns the SEV encryption mask. >>> >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c >>> index 56d8f3f318..b926c7b786 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c >>> @@ -100,6 +100,44 @@ MemEncryptSevIsEnabled ( >>> return Msr.Bits.SevBit ? TRUE : FALSE; >>> } >>> >>> +/** >>> + Interface exposed by the ASM implementation of the core hypercall >>> + >>> + @retval Hypercall returned status. >>> +**/ >>> +UINTN >>> +EFIAPI >>> +SetMemoryEncDecHypercall3AsmStub ( >>> + IN UINTN HypercallNum, >>> + IN UINTN PhysicalAddress, >>> + IN UINTN Pages, >>> + IN UINTN Attributes >>> + ) >>> +{ >>> + // >>> + // Not used in SEC phase. >>> + // >>> + return RETURN_UNSUPPORTED; >>> +} >>> + >>> +/** >>> + Returns a boolean to indicate whether SEV live migration is enabled. >>> + >>> + @retval TRUE SEV live migration is enabled >>> + @retval FALSE SEV live migration is not enabled >>> +**/ >>> +BOOLEAN >>> +EFIAPI >>> +MemEncryptSevLiveMigrationIsEnabled ( >>> + VOID >>> + ) >>> +{ >>> + // >>> + // Not used in SEC phase. >>> + // >>> + return FALSE; >>> +} >>> + >>> /** >>> Returns the SEV encryption mask. >>> >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm >>> new file mode 100644 >>> index 0000000000..c7c11f77f1 >>> --- /dev/null >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/AsmHelperStub.nasm >>> @@ -0,0 +1,33 @@ >>> +/** @file >>> + >>> + ASM helper stub to invoke hypercall >>> + >>> + Copyright (c) 2021, AMD Incorporated. All rights reserved.
>>> + >>> + SPDX-License-Identifier: BSD-2-Clause-Patent >>> + >>> +**/ >>> + >>> +DEFAULT REL >>> +SECTION .text >>> + >>> +; UINTN >>> +; EFIAPI >>> +; SetMemoryEncDecHypercall3AsmStub ( >>> +; IN UINTN HypercallNum, >>> +; IN UINTN Arg1, >>> +; IN UINTN Arg2, >>> +; IN UINTN Arg3 >>> +; ); >>> +global ASM_PFX(SetMemoryEncDecHypercall3AsmStub) >>> +ASM_PFX(SetMemoryEncDecHypercall3AsmStub): >>> + ; UEFI calling conventions require RBX to >>> + ; be nonvolatile/callee-saved. >>> + push rbx >>> + mov rax, rcx ; Copy HypercallNumber to rax >>> + mov rbx, rdx ; Copy Arg1 to the register expected by KVM >>> + mov rcx, r8 ; Copy Arg2 to register expected by KVM >>> + mov rdx, r9 ; Copy Arg2 to register expected by KVM >> >> Comment: s/Arg2/Arg3/ >> > Yes. >>> + vmmcall ; Call VMMCALL >>> + pop rbx >>> + ret >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> index a57e8fd37f..57447e69dc 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c >>> @@ -143,3 +143,57 @@ MemEncryptSevClearMmioPageEncMask ( >>> ); >>> >>> } >>> + >>> +/** >>> + This hyercall is used to notify hypervisor when the page's encryption >>> + state changes. >>> + >>> + @param[in] PhysicalAddress The physical address that is the start address >>> + of a memory region. The physical address is >>> + expected to be PAGE_SIZE aligned. >>> + @param[in] Pages Number of Pages in the memory region. >>> + @param[in] Status Encrypted(1) or Decrypted(0). >>> + >>> +@retval RETURN_SUCCESS Hypercall returned success. >> >> or RETURN_UNSUPPORTED or RETURN_NO_MAPPING. >> > > Ok. > >>> +**/ >>> +RETURN_STATUS >>> +EFIAPI >>> +SetMemoryEncDecHypercall3 ( >>> + IN UINTN PhysicalAddress, >>> + IN UINTN Pages, >>> + IN UINTN Status >> >> Consider: >> >> IN BOOL IsEncrypted >> >> or: >> >> IN MAP_RANGE_MODE EncMode >> >> (it's not a Status in the EFI_STATUS sense that appears all around edk2). >> > Ok, i think i will prefer something like a MAP_RANGE_MODE. >> >>> + ) >>> +{ >>> + RETURN_STATUS Ret; >>> + INTN Error; >>> + >> >> Add assert for the expected alignment of PhysicalAddress, and then >> you don't need to round it down when calling SetMemoryEncDecHypercall3AsmStub. >> > > Cannot really use an assert here, as when the GCD map is being walked > and the c-bit being cleared from MMIO and NonExistent memory spaces, the > physical address range being passed may not be page-aligned, so adding > an assert here prevents booting. Hence, rounding it down when calling > SetMemoryEncDecHypercall3AsmStub below. > OK. So fix the comment above the function (which says: "The physical address is expected to be PAGE_SIZE aligned.") >> >>> + Ret = RETURN_UNSUPPORTED; >>> + >>> + if (MemEncryptSevLiveMigrationIsEnabled ()) { >>> + Ret = EFI_SUCCESS; >>> + // >>> + // The encryption bit is set/clear on the smallest page size, hence >>> + // use the 4k page size in MAP_GPA_RANGE hypercall below. >>> + // Also, the hypercall expects the guest physical address to be >>> + // page-aligned. >>> + // >>> + Error = SetMemoryEncDecHypercall3AsmStub ( >>> + KVM_HC_MAP_GPA_RANGE, >>> + (PhysicalAddress & (~(EFI_PAGE_SIZE-1))), >> >> Simpler: >> >> PhysicalAddress & ~EFI_PAGE_MASK >> >> > > Ok. > >>> + Pages, >>> + KVM_MAP_GPA_RANGE_PAGE_SZ_4K | KVM_MAP_GPA_RANGE_ENC_STAT(Status) >>> + ); >>> + >>> + if (Error != 0) { >>> + DEBUG ((DEBUG_ERROR, >>> + "SetMemoryEncDecHypercall3 failed, Phys = %Lx, Pages = %Ld, Err = %Ld\n", >>> + PhysicalAddress, >>> + Pages, >>> + (INT64)Error)); >>> + >>> + Ret = RETURN_NO_MAPPING; >>> + } >>> + } >>> + >>> + return Ret; >>> +} >>> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> index c696745f9d..0b1588a4c1 100644 >>> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c >>> @@ -536,7 +536,6 @@ EnableReadOnlyPageWriteProtect ( >>> AsmWriteCr0 (AsmReadCr0() | BIT16); >>> } >>> >>> - >>> /** >>> This function either sets or clears memory encryption bit for the memory >>> region specified by PhysicalAddress and Length from the current page table >>> @@ -585,6 +584,9 @@ SetMemoryEncDec ( >>> UINT64 AddressEncMask; >>> BOOLEAN IsWpEnabled; >>> RETURN_STATUS Status; >>> + UINTN Size; >>> + BOOLEAN CBitChanged; >>> + PHYSICAL_ADDRESS OrigPhysicalAddress; >>> >>> // >>> // Set PageMapLevel4Entry to suppress incorrect compiler/analyzer warnings. >>> @@ -636,6 +638,10 @@ SetMemoryEncDec ( >>> >>> Status = EFI_SUCCESS; >>> >>> + Size = Length; >>> + CBitChanged = FALSE; >>> + OrigPhysicalAddress = PhysicalAddress; >>> + >>> while (Length != 0) >>> { >>> // >>> @@ -695,6 +701,7 @@ SetMemoryEncDec ( >>> )); >>> PhysicalAddress += BIT30; >>> Length -= BIT30; >>> + CBitChanged = TRUE; >>> } else { >>> // >>> // We must split the page >>> @@ -749,6 +756,7 @@ SetMemoryEncDec ( >>> SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode); >>> PhysicalAddress += BIT21; >>> Length -= BIT21; >>> + CBitChanged = TRUE; >>> } else { >>> // >>> // We must split up this page into 4K pages >>> @@ -791,6 +799,7 @@ SetMemoryEncDec ( >>> SetOrClearCBit (&PageTableEntry->Uint64, Mode); >>> PhysicalAddress += EFI_PAGE_SIZE; >>> Length -= EFI_PAGE_SIZE; >>> + CBitChanged = TRUE; >>> } >>> } >>> } >>> @@ -808,6 +817,17 @@ SetMemoryEncDec ( >>> // >>> CpuFlushTlb(); >>> >>> + // >>> + // Notify Hypervisor on C-bit status >>> + // >>> + if (CBitChanged) { >>> + Status = SetMemoryEncDecHypercall3 ( >>> + OrigPhysicalAddress, >>> + EFI_SIZE_TO_PAGES(Size), >>> + !Mode >> >> Here you pass !Mode (which is 0 or 1) as the third argument to >> SetMemoryEncDecHypercall3 . >> >> But on patch 3/4 you pass KVM_MAP_GPA_RANGE_DECRYPTED (which is 0<<4); >> but that hints that you expect either 0<<4 or 1<<4 as this third argument. >> If this is the case, then here it should be: >> >> (Mode == SetCBit) ? KVM_MAP_GPA_RANGE_ENCRYPTED : KVM_MAP_GPA_RANGE_DECRYPTED >> >> If it's the other way around, then patch 3/4 needs to pass a simple 0 as >> the third argument. >> > > Yes, i need to pass a 0 (decrypted) as the third argument in that patch. > Even clearer than a literal 0 -- pass a ClearCBit as the third argument (assuming you're changing the argument type to MAP_RANGE_MODE). -Dov > Thanks, > Ashish >> >> >> >>> + ); >>> + } >>> + >>> Done: >>> // >>> // Restore page table write protection, if any. >>>