[PATCH v4 0/4] SEV Live Migration support for OVMF.


Ashish Kalra
 

From: Ashish Kalra <ashish.kalra@amd.com>

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.

The patch-set adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

The patch-set detects if it is running under KVM hypervisor and then
checks for SEV live migration feature support via KVM_FEATURE_CPUID,
if detected setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

A branch containing these patches is available here:
https://github.com/ashkalra/edk2/tree/sev_live_migration_v4

Changes since v3:
- Fix all DSC files under OvmfPkg except X64 to add support for
BaseMemEncryptLib and add NULL instance of BaseMemEncryptLib
for 32 bit platforms.
- Add the MemEncryptHypercallLib-related files to Maintainers.txt,
in section "OvmfPkg: Confidential Computing".
- Add support for the new KVM_HC_MAP_GPA_RANGE hypercall interface.
- Add patch for SEV live migration support.

Changes since v2:
- GHCB_BASE setup during reset-vector as decrypted is marked explicitly
in the hypervisor page encryption bitmap after setting the
PcdSevEsIsEnabled PCD.

Changes since v1:
- Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
the hypervisor page encryption bitmap.
- Resending the series with correct shallow threading.

Ashish Kalra (3):
OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Maintainers.txt | 2 +
OvmfPkg/Include/Guid/MemEncryptLib.h | 20 ++++
.../Include/Library/MemEncryptHypercallLib.h | 43 +++++++
.../DxeMemEncryptSevLib.inf | 1 +
.../PeiMemEncryptSevLib.inf | 1 +
.../X64/PeiDxeVirtualMemory.c | 22 ++++
.../Ia32/MemEncryptHypercallLib.c | 37 ++++++
.../MemEncryptHypercallLib.inf | 42 +++++++
.../X64/AsmHelperStub.nasm | 28 +++++
.../X64/MemEncryptHypercallLib.c | 105 +++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/PlatformDxe/AmdSev.c | 108 ++++++++++++++++++
OvmfPkg/PlatformDxe/Platform.c | 5 +
OvmfPkg/PlatformDxe/Platform.inf | 2 +
OvmfPkg/PlatformDxe/PlatformConfig.h | 5 +
OvmfPkg/PlatformPei/AmdSev.c | 10 ++
20 files changed, 436 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c

--
2.17.1


Laszlo Ersek
 

Hi Ashish,

(+Dave, +Paolo)

On 06/21/21 15:56, Ashish Kalra wrote:
From: Ashish Kalra <ashish.kalra@amd.com>

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.

The patch-set adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

The patch-set detects if it is running under KVM hypervisor and then
checks for SEV live migration feature support via KVM_FEATURE_CPUID,
if detected setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

A branch containing these patches is available here:
https://github.com/ashkalra/edk2/tree/sev_live_migration_v4

Changes since v3:
- Fix all DSC files under OvmfPkg except X64 to add support for
BaseMemEncryptLib and add NULL instance of BaseMemEncryptLib
for 32 bit platforms.
- Add the MemEncryptHypercallLib-related files to Maintainers.txt,
in section "OvmfPkg: Confidential Computing".
- Add support for the new KVM_HC_MAP_GPA_RANGE hypercall interface.
- Add patch for SEV live migration support.
I have absolutely zero context in my mind about this work.

By v1 / v2 / v3, are you referring to the following patch series (from December 2020):

- [PATCH v1 0/2] SEV Page Encryption Bitmap support for OVMF.
https://listman.redhat.com/archives/edk2-devel-archive/2020-December/msg00081.html

- [PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.
https://listman.redhat.com/archives/edk2-devel-archive/2020-December/msg00198.html

- [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
https://listman.redhat.com/archives/edk2-devel-archive/2020-December/msg00202.html

We certainly need a new TianoCore BZ for tracking this feature; I only found the above patch set versions because I have full text search for my complete email traffic on my laptop. Sending v4 after half a year hiatus is like sending it in the next century. :)

Anyway, where I'm particularly lost is that I (very vaguely) recall conflicting approaches from AMD and IBM on migration. Has an agreement been reached there?

I certainly apologize for missing the context here; had someone asked me if I had seen any version of this patch set before, I would have *sworn* that I hadn't.

I'm basically incapable of tracking this volume of development around confidential computing; sorry.

Laszlo


Changes since v2:
- GHCB_BASE setup during reset-vector as decrypted is marked explicitly
in the hypervisor page encryption bitmap after setting the
PcdSevEsIsEnabled PCD.

Changes since v1:
- Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
the hypervisor page encryption bitmap.
- Resending the series with correct shallow threading.

Ashish Kalra (3):
OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Maintainers.txt | 2 +
OvmfPkg/Include/Guid/MemEncryptLib.h | 20 ++++
.../Include/Library/MemEncryptHypercallLib.h | 43 +++++++
.../DxeMemEncryptSevLib.inf | 1 +
.../PeiMemEncryptSevLib.inf | 1 +
.../X64/PeiDxeVirtualMemory.c | 22 ++++
.../Ia32/MemEncryptHypercallLib.c | 37 ++++++
.../MemEncryptHypercallLib.inf | 42 +++++++
.../X64/AsmHelperStub.nasm | 28 +++++
.../X64/MemEncryptHypercallLib.c | 105 +++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/PlatformDxe/AmdSev.c | 108 ++++++++++++++++++
OvmfPkg/PlatformDxe/Platform.c | 5 +
OvmfPkg/PlatformDxe/Platform.inf | 2 +
OvmfPkg/PlatformDxe/PlatformConfig.h | 5 +
OvmfPkg/PlatformPei/AmdSev.c | 10 ++
20 files changed, 436 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c


Brijesh Singh
 

Hi Ashish,

I have queue'd to review this series for later part of the week.
Just curious, did you run CI on this series ? A quick glance hints that this
series may fail to build on some platforms and additionally have formatting
error.

P.S: If you don't know how to use EDK2 CI then buzz me off-list.

thanks

On 6/22/2021 12:20 PM, Laszlo Ersek wrote:
Hi Ashish,

(+Dave, +Paolo)

On 06/21/21 15:56, Ashish Kalra wrote:
From: Ashish Kalra <ashish.kalra@amd.com>

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.

The patch-set adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

The patch-set detects if it is running under KVM hypervisor and then
checks for SEV live migration feature support via KVM_FEATURE_CPUID,
if detected setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

A branch containing these patches is available here:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_live_migration_v4&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb6f0cd9ca0cb4203327908d935a21cb3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792656890122%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=zwiAg6jzSPYtUA8UARYE6K39Q3VCJkhm9Ey00aGYC10%3D&amp;reserved=0

Changes since v3:
- Fix all DSC files under OvmfPkg except X64 to add support for
BaseMemEncryptLib and add NULL instance of BaseMemEncryptLib
for 32 bit platforms.
- Add the MemEncryptHypercallLib-related files to Maintainers.txt,
in section "OvmfPkg: Confidential Computing".
- Add support for the new KVM_HC_MAP_GPA_RANGE hypercall interface.
- Add patch for SEV live migration support.
I have absolutely zero context in my mind about this work.

By v1 / v2 / v3, are you referring to the following patch series (from December 2020):

- [PATCH v1 0/2] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00081.html&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb6f0cd9ca0cb4203327908d935a21cb3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792656890122%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=QkZUdYyeWREfXyx2%2B32chbp7dMzEVfBb78dEsecduFw%3D&amp;reserved=0

- [PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00198.html&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb6f0cd9ca0cb4203327908d935a21cb3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792656900118%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=TH%2BbYo%2B2CZyOunhIpegEjqQkdXlBuZsiyWz1k%2BGXtQc%3D&amp;reserved=0

- [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00202.html&;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cb6f0cd9ca0cb4203327908d935a21cb3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792656900118%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=css90wZ%2BFgbYm%2FQjvCLIFZwwozZz3dfzaVPDpsQsCsk%3D&amp;reserved=0

We certainly need a new TianoCore BZ for tracking this feature; I only found the above patch set versions because I have full text search for my complete email traffic on my laptop. Sending v4 after half a year hiatus is like sending it in the next century. :)

Anyway, where I'm particularly lost is that I (very vaguely) recall conflicting approaches from AMD and IBM on migration. Has an agreement been reached there?

I certainly apologize for missing the context here; had someone asked me if I had seen any version of this patch set before, I would have *sworn* that I hadn't.

I'm basically incapable of tracking this volume of development around confidential computing; sorry.

Laszlo


Changes since v2:
- GHCB_BASE setup during reset-vector as decrypted is marked explicitly
in the hypervisor page encryption bitmap after setting the
PcdSevEsIsEnabled PCD.

Changes since v1:
- Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
the hypervisor page encryption bitmap.
- Resending the series with correct shallow threading.

Ashish Kalra (3):
OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Maintainers.txt | 2 +
OvmfPkg/Include/Guid/MemEncryptLib.h | 20 ++++
.../Include/Library/MemEncryptHypercallLib.h | 43 +++++++
.../DxeMemEncryptSevLib.inf | 1 +
.../PeiMemEncryptSevLib.inf | 1 +
.../X64/PeiDxeVirtualMemory.c | 22 ++++
.../Ia32/MemEncryptHypercallLib.c | 37 ++++++
.../MemEncryptHypercallLib.inf | 42 +++++++
.../X64/AsmHelperStub.nasm | 28 +++++
.../X64/MemEncryptHypercallLib.c | 105 +++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/PlatformDxe/AmdSev.c | 108 ++++++++++++++++++
OvmfPkg/PlatformDxe/Platform.c | 5 +
OvmfPkg/PlatformDxe/Platform.inf | 2 +
OvmfPkg/PlatformDxe/PlatformConfig.h | 5 +
OvmfPkg/PlatformPei/AmdSev.c | 10 ++
20 files changed, 436 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c


Ashish Kalra
 

Hello Laszlo,

Please see my replies below :

On Tue, Jun 22, 2021 at 07:20:53PM +0200, Laszlo Ersek wrote:
Hi Ashish,

(+Dave, +Paolo)

On 06/21/21 15:56, Ashish Kalra wrote:
From: Ashish Kalra <ashish.kalra@amd.com>

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.

The patch-set adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

The patch-set detects if it is running under KVM hypervisor and then
checks for SEV live migration feature support via KVM_FEATURE_CPUID,
if detected setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

A branch containing these patches is available here:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_live_migration_v4&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=41Jj%2BGNUyEL43UhZgI19iwGcOJcP%2FWg94D8fTopaZxQ%3D&amp;reserved=0

Changes since v3:
- Fix all DSC files under OvmfPkg except X64 to add support for
BaseMemEncryptLib and add NULL instance of BaseMemEncryptLib
for 32 bit platforms.
- Add the MemEncryptHypercallLib-related files to Maintainers.txt,
in section "OvmfPkg: Confidential Computing".
- Add support for the new KVM_HC_MAP_GPA_RANGE hypercall interface.
- Add patch for SEV live migration support.
I have absolutely zero context in my mind about this work.

By v1 / v2 / v3, are you referring to the following patch series (from December 2020):

- [PATCH v1 0/2] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00081.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=cVUvWe6VgjR78OAk5LXgBQiKon4Gp%2F62a2hc%2FKwoLw4%3D&amp;reserved=0

- [PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00198.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=IXLGA2ttyxdVoC63HeCkPVNuUMH3u5Vd3U1fc6c8LQc%3D&amp;reserved=0

- [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00202.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=cOJBqGyM3a7xMbfhbBAh3IEm7IBJFOGu2ReQMSFJ%2BDw%3D&amp;reserved=0
Yes, actually the guest kernel API for SEV live migration was not
decided at the time of the last patch-set submission, hence, i am now
submitting this patch-set as the guest kernel API has been discussed and
closed and the corresponding KVM/kernel patches have been queued now.

And therefore, this is simply not the SEV Page Encryption Bitmap support
anymore, but the SEV live migration support which includes the guest page
encryption status tracking.

We certainly need a new TianoCore BZ for tracking this feature; I only found the above patch set versions because I have full text search for my complete email traffic on my laptop. Sending v4 after half a year hiatus is like sending it in the next century. :)
Ok.

As i mentioned above it took such a long time to re-submit the
patch-set because of the guest kernel API discussions taking some
time and getting closed.

Anyway, where I'm particularly lost is that I (very vaguely) recall conflicting approaches from AMD and IBM on migration. Has an agreement been reached there?
Yes, these are the slow SEV live migration patches. The fast migration
support is being developed by IBM and that is a separate effort.

As this slow live migration support has now been included in KVM, we
will need the corresponding OVMF and QEMU support now.

I certainly apologize for missing the context here; had someone asked me if I had seen any version of this patch set before, I would have *sworn* that I hadn't.

I'm basically incapable of tracking this volume of development around confidential computing; sorry.

Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.

I believe that they are anyway the maintainers for confidential computing related stuff.

Thanks,
Ashish


Changes since v2:
- GHCB_BASE setup during reset-vector as decrypted is marked explicitly
in the hypervisor page encryption bitmap after setting the
PcdSevEsIsEnabled PCD.

Changes since v1:
- Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
the hypervisor page encryption bitmap.
- Resending the series with correct shallow threading.

Ashish Kalra (3):
OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Maintainers.txt | 2 +
OvmfPkg/Include/Guid/MemEncryptLib.h | 20 ++++
.../Include/Library/MemEncryptHypercallLib.h | 43 +++++++
.../DxeMemEncryptSevLib.inf | 1 +
.../PeiMemEncryptSevLib.inf | 1 +
.../X64/PeiDxeVirtualMemory.c | 22 ++++
.../Ia32/MemEncryptHypercallLib.c | 37 ++++++
.../MemEncryptHypercallLib.inf | 42 +++++++
.../X64/AsmHelperStub.nasm | 28 +++++
.../X64/MemEncryptHypercallLib.c | 105 +++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/PlatformDxe/AmdSev.c | 108 ++++++++++++++++++
OvmfPkg/PlatformDxe/Platform.c | 5 +
OvmfPkg/PlatformDxe/Platform.inf | 2 +
OvmfPkg/PlatformDxe/PlatformConfig.h | 5 +
OvmfPkg/PlatformPei/AmdSev.c | 10 ++
20 files changed, 436 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c


Dov Murik
 

+cc Tobin


On 22/06/2021 20:46, Ashish Kalra via groups.io wrote:
Hello Laszlo,

Please see my replies below :

On Tue, Jun 22, 2021 at 07:20:53PM +0200, Laszlo Ersek wrote:
-snip-



Anyway, where I'm particularly lost is that I (very vaguely) recall conflicting approaches from AMD and IBM on migration. Has an agreement been reached there?
Yes, these are the slow SEV live migration patches. The fast migration
support is being developed by IBM and that is a separate effort.

As this slow live migration support has now been included in KVM, we
will need the corresponding OVMF and QEMU support now.

I'll also add that the fast (guest-assisted) SEV migration we (IBM) are
working on uses some of the features of the "slow" migration. For
example, for sure, patches 1-3 in this series will be needed to keep the
record of which of the guest pages are encrypted and which are shared
(also in fast migration).

-Dov


Laszlo Ersek
 

On 06/22/21 19:46, Ashish Kalra wrote:
Hello Laszlo,

Please see my replies below :

On Tue, Jun 22, 2021 at 07:20:53PM +0200, Laszlo Ersek wrote:
Hi Ashish,

(+Dave, +Paolo)

On 06/21/21 15:56, Ashish Kalra wrote:
From: Ashish Kalra <ashish.kalra@amd.com>

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.

The patch-set adds a new SEV and SEV-ES hypercall abstraction
library to support SEV Page encryption/decryption status hypercalls
for SEV and SEV-ES guests.

BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.

The patch-set detects if it is running under KVM hypervisor and then
checks for SEV live migration feature support via KVM_FEATURE_CPUID,
if detected setup a new UEFI enviroment variable to indicate OVMF
support for SEV live migration.

A branch containing these patches is available here:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_live_migration_v4&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=41Jj%2BGNUyEL43UhZgI19iwGcOJcP%2FWg94D8fTopaZxQ%3D&amp;reserved=0

Changes since v3:
- Fix all DSC files under OvmfPkg except X64 to add support for
BaseMemEncryptLib and add NULL instance of BaseMemEncryptLib
for 32 bit platforms.
- Add the MemEncryptHypercallLib-related files to Maintainers.txt,
in section "OvmfPkg: Confidential Computing".
- Add support for the new KVM_HC_MAP_GPA_RANGE hypercall interface.
- Add patch for SEV live migration support.
I have absolutely zero context in my mind about this work.

By v1 / v2 / v3, are you referring to the following patch series (from December 2020):

- [PATCH v1 0/2] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00081.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=cVUvWe6VgjR78OAk5LXgBQiKon4Gp%2F62a2hc%2FKwoLw4%3D&amp;reserved=0

- [PATCH v2 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00198.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=IXLGA2ttyxdVoC63HeCkPVNuUMH3u5Vd3U1fc6c8LQc%3D&amp;reserved=0

- [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flistman.redhat.com%2Farchives%2Fedk2-devel-archive%2F2020-December%2Fmsg00202.html&;data=04%7C01%7Cashish.kalra%40amd.com%7Ca402470f4cba428feaac08d935a21c79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637599792647899814%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=cOJBqGyM3a7xMbfhbBAh3IEm7IBJFOGu2ReQMSFJ%2BDw%3D&amp;reserved=0
Yes, actually the guest kernel API for SEV live migration was not
decided at the time of the last patch-set submission, hence, i am now
submitting this patch-set as the guest kernel API has been discussed and
closed and the corresponding KVM/kernel patches have been queued now.

And therefore, this is simply not the SEV Page Encryption Bitmap support
anymore, but the SEV live migration support which includes the guest page
encryption status tracking.

We certainly need a new TianoCore BZ for tracking this feature; I only found the above patch set versions because I have full text search for my complete email traffic on my laptop. Sending v4 after half a year hiatus is like sending it in the next century. :)
Ok.

As i mentioned above it took such a long time to re-submit the
patch-set because of the guest kernel API discussions taking some
time and getting closed.

Anyway, where I'm particularly lost is that I (very vaguely) recall conflicting approaches from AMD and IBM on migration. Has an agreement been reached there?
Yes, these are the slow SEV live migration patches. The fast migration
support is being developed by IBM and that is a separate effort.

As this slow live migration support has now been included in KVM, we
will need the corresponding OVMF and QEMU support now.

I certainly apologize for missing the context here; had someone asked me if I had seen any version of this patch set before, I would have *sworn* that I hadn't.

I'm basically incapable of tracking this volume of development around confidential computing; sorry.

Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.
Indeed. This helps. Thanks.

I'll keep this patch set in my review queue then, for said "formalities
review".

Thanks.
Laszlo


I believe that they are anyway the maintainers for confidential computing related stuff.

Thanks,
Ashish


Changes since v2:
- GHCB_BASE setup during reset-vector as decrypted is marked explicitly
in the hypervisor page encryption bitmap after setting the
PcdSevEsIsEnabled PCD.

Changes since v1:
- Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
the hypervisor page encryption bitmap.
- Resending the series with correct shallow threading.

Ashish Kalra (3):
OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall
OvmfPkg/PlatformDxe: Add support for SEV live migration.

Brijesh Singh (1):
OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall

Maintainers.txt | 2 +
OvmfPkg/Include/Guid/MemEncryptLib.h | 20 ++++
.../Include/Library/MemEncryptHypercallLib.h | 43 +++++++
.../DxeMemEncryptSevLib.inf | 1 +
.../PeiMemEncryptSevLib.inf | 1 +
.../X64/PeiDxeVirtualMemory.c | 22 ++++
.../Ia32/MemEncryptHypercallLib.c | 37 ++++++
.../MemEncryptHypercallLib.inf | 42 +++++++
.../X64/AsmHelperStub.nasm | 28 +++++
.../X64/MemEncryptHypercallLib.c | 105 +++++++++++++++++
OvmfPkg/OvmfPkg.dec | 1 +
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/PlatformDxe/AmdSev.c | 108 ++++++++++++++++++
OvmfPkg/PlatformDxe/Platform.c | 5 +
OvmfPkg/PlatformDxe/Platform.inf | 2 +
OvmfPkg/PlatformDxe/PlatformConfig.h | 5 +
OvmfPkg/PlatformPei/AmdSev.c | 10 ++
20 files changed, 436 insertions(+)
create mode 100644 OvmfPkg/Include/Guid/MemEncryptLib.h
create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/Ia32/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/MemEncryptHypercallLib.c
create mode 100644 OvmfPkg/PlatformDxe/AmdSev.c


Laszlo Ersek
 

On 06/23/21 18:42, Laszlo Ersek wrote:
On 06/22/21 19:46, Ashish Kalra wrote:
Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.
Indeed. This helps. Thanks.

I'll keep this patch set in my review queue then, for said "formalities
review".
Please do file a TianoCore Feature Request BZ for this, and reference
the bug URL in the commit messages. One important purpose of such a BZ
is to succinctly link together all versions of a patch set -- that way
poor maintainers know where to find previous versions, even if the blurb
subject line changes over time. I also like to capture "permanent
workflow notes" like the above in BZs (basically a high-level summary of
who does what).

For now it seems that a v5 will be necessary. Please keep me on CC, and
when you have the BZ filed, we should link all past and future versions
into it.

Thanks
Laszlo


Ashish Kalra
 

Hello Laszlo,

Yes i will file a TianoCore Feature Request BZ for this and i am working
on a v5 for this patch-set.

Thanks,
Ashish

On Wed, Jun 23, 2021 at 06:49:06PM +0200, Laszlo Ersek wrote:
On 06/23/21 18:42, Laszlo Ersek wrote:
On 06/22/21 19:46, Ashish Kalra wrote:
Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.
Indeed. This helps. Thanks.

I'll keep this patch set in my review queue then, for said "formalities
review".
Please do file a TianoCore Feature Request BZ for this, and reference
the bug URL in the commit messages. One important purpose of such a BZ
is to succinctly link together all versions of a patch set -- that way
poor maintainers know where to find previous versions, even if the blurb
subject line changes over time. I also like to capture "permanent
workflow notes" like the above in BZs (basically a high-level summary of
who does what).

For now it seems that a v5 will be necessary. Please keep me on CC, and
when you have the BZ filed, we should link all past and future versions
into it.

Thanks
Laszlo


Ashish Kalra
 

Hello Laszlo,

On Wed, Jun 23, 2021 at 06:49:06PM +0200, Laszlo Ersek wrote:
On 06/23/21 18:42, Laszlo Ersek wrote:
On 06/22/21 19:46, Ashish Kalra wrote:
Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.
Indeed. This helps. Thanks.

I'll keep this patch set in my review queue then, for said "formalities
review".
Please do file a TianoCore Feature Request BZ for this, and reference
the bug URL in the commit messages. One important purpose of such a BZ
is to succinctly link together all versions of a patch set -- that way
poor maintainers know where to find previous versions, even if the blurb
subject line changes over time. I also like to capture "permanent
workflow notes" like the above in BZs (basically a high-level summary of
who does what).
I have filed a new TianoCore Feature request BZ for this.
https://bugzilla.tianocore.org/show_bug.cgi?id=3467

I will refer this bug in future commit messages for this patch-set.

Please let me know if you want me to add additional contents and
comments to this bug.

Thanks,
Ashish

For now it seems that a v5 will be necessary. Please keep me on CC, and
when you have the BZ filed, we should link all past and future versions
into it.

Thanks
Laszlo


Laszlo Ersek
 

On 06/30/21 11:11, Ashish Kalra via groups.io wrote:
Hello Laszlo,

On Wed, Jun 23, 2021 at 06:49:06PM +0200, Laszlo Ersek wrote:
On 06/23/21 18:42, Laszlo Ersek wrote:
On 06/22/21 19:46, Ashish Kalra wrote:
Please find below your reply on v3 of this patch-set :

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for formalities. I'd like to receive an R-b from Tom or Brijesh [*], and another from Dov or a colleague at IBM, for this series; those together should suffice for merging the library.

So, if you are fine with this approach, then probably Tom or Brijesh can
take these patches under their reviewership and provide their R-b for
this patch-set to be accepted and merged.
Indeed. This helps. Thanks.

I'll keep this patch set in my review queue then, for said "formalities
review".
Please do file a TianoCore Feature Request BZ for this, and reference
the bug URL in the commit messages. One important purpose of such a BZ
is to succinctly link together all versions of a patch set -- that way
poor maintainers know where to find previous versions, even if the blurb
subject line changes over time. I also like to capture "permanent
workflow notes" like the above in BZs (basically a high-level summary of
who does what).
I have filed a new TianoCore Feature request BZ for this.
https://bugzilla.tianocore.org/show_bug.cgi?id=3467

I will refer this bug in future commit messages for this patch-set.

Please let me know if you want me to add additional contents and
comments to this bug.
Thanks for filing the BZ, I've captured my previous statements / wishes
in some new BZ comments now.

Laszlo


Thanks,
Ashish

For now it seems that a v5 will be necessary. Please keep me on CC, and
when you have the BZ filed, we should link all past and future versions
into it.

Thanks
Laszlo