From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Oliver Upton <oupton@google.com>,
kvm@vger.kernel.org, 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 v4 1/8] KVM: nVMX: Keep KVM updates to BNDCFGS ctrl bits across MSR write
Date: Thu, 3 Mar 2022 16:15:47 +0000 [thread overview]
Message-ID: <YiDps0lOKITPn4gv@google.com> (raw)
In-Reply-To: <78abcc19-0a79-4f8b-2eaf-c99b96efea42@redhat.com>
On Thu, Mar 03, 2022, Paolo Bonzini wrote:
> On 3/3/22 02:43, Sean Christopherson wrote:
> > > Maybe I can redirect you to a test case to highlight a possible
> > > regression in KVM, as seen by userspace;-)
> > Regressions aside, VMCS controls are not tied to CPUID, KVM should not be mucking
> > with unrelated things. The original hack was to fix a userspace bug and should
> > never have been mreged.
>
> Note that it dates back to:
>
> commit 5f76f6f5ff96587af5acd5930f7d9fea81e0d1a8
> Author: Liran Alon <liran.alon@oracle.com>
> Date: Fri Sep 14 03:25:52 2018 +0300
>
> KVM: nVMX: Do not expose MPX VMX controls when guest MPX disabled
> Before this commit, KVM exposes MPX VMX controls to L1 guest only based
> on if KVM and host processor supports MPX virtualization.
> However, these controls should be exposed to guest only in case guest
> vCPU supports MPX.
>
> It's not to fix a userspace bug, it's to support userspace that doesn't
> know about using KVM_SET_MSR for VMX features---which is okay since unlike
> KVM_SET_CPUID2 it's not a mandatory call.
I disagree, IMO failure to properly configure the vCPU model is a userspace bug.
Maybe it was a userspace bug induced by a haphazard and/or poorly documented KVM
ABI, but it's still a userspace bug. One could argue that KVM should disable/clear
VMX features if userspace clears a related CPUID feature, but _setting_ a VMX
feature based on CPUID is architecturally wrong. Even if we consider one or both
cases to be desirable behavior in terms of creating a consistent vCPU model, forcing
a consistent vCPU model for this one case goes against every other ioctl in KVM's
ABI.
If we consider it KVM's responsibility to propagate CPUID state to VMX MSRs, then
KVM has a bunch of "bugs".
X86_FEATURE_LM => VM_EXIT_HOST_ADDR_SPACE_SIZE, VM_ENTRY_IA32E_MODE, VMX_MISC_SAVE_EFER_LMA
X86_FEATURE_TSC => CPU_BASED_RDTSC_EXITING, CPU_BASED_USE_TSC_OFFSETTING,
SECONDARY_EXEC_TSC_SCALING
X86_FEATURE_INVPCID_SINGLE => SECONDARY_EXEC_ENABLE_INVPCID
X86_FEATURE_MWAIT => CPU_BASED_MONITOR_EXITING, CPU_BASED_MWAIT_EXITING
X86_FEATURE_INTEL_PT => SECONDARY_EXEC_PT_CONCEAL_VMX, SECONDARY_EXEC_PT_USE_GPA,
VM_EXIT_CLEAR_IA32_RTIT_CTL, VM_ENTRY_LOAD_IA32_RTIT_CTL
X86_FEATURE_XSAVES => SECONDARY_EXEC_XSAVES
next prev parent reply other threads:[~2022-03-03 16:15 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-01 6:03 [PATCH v4 0/8] KVM: x86: VMX ctrl MSR + KVM quirk fixes Oliver Upton
2022-03-01 6:03 ` [PATCH v4 1/8] KVM: nVMX: Keep KVM updates to BNDCFGS ctrl bits across MSR write Oliver Upton
2022-03-01 18:00 ` Paolo Bonzini
2022-03-01 18:43 ` Oliver Upton
2022-03-02 12:21 ` Paolo Bonzini
2022-03-02 20:51 ` Oliver Upton
2022-03-02 21:22 ` Paolo Bonzini
2022-03-02 21:54 ` Oliver Upton
2022-03-03 1:43 ` Sean Christopherson
2022-03-03 6:29 ` Paolo Bonzini
2022-03-03 16:15 ` Sean Christopherson [this message]
2022-03-03 21:44 ` Jim Mattson
2022-03-03 23:44 ` Sean Christopherson
2022-03-04 15:50 ` Paolo Bonzini
2022-04-07 0:26 ` Sean Christopherson
2022-04-07 0:29 ` Oliver Upton
2022-04-07 0:32 ` Oliver Upton
2022-04-07 0:34 ` Sean Christopherson
2022-05-27 16:55 ` Sean Christopherson
2022-03-01 6:03 ` [PATCH v4 2/8] KVM: nVMX: Keep KVM updates to PERF_GLOBAL_CTRL " Oliver Upton
2022-03-01 18:01 ` Paolo Bonzini
2022-04-07 0:21 ` Sean Christopherson
2022-03-01 6:03 ` [PATCH v4 3/8] KVM: nVMX: Drop nested_vmx_pmu_refresh() Oliver Upton
2022-03-01 6:03 ` [PATCH v4 4/8] KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2 Oliver Upton
2022-03-09 16:01 ` Paolo Bonzini
2022-03-01 6:03 ` [PATCH v4 5/8] KVM: nVMX: Add a quirk for KVM tweaks to VMX control MSRs Oliver Upton
2022-04-07 0:28 ` Sean Christopherson
2022-03-01 6:03 ` [PATCH v4 6/8] selftests: KVM: Separate static alloc from KVM_GET_SUPPORTED_CPUID call Oliver Upton
2022-03-01 6:03 ` [PATCH v4 7/8] selftests: KVM: Add test for PERF_GLOBAL_CTRL VMX control MSR bits Oliver Upton
2022-03-01 16:59 ` David Dunn
2022-03-01 6:03 ` [PATCH v4 8/8] selftests: KVM: Add test for BNDCFGS " Oliver Upton
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=YiDps0lOKITPn4gv@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).