From: Marc Orr <marcorr@google.com>
To: Peter Gonda <pgonda@google.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
Paolo Bonzini <pbonzini@redhat.com>,
Sean Christopherson <seanjc@google.com>
Subject: Re: [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests
Date: Wed, 8 Dec 2021 21:53:25 -0800 [thread overview]
Message-ID: <CAA03e5H6TxcL6WVYcBs5aX5zHLB=sCYcrBLggAtmLZADn_BHyA@mail.gmail.com> (raw)
In-Reply-To: <20211208191642.3792819-4-pgonda@google.com>
On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote:
>
> Add tests to confirm mirror vms can only run correct subset of commands.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Marc Orr <marcorr@google.com>
> Signed-off-by: Peter Gonda <pgonda@google.com>
> ---
> .../selftests/kvm/x86_64/sev_migrate_tests.c | 55 +++++++++++++++++--
> 1 file changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
> index 4bb960ca6486..80056bbbb003 100644
> --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
> +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c
> @@ -21,7 +21,7 @@
> #define NR_LOCK_TESTING_THREADS 3
> #define NR_LOCK_TESTING_ITERATIONS 10000
>
> -static void sev_ioctl(int vm_fd, int cmd_id, void *data)
> +static int __sev_ioctl(int vm_fd, int cmd_id, void *data, __u32 *fw_error)
> {
> struct kvm_sev_cmd cmd = {
> .id = cmd_id,
> @@ -30,11 +30,20 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data)
> };
> int ret;
>
> -
> ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd);
> - TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS,
> + *fw_error = cmd.error;
> + return ret;
> +}
> +
> +static void sev_ioctl(int vm_fd, int cmd_id, void *data)
> +{
> + int ret;
> + __u32 fw_error;
> +
> + ret = __sev_ioctl(vm_fd, cmd_id, data, &fw_error);
> + TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS,
> "%d failed: return code: %d, errno: %d, fw error: %d",
> - cmd_id, ret, errno, cmd.error);
> + cmd_id, ret, errno, fw_error);
> }
>
> static struct kvm_vm *sev_vm_create(bool es)
> @@ -226,6 +235,42 @@ static void sev_mirror_create(int dst_fd, int src_fd)
> TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno);
> }
>
> +static void verify_mirror_allowed_cmds(int vm_fd)
> +{
> + struct kvm_sev_guest_status status;
> +
> + for (int cmd_id = KVM_SEV_INIT; cmd_id < KVM_SEV_NR_MAX; ++cmd_id) {
> + int ret;
> + __u32 fw_error;
> +
> + /*
> + * These commands are allowed for mirror VMs, all others are
> + * not.
> + */
> + switch (cmd_id) {
> + case KVM_SEV_LAUNCH_UPDATE_VMSA:
> + case KVM_SEV_GUEST_STATUS:
> + case KVM_SEV_DBG_DECRYPT:
> + case KVM_SEV_DBG_ENCRYPT:
> + continue;
> + default:
> + break;
> + }
> +
> + /*
> + * These commands should be disallowed before the data
> + * parameter is examined so NULL is OK here.
> + */
> + ret = __sev_ioctl(vm_fd, cmd_id, NULL, &fw_error);
> + TEST_ASSERT(
> + ret == -1 && errno == EINVAL,
> + "Should not be able call command: %d. ret: %d, errno: %d\n",
> + cmd_id, ret, errno);
> + }
> +
> + sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status);
Why is this here? I'd either delete it or maybe alternatively move it
into the `case KVM_SEV_GUEST_STATUS` with a corresponding TEST_ASSERT
to check that the command succeeded. Something like:
...
switch (cmd_id) {
case KVM_SEV_GUEST_STATUS:
sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status);
TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, ...);
continue;
case KVM_SEV_LAUNCH_UPDATE_VMSA:
case KVM_SEV_DBG_DECRYPT:
case KVM_SEV_DBG_ENCRYPT:
continue;
default:
break;
}
> +}
> +
> static void test_sev_mirror(bool es)
> {
> struct kvm_vm *src_vm, *dst_vm;
> @@ -243,6 +288,8 @@ static void test_sev_mirror(bool es)
> if (es)
> sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL);
>
> + verify_mirror_allowed_cmds(dst_vm->fd);
> +
> kvm_vm_free(src_vm);
> kvm_vm_free(dst_vm);
> }
> --
> 2.34.1.400.ga245620fadb-goog
>
next prev parent reply other threads:[~2021-12-09 5:53 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 19:16 [PATCH 0/3] Fixes for SEV mirror VM tests Peter Gonda
2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda
2021-12-09 5:43 ` Marc Orr
2021-12-08 19:16 ` [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() Peter Gonda
2021-12-09 5:45 ` Marc Orr
2021-12-09 18:25 ` Peter Gonda
2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda
2021-12-09 5:53 ` Marc Orr [this message]
2021-12-09 20:45 ` Marc Orr
2021-12-09 11:27 ` Paolo Bonzini
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-all 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='CAA03e5H6TxcL6WVYcBs5aX5zHLB=sCYcrBLggAtmLZADn_BHyA@mail.gmail.com' \
--to=marcorr@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=pgonda@google.com \
--cc=seanjc@google.com \
/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;
as well as URLs for NNTP newsgroup(s).