All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Oliver Upton <oupton@google.com>
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	David Dunn <daviddunn@google.com>
Subject: Re: [PATCH] KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2
Date: Mon, 28 Feb 2022 18:22:02 +0000	[thread overview]
Message-ID: <Yh0SyrFvu6UBwsj3@google.com> (raw)
In-Reply-To: <Yh0NyuuzIymt9mgt@google.com>

On Mon, Feb 28, 2022, Oliver Upton wrote:
> On Mon, Feb 28, 2022 at 04:33:57PM +0000, Sean Christopherson wrote:
> > 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.
> 
> I had considered this before sending out v1, but was concerned if a
> userspace didn't correctly handle a return value >1 from
> KVM_CHECK_EXTENSION.

Ah, right, userspace could theoretically check "== 1".  Blech.

> Turns out, I can't even find any evidence of the KVM_CAP_DISABLE_QUIRKS used
> by userspace. I spot checked QEMU, kvmtool,
> and a couple of the rusty ones.
> 
> The only other thing that comes to mind is it's a bit gross for userspace
> to do a graceful fallback if KVM_QUIRK_ENFORCE_QUIRKS isn't valid, since
> most userspace would just error out on -EINVAL. At least with a new cap
> userspace could follow a somewhat standardized way to discover if the
> kernel supports enforced quirks.

Yeah, a QUIRKS2 is probably easier for everyone.

      reply	other threads:[~2022-02-28 18:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-26  0:21 [PATCH] KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2 Oliver Upton
2022-02-26  0:22 ` Oliver Upton
2022-02-28 16:33 ` Sean Christopherson
2022-02-28 18:00   ` Oliver Upton
2022-02-28 18:22     ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yh0SyrFvu6UBwsj3@google.com \
    --to=seanjc@google.com \
    --cc=daviddunn@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=oupton@google.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.