The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: Chen Yu <yu.c.chen@intel.com>, <tony.luck@intel.com>
Cc: <tglx@kernel.org>, <bp@alien8.de>, <mingo@redhat.com>,
	<dave.hansen@linux.intel.com>, <hpa@zytor.com>,
	<fenghuay@nvidia.com>, <babu.moger@amd.com>, <chen.yu@linux.dev>,
	<x86@kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6 9/9] x86/resctrl: Add MMIO-based LLC occupancy monitoring support
Date: Wed, 19 Aug 2026 16:10:43 -0700	[thread overview]
Message-ID: <9c33a763-e39f-45e1-a6b7-297dc79263f7@intel.com> (raw)
In-Reply-To: <ae3f0f81e13f43061e6885dfd7a460b6b61f2c4a.1784968626.git.yu.c.chen@intel.com>

Hi Chenyu,

On 7/25/26 2:23 AM, Chen Yu wrote:
> Add erdt_mon_read() to read LLC occupancy via MMIO and use it when
> the platform supports ERDT. Register the L3 occupancy event with
> ERDT enabled when available, falling back to the MSR-based path
> otherwise.
> 
> Use the CMRC (Cache Monitoring Registers for CPU Agents Description)
> ACPI sub-table to read LLC occupancy counters for each RMID via MMIO
> when ERDT is enabled. This CMRC information is stored in the
> rdt_hw_l3_mon_domain, which could be accessed directly.

Please write in imperative tone.

> 
> Currently, the per-domain limbo handler is still in use. There is no need
> to switch to a global limbo handler, because even after such a switch, the
> worker thread would still have to iterate through all domains one by one.
> The per-domain handler already accomplishes this using a worker thread rather
> than costly IPIs, so there is no clear benefit to switching to a global handler.

Could you please elaborate how a global limbo handler would require IPIs?

> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h
> index 5491853113dd..0948f64856ef 100644
> --- a/arch/x86/include/asm/resctrl.h
> +++ b/arch/x86/include/asm/resctrl.h
> @@ -132,7 +132,13 @@ static inline void __resctrl_sched_in(struct task_struct *tsk)
>  
>  static inline unsigned int resctrl_arch_round_mon_val(unsigned int val)
>  {
> -	unsigned int scale = boot_cpu_data.x86_cache_occ_scale;
> +	unsigned int scale = boot_cpu_data.x86_cache_occ_scale, escale;

related to earlier topic, "scale" being unsigned int is ok since 
x86_cache_occ_scale is initialized from 32bits. As I understand it the
ERDT scale value is initialized from 64 bits instead so the existing
types do not seem to accommodate?

> +
> +	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +		escale = erdt_get_scale();
> +		if (escale)
> +			scale = escale;
> +	}
>  
>  	/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
>  	val /= scale;
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 74087d04f1da..b528023a7bcc 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -997,7 +997,10 @@ static __init bool get_rdt_mon_resources(void)
>  	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>  	bool ret = false;
>  
> -	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +	if (erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
> +		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, true, 0, NULL);
> +		ret = true;
> +	} else if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
>  		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false, 0, NULL);
>  		ret = true;
>  	}
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index 167d2dfc368d..5f9377c80be6 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -26,6 +26,10 @@ static bool erdt_enabled;
>  #define CMRC_SUPPORTED_INDEX_FN		1
>  #define RMDD_FLAG_CPU_L3_DOMAIN		BIT(0)
>  
> +/* Set in a monitoring counter when it holds no valid data to report. */
> +#define UNAVAILABLE_COUNTER		BIT_ULL(63)
> +#define CMRC_FLAG_UNAVAILABLE_BIT	BIT(0)
> +
>  /* Bitmask of valid sub-tables found in the first RMDD, used to ensure all RMDDs match. */
>  static u32 valid_subtbl_mask;
>  
> @@ -39,6 +43,9 @@ static int erdt_scale;
>  
>  bool erdt_support(int flag)
>  {
> +	if (flag == X86_FEATURE_CQM_OCCUP_LLC)
> +		return valid_subtbl_mask & BIT(ACPI_ERDT_TYPE_CMRC);
> +

Is the plan to keep adding more if() statements as new flags need to be tested?


>  	return false;
>  }
>  
> @@ -52,6 +59,58 @@ int erdt_get_scale(void)
>  	return erdt_scale;
>  }
>  
> +static u32 cmrc_index_function_1(struct acpi_erdt_cmrc *cmrc, u32 rmid)
> +{
> +	/*
> +	 * MMIO_offset_for_RMID# =
> +	 *   (RMID / ClumpSize) * Stride +
> +	 *   (RMID % ClumpSize) * 8
> +	 */
> +	return (rmid / cmrc->clump_size) * cmrc->clump_stride +
> +	       (rmid % cmrc->clump_size) * 8;
> +}
> +
> +static int erdt_read_l3_occupancy(const struct erdt_domain_info *d, u32 rmid, u64 *val)
> +{
> +	struct acpi_erdt_cmrc *cmrc;
> +	u64 l3_cmt_count;
> +	u32 offset;
> +
> +	cmrc = d->cmrc;
> +	if (!cmrc)
> +		return -EIO;
> +
> +	offset = cmrc_index_function_1(cmrc, rmid);
> +	/* Overflow of cmt_reg_size * SZ_4K already validated in erdt_ioremap(). */
> +	if (offset + sizeof(u64) > (u32)cmrc->cmt_reg_size * SZ_4K)
> +		return -EINVAL;
> +
> +	l3_cmt_count = readq(d->base[ERDT_MMIO_CMRC_BASE] + offset);
> +	if ((cmrc->flags & CMRC_FLAG_UNAVAILABLE_BIT) &&
> +	    (l3_cmt_count & UNAVAILABLE_COUNTER))
> +		return -EINVAL;
> +
> +	*val = l3_cmt_count * cmrc->up_scale;
> +
> +	return 0;
> +}
> +
> +int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val)
> +{
> +	struct rdt_hw_l3_mon_domain *hw_dom;
> +	const struct erdt_domain_info *d;
> +
> +	hw_dom = resctrl_to_arch_mon_dom(container_of(hdr, struct rdt_l3_mon_domain, hdr));
> +	d = hw_dom->d_info;
> +	if (!d)
> +		return -EIO;
> +
> +	if (evtid == QOS_L3_OCCUP_EVENT_ID)
> +		return erdt_read_l3_occupancy(d, rmid, val);
> +
> +	return -EIO;
> +}
> +
>  static void __iomem *erdt_ioremap(phys_addr_t base, u32 num_pages, const char *desc)
>  {
>  	void __iomem *addr;
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index d14a36d65b05..a3cb32e8fabc 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -158,7 +158,11 @@ struct msr_param {
>   *			which has been corrected for features like CDP.
>   * @msr_base:		Base MSR address for CBMs
>   * @msr_update:		Function pointer to update QOS MSRs
> - * @mon_scale:		cqm counter * mon_scale = occupancy in bytes
> + * @mon_scale:		Scale factor applied to a raw counter value on the
> + *			MSR-based read path: CMT occupancy counter * mon_scale =
> + *			occupancy in bytes, and MBM chunk count * mon_scale = bytes
> + *			transferred. ERDT reads occupancy via MMIO and applies its
> + *			own firmware-provided scale instead.
>   * @mbm_width:		Monitor width, to detect and correct for overflow.
>   * @cdp_enabled:	CDP state of this resource
>   * @mbm_cntr_assign_enabled:	ABMC feature is enabled
> @@ -292,6 +296,7 @@ static inline bool intel_handle_aet_option(bool force_off, char *tok) { return f
>  bool erdt_support(int flag);
>  bool erdt_cpu_has(int flag);
>  int erdt_get_max_rmid(void);
> +int erdt_mon_read(struct rdt_domain_hdr *hdr, enum resctrl_event_id evtid, u32 rmid, u64 *val);
>  int erdt_init(void);
>  void erdt_exit(void);
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 786828a0a3c2..d33a865e59d4 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -284,6 +284,10 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain_hdr *hdr,
>  
>  	switch (r->rid) {
>  	case RDT_RESOURCE_L3:
> +		if (eventid == QOS_L3_OCCUP_EVENT_ID &&
> +		    erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
> +			return erdt_mon_read(hdr, eventid, rmid, val);
> +
>  		return arch_l3_read_event(hdr, rmid, eventid, val, r);
>  	case RDT_RESOURCE_PERF_PKG:
>  		return intel_aet_read_event(hdr->id, rmid, arch_priv, val);
> @@ -430,12 +434,15 @@ int __init rdt_get_l3_mon_config(struct rdt_resource *r)
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  	unsigned int threshold;
>  	u32 eax, ebx, ecx, edx;
> +	int max_rmid;
>  
>  	snc_nodes_per_l3_cache = snc_get_config();
>  
> +	max_rmid = erdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC) ?
> +						erdt_get_max_rmid() : boot_cpu_data.x86_cache_max_rmid;

This does not look right. Wouldn't this use the ERDT supported RMID for the MBM events also
even though they are read via MSR?

>  	resctrl_rmid_realloc_limit = boot_cpu_data.x86_cache_size * 1024;
>  	hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale / snc_nodes_per_l3_cache;

Should the scale used by ERDT also be adjusted when SNC enabled?

> -	r->mon.num_rmid = (boot_cpu_data.x86_cache_max_rmid + 1) / snc_nodes_per_l3_cache;
> +	r->mon.num_rmid = (max_rmid + 1) / snc_nodes_per_l3_cache;
>  	hw_res->mbm_width = MBM_CNTR_WIDTH_BASE;
>  
>  	if (mbm_offset > 0 && mbm_offset <= MBM_CNTR_WIDTH_OFFSET_MAX)

Reinette

  reply	other threads:[~2026-08-19 23:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-25  9:20 [PATCH v6 0/9] Introduce MMIO-based CMT access for Enhanced RDT Chen Yu
2026-07-25  9:22 ` [PATCH v6 1/9] x86/topology: Export topo_lookup_cpuid() for resctrl use Chen Yu
2026-08-19 22:55   ` Reinette Chatre
2026-07-25  9:22 ` [PATCH v6 2/9] x86/resctrl: Require 64-bit x86 for resctrl support Chen Yu
2026-08-19 22:55   ` Reinette Chatre
2026-07-25  9:22 ` [PATCH v6 3/9] x86/resctrl: Parse ACPI ERDT table and save CACD cpumask for RMDD domains Chen Yu
2026-08-19 23:01   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 4/9] x86/resctrl: Attach ACPI ERDT information to L3 mon domain on CPU online Chen Yu
2026-08-19 23:04   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 5/9] x86/resctrl: Parse ACPI CMRC table Chen Yu
2026-08-19 23:06   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 6/9] x86/resctrl: Refactor the monitor read function Chen Yu
2026-08-19 23:07   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 7/9] fs/resctrl: Do not invoke smp_processor_id() in preemptible context Chen Yu
2026-08-19 23:08   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 8/9] x86/resctrl: Introduce erdt_cpu_has() and erdt_support() Chen Yu
2026-08-19 23:08   ` Reinette Chatre
2026-07-25  9:23 ` [PATCH v6 9/9] x86/resctrl: Add MMIO-based LLC occupancy monitoring support Chen Yu
2026-08-19 23:10   ` Reinette Chatre [this message]
2026-08-13  6:43 ` [PATCH v6 0/9] Introduce MMIO-based CMT access for Enhanced RDT Chen Yu

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=9c33a763-e39f-45e1-a6b7-297dc79263f7@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=chen.yu@linux.dev \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghuay@nvidia.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=yu.c.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