Re: [PATCH v7 6/7] KVM: x86: Introduce MSR filtering

[Aaron Lewis <aaronlewis@google.com>]

+4.126 KVM_X86_SET_MSR_FILTER
+----------------------------
+
+:Capability: KVM_X86_SET_MSR_FILTER
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_msr_filter
+:Returns: 0 on success, < 0 on error
+
+::
+
+  struct kvm_msr_filter_range {
+  #define KVM_MSR_FILTER_READ  (1 << 0)
+  #define KVM_MSR_FILTER_WRITE (1 << 1)
+       __u32 flags;
+       __u32 nmsrs; /* number of msrs in bitmap */
+       __u32 base;  /* MSR index the bitmap starts at */
+       __u8 *bitmap; /* a 1 bit allows the operations in flags, 0 denies */
+  };
+
+  struct kvm_msr_filter {
+  #define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
+  #define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
+       __u32 flags;
+       struct kvm_msr_filter_range ranges[16];

Can we replace 16 with something more meaningful.  Prehaps
KVM_MSR_FILTER_MAX_RANGES.

+  };
+
+flags values:
+
+KVM_MSR_FILTER_READ
+
+  Filter read accesses to MSRs using the given bitmap. A 0 in the bitmap
+  indicates that a read should immediately fail, while a 1 indicates that
+  a read should be handled like without the filter.

nit: not sure you need 'like without the filter'

+
+KVM_MSR_FILTER_WRITE
+
+  Filter write accesses to MSRs using the given bitmap. A 0 in the bitmap
+  indicates that a write should immediately fail, while a 1 indicates that
+  a write should be handled like without the filter.

nit: again, not sure you need 'like without the filter'

+
+KVM_MSR_FILTER_READ | KVM_MSR_FILTER_WRITE
+
+  Filter booth read and write accesses to MSRs using the given bitmap. A 0

nit: replace 'booth' for 'both'

+  in the bitmap indicates that both reads and writes should immediately fail,
+  while a 1 indicates that reads and writes should be handled like without
+  the filter.

nit: again, not sure you need 'like without the filter'

+
+KVM_MSR_FILTER_DEFAULT_ALLOW
+
+  If no filter range matches an MSR index that is getting accessed, KVM will
+  fall back to allowing access to the MSR.
+
+KVM_MSR_FILTER_DEFAULT_DENY
+
+  If no filter range matches an MSR index that is getting accessed, KVM will
+  fall back to rejecting access to the MSR. In this mode, all MSRs that should
+  be processed by KVM need to explicitly be marked as allowed in the bitmaps.
+
+This ioctl allows user space to define a up to 16 bitmaps of MSR ranges to

nit: "define up to" remove 'a'

+specify whether a certain MSR access should be explicitly rejected or not.
+
+If this ioctl has never been invoked, MSR accesses are not guarded and the
+old KVM in-kernel emulation behavior is fully preserved.
+
+As soon as the filtering is in place, every MSR access is precessed through

nit: processed

+the filtering. If a bit is within one of the defined ranges, read and write
+accesses are guarded by the bitmap's value for the MSR index. If it is not
+defined in any range, whether MSR access is rejected is determined by the flags
+field of in the kvm_msr_filter struct: KVM_MSR_FILTER_DEFAULT_ALLOW and

nit: remove 'of' or 'in'

+KVM_MSR_FILTER_DEFAULT_DENY.
+
+Calling this ioctl with an empty set of ranges (all nmsrs == 0) disables MSR
+filtering. In that mode, KVM_MSR_FILTER_DEFAULT_DENY no longer has any effect.


+struct msr_bitmap_range {
+       u32 flags;
+       u32 nmsrs;
+       u32 base;
+       unsigned long *bitmap;
+};
+
 enum kvm_irqchip_mode {
        KVM_IRQCHIP_NONE,
        KVM_IRQCHIP_KERNEL,       /* created with KVM_CREATE_IRQCHIP */
@@ -964,6 +972,12 @@ struct kvm_arch {
        /* Deflect RDMSR and WRMSR to user space when they trigger a #GP */
        u32 user_space_msr_mask;

+       struct {
+               u8 count;
+               bool default_allow:1;
+               struct msr_bitmap_range ranges[16];

Replace 16 with macro

+       } msr_filter;
+
        struct kvm_pmu_event_filter *pmu_event_filter;
        struct task_struct *nx_lpage_recovery_thread;
 };
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 50650cfd235a..66bba91e1bb8 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -192,8 +192,25 @@ struct kvm_msr_list {
        __u32 indices[0];
 };

-#define KVM_MSR_ALLOW_READ  (1 << 0)
-#define KVM_MSR_ALLOW_WRITE (1 << 1)
+/* Maximum size of any access bitmap in bytes */
+#define KVM_MSR_FILTER_MAX_BITMAP_SIZE 0x600

Add a macro.  Prehaps, #define KVM_MSR_FILTER_MAX_RANGES 16

+
+/* for KVM_X86_SET_MSR_FILTER */
+struct kvm_msr_filter_range {
+#define KVM_MSR_FILTER_READ  (1 << 0)
+#define KVM_MSR_FILTER_WRITE (1 << 1)
+       __u32 flags;
+       __u32 nmsrs; /* number of msrs in bitmap */
+       __u32 base;  /* MSR index the bitmap starts at */
+       __u8 *bitmap; /* a 1 bit allows the operations in flags, 0 denies */
+};
+
+struct kvm_msr_filter {
+#define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
+#define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
+       __u32 flags;
+       struct kvm_msr_filter_range ranges[16];

Replace 16 with macro

+};

 struct kvm_cpuid_entry {
        __u32 function;

@@ -3603,6 +3639,7 @@ int kvm_vm_ioctl_check_extension(struct kvm
*kvm, long ext)
        case KVM_CAP_SET_GUEST_DEBUG:
        case KVM_CAP_LAST_CPU:
        case KVM_CAP_X86_USER_SPACE_MSR:
+       case KVM_CAP_X86_MSR_FILTER:
                r = 1;
                break;
        case KVM_CAP_SYNC_REGS:
@@ -5134,6 +5171,103 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
        return r;
 }

+static void kvm_clear_msr_filter(struct kvm *kvm)
+{
+       u32 i;
+       u32 count = kvm->arch.msr_filter.count;
+       struct msr_bitmap_range ranges[16];

Replace 16 with macro

+
+       mutex_lock(&kvm->lock);
+       kvm->arch.msr_filter.count = 0;
+       memcpy(ranges, kvm->arch.msr_filter.ranges, count * sizeof(ranges[0]));
+       mutex_unlock(&kvm->lock);
+       synchronize_srcu(&kvm->srcu);
+
+       for (i = 0; i < count; i++)
+               kfree(ranges[i].bitmap);
+}