public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Dapeng Mi <dapeng1.mi@linux.intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Jim Mattson <jmattson@google.com>,
	Mingwei Zhang <mizhang@google.com>,
	 Zide Chen <zide.chen@intel.com>,
	Das Sandipan <Sandipan.Das@amd.com>,
	 Shukla Manali <Manali.Shukla@amd.com>,
	Xiaoyao Li <xiaoyao.li@intel.com>,
	 Dapeng Mi <dapeng1.mi@intel.com>,
	dongsheng <dongsheng.x.zhang@intel.com>,
	 Yi Lai <yi1.lai@intel.com>
Subject: Re: [kvm-unit-tests patch v3 1/8] x86/pmu: Add helper to detect Intel overcount issues
Date: Thu, 20 Nov 2025 14:27:02 -0800	[thread overview]
Message-ID: <aR-VtupdTy4vHvSz@google.com> (raw)
In-Reply-To: <20250903064601.32131-2-dapeng1.mi@linux.intel.com>

On Wed, Sep 03, 2025, Dapeng Mi wrote:
> From: dongsheng <dongsheng.x.zhang@intel.com>
> 
> For Intel Atom CPUs, the PMU events "Instruction Retired" or
> "Branch Instruction Retired" may be overcounted for some certain
> instructions, like FAR CALL/JMP, RETF, IRET, VMENTRY/VMEXIT/VMPTRLD
> and complex SGX/SMX/CSTATE instructions/flows.
> 
> The detailed information can be found in the errata (section SRF7):
> https://edc.intel.com/content/www/us/en/design/products-and-solutions/processors-and-chipsets/sierra-forest/xeon-6700-series-processor-with-e-cores-specification-update/errata-details/
> 
> For the Atom platforms before Sierra Forest (including Sierra Forest),
> Both 2 events "Instruction Retired" and "Branch Instruction Retired" would
> be overcounted on these certain instructions, but for Clearwater Forest
> only "Instruction Retired" event is overcounted on these instructions.
> 
> So add a helper detect_inst_overcount_flags() to detect whether the
> platform has the overcount issue and the later patches would relax the
> precise count check by leveraging the gotten overcount flags from this
> helper.
> 
> Signed-off-by: dongsheng <dongsheng.x.zhang@intel.com>
> [Rewrite comments and commit message - Dapeng]
> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
> Tested-by: Yi Lai <yi1.lai@intel.com>
> ---
>  lib/x86/processor.h | 27 ++++++++++++++++++++++++++
>  x86/pmu.c           | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> index 62f3d578..937f75e4 100644
> --- a/lib/x86/processor.h
> +++ b/lib/x86/processor.h
> @@ -1188,4 +1188,31 @@ static inline bool is_lam_u57_enabled(void)
>  	return !!(read_cr3() & X86_CR3_LAM_U57);
>  }
>  
> +/* Copy from kernel arch/x86/lib/cpu.c */

Eh, just drop this, we don't care if the kernel code changes, this is all based
on architectural behavior.

> +static inline u32 x86_family(u32 sig)
> +{
> +	u32 x86;
> +
> +	x86 = (sig >> 8) & 0xf;
> +
> +	if (x86 == 0xf)
> +		x86 += (sig >> 20) & 0xff;
> +
> +	return x86;
> +}
> +
> +static inline u32 x86_model(u32 sig)
> +{
> +	u32 fam, model;
> +
> +	fam = x86_family(sig);
> +
> +	model = (sig >> 4) & 0xf;
> +
> +	if (fam >= 0x6)
> +		model += ((sig >> 16) & 0xf) << 4;
> +
> +	return model;
> +}

We should place these up near is_intel() so that it's more obviously what "family"
and "model" mean (should be obvious already, but it's an easy thing to do).
> +/*
> + * For Intel Atom CPUs, the PMU events "Instruction Retired" or
> + * "Branch Instruction Retired" may be overcounted for some certain
> + * instructions, like FAR CALL/JMP, RETF, IRET, VMENTRY/VMEXIT/VMPTRLD
> + * and complex SGX/SMX/CSTATE instructions/flows.
> + *
> + * The detailed information can be found in the errata (section SRF7):
> + * https://edc.intel.com/content/www/us/en/design/products-and-solutions/processors-and-chipsets/sierra-forest/xeon-6700-series-processor-with-e-cores-specification-update/errata-details/
> + *
> + * For the Atom platforms before Sierra Forest (including Sierra Forest),
> + * Both 2 events "Instruction Retired" and "Branch Instruction Retired" would
> + * be overcounted on these certain instructions, but for Clearwater Forest
> + * only "Instruction Retired" event is overcounted on these instructions.
> + */
> +static u32 detect_inst_overcount_flags(void)
> +{
> +	u32 flags = 0;
> +	struct cpuid c = cpuid(1);
> +
> +	if (x86_family(c.a) == 0x6) {
> +		switch (x86_model(c.a)) {
> +		case 0xDD: /* Clearwater Forest */
> +			flags = INST_RETIRED_OVERCOUNT;
> +			break;
> +
> +		case 0xAF: /* Sierra Forest */
> +		case 0x4D: /* Avaton, Rangely */
> +		case 0x5F: /* Denverton */
> +		case 0x86: /* Jacobsville */
> +			flags = INST_RETIRED_OVERCOUNT | BR_RETIRED_OVERCOUNT;
> +			break;
> +		}
> +	}
> +
> +	return flags;
> +}

The errata tracking definitely belongs "struct pmu_caps pmu", and the init in
pmu_init().

  reply	other threads:[~2025-11-20 22:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-03  6:45 [kvm-unit-tests patch v3 0/8] Fix pmu test errors on GNR/SRF/CWF Dapeng Mi
2025-09-03  6:45 ` [kvm-unit-tests patch v3 1/8] x86/pmu: Add helper to detect Intel overcount issues Dapeng Mi
2025-11-20 22:27   ` Sean Christopherson [this message]
2025-11-21  1:18     ` Mi, Dapeng
2025-09-03  6:45 ` [kvm-unit-tests patch v3 2/8] x86/pmu: Relax precise count validation for Intel overcounted platforms Dapeng Mi
2025-09-03  6:45 ` [kvm-unit-tests patch v3 3/8] x86/pmu: Fix incorrect masking of fixed counters Dapeng Mi
2025-11-20 22:28   ` Sean Christopherson
2025-11-21  1:25     ` Mi, Dapeng
2025-09-03  6:45 ` [kvm-unit-tests patch v3 4/8] x86/pmu: Handle instruction overcount issue in overflow test Dapeng Mi
2025-09-03  6:45 ` [kvm-unit-tests patch v3 5/8] x86/pmu: Relax precise count check for emulated instructions tests Dapeng Mi
2025-11-20 22:29   ` Sean Christopherson
2025-11-21  0:53     ` Mi, Dapeng
2025-09-03  6:45 ` [kvm-unit-tests patch v3 6/8] x86/pmu: Expand "llc references" upper limit for broader compatibility Dapeng Mi
2025-09-03  6:46 ` [kvm-unit-tests patch v3 7/8] x86: pmu_pebs: Remove abundant data_cfg_match calculation Dapeng Mi
2025-09-03  6:46 ` [kvm-unit-tests patch v3 8/8] x86: pmu_pebs: Support to validate timed PEBS record on GNR/SRF Dapeng Mi
2025-11-20 22:30 ` [kvm-unit-tests patch v3 0/8] Fix pmu test errors on GNR/SRF/CWF 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=aR-VtupdTy4vHvSz@google.com \
    --to=seanjc@google.com \
    --cc=Manali.Shukla@amd.com \
    --cc=Sandipan.Das@amd.com \
    --cc=dapeng1.mi@intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=dongsheng.x.zhang@intel.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mizhang@google.com \
    --cc=pbonzini@redhat.com \
    --cc=xiaoyao.li@intel.com \
    --cc=yi1.lai@intel.com \
    --cc=zide.chen@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