public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ashish Kalra" <ashish.kalra@amd.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: devel@edk2.groups.io, brijesh.singh@amd.com,
	Thomas.Lendacky@amd.com, jejb@linux.ibm.com,
	erdemaktas@google.com, jiewen.yao@intel.com, min.m.xu@intel.com,
	jordan.l.justen@intel.com, ard.biesheuvel@arm.com,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v4 0/4] SEV Live Migration support for OVMF.
Date: Tue, 22 Jun 2021 17:46:03 +0000	[thread overview]
Message-ID: <20210622174603.GA6366@ashkalra_ubuntu_server> (raw)
In-Reply-To: <dffa37d8-9139-4d55-18c1-e88bc081def4@redhat.com>

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&amp;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&amp;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&amp;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&amp;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
> > 
> 

  parent reply	other threads:[~2021-06-22 17:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21 13:56 [PATCH v4 0/4] SEV Live Migration support for OVMF Ashish Kalra
2021-06-21 13:56 ` [PATCH v4 1/4] OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls Ashish Kalra
2021-06-22 19:47   ` Brijesh Singh
2021-06-22 19:58     ` Brijesh Singh
2021-06-22 22:47   ` Lendacky, Thomas
2021-06-22 23:20     ` Ashish Kalra
2021-06-22 23:38       ` Brijesh Singh
2021-06-23  1:47     ` Ashish Kalra
2021-06-23 15:02       ` Ashish Kalra
2021-06-21 13:57 ` [PATCH v4 2/4] OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall Ashish Kalra
2021-06-22 22:50   ` Lendacky, Thomas
2021-06-21 13:57 ` [PATCH v4 3/4] OvmfPkg/PlatformPei: Mark SEC GHCB page as unencrypted via hypercall Ashish Kalra
2021-06-22 20:35   ` Brijesh Singh
2021-06-21 13:57 ` [PATCH v4 4/4] OvmfPkg/PlatformDxe: Add support for SEV live migration Ashish Kalra
2021-06-22 23:06   ` Lendacky, Thomas
2021-06-24 16:29     ` Ashish Kalra
2021-06-22 17:20 ` [PATCH v4 0/4] SEV Live Migration support for OVMF Laszlo Ersek
2021-06-22 17:45   ` Brijesh Singh
2021-06-22 17:46   ` Ashish Kalra [this message]
2021-06-23 13:18     ` [edk2-devel] " Dov Murik
2021-06-23 16:42     ` Laszlo Ersek
2021-06-23 16:49       ` Laszlo Ersek
2021-06-23 17:03         ` Ashish Kalra
2021-06-30  9:11         ` Ashish Kalra
2021-06-30 16:25           ` [edk2-devel] " Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210622174603.GA6366@ashkalra_ubuntu_server \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox