Re: [PATCH] KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2

[Sean Christopherson <seanjc@google.com>]

On Sat, Feb 26, 2022, Oliver Upton wrote:
> KVM_CAP_DISABLE_QUIRKS is irrevocably broken. The capability does not
> advertise the set of quirks which may be disabled to userspace, so it is
> impossible to predict the behavior of KVM. Worse yet,
> KVM_CAP_DISABLE_QUIRKS will tolerate any value for cap->args[0], meaning
> it fails to reject attempts to set invalid quirk bits.

FWIW, we do have a way out without adding another capability.  The 'flags' field
is enforced for all capabilities, we could use a bit there to add "v2" functionality.
Userspace can assume KVM_QUIRK_ENFORCE_QUIRKS is allowed if the return from probing
the capability is >1.

It's gross and forced, just an idea if we want to avoid yet another cap.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c712c33c1521..2a8449d1cf24 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4229,7 +4229,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
        case KVM_CAP_HYPERV_TIME:
        case KVM_CAP_IOAPIC_POLARITY_IGNORED:
        case KVM_CAP_TSC_DEADLINE_TIMER:
-       case KVM_CAP_DISABLE_QUIRKS:
        case KVM_CAP_SET_BOOT_CPU_ID:
        case KVM_CAP_SPLIT_IRQCHIP:
        case KVM_CAP_IMMEDIATE_EXIT:
@@ -4254,6 +4253,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
        case KVM_CAP_VAPIC:
                r = 1;
                break;
+       case KVM_CAP_DISABLE_QUIRKS:
+               r = KVM_X86_VALID_QUIRKS;
+               break;
        case KVM_CAP_EXIT_HYPERCALL:
                r = KVM_EXIT_HYPERCALL_VALID_MASK;
                break;
@@ -5892,11 +5894,19 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 {
        int r;

-       if (cap->flags)
+       if (cap->flags && cap->cap != KVM_CAP_DISABLE_QUIRKS)
                return -EINVAL;

        switch (cap->cap) {
        case KVM_CAP_DISABLE_QUIRKS:
+               r = -EINVAL;
+               if (cap->flags & ~KVM_QUIRK_ENFORCE_QUIRKS)
+                       break;
+
+               if ((cap->flags & KVM_QUIRK_ENFORCE_QUIRKS) &&
+                   (cap->args[0] & ~KVM_X86_VALID_QUIRKS))
+                       break;
+
                kvm->arch.disabled_quirks = cap->args[0];
                r = 0;
                break;

> +7.30 KVM_CAP_DISABLE_QUIRKS2
> +----------------------------
> +
> +:Capability: KVM_CAP_DISABLE_QUIRKS2
> +:Parameters: args[0] - set of KVM quirks to disable
> +:Architectures: x86
> +:Type: vm
> +
> +This capability, if enabled, will cause KVM to disable some behavior
> +quirks.
> +
> +Calling KVM_CHECK_EXTENSION for this capability returns a bitmask of
> +quirks that can be disabled in KVM.
> +
> +The argument to KVM_ENABLE_CAP for this capability is a bitmask of
> +quirks to disable, and must be a subset of the bitmask returned by
> +KVM_CHECK_EXTENSION.
> +
> +The valid bits in cap.args[0] are:
> +
> +=================================== ============================================
> + KVM_X86_QUIRK_LINT0_ENABLED        By default, the reset value for the LVT

LINT0_REEANBLED.