[PATCH v4 0/5] MSR filtering and MSR exiting flag clean up

[PATCH v4 0/5] MSR filtering and MSR exiting flag clean up

[Aaron Lewis]

The changes in this series were intended to be accepted at the same time as
commit cf5029d5dd7c ("KVM: x86: Protect the unused bits in MSR exiting
flags").  With that already accepted this series is the rest of the changes
that evolved from the code review.  The RFC tag has been removed because
that part of the change has already been accepted.  All that's left is the
clean up and the selftest.

v3 -> v4
 - Patches 2 and 3 are new.  They were intended to be a part of commit
   cf5029d5dd7c ("KVM: x86: Protect the unused bits in MSR exiting flags"),
   but with that accepted it made sense to split what remained into two.

v2 -> v3
 - Added patch 1/4 to prevent the kernel from using the flag
   KVM_MSR_FILTER_DEFAULT_ALLOW.
 - Cleaned up the selftest code based on feedback.

v1 -> v2
 - Added valid masks KVM_MSR_FILTER_VALID_MASK and
   KVM_MSR_EXIT_REASON_VALID_MASK.
 - Added patch 2/3 to add valid mask KVM_MSR_FILTER_RANGE_VALID_MASK, and
   use it.
 - Added testing to demonstrate flag protection when calling the ioctl for
   KVM_X86_SET_MSR_FILTER or KVM_CAP_X86_USER_SPACE_MSR.

Aaron Lewis (5):
  KVM: x86: Disallow the use of KVM_MSR_FILTER_DEFAULT_ALLOW in the kernel
  KVM: x86: Add a VALID_MASK for the MSR exit reason flags
  KVM: x86: Add a VALID_MASK for the flag in kvm_msr_filter
  KVM: x86: Add a VALID_MASK for the flags in kvm_msr_filter_range
  selftests: kvm/x86: Test the flags in MSR filtering and MSR exiting

 arch/x86/include/uapi/asm/kvm.h               |  5 ++
 arch/x86/kvm/x86.c                            |  8 +-
 include/uapi/linux/kvm.h                      |  3 +
 .../kvm/x86_64/userspace_msr_exit_test.c      | 85 +++++++++++++++++++
 4 files changed, 96 insertions(+), 5 deletions(-)

-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v4 1/5] KVM: x86: Disallow the use of KVM_MSR_FILTER_DEFAULT_ALLOW in the kernel

[Aaron Lewis]

Protect the kernel from using the flag KVM_MSR_FILTER_DEFAULT_ALLOW.
Its value is 0, and using it incorrectly could have unintended
consequences. E.g. prevent someone in the kernel from writing something
like this.

if (filter.flags & KVM_MSR_FILTER_DEFAULT_ALLOW)
        <allow the MSR>

and getting confused when it doesn't work.

It would be more ideal to remove this flag altogether, but userspace
may already be using it, so protecting the kernel is all that can
reasonably be done at this point.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---

Google's VMM is already using this flag, so we *know* that dropping the
flag entirely will break userspace.  All we can do at this point is
prevent the kernel from using it.

 arch/x86/include/uapi/asm/kvm.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 46de10a809ec..73ad693aa653 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -222,7 +222,9 @@ struct kvm_msr_filter_range {
 
 #define KVM_MSR_FILTER_MAX_RANGES 16
 struct kvm_msr_filter {
+#ifndef __KERNEL__
 #define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
+#endif
 #define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
 	__u32 flags;
 	struct kvm_msr_filter_range ranges[KVM_MSR_FILTER_MAX_RANGES];
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v4 2/5] KVM: x86: Add a VALID_MASK for the MSR exit reason flags

[Aaron Lewis]

Add the mask KVM_MSR_EXIT_REASON_VALID_MASK for the MSR exit reason
flags.  This simplifies checks that validate these flags, and makes it
easier to introduce new flags in the future.

No functional change intended.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 arch/x86/kvm/x86.c       | 4 +---
 include/uapi/linux/kvm.h | 3 +++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d7374d768296..852614246825 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6182,9 +6182,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		break;
 	case KVM_CAP_X86_USER_SPACE_MSR:
 		r = -EINVAL;
-		if (cap->args[0] & ~(KVM_MSR_EXIT_REASON_INVAL |
-				     KVM_MSR_EXIT_REASON_UNKNOWN |
-				     KVM_MSR_EXIT_REASON_FILTER))
+		if (cap->args[0] & ~KVM_MSR_EXIT_REASON_VALID_MASK)
 			break;
 		kvm->arch.user_space_msr_mask = cap->args[0];
 		r = 0;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index eed0315a77a6..44d476c3143a 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -485,6 +485,9 @@ struct kvm_run {
 #define KVM_MSR_EXIT_REASON_INVAL	(1 << 0)
 #define KVM_MSR_EXIT_REASON_UNKNOWN	(1 << 1)
 #define KVM_MSR_EXIT_REASON_FILTER	(1 << 2)
+#define KVM_MSR_EXIT_REASON_VALID_MASK	(KVM_MSR_EXIT_REASON_INVAL   |	\
+					 KVM_MSR_EXIT_REASON_UNKNOWN |	\
+					 KVM_MSR_EXIT_REASON_FILTER)
 			__u32 reason; /* kernel -> user */
 			__u32 index; /* kernel -> user */
 			__u64 data; /* kernel <-> user */
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v4 3/5] KVM: x86: Add a VALID_MASK for the flag in kvm_msr_filter

[Aaron Lewis]

Add the mask KVM_MSR_FILTER_VALID_MASK for the flag in the struct
kvm_msr_filter.  This makes it easier to introduce new flags in the
future.

No functional change intended.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 arch/x86/include/uapi/asm/kvm.h | 1 +
 arch/x86/kvm/x86.c              | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 73ad693aa653..ae4324674c49 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -226,6 +226,7 @@ struct kvm_msr_filter {
 #define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
 #endif
 #define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
+#define KVM_MSR_FILTER_VALID_MASK (KVM_MSR_FILTER_DEFAULT_DENY)
 	__u32 flags;
 	struct kvm_msr_filter_range ranges[KVM_MSR_FILTER_MAX_RANGES];
 };
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 852614246825..670ae38f8f3e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6397,7 +6397,7 @@ static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
 	if (copy_from_user(&filter, user_msr_filter, sizeof(filter)))
 		return -EFAULT;
 
-	if (filter.flags & ~KVM_MSR_FILTER_DEFAULT_DENY)
+	if (filter.flags & ~KVM_MSR_FILTER_VALID_MASK)
 		return -EINVAL;
 
 	for (i = 0; i < ARRAY_SIZE(filter.ranges); i++)
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v4 4/5] KVM: x86: Add a VALID_MASK for the flags in kvm_msr_filter_range

[Aaron Lewis]

Add the mask KVM_MSR_FILTER_RANGE_VALID_MASK for the flags in the
struct kvm_msr_filter_range.  This simplifies checks that validate
these flags, and makes it easier to introduce new flags in the future.

No functional change intended.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 arch/x86/include/uapi/asm/kvm.h | 2 ++
 arch/x86/kvm/x86.c              | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index ae4324674c49..c6df6b16a088 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -214,6 +214,8 @@ struct kvm_msr_list {
 struct kvm_msr_filter_range {
 #define KVM_MSR_FILTER_READ  (1 << 0)
 #define KVM_MSR_FILTER_WRITE (1 << 1)
+#define KVM_MSR_FILTER_RANGE_VALID_MASK (KVM_MSR_FILTER_READ | \
+					 KVM_MSR_FILTER_WRITE)
 	__u32 flags;
 	__u32 nmsrs; /* number of msrs in bitmap */
 	__u32 base;  /* MSR index the bitmap starts at */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 670ae38f8f3e..48fe6a5e625a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6359,7 +6359,7 @@ static int kvm_add_msr_filter(struct kvm_x86_msr_filter *msr_filter,
 	if (!user_range->nmsrs)
 		return 0;
 
-	if (user_range->flags & ~(KVM_MSR_FILTER_READ | KVM_MSR_FILTER_WRITE))
+	if (user_range->flags & ~KVM_MSR_FILTER_RANGE_VALID_MASK)
 		return -EINVAL;
 
 	if (!user_range->flags)
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v4 5/5] selftests: kvm/x86: Test the flags in MSR filtering and MSR exiting

[Aaron Lewis]

When using the flags in KVM_X86_SET_MSR_FILTER and
KVM_CAP_X86_USER_SPACE_MSR it is expected that an attempt to write to
any of the unused bits will fail.  Add testing to walk over every bit
in each of the flag fields in MSR filtering and MSR exiting to verify
that unused bits return and error and used bits, i.e. valid bits,
succeed.

Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---
 .../kvm/x86_64/userspace_msr_exit_test.c      | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
index a4f06370a245..fae95089e655 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
@@ -733,6 +733,89 @@ static void test_msr_permission_bitmap(void)
 	kvm_vm_free(vm);
 }
 
+#define test_user_exit_msr_ioctl(vm, cmd, arg, flag, valid_mask)	\
+({									\
+	int r = __vm_ioctl(vm, cmd, arg);				\
+									\
+	if (flag & valid_mask)						\
+		TEST_ASSERT(!r, __KVM_IOCTL_ERROR(#cmd, r));		\
+	else								\
+		TEST_ASSERT(r == -1 && errno == EINVAL,			\
+			    "Wanted EINVAL for %s with flag = 0x%llx, got  rc: %i errno: %i (%s)", \
+			    #cmd, flag, r, errno,  strerror(errno));	\
+})
+
+static void run_user_space_msr_flag_test(struct kvm_vm *vm)
+{
+	struct kvm_enable_cap cap = { .cap = KVM_CAP_X86_USER_SPACE_MSR };
+	int nflags = sizeof(cap.args[0]) * BITS_PER_BYTE;
+	int rc;
+	int i;
+
+	rc = kvm_check_cap(KVM_CAP_X86_USER_SPACE_MSR);
+	TEST_ASSERT(rc, "KVM_CAP_X86_USER_SPACE_MSR is available");
+
+	for (i = 0; i < nflags; i++) {
+		cap.args[0] = BIT_ULL(i);
+		test_user_exit_msr_ioctl(vm, KVM_ENABLE_CAP, &cap,
+			   BIT_ULL(i), KVM_MSR_EXIT_REASON_VALID_MASK);
+	}
+}
+
+static void run_msr_filter_flag_test(struct kvm_vm *vm)
+{
+	u64 deny_bits = 0;
+	struct kvm_msr_filter filter = {
+		.flags = KVM_MSR_FILTER_DEFAULT_ALLOW,
+		.ranges = {
+			{
+				.flags = KVM_MSR_FILTER_READ,
+				.nmsrs = 1,
+				.base = 0,
+				.bitmap = (uint8_t *)&deny_bits,
+			},
+		},
+	};
+	int nflags;
+	int rc;
+	int i;
+
+	rc = kvm_check_cap(KVM_CAP_X86_MSR_FILTER);
+	TEST_ASSERT(rc, "KVM_CAP_X86_MSR_FILTER is available");
+
+	nflags = sizeof(filter.flags) * BITS_PER_BYTE;
+	for (i = 0; i < nflags; i++) {
+		filter.flags = BIT_ULL(i);
+		test_user_exit_msr_ioctl(vm, KVM_X86_SET_MSR_FILTER, &filter,
+			   BIT_ULL(i), KVM_MSR_FILTER_VALID_MASK);
+	}
+
+	filter.flags = KVM_MSR_FILTER_DEFAULT_ALLOW;
+	nflags = sizeof(filter.ranges[0].flags) * BITS_PER_BYTE;
+	for (i = 0; i < nflags; i++) {
+		filter.ranges[0].flags = BIT_ULL(i);
+		test_user_exit_msr_ioctl(vm, KVM_X86_SET_MSR_FILTER, &filter,
+			   BIT_ULL(i), KVM_MSR_FILTER_RANGE_VALID_MASK);
+	}
+}
+
+/* Test that attempts to write to the unused bits in a flag fails. */
+static void test_user_exit_msr_flags(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+
+	vm = vm_create_with_one_vcpu(&vcpu, NULL);
+
+	/* Test flags for KVM_CAP_X86_USER_SPACE_MSR. */
+	run_user_space_msr_flag_test(vm);
+
+	/* Test flags and range flags for KVM_X86_SET_MSR_FILTER. */
+	run_msr_filter_flag_test(vm);
+
+	kvm_vm_free(vm);
+}
+
 int main(int argc, char *argv[])
 {
 	/* Tell stdout not to buffer its content */
@@ -744,5 +827,7 @@ int main(int argc, char *argv[])
 
 	test_msr_permission_bitmap();
 
+	test_user_exit_msr_flags();
+
 	return 0;
 }
-- 
2.37.3.968.ga6b4b080e4-goog

Re: [PATCH v4 1/5] KVM: x86: Disallow the use of KVM_MSR_FILTER_DEFAULT_ALLOW in the kernel

[Sean Christopherson]

On Wed, Sep 21, 2022, Aaron Lewis wrote:
> Protect the kernel from using the flag KVM_MSR_FILTER_DEFAULT_ALLOW.
> Its value is 0, and using it incorrectly could have unintended
> consequences. E.g. prevent someone in the kernel from writing something
> like this.
> 
> if (filter.flags & KVM_MSR_FILTER_DEFAULT_ALLOW)
>         <allow the MSR>
> 
> and getting confused when it doesn't work.
> 
> It would be more ideal to remove this flag altogether, but userspace
> may already be using it, so protecting the kernel is all that can
> reasonably be done at this point.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>

> Google's VMM is already using this flag, so we *know* that dropping the
> flag entirely will break userspace.  All we can do at this point is
> prevent the kernel from using it.

LOL, nice.

Re: [PATCH v4 2/5] KVM: x86: Add a VALID_MASK for the MSR exit reason flags

[Sean Christopherson]

On Wed, Sep 21, 2022, Aaron Lewis wrote:
> Add the mask KVM_MSR_EXIT_REASON_VALID_MASK for the MSR exit reason

Uber nit, "the mask" is rather redundant, e.g.

Add KVM_MSR_EXIT_REASON_VALID_MASK to track the allowed MSR exit reason
flags.

> flags.  This simplifies checks that validate these flags, and makes it
> easier to introduce new flags in the future.
> 
> No functional change intended.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
>  arch/x86/kvm/x86.c       | 4 +---
>  include/uapi/linux/kvm.h | 3 +++
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index d7374d768296..852614246825 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6182,9 +6182,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		break;
>  	case KVM_CAP_X86_USER_SPACE_MSR:
>  		r = -EINVAL;
> -		if (cap->args[0] & ~(KVM_MSR_EXIT_REASON_INVAL |
> -				     KVM_MSR_EXIT_REASON_UNKNOWN |
> -				     KVM_MSR_EXIT_REASON_FILTER))
> +		if (cap->args[0] & ~KVM_MSR_EXIT_REASON_VALID_MASK)
>  			break;
>  		kvm->arch.user_space_msr_mask = cap->args[0];
>  		r = 0;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index eed0315a77a6..44d476c3143a 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -485,6 +485,9 @@ struct kvm_run {
>  #define KVM_MSR_EXIT_REASON_INVAL	(1 << 0)
>  #define KVM_MSR_EXIT_REASON_UNKNOWN	(1 << 1)
>  #define KVM_MSR_EXIT_REASON_FILTER	(1 << 2)
> +#define KVM_MSR_EXIT_REASON_VALID_MASK	(KVM_MSR_EXIT_REASON_INVAL   |	\
> +					 KVM_MSR_EXIT_REASON_UNKNOWN |	\
> +					 KVM_MSR_EXIT_REASON_FILTER)

Put all of these VALID_MASK defines in arch/x86/include/asm/kvm_host.h so that
they aren't exposed to userspace, e.g. see KVM_X86_NOTIFY_VMEXIT_VALID_BITS.
Generally speaking, things should be kept in-kernel unless there's an actual need
to expose something to userspace.  Once something is exposed to userspace, our
options become much more limited.

E.g. if userspace does something silly like:

	filters = KVM_MSR_EXIT_REASON_VALID_MASK;

then upgrading kernel headers will unexpectedly change and potentially break
userspace, and then KVM is stuck having a bogus VALID_MASK.

Re: [PATCH v4 5/5] selftests: kvm/x86: Test the flags in MSR filtering and MSR exiting

[Sean Christopherson]

On Wed, Sep 21, 2022, Aaron Lewis wrote:
> +#define test_user_exit_msr_ioctl(vm, cmd, arg, flag, valid_mask)	\

There's nothing specific to userspace MSR exiting in this macro.  To keep the
name short, and to potentially allow moving it to common code in the future, how
about test_ioctl_flags()?

> +({									\
> +	int r = __vm_ioctl(vm, cmd, arg);				\
> +									\
> +	if (flag & valid_mask)						\
> +		TEST_ASSERT(!r, __KVM_IOCTL_ERROR(#cmd, r));		\
> +	else								\
> +		TEST_ASSERT(r == -1 && errno == EINVAL,			\
> +			    "Wanted EINVAL for %s with flag = 0x%llx, got  rc: %i errno: %i (%s)", \
> +			    #cmd, flag, r, errno,  strerror(errno));	\
> +})
> +
> +static void run_user_space_msr_flag_test(struct kvm_vm *vm)
> +{
> +	struct kvm_enable_cap cap = { .cap = KVM_CAP_X86_USER_SPACE_MSR };
> +	int nflags = sizeof(cap.args[0]) * BITS_PER_BYTE;
> +	int rc;
> +	int i;

These declarations can go on a single line.

> +
> +	rc = kvm_check_cap(KVM_CAP_X86_USER_SPACE_MSR);
> +	TEST_ASSERT(rc, "KVM_CAP_X86_USER_SPACE_MSR is available");
> +
> +	for (i = 0; i < nflags; i++) {
> +		cap.args[0] = BIT_ULL(i);
> +		test_user_exit_msr_ioctl(vm, KVM_ENABLE_CAP, &cap,
> +			   BIT_ULL(i), KVM_MSR_EXIT_REASON_VALID_MASK);

Align params.  With a shorter macro name, that's easy to do without creating
massively long lines.

And pass in the actual flags, e.g. cap.args[0] here, so that it's slightly more
obvious what is being tested, and to minimize the risk of mixups.

E.g.

		test_ioctl_flags(vm, KVM_ENABLE_CAP, &cap, cap.args[0],
				 KVM_MSR_EXIT_REASON_VALID_MASK);

> +	}
> +}
> +
> +static void run_msr_filter_flag_test(struct kvm_vm *vm)
> +{
> +	u64 deny_bits = 0;
> +	struct kvm_msr_filter filter = {
> +		.flags = KVM_MSR_FILTER_DEFAULT_ALLOW,
> +		.ranges = {
> +			{
> +				.flags = KVM_MSR_FILTER_READ,
> +				.nmsrs = 1,
> +				.base = 0,
> +				.bitmap = (uint8_t *)&deny_bits,
> +			},
> +		},
> +	};
> +	int nflags;
> +	int rc;
> +	int i;

	int nflags, rc, i;

> +
> +	rc = kvm_check_cap(KVM_CAP_X86_MSR_FILTER);
> +	TEST_ASSERT(rc, "KVM_CAP_X86_MSR_FILTER is available");
> +
> +	nflags = sizeof(filter.flags) * BITS_PER_BYTE;
> +	for (i = 0; i < nflags; i++) {
> +		filter.flags = BIT_ULL(i);
> +		test_user_exit_msr_ioctl(vm, KVM_X86_SET_MSR_FILTER, &filter,
> +			   BIT_ULL(i), KVM_MSR_FILTER_VALID_MASK);

		test_ioctl_flags(vm, KVM_X86_SET_MSR_FILTER, &filter,
				 filter.flags, KVM_MSR_FILTER_VALID_MASK);

> +	}
> +
> +	filter.flags = KVM_MSR_FILTER_DEFAULT_ALLOW;
> +	nflags = sizeof(filter.ranges[0].flags) * BITS_PER_BYTE;
> +	for (i = 0; i < nflags; i++) {
> +		filter.ranges[0].flags = BIT_ULL(i);
> +		test_user_exit_msr_ioctl(vm, KVM_X86_SET_MSR_FILTER, &filter,
> +			   BIT_ULL(i), KVM_MSR_FILTER_RANGE_VALID_MASK);

		test_ioctl_flags(vm, KVM_X86_SET_MSR_FILTER, &filter,
				 filter.ranges[0].flags,
				 KVM_MSR_FILTER_RANGE_VALID_MASK);

> +	}
> +}

Nits aside, nice test!

Re: [PATCH v4 0/5] MSR filtering and MSR exiting flag clean up

[Paolo Bonzini]

On 9/21/22 17:15, Aaron Lewis wrote:
> The changes in this series were intended to be accepted at the same time as
> commit cf5029d5dd7c ("KVM: x86: Protect the unused bits in MSR exiting
> flags").  With that already accepted this series is the rest of the changes
> that evolved from the code review.  The RFC tag has been removed because
> that part of the change has already been accepted.  All that's left is the
> clean up and the selftest.
> 
> v3 -> v4
>   - Patches 2 and 3 are new.  They were intended to be a part of commit
>     cf5029d5dd7c ("KVM: x86: Protect the unused bits in MSR exiting flags"),
>     but with that accepted it made sense to split what remained into two.
> 
> v2 -> v3
>   - Added patch 1/4 to prevent the kernel from using the flag
>     KVM_MSR_FILTER_DEFAULT_ALLOW.
>   - Cleaned up the selftest code based on feedback.
> 
> v1 -> v2
>   - Added valid masks KVM_MSR_FILTER_VALID_MASK and
>     KVM_MSR_EXIT_REASON_VALID_MASK.
>   - Added patch 2/3 to add valid mask KVM_MSR_FILTER_RANGE_VALID_MASK, and
>     use it.
>   - Added testing to demonstrate flag protection when calling the ioctl for
>     KVM_X86_SET_MSR_FILTER or KVM_CAP_X86_USER_SPACE_MSR.
> 
> Aaron Lewis (5):
>    KVM: x86: Disallow the use of KVM_MSR_FILTER_DEFAULT_ALLOW in the kernel
>    KVM: x86: Add a VALID_MASK for the MSR exit reason flags
>    KVM: x86: Add a VALID_MASK for the flag in kvm_msr_filter
>    KVM: x86: Add a VALID_MASK for the flags in kvm_msr_filter_range
>    selftests: kvm/x86: Test the flags in MSR filtering and MSR exiting
> 
>   arch/x86/include/uapi/asm/kvm.h               |  5 ++
>   arch/x86/kvm/x86.c                            |  8 +-
>   include/uapi/linux/kvm.h                      |  3 +
>   .../kvm/x86_64/userspace_msr_exit_test.c      | 85 +++++++++++++++++++
>   4 files changed, 96 insertions(+), 5 deletions(-)

Queued, thanks.

Paolo