public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, kvm <kvm@vger.kernel.org>,
	Jim Mattson <jmattson@google.com>
Subject: Re: [PATCH] kvm: x86: Make traced and returned value of kvm_cpuid consistent again
Date: Mon, 2 Mar 2020 09:55:39 -0800	[thread overview]
Message-ID: <20200302175539.GB6244@linux.intel.com> (raw)
In-Reply-To: <27b0a092-dae4-157b-7c56-7a757a680217@siemens.com>

On Mon, Mar 02, 2020 at 05:46:01PM +0100, Jan Kiszka wrote:
> On 02.03.20 17:38, Sean Christopherson wrote:
> >On Mon, Mar 02, 2020 at 05:11:57PM +0100, Paolo Bonzini wrote:
> >>Queued, thanks.
> >
> >Too fast, too fast!
> >
> >On Sun, Mar 01, 2020 at 11:47:20AM +0100, Jan Kiszka wrote:
> >>From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >>After 43561123ab37, found is not set correctly in case of leaves 0BH,
> >>1FH, or anything out-of-range.
> >
> >No, found is set correctly, kvm_cpuid() should return true if and only if
> >an exact match for the requested function is found, and that's the original
> >tracing behavior of "found" (pre-43561123ab37).
> >
> >>This is currently harmless for the return value because the only caller
> >>evaluating it passes leaf 0x80000008.
> >
> >No, it's 100% correct.  Well, technically it's irrelevant because the only
> >caller, check_cr_write(), passes %false for check_limit, i.e. found will be
> >true if and only if entry 0x80000008 exists.  But, in a purely hypothetical
> >scenario where the emulator passed check_limit=%true, the intent of "found"
> >is to report that the exact leaf was found, not if some random entry was
> >found.
> 
> Nicely non-intuitive semantics. Should definitely be documented.
> 
> And then it's questionable to me what value tracing such a return code has.

There's value in knowing the the output came from the actual requested leaf
as opposed to the max basic leaf, e.g. if the guest is seeing weird CPUID
output in the guest, knowing whether it was explicitly configured by the
userspace VMM versus coming from KVM's emulation of Intel's wonderful CPUID
behavior.

> At the bare minimum, "found" should be renamed to something like
> "exact_match".

I can do something along these lines.  kvm_cpuid() really doesn't need to
be returning a value, i.e. the emulator shouldn't be manually calculating
maxphyaddr anyways.

      reply	other threads:[~2020-03-02 17:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-01 10:47 [PATCH] kvm: x86: Make traced and returned value of kvm_cpuid consistent again Jan Kiszka
2020-03-01 12:25 ` Xiaoyao Li
2020-03-02  6:19   ` Jan Kiszka
2020-03-02 16:11 ` Paolo Bonzini
2020-03-02 16:38   ` Sean Christopherson
2020-03-02 16:46     ` Jan Kiszka
2020-03-02 17:55       ` 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=20200302175539.GB6244@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.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