On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@xxxxxxxxxx> wrote:
>
> Add tests to confirm mirror vms can only run correct subset of commands.
>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
> Cc: Marc Orr <marcorr@xxxxxxxxxx>
> Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
> ---
> .../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
>
