lkml.org 
[lkml]   [2021]   [Oct]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC 06/16] KVM: selftests: add library for creating/interacting with SEV guests
On Mon, Oct 11, 2021 at 08:15:37PM -0500, Michael Roth wrote:
> On Sun, Oct 10, 2021 at 08:17:00PM -0700, Marc Orr wrote:
> > On Wed, Oct 6, 2021 at 1:40 PM Michael Roth <michael.roth@amd.com> wrote:
> > >
> > > Add interfaces to allow tests to create/manage SEV guests. The
> > > additional state associated with these guests is encapsulated in a new
> > > struct sev_vm, which is a light wrapper around struct kvm_vm. These
> > > VMs will use vm_set_memory_encryption() and vm_get_encrypted_phy_pages()
> > > under the covers to configure and sync up with the core kvm_util
> > > library on what should/shouldn't be treated as encrypted memory.
> > >
> > > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > > ---
> > > tools/testing/selftests/kvm/Makefile | 1 +
> > > .../selftests/kvm/include/x86_64/sev.h | 62 ++++
> > > tools/testing/selftests/kvm/lib/x86_64/sev.c | 303 ++++++++++++++++++
> > > 3 files changed, 366 insertions(+)
> > > create mode 100644 tools/testing/selftests/kvm/include/x86_64/sev.h
> > > create mode 100644 tools/testing/selftests/kvm/lib/x86_64/sev.c
> > >
> > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > > index 5832f510a16c..c7a5e1c69e0c 100644
> > > --- a/tools/testing/selftests/kvm/Makefile
> > > +++ b/tools/testing/selftests/kvm/Makefile
> > > @@ -35,6 +35,7 @@ endif
> > >
> > > LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/rbtree.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
> > > LIBKVM_x86_64 = lib/x86_64/apic.c lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
> > > +LIBKVM_x86_64 += lib/x86_64/sev.c
> >
> > Regarding RFC-level feedback: First off, I'm super jazzed with what
> > I'm seeing so far! (While this is my first review, I've been studying
> > the patches up through the SEV boot test, i.e., patch #7). One thing
> > I'm wondering is: the way this is structured is to essentially split
> > the test cases into non-SEV and SEV. I'm wondering how hard it would
> > be to add some flag or environment variable to set up pre-existing
> > tests to run under SEV. Or is this something you all thought about,
> > and decided that it does not make sense?
> >
> > Looking at how the guest memory is handled, it seems like it's not far
> > off from handling SEV transparently across all test cases. I'd think
> > that we could just default all memory to use the encryption bit, and
> > then have test cases, such as the test case in patch #7, clear the
> > encryption bit for shared pages. However, I think the VM creation
> > would need a bit more refactoring to work with other test cases.
>
> I think it's possible, but there's a few missing pieces:
>
> 1) As you indicated, existing tests which rely on vm_create(),
> vm_create_default(), vm_create_default_with_vcpus(), etc. would either
> need to be updated with whatever new interface provides this 'use-sev'
> flag, or it would need to happen underneath the covers based on said
> environment variable/global/etc. There's also the question of where
> to hook in the sev_vm_launch_start() hooks. Maybe the first time a
> vcpu_run() is issued? Or maybe some explict call each test will need
> to be updated to call just prior to initial execution.
>
> 2) Many of the existing tests use the GUESY_SYNC/ucall stuff to handle
> synchronization between host userspace and guest kernel, which relies on
> guests issuing PIO instructions to particular port addresses to cause an
> exit back to host userspace, with various parameters passed via register
> arguments.
>
> - For SEV this would almost work as-is, but some tests might rely on
> things like memory addresses being passed in this way so would need
> to audit the code and mark that memory as shared where needed.
>
> - For SEV-ES/SEV-SNP, there's a bit more work since:
>
> - The registers will not be accessible through the existing
> KVM_GET_REGS mechanism. It may be possible to set some flag/hook to
> set/access arguments through some other mechanism like a shared
> buffer for certain VM types though.
>
> - Additionally, the #VC handler only supports CPUID currently, and
> leverages that fact to avoid doing any significant instruction
> decoding. Instead the SEV tests use HLT instructions to handle exits
> to host userspace, which may not work for some tests. So unless
> there's some other mechanism that SEV/non-SEV tests could utilize
> rather that PIO, the #VC handler would need to support PIO, which
> would be nice to have either way, but would likely involve
> pulling in the intruction decoder library used in the kernel, or
> some subset/re-implementation of it at least.
>
> 3) Similar to SEV-ES/SEV-SNP requirements for 1), tests which generate
> PIO/MMIO and other NAE events would need appropriate support for those
> events in the #VC handler. Nice-to-have either way, but not sure atm
> how much it would be to implement all of that. Also any tests relying
> on things like KVM_GET_REGS/KVM_GET_SREGS are non-starters.

One more I should mention:

4) After encryption, the page table is no longer usable for translations by
stuff like addr_gva2gpa(), so tests would either need to be
audited/updated to do these translations upfront and only rely on
cached/stored values thereafter, or perhaps a "shadow" copy could be
maintained by kvm_util so the translations will continue to work
after encryption.

\
 
 \ /
  Last update: 2021-10-12 14:56    [W:0.139 / U:1.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site