| 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: <f77737ac-d3f6-3e4b-3565-564f79c86ca8@amd.com>
Date: Fri, 27 Sep 2024 11:22:25 -0500
From: "Moger, Babu" <bmoger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>,
Babu Moger <babu.moger@....com>, corbet@....net, fenghua.yu@...el.com,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, paulmck@...nel.org, rdunlap@...radead.org,
tj@...nel.org, peterz@...radead.org, yanjiewtw@...il.com,
kim.phillips@....com, lukas.bulwahn@...il.com, seanjc@...gle.com,
jmattson@...gle.com, leitao@...ian.org, jpoimboe@...nel.org,
rick.p.edgecombe@...el.com, kirill.shutemov@...ux.intel.com,
jithu.joseph@...el.com, kai.huang@...el.com, kan.liang@...ux.intel.com,
daniel.sneddon@...ux.intel.com, pbonzini@...hat.com, sandipan.das@....com,
ilpo.jarvinen@...ux.intel.com, peternewman@...gle.com,
maciej.wieczor-retman@...el.com, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, eranian@...gle.com, james.morse@....com
Subject: Re: [PATCH v7 22/24] x86/resctrl: Update assignments on event
configuration changes
Hi Reinitte,
On 9/19/2024 12:45 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 9/4/24 3:21 PM, Babu Moger wrote:
>> Users can modify the configuration of assignable events. Whenever the
>> event configuration is updated, MBM assignments must be revised across
>> all monitor groups within the impacted domains.
>>
>> Signed-off-by: Babu Moger <babu.moger@....com>
>> ---
>> v7: New patch to update the assignments. Missed it earlier.
>> ---
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 53 ++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 1054583bef9d..0b1490d71e77 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -871,6 +871,15 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of,
>> */
>> #define MBM_EVENT_ARRAY_INDEX(_event) ((_event) - 2)
>>
>> +static bool resctrl_mbm_event_assigned(struct rdtgroup *rdtg,
>> + struct rdt_mon_domain *d, u32 evtid)
>> +{
>> + int index = MBM_EVENT_ARRAY_INDEX(evtid);
>> + int cntr_id = rdtg->mon.cntr_id[index];
>> +
>> + return (cntr_id != MON_CNTR_UNSET && test_bit(cntr_id, d->mbm_cntr_map));
>
> (Please check spaces and paren use.)
Sure.
>
>> +}
>> +
>> static int rdtgroup_mbm_assign_mode_show(struct kernfs_open_file *of,
>> struct seq_file *s, void *v)
>> {
>> @@ -1793,12 +1802,48 @@ static int mbm_local_bytes_config_show(struct kernfs_open_file *of,
>> return 0;
>> }
>>
>> +static int resctrl_mbm_event_update_assign(struct rdt_resource *r,
>> + struct rdt_mon_domain *d, u32 evtid)
>> +{
>> + struct rdt_mon_domain *dom;
>> + struct rdtgroup *rdtg;
>> + int ret = 0;
>> +
>> + if (!resctrl_arch_mbm_cntr_assign_enabled(r))
>> + return ret;
>> +
>> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
>> + struct rdtgroup *crg;
>> +
>> + list_for_each_entry(dom, &r->mon_domains, hdr.list) {
>> + if (d == dom && resctrl_mbm_event_assigned(rdtg, dom, evtid)) {
>> + ret = rdtgroup_assign_cntr(r, rdtg, dom, evtid);
>> + if (ret)
>> + goto out_done;
>> + }
>> + }
>> +
>> + list_for_each_entry(crg, &rdtg->mon.crdtgrp_list, mon.crdtgrp_list) {
>> + list_for_each_entry(dom, &r->mon_domains, hdr.list) {
>> + if (d == dom && resctrl_mbm_event_assigned(crg, dom, evtid)) {
>> + ret = rdtgroup_assign_cntr(r, crg, dom, evtid);
>> + if (ret)
>> + goto out_done;
>> + }
>> + }
>> + }
>> + }
>> +
>> +out_done:
>> + return ret;
>> +}
>>
>> static void mbm_config_write_domain(struct rdt_resource *r,
>> struct rdt_mon_domain *d, u32 evtid, u32 val)
>> {
>> struct mon_config_info mon_info = {0};
>> u32 config_val;
>> + int ret;
>>
>> /*
>> * Check the current config value first. If both are the same then
>> @@ -1822,6 +1867,14 @@ static void mbm_config_write_domain(struct rdt_resource *r,
>> resctrl_arch_event_config_set,
>> &mon_info, 1);
>>
>> + /*
>> + * Counter assignments needs to be updated to match the event
>> + * configuration.
>> + */
>> + ret = resctrl_mbm_event_update_assign(r, d, evtid);
>> + if (ret)
>> + rdt_last_cmd_puts("Assign failed, event will be Unavailable\n");
>> +
>
> This does not look right. This function _just_ returned from an IPI on appropriate CPU and then
> starts flow to do _another_ IPI to run code that could have just been run during previous IPI.
> The whole flow to call rdgroup_assign_cntr() also seems like an obfuscated way to call resctrl_arch_assign_cntr()
> to just reconfigure the counter (not actually assign it).
> Could it perhaps call some resctrl fs code via single IPI that in turn calls the appropriate arch code to set the new
> mon event config and re-configures the counter?
>
I think we can simplify this. We dont have to go thru all the rdtgroups
to figure out if the counter is assigned or not.
I can move the code inside mon_config_write() after the call
mbm_config_write_domain().
Using the domain bitmap we can figure out which of the counters are
assigned in the domain. I can use the hardware help to update the
assignment for each counter. This has to be done via IPI.
Something like this.
static void rdtgroup_abmc_dom_cfg(void *info)
{
union l3_qos_abmc_cfg *abmc_cfg = info;
u32 val = abmc_cfg->bw_type;
/* Get the counter configuration */
wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *abmc_cfg);
rdmsrl(MGR_IA32_L3_QOS_ABMC_DSC, *abmc_cfg);
/* update the counter configuration */
if (abmc_cfg->bw_type != val) {
abmc_cfg->bw_type = val;
abmc_cfg.split.cfg_en = 1;
wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *abmc_cfg);
}
}
--
- Babu Moger
Powered by blists - more mailing lists