mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Sean Christopherson <seanjc@google.com>,
	Mingwei Zhang <mizhang@google.com>
Cc: Aaron Lewis <aaronlewis@google.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] KVM: x86/pmu: Reset perf_capabilities in vcpu to 0 if PDCM is disabled
Date: Mon, 29 Jan 2024 15:39:07 +0100	[thread overview]
Message-ID: <b0b5ba26-505e-4247-b30d-9ba2bb0301c1@redhat.com> (raw)
In-Reply-To: <ZbGUfmn-ZAe4lkiN@google.com>

On 1/24/24 23:51, Sean Christopherson wrote:
>> If we follow the suggestion by removing the initial value at vCPU
>> creation time, then I think it breaks the existing VMM code, since that
>> requires VMM to explicitly set the MSR, which I am not sure we do today.
> Yeah, I'm hoping we can squeak by without breaking existing setups.
> 
> I'm 99% certain QEMU is ok, as QEMU has explicitly set MSR_IA32_PERF_CAPABILITIES
> since support for PDCM/PERF_CAPABILITIES was added by commit ea39f9b643
> ("target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES").
> 
> Frankly, if our VMM doesn't do the same, then it's wildly busted.  Relying on
> KVM to define the vCPU is irresponsible, to put it nicely.

Yes, I tend to agree.

What QEMU does goes from the squeaky clean to the very debatable 
depending on the parameters you give it.

With "-cpu Haswell" and similar, it will provide values for all CPUID 
and MSR bits that match as much as possible values from an actual CPU 
model.  It will complain if there are some values that do not match[1].

With "-cpu host", it will copy values from KVM_GET_SUPPORTED_CPUID and 
from the feature MSRs, but only for features that it knows about.

With "-cpu host,migratable=no", it will copy values from 
KVM_GET_SUPPORTED_CPUID and from the feature MSRs, but only for *feature 
words* (CPUID registers, or MSRs) that it knows about.  This is where it 
becomes debatable, because a CPUID bit could be added without QEMU 
knowing the corresponding MSR.  In this case, the user probably expects 
the MSR to have a nonzero.  On one hand I agree that it would be 
irresponsible, on the other hand that's the point of "-cpu 
host,migratable=no".

If you want to proceed with the change, I don't have any problem with 
considering it a QEMU bug that it doesn't copy over to the guest any 
unknown leaves or MSRs.

Paolo

[1] Unfortunately it's not fatal because there are way way too many 
models, and also because until recently TCG lacked AVX---and therefore 
could only emulate completely some very old CPU models.  But with "-cpu 
Haswell,enforce" then everything's clean.


  parent reply	other threads:[~2024-01-29 14:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-24  0:38 [PATCH 0/2] minor fix on perf_capabilities in KVM/x86 Mingwei Zhang
2024-01-24  0:38 ` [PATCH 1/2] KVM: x86/pmu: Reset perf_capabilities in vcpu to 0 if PDCM is disabled Mingwei Zhang
2024-01-24 15:49   ` Sean Christopherson
2024-01-24 21:04     ` Aaron Lewis
2024-01-24 21:25       ` Sean Christopherson
2024-01-24 22:24         ` Mingwei Zhang
2024-01-24 22:51           ` Sean Christopherson
2024-01-25  0:14             ` Mingwei Zhang
2024-01-26 18:33               ` Sean Christopherson
2024-01-26 19:30                 ` Mingwei Zhang
2024-01-26 19:34                   ` Sean Christopherson
2024-01-29 14:40                   ` Paolo Bonzini
2024-01-29 14:39             ` Paolo Bonzini [this message]
2024-01-31 19:43               ` Mingwei Zhang
2024-01-24  0:38 ` [PATCH 2/2] KVM: x86/pmu: Remove vcpu_get_perf_capabilities() Mingwei Zhang
2024-01-24 15:52   ` 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=b0b5ba26-505e-4247-b30d-9ba2bb0301c1@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=aaronlewis@google.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=seanjc@google.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

all inboxes | Powered by JetHome®