From: Yang Weijiang <weijiang.yang@intel.com>
To: Like Xu <like.xu.linux@gmail.com>
Cc: Yang Weijiang <weijiang.yang@intel.com>,
pbonzini@redhat.com, jmattson@google.com, seanjc@google.com,
vkuznets@redhat.com, wei.w.wang@intel.com, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
Date: Tue, 10 Aug 2021 16:30:40 +0800 [thread overview]
Message-ID: <20210810083040.GB2970@intel.com> (raw)
In-Reply-To: <59ef2c3c-0997-d6a5-0d4a-4e777206a665@gmail.com>
On Mon, Aug 09, 2021 at 09:36:49PM +0800, Like Xu wrote:
> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> >From: Like Xu <like.xu@linux.intel.com>
> >
> >Arch LBRs are enabled by setting MSR_ARCH_LBR_CTL.LBREn to 1. A new guest
> >state field named "Guest IA32_LBR_CTL" is added to enhance guest LBR usage.
> >When guest Arch LBR is enabled, a guest LBR event will be created like the
> >model-specific LBR does. Clear guest LBR enable bit on host PMI handling so
> >guest can see expected config.
> >
> >On processors that support Arch LBR, MSR_IA32_DEBUGCTLMSR[bit 0] has no
> >meaning. It can be written to 0 or 1, but reads will always return 0.
> >Like IA32_DEBUGCTL, IA32_ARCH_LBR_CTL msr is also preserved on INIT.
> >
> >Regardless of the Arch LBR or legacy LBR, when the LBR_EN bit 0 of the
> >corresponding control MSR is set to 1, LBR recording will be enabled.
> >
> >Signed-off-by: Like Xu <like.xu@linux.intel.com>
> >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> >---
> > arch/x86/events/intel/lbr.c | 2 --
> > arch/x86/include/asm/msr-index.h | 1 +
> > arch/x86/include/asm/vmx.h | 2 ++
> > arch/x86/kvm/vmx/pmu_intel.c | 48 ++++++++++++++++++++++++++++----
> > arch/x86/kvm/vmx/vmx.c | 9 ++++++
> > 5 files changed, 55 insertions(+), 7 deletions(-)
> >
[...]
> >+static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
> >+{
> >+ unsigned int eax, ebx, ecx, edx;
> >+
> >+ if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >+ return false;
> >+
> >+ cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> >+ if (!(ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
> >+ return false;
> >+ if (!(ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
> >+ return false;
> >+ if (!(ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_BRN_MASK))
> >+ return false;
> >+
> >+ return !(ctl & ~KVM_ARCH_LBR_CTL_MASK);
> >+}
>
> Please check it with the *guest* cpuid entry.
If KVM "trusts" user-space, then check with guest cpuid is OK.
But if user-space enable excessive controls, then check against guest
cpuid could make things mess.
>
> And it should remove the bits that are not supported by x86_pmu.lbr_ctl_mask before
> vmcs_write64(...) if the guest value is a superset of the host value with
> warning message.
Then I think it makes more sense to check against x86_pmu.lbr_xxx masks in above function
for compatibility. What do you think of it?
>
> >+
> > static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > {
> > struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >@@ -392,6 +414,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > case MSR_ARCH_LBR_DEPTH:
> > msr_info->data = lbr_desc->records.nr;
> > return 0;
> >+ case MSR_ARCH_LBR_CTL:
> >+ msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
> >+ return 0;
> > default:
> > if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> > (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> > vmcs_write64(GUEST_IA32_DEBUGCTL, data);
[...]
> > if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
> > (data & DEBUGCTLMSR_LBR))
> >@@ -4441,6 +4448,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> > vmcs_writel(GUEST_SYSENTER_ESP, 0);
> > vmcs_writel(GUEST_SYSENTER_EIP, 0);
> > vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
> >+ if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> >+ vmcs_write64(GUEST_IA32_LBR_CTL, 0);
>
> Please update dump_vmcs() to dump GUEST_IA32_LBR_CTL as well.
OK, will add it.
>
> How about update the load_vmcs12_host_state() for GUEST_IA32_LBR_CTL
> since you enabled the nested case in this patch set ?
No, I didn't enable nested Arch LBR but unblocked some issues for nested case.
>
> > }
> > kvm_set_rflags(vcpu, X86_EFLAGS_FIXED);
> >
next prev parent reply other threads:[~2021-08-10 8:17 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-06 7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 02/15] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
2021-08-09 13:07 ` Like Xu
2021-08-06 7:42 ` [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
2021-08-09 13:16 ` Like Xu
2021-08-10 7:38 ` Yang Weijiang
2021-08-10 7:54 ` Like Xu
2021-08-10 9:08 ` Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
2021-08-06 16:26 ` kernel test robot
2021-08-09 13:36 ` Like Xu
2021-08-10 8:30 ` Yang Weijiang [this message]
2021-08-10 8:37 ` Like Xu
2021-08-06 7:42 ` [PATCH v7 06/15] KVM: x86/pmu: Refactor code to support " Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 07/15] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 08/15] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 09/15] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 10/15] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 11/15] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 12/15] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
2021-08-09 5:08 ` Like Xu
2021-08-09 9:02 ` Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 14/15] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
2021-08-06 7:42 ` [PATCH v7 15/15] KVM: x86/cpuid: Advise Arch LBR feature in CPUID Yang Weijiang
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=20210810083040.GB2970@intel.com \
--to=weijiang.yang@intel.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=like.xu.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=vkuznets@redhat.com \
--cc=wei.w.wang@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).