| Message ID | 20220921151525.904162-3-aaronlewis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MSR filtering and MSR exiting flag clean up | expand |
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.
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 */
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(-)