| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b3b3fdde-ce64-dbd2-00a2-a5d75259e558@intel.com>
Date: Tue, 3 Dec 2024 20:16:12 -0800
From: Fenghua Yu <fenghua.yu@...el.com>
To: Babu Moger <babu.moger@....com>, <corbet@....net>,
<reinette.chatre@...el.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
<bp@...en8.de>, <dave.hansen@...ux.intel.com>
CC: <x86@...nel.org>, <hpa@...or.com>, <thuth@...hat.com>,
<paulmck@...nel.org>, <rostedt@...dmis.org>, <akpm@...ux-foundation.org>,
<xiongwei.song@...driver.com>, <pawan.kumar.gupta@...ux.intel.com>,
<daniel.sneddon@...ux.intel.com>, <perry.yuan@....com>,
<sandipan.das@....com>, <kai.huang@...el.com>, <xiaoyao.li@...el.com>,
<seanjc@...gle.com>, <jithu.joseph@...el.com>, <brijesh.singh@....com>,
<xin3.li@...el.com>, <ebiggers@...gle.com>, <andrew.cooper3@...rix.com>,
<mario.limonciello@....com>, <james.morse@....com>,
<tan.shaopeng@...itsu.com>, <tony.luck@...el.com>,
<vikas.shivappa@...ux.intel.com>, <linux-doc@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <peternewman@...gle.com>,
<maciej.wieczor-retman@...el.com>, <eranian@...gle.com>,
<jpoimboe@...nel.org>, <thomas.lendacky@....com>
Subject: Re: [PATCH v9 18/26] x86/resctrl: Add the interface to assign/update
counter assignment
Hi, Babu,
On 10/29/24 16:21, Babu Moger wrote:
> The mbm_cntr_assign mode offers several hardware counters that can be
> assigned to an RMID, event pair and monitor the bandwidth as long as it
> is assigned.
>
> Counters are managed at two levels. The global assignment is tracked
> using the mbm_cntr_free_map field in the struct resctrl_mon, while
> domain-specific assignments are tracked using the mbm_cntr_map field
> in the struct rdt_mon_domain. Allocation begins at the global level
> and is then applied individually to each domain.
>
> Introduce an interface to allocate these counters and update the
> corresponding domains accordingly.
>
> Signed-off-by: Babu Moger <babu.moger@....com>
> ---
> v9: Introduced new function resctrl_config_cntr to assign the counter, update
> the bitmap and reset the architectural state.
> Taken care of error handling(freeing the counter) when assignment fails.
> Moved mbm_cntr_assigned_to_domain here as it used in this patch.
> Minor text changes.
>
> v8: Renamed rdtgroup_assign_cntr() to rdtgroup_assign_cntr_event().
> Added the code to return the error if rdtgroup_assign_cntr_event fails.
> Moved definition of MBM_EVENT_ARRAY_INDEX to resctrl/internal.h.
> Updated typo in the comments.
>
> v7: New patch. Moved all the FS code here.
> Merged rdtgroup_assign_cntr and rdtgroup_alloc_cntr.
> Adde new #define MBM_EVENT_ARRAY_INDEX.
> ---
> arch/x86/kernel/cpu/resctrl/internal.h | 2 +
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 87 ++++++++++++++++++++++++++
> 2 files changed, 89 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 00f7bf60e16a..cb496bd97007 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -717,6 +717,8 @@ unsigned int mon_event_config_index_get(u32 evtid);
> int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
> enum resctrl_event_id evtid, u32 rmid, u32 closid,
> u32 cntr_id, bool assign);
> +int rdtgroup_assign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
> + struct rdt_mon_domain *d, enum resctrl_event_id evtid);
> void rdt_staged_configs_clear(void);
> bool closid_allocated(unsigned int closid);
> int resctrl_find_cleanest_closid(void);
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 1b5529c212f5..bc3752967c44 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1924,6 +1924,93 @@ int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
> return 0;
> }
>
> +/*
> + * Configure the counter for the event, RMID pair for the domain.
> + * Update the bitmap and reset the architectural state.
> + */
> +static int resctrl_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
> + enum resctrl_event_id evtid, u32 rmid, u32 closid,
> + u32 cntr_id, bool assign)
> +{
> + int ret;
> +
> + ret = resctrl_arch_config_cntr(r, d, evtid, rmid, closid, cntr_id, assign);
> + if (ret)
> + return ret;
> +
> + if (assign)
> + __set_bit(cntr_id, d->mbm_cntr_map);
> + else
> + __clear_bit(cntr_id, d->mbm_cntr_map);
> +
> + /*
> + * Reset the architectural state so that reading of hardware
> + * counter is not considered as an overflow in next update.
> + */
> + resctrl_arch_reset_rmid(r, d, closid, rmid, evtid);
> +
> + return ret;
> +}
> +
> +static bool mbm_cntr_assigned_to_domain(struct rdt_resource *r, u32 cntr_id)
> +{
> + struct rdt_mon_domain *d;
> +
> + list_for_each_entry(d, &r->mon_domains, hdr.list)
> + if (test_bit(cntr_id, d->mbm_cntr_map))
> + return 1;
> +
> + return 0;
> +}
> +
> +/*
> + * Assign a hardware counter to event @evtid of group @rdtgrp.
> + * Counter will be assigned to all the domains if rdt_mon_domain is NULL
> + * else the counter will be assigned to specific domain.
> + */
> +int rdtgroup_assign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
> + struct rdt_mon_domain *d, enum resctrl_event_id evtid)
> +{
> + int index = MBM_EVENT_ARRAY_INDEX(evtid);
> + int cntr_id = rdtgrp->mon.cntr_id[index];
> + int ret;
> +
> + /*
> + * Allocate a new counter id to the event if the counter is not
> + * assigned already.
> + */
> + if (cntr_id == MON_CNTR_UNSET) {
> + cntr_id = mbm_cntr_alloc(r);
> + if (cntr_id < 0) {
> + rdt_last_cmd_puts("Out of MBM assignable counters\n");
> + return -ENOSPC;
> + }
> + rdtgrp->mon.cntr_id[index] = cntr_id;
> + }
> +
> + if (!d) {
Should assert cpus are locked here?
/* Walking r->domains, ensure it can't race with cpuhp */
lockdep_assert_cpus_held();
Please see more comments on patch #20.
> + list_for_each_entry(d, &r->mon_domains, hdr.list) {
> + ret = resctrl_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
> + rdtgrp->closid, cntr_id, true);
> + if (ret)
> + goto out_done_assign;
> + }
> + } else {
> + ret = resctrl_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
> + rdtgrp->closid, cntr_id, true);
> + if (ret)
> + goto out_done_assign;
> + }
> +
> +out_done_assign:
> + if (ret && !mbm_cntr_assigned_to_domain(r, cntr_id)) {
> + mbm_cntr_free(r, cntr_id);
> + rdtgroup_cntr_id_init(rdtgrp, evtid);
> + }
> +
> + return ret;
> +}
> +
> /* rdtgroup information files for one cache resource. */
> static struct rftype res_common_files[] = {
> {
Thanks.
-Fenghua
Powered by blists - more mailing lists