| 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: <efa7aee8-d1f3-4d15-9a6e-09b19c296e47@intel.com>
Date: Tue, 15 Apr 2025 09:53:11 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: <babu.moger@....com>, <tony.luck@...el.com>, <peternewman@...gle.com>
CC: <corbet@....net>, <tglx@...utronix.de>, <mingo@...hat.com>,
<bp@...en8.de>, <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
<hpa@...or.com>, <paulmck@...nel.org>, <akpm@...ux-foundation.org>,
<thuth@...hat.com>, <rostedt@...dmis.org>, <ardb@...nel.org>,
<gregkh@...uxfoundation.org>, <daniel.sneddon@...ux.intel.com>,
<jpoimboe@...nel.org>, <alexandre.chartre@...cle.com>,
<pawan.kumar.gupta@...ux.intel.com>, <thomas.lendacky@....com>,
<perry.yuan@....com>, <seanjc@...gle.com>, <kai.huang@...el.com>,
<xiaoyao.li@...el.com>, <kan.liang@...ux.intel.com>, <xin3.li@...el.com>,
<ebiggers@...gle.com>, <xin@...or.com>, <sohil.mehta@...el.com>,
<andrew.cooper3@...rix.com>, <mario.limonciello@....com>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<maciej.wieczor-retman@...el.com>, <eranian@...gle.com>
Subject: Re: [PATCH v12 14/26] x86/resctrl: Add the functionality to assign
MBM events
Hi Babu,
On 4/15/25 7:20 AM, Moger, Babu wrote:
> Hi Reinette,
>
> On 4/11/25 16:04, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 4/3/25 5:18 PM, Babu Moger wrote:
>>> The mbm_cntr_assign mode offers "num_mbm_cntrs" number of counters that
>>> can be assigned to an RMID, event pair and monitor the bandwidth as long
>>> as it is assigned.
>>
>> Above makes it sound as though multiple counters can be assigned to
>> an RMID, event pair.
>>
>
> Yes. Multiple counter-ids can be assigned to RMID, event pair.
oh, are you referring to the assignments of different counters across multiple
domains?
>
>>>
>>> Add the functionality to allocate and assign the counters to RMID, event
>>> pair in the domain.
>>
>> "assign *a* counter to an RMID, event pair"?
>
> Sure.
>
>>
>>>
>>> If all the counters are in use, the kernel will log the error message
>>> "Unable to allocate counter in domain" in /sys/fs/resctrl/info/
>>> last_cmd_status when a new assignment is requested. Exit on the first
>>> failure when assigning counters across all the domains.
>>>
>>> Signed-off-by: Babu Moger <babu.moger@....com>
>>> ---
>>
>> ...
>>
>>> ---
>>> arch/x86/kernel/cpu/resctrl/internal.h | 2 +
>>> arch/x86/kernel/cpu/resctrl/monitor.c | 124 +++++++++++++++++++++++++
>>> 2 files changed, 126 insertions(+)
>>>
>>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>>> index 0b73ec451d2c..1a8ac511241a 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>>> @@ -574,6 +574,8 @@ bool closid_allocated(unsigned int closid);
>>> int resctrl_find_cleanest_closid(void);
>>> void arch_mbm_evt_config_init(struct rdt_hw_mon_domain *hw_dom);
>>> unsigned int mon_event_config_index_get(u32 evtid);
>>> +int resctrl_assign_cntr_event(struct rdt_resource *r, struct rdt_mon_domain *d,
>>> + struct rdtgroup *rdtgrp, enum resctrl_event_id evtid, u32 evt_cfg);
>>
>> This is internal to resctrl fs. Why is it needed to provide both the event id
>> and the event configuration? Event configuration can be determined from event ID?
>
> Yes. It can be done. Then I have to export the functions like
> mbm_get_assign_config() into monitor.c. To avoid that I passed it from
> here which I felt much more cleaner.
>From what I can tell, for example by looking at patch #22, callers of
resctrl_assign_cntr_event() now need to call mbm_get_assign_config()
every time before calling resctrl_assign_cntr_event(). Calling
mbm_get_assign_config() from within resctrl_assign_cntr_event() seems
simpler to me and that may result in mbm_get_assign_config() moving to
monitor.c as an extra benefit.
...
>>> +static int mbm_cntr_get(struct rdt_resource *r, struct rdt_mon_domain *d,
>>> + struct rdtgroup *rdtgrp, enum resctrl_event_id evtid)
>>> +{
>>> + int cntr_id;
>>> +
>>> + for (cntr_id = 0; cntr_id < r->mon.num_mbm_cntrs; cntr_id++) {
>>> + if (d->cntr_cfg[cntr_id].rdtgrp == rdtgrp &&
>>> + d->cntr_cfg[cntr_id].evtid == evtid)
>>> + return cntr_id;
>>> + }
>>> +
>>> + return -ENOENT;
>>> +}
>>> +
>>> +static int mbm_cntr_alloc(struct rdt_resource *r, struct rdt_mon_domain *d,
>>> + struct rdtgroup *rdtgrp, enum resctrl_event_id evtid)
>>> +{
>>> + int cntr_id;
>>> +
>>> + for (cntr_id = 0; cntr_id < r->mon.num_mbm_cntrs; cntr_id++) {
>>> + if (!d->cntr_cfg[cntr_id].rdtgrp) {
>>> + d->cntr_cfg[cntr_id].rdtgrp = rdtgrp;
>>> + d->cntr_cfg[cntr_id].evtid = evtid;
>>> + return cntr_id;
>>> + }
>>> + }
>>> +
>>> + return -ENOSPC;
>>> +}
>>> +
>>> +static void mbm_cntr_free(struct rdt_mon_domain *d, int cntr_id)
>>> +{
>>> + memset(&d->cntr_cfg[cntr_id], 0, sizeof(struct mbm_cntr_cfg));
>>> +}
>>> +
>>> +/*
>>> + * Allocate a fresh counter and configure the event if not assigned already.
>>> + */
>>> +static int resctrl_alloc_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
>>> + struct rdtgroup *rdtgrp, enum resctrl_event_id evtid,
>>> + u32 evt_cfg)
>>
>> Same here, why are both evtid and evt_cfg provided as arguments?
>
> Yes. It can be done. Then I have to export the functions like
> mbm_get_assign_config() into monitor.c. To avoid that I passed it from
> here which I felt much more cleaner.
Maybe even resctrl_assign_cntr_event() does not need to call mbm_get_assign_config()
but only resctrl_alloc_config_cntr() needs to call mbm_get_assign_config(). Doing so
may avoid more burden on callers while reducing parameters needed throughout.
Reinette
Powered by blists - more mailing lists