From: Sean Christopherson <seanjc@google.com>
To: Like Xu <like.xu@linux.intel.com>
Cc: 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>,
kvm@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] KVM: vmx/pmu: Fix dummy check if lbr_desc->event is created
Date: Tue, 23 Feb 2021 09:15:12 -0800 [thread overview]
Message-ID: <YDU4II6Jt+E5nFmG@google.com> (raw)
In-Reply-To: <20210223013958.1280444-1-like.xu@linux.intel.com>
On Tue, Feb 23, 2021, Like Xu wrote:
> If lbr_desc->event is successfully created, the intel_pmu_create_
> guest_lbr_event() will return 0, otherwise it will return -ENOENT,
> and then jump to LBR msrs dummy handling.
>
> Fixes: 1b5ac3226a1a ("KVM: vmx/pmu: Pass-through LBR msrs when the guest LBR event is ACTIVE")
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
> arch/x86/kvm/vmx/pmu_intel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index d1df618cb7de..d6a5fe19ff09 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -320,7 +320,7 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
> if (!intel_pmu_is_valid_lbr_msr(vcpu, index))
> return false;
>
> - if (!lbr_desc->event && !intel_pmu_create_guest_lbr_event(vcpu))
> + if (!lbr_desc->event && intel_pmu_create_guest_lbr_event(vcpu))
> goto dummy;
Wouldn't it be better to create an event only on write? And really, why create
the event in this flow in the first place? In normal operation, can't event
creation be deferred until GUEST_IA32_DEBUGCTL.DEBUGCTLMSR_LBR=1? If event
creation fails in that flow, I would think KVM would do its best to create an
event in future runs without waiting for additional actions from the guest.
Also, this bug suggests there's a big gaping hole in the test coverage. AFAICT,
event contention would lead to a #GP crash in the host due to lbr_desc->event
being dereferenced, no?
>
> /*
> --
> 2.29.2
>
next prev parent reply other threads:[~2021-02-23 17:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-23 1:39 [PATCH 1/2] KVM: vmx/pmu: Fix dummy check if lbr_desc->event is created Like Xu
2021-02-23 1:39 ` [PATCH 2/2] KVM: vmx/pmu: Clear DEBUGCTLMSR_LBR bit on the debug breakpoint event Like Xu
2021-02-23 16:38 ` Sean Christopherson
2021-02-23 17:06 ` Paolo Bonzini
2021-02-23 17:06 ` [PATCH 1/2] KVM: vmx/pmu: Fix dummy check if lbr_desc->event is created Paolo Bonzini
2021-02-23 17:15 ` Sean Christopherson [this message]
2021-02-23 17:19 ` Paolo Bonzini
2021-02-24 2:29 ` Xu, Like
2021-02-26 22:46 ` 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=YDU4II6Jt+E5nFmG@google.com \
--to=seanjc@google.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=like.xu@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/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