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@xxxxxxxxxx>
> ---
> 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.
