[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b651b14f-9ad2-811d-0d16-21d23a65eba4@redhat.com>
Date: Wed, 22 Dec 2021 15:46:24 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Michael Roth <michael.roth@....com>,
linux-kselftest@...r.kernel.org
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, x86@...nel.org,
Nathan Tempelman <natet@...gle.com>,
Marc Orr <marcorr@...gle.com>,
Steve Rutherford <srutherford@...gle.com>,
Sean Christopherson <seanjc@...gle.com>,
Mingwei Zhang <mizhang@...gle.com>,
Brijesh Singh <brijesh.singh@....com>,
Tom Lendacky <thomas.lendacky@....com>,
Varad Gautam <varad.gautam@...e.com>,
Shuah Khan <shuah@...nel.org>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
David Woodhouse <dwmw@...zon.co.uk>,
Ricardo Koller <ricarkol@...gle.com>,
Jim Mattson <jmattson@...gle.com>,
Joerg Roedel <joro@...tes.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
"H . Peter Anvin" <hpa@...or.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Janosch Frank <frankja@...ux.ibm.com>,
David Hildenbrand <david@...hat.com>,
Claudio Imbrenda <imbrenda@...ux.ibm.com>,
Marc Zyngier <maz@...nel.org>,
James Morse <james.morse@....com>,
Alexandru Elisei <alexandru.elisei@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
kvmarm@...ts.cs.columbia.edu
Subject: Re: [RFC PATCH 00/10] KVM: selftests: Add support for test-selectable
ucall implementations
On 12/10/21 17:46, Michael Roth wrote:
> These patches are based on kvm/next, and are also available at:
>
> https://github.com/mdroth/linux/commits/sev-selftests-ucall-rfc1
Thanks, this is a nice implementation of the concept. I'll check s390
before the next merge window, as I intend to merge this and the SEV
tests (which I have already confirmed to work; I haven't yet checked
SEV-ES because it's a bit harder for me to install new kernels on the
SEV-ES machine I have access to).
Paolo
> == BACKGROUND ==
>
> These patches are a prerequisite for adding selftest support for SEV guests
> and possibly other confidential computing implementations in the future.
> They were motivated by a suggestion Paolo made in response to the initial
> SEV selftest RFC:
>
> https://lore.kernel.org/lkml/20211025035833.yqphcnf5u3lk4zgg@amd.com/T/#m959b56f9fb4ae6ab973f6ab50fe3ddfacd7c5617
>
> Since the changes touch multiple archs and ended up creating a bit more churn
> than expected, I thought it would be a good idea to carve this out into a
> separate standalone series for reviewers who may be more interested in the
> ucall changes than anything SEV-related.
>
> To summarize, x86 relies on a ucall based on using PIO intructions to generate
> an exit to userspace and provide the GVA of a dynamically-allocated ucall
> struct that resides in guest memory and contains information about how to
> handle/interpret the exit. This doesn't work for SEV guests for 3 main reasons:
>
> 1) The guest memory is generally encrypted during run-time, so the guest
> needs to ensure the ucall struct is allocated in shared memory.
> 2) The guest page table is also encrypted, so the address would need to be a
> GPA instead of a GVA.
> 3) The guest vCPU register may also be encrypted in the case of
> SEV-ES/SEV-SNP, so the approach of examining vCPU register state has
> additional requirements such as requiring guest code to implement a #VC
> handler that can provide the appropriate registers via a vmgexit.
>
> To address these issues, the SEV selftest RFC1 patchset introduced a set of new
> SEV-specific interfaces that closely mirrored the functionality of
> ucall()/get_ucall(), but relied on a pre-allocated/static ucall buffer in
> shared guest memory so it that guest code could pass messages/state to the host
> by simply writing to this pre-arranged shared memory region and then generating
> an exit to userspace (via a halt instruction).
>
> Paolo suggested instead implementing support for test/guest-specific ucall
> implementations that could be used as an alternative to the default PIO-based
> ucall implementations as-needed based on test/guest requirements, while still
> allowing for tests to use a common set interfaces like ucall()/get_ucall().
>
> == OVERVIEW ==
>
> This series implements the above functionality by introducing a new ucall_ops
> struct that can be used to register a particular ucall implementation as need,
> then re-implements x86/arm64/s390x in terms of the ucall_ops.
>
> But for the purposes of introducing a new ucall_ops implementation appropriate
> for SEV, there are a couple issues that resulted in the need for some additional
> ucall interfaces as well:
>
> a) ucall() doesn't take a pointer to the ucall struct it modifies, so to make
> it work in the case of an implementation that relies a pre-allocated ucall
> struct in shared guest memory some sort of global lookup functionality
> would be needed to locate the appropriate ucall struct for a particular
> VM/vcpu combination, and this would need to be made accessible for use by
> the guest as well. guests would then need some way of determining what
> VM/vcpu identifiers they need to use to do the lookup, which to do reliably
> would likely require seeding the guest with those identifiers in advance,
> which is possible, but much more easily achievable by simply adding a
> ucall() alternative that accepts a pointer to the ucall struct for that
> particular VM/vcpu.
>
> b) get_ucall() *does* take a pointer to a ucall struct, but currently zeroes
> it out and uses it to copy the guest's ucall struct into. It *could* be
> re-purposed to handle the case where the pointer is an actual pointer to
> the ucall struct in shared guest memory, but that could cause problems
> since callers would need some idea of what the underlying ucall
> implementation expects. Ideally the interfaces would be agnostic to the
> ucall implementation.
>
> So to address those issues, this series also allows ucall implementations to
> optionally be extended to support a set of 'shared' ops that are used in the
> following manner:
>
> host:
> uc_gva = ucall_shared_alloc()
> setup_vm_args(vm, uc_gva)
>
> guest:
> ucall_shared(uc_gva, ...)
>
> host:
> uget_ucall_shared(uc_gva, ...)
>
> and then implements a new ucall implementation, ucall_ops_halt, based around
> these shared interfaces and halt instructions.
>
> While this doesn't really meet the initial goal of re-using the existing
> ucall interfaces as-is, the hope is that these *_shared interfaces are
> general enough to be re-usable things other than SEV, or at least improve on
> code readability over the initial SEV-specific interfaces.
>
> Any review/comments are greatly appreciated!
>
> ----------------------------------------------------------------
> Michael Roth (10):
> kvm: selftests: move base kvm_util.h declarations to kvm_util_base.h
> kvm: selftests: move ucall declarations into ucall_common.h
> kvm: selftests: introduce ucall_ops for test/arch-specific ucall implementations
> kvm: arm64: selftests: use ucall_ops to define default ucall implementation
> (COMPILE-TESTED ONLY) kvm: s390: selftests: use ucall_ops to define default ucall implementation
> kvm: selftests: add ucall interfaces based around shared memory
> kvm: selftests: add ucall_shared ops for PIO
> kvm: selftests: introduce ucall implementation based on halt instructions
> kvm: selftests: add GUEST_SHARED_* macros for shared ucall implementations
> kvm: selftests: add ucall_test to test various ucall functionality
>
> tools/testing/selftests/kvm/.gitignore | 1 +
> tools/testing/selftests/kvm/Makefile | 5 +-
> .../testing/selftests/kvm/include/aarch64/ucall.h | 18 +
> tools/testing/selftests/kvm/include/kvm_util.h | 408 +--------------------
> .../testing/selftests/kvm/include/kvm_util_base.h | 368 +++++++++++++++++++
> tools/testing/selftests/kvm/include/s390x/ucall.h | 18 +
> tools/testing/selftests/kvm/include/ucall_common.h | 147 ++++++++
> tools/testing/selftests/kvm/include/x86_64/ucall.h | 19 +
> tools/testing/selftests/kvm/lib/aarch64/ucall.c | 43 +--
> tools/testing/selftests/kvm/lib/s390x/ucall.c | 45 +--
> tools/testing/selftests/kvm/lib/ucall_common.c | 133 +++++++
> tools/testing/selftests/kvm/lib/x86_64/ucall.c | 82 +++--
> tools/testing/selftests/kvm/ucall_test.c | 182 +++++++++
> 13 files changed, 982 insertions(+), 487 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/ucall.h
> create mode 100644 tools/testing/selftests/kvm/include/kvm_util_base.h
> create mode 100644 tools/testing/selftests/kvm/include/s390x/ucall.h
> create mode 100644 tools/testing/selftests/kvm/include/ucall_common.h
> create mode 100644 tools/testing/selftests/kvm/include/x86_64/ucall.h
> create mode 100644 tools/testing/selftests/kvm/lib/ucall_common.c
> create mode 100644 tools/testing/selftests/kvm/ucall_test.c
>
>
Powered by blists - more mailing lists