linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Jim Mattson <jmattson@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Joerg Roedel <joro@8bytes.org>, kvm list <kvm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Xiaoyao Li <xiaoyao.li@intel.com>
Subject: Re: [PATCH 2/6] KVM: x86: Fix CPUID range check for Centaur and Hypervisor ranges
Date: Mon, 2 Mar 2020 20:58:38 -0800	[thread overview]
Message-ID: <20200303045838.GF27842@linux.intel.com> (raw)
In-Reply-To: <CALMp9eR0Mw8iPv_Z43gfCEbErHQ6EXX8oghJJb5Xge+47ZU9yQ@mail.gmail.com>

On Mon, Mar 02, 2020 at 08:25:31PM -0800, Jim Mattson wrote:
> On Mon, Mar 2, 2020 at 7:25 PM Jim Mattson <jmattson@google.com> wrote:
> >
> > On Mon, Mar 2, 2020 at 11:57 AM Sean Christopherson
> > <sean.j.christopherson@intel.com> wrote:
> >
> > > The bad behavior can be visually confirmed by dumping CPUID output in
> > > the guest when running Qemu with a stable TSC, as Qemu extends the limit
> > > of range 0x40000000 to 0x40000010 to advertise VMware's cpuid_freq,
> > > without defining zeroed entries for 0x40000002 - 0x4000000f.
> >
> > I think it could be reasonably argued that this is a userspace bug.
> > Clearly, when userspace explicitly supplies the results for a leaf,
> > those results override the default CPUID values for that leaf. But I
> > haven't seen it documented anywhere that leaves *not* explicitly
> > supplied by userspace will override the default CPUID values, just
> > because they happen to appear in some magic range.
> 
> In fact, the more I think about it, the original change is correct, at
> least in this regard. Your "fix" introduces undocumented and
> unfathomable behavior.

Heh, the takeaway from this is that whatever we decide on needs to be
documented somewhere :-)

I wouldn't say it's unfathomable, conceptually it seems like the intent
of the hypervisor range was to mimic the basic and extended ranges.  The
whole thing is arbitrary behavior.  Of course if Intel CPUs would just
return 0s on undefined leafs it would be a lot less arbitrary :-)
 
Anyways, I don't have a strong opinion on whether this patch stays or goes.

  reply	other threads:[~2020-03-03  4:58 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 19:57 [PATCH 0/6] KVM: x86: CPUID emulation and tracing fixes Sean Christopherson
2020-03-02 19:57 ` [PATCH 1/6] KVM: x86: Fix tracing of CPUID.function when function is out-of-range Sean Christopherson
2020-03-02 20:26   ` Jan Kiszka
2020-03-02 20:49     ` Sean Christopherson
2020-03-02 20:59       ` Jan Kiszka
2020-03-03  2:27       ` Xiaoyao Li
2020-03-03  3:45         ` Sean Christopherson
2020-03-03  4:02           ` Xiaoyao Li
2020-03-03  4:12             ` Sean Christopherson
2020-03-03  4:30               ` Xiaoyao Li
2020-03-03  2:50   ` Xiaoyao Li
2020-03-03  4:08     ` Sean Christopherson
2020-03-03  4:16       ` Xiaoyao Li
2020-03-02 19:57 ` [PATCH 2/6] KVM: x86: Fix CPUID range check for Centaur and Hypervisor ranges Sean Christopherson
2020-03-02 21:59   ` Jim Mattson
2020-03-03  0:57     ` Sean Christopherson
2020-03-03  3:25   ` Jim Mattson
2020-03-03  4:25     ` Jim Mattson
2020-03-03  4:58       ` Sean Christopherson [this message]
2020-03-03 17:42         ` Jim Mattson
2020-03-03 18:01           ` Sean Christopherson
2020-03-03 18:08             ` Jim Mattson
2020-03-04 11:18             ` Paolo Bonzini
2020-03-02 19:57 ` [PATCH 3/6] KVM: x86: Add dedicated emulator helper for grabbing CPUID.maxphyaddr Sean Christopherson
2020-03-03  8:48   ` Paolo Bonzini
2020-03-03  9:48     ` Jan Kiszka
2020-03-03 10:14       ` Paolo Bonzini
2020-03-04 20:47         ` Sean Christopherson
2020-03-03 16:28     ` Sean Christopherson
2020-03-03 17:21       ` Paolo Bonzini
2020-03-02 19:57 ` [PATCH 4/6] KVM: x86: Drop return value from kvm_cpuid() Sean Christopherson
2020-03-02 19:57 ` [PATCH 5/6] KVM: x86: Rename "found" variable in kvm_cpuid() to "exact_entry_exists" Sean Christopherson
2020-03-02 20:20   ` Jan Kiszka
2020-03-02 20:35     ` Sean Christopherson
2020-03-02 20:48       ` Jan Kiszka
2020-03-02 19:57 ` [PATCH 6/6] KVM: x86: Add requested index to the CPUID tracepoint Sean Christopherson
2020-03-07  9:48   ` Jan Kiszka
2020-03-10  4:00     ` Sean Christopherson
2020-03-03  8:48 ` [PATCH 0/6] KVM: x86: CPUID emulation and tracing fixes Paolo Bonzini
2020-03-03 16:38   ` Sean Christopherson

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=20200303045838.GF27842@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=xiaoyao.li@intel.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).