| 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: <2ef3c008-81db-a3b1-19bb-6f60c6aa617c@amd.com>
Date: Fri, 18 Oct 2024 10:50:59 -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,
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 v8 23/25] x86/resctrl: Update assignments on event
configuration changes
Hi Reinette,
On 10/15/2024 10:40 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 10/9/24 10:39 AM, 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>
>> ---
> ...
>
>> ---
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 49 ++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index f890d294e002..cf2e0ad0e4f4 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -1669,6 +1669,7 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
>> }
>>
>> struct mon_config_info {
>> + struct rdt_resource *r;
>> struct rdt_mon_domain *d;
>> u32 evtid;
>> u32 mon_config;
>> @@ -1694,11 +1695,46 @@ u32 resctrl_arch_mon_event_config_get(struct rdt_mon_domain *d,
>> return INVALID_CONFIG_VALUE;
>> }
>>
>> +static void mbm_cntr_event_update(int cntr_id, unsigned int index, u32 val)
>> +{
>> + union l3_qos_abmc_cfg abmc_cfg = { 0 };
>> + struct rdtgroup *prgrp, *crgrp;
>> + int update = 0;
>> +
>> + /* Check if the cntr_id is associated to the event type updated */
>> + list_for_each_entry(prgrp, &rdt_all_groups, rdtgroup_list) {
>> + if (prgrp->mon.cntr_id[index] == cntr_id) {
>> + abmc_cfg.split.bw_src = prgrp->mon.rmid;
>> + update = 1;
>> + goto out_update;
>> + }
>> + list_for_each_entry(crgrp, &prgrp->mon.crdtgrp_list, mon.crdtgrp_list) {
>> + if (crgrp->mon.cntr_id[index] == cntr_id) {
>> + abmc_cfg.split.bw_src = crgrp->mon.rmid;
>> + update = 1;
>> + goto out_update;
>> + }
>> + }
>
> This code looks like it is better suited for resctrl fs. Note that
> after the arch fs split struct rdtgroup is private to resctrl fs.
ok
>
>> + }
>> +
>> +out_update:
>> + if (update) {
>> + abmc_cfg.split.cfg_en = 1;
>> + abmc_cfg.split.cntr_en = 1;
>> + abmc_cfg.split.cntr_id = cntr_id;
>> + abmc_cfg.split.bw_type = val;
>> + wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, abmc_cfg.full);
>> + }
>> +}
>> +
>> void resctrl_arch_mon_event_config_set(void *info)
>> {
>> struct mon_config_info *mon_info = info;
>> + struct rdt_mon_domain *d = mon_info->d;
>> + struct rdt_resource *r = mon_info->r;
>> struct rdt_hw_mon_domain *hw_dom;
>> unsigned int index;
>> + int cntr_id;
>>
>> index = mon_event_config_index_get(mon_info->evtid);
>> if (index == INVALID_CONFIG_INDEX)
>> @@ -1718,6 +1754,18 @@ void resctrl_arch_mon_event_config_set(void *info)
>> hw_dom->mbm_local_cfg = mon_info->mon_config;
>> break;
>> }
>> +
>> + /*
>> + * Update the assignment if the domain has the cntr_id's assigned
>> + * to event type updated.
>> + */
>> + if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
>> + for (cntr_id = 0; cntr_id < r->mon.num_mbm_cntrs; cntr_id++) {
>> + if (test_bit(cntr_id, d->mbm_cntr_map))
>> + mbm_cntr_event_update(cntr_id, index,
>> + mon_info->mon_config);
>> + }
>> + }
>> }
>>
>> /**
>> @@ -1805,6 +1853,7 @@ static void mbm_config_write_domain(struct rdt_resource *r,
>> mon_info.d = d;
>> mon_info.evtid = evtid;
>> mon_info.mon_config = val;
>> + mon_info.r = r;
>>
>> /*
>> * Update MSR_IA32_EVT_CFG_BASE MSR on one of the CPUs in the
>
> If I understand correctly, mbm_config_write_domain() paints itself into a corner by
> calling arch code via IPI. As seen above it needs resctrl help to get all the information
> and doing so from the arch helper is not appropriate.
>
> How about calling a resctrl fs helper via IPI instead? For example:
>
> resctrl_mon_event_config_set() {
>
> resctrl_arch_mon_event_config_set();
>
> if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
> for (cntr_id = 0; cntr_id < r->mon.num_mbm_cntrs; cntr_id++) {
> if (test_bit(cntr_id, d->mbm_cntr_map)) {
> /* determine rmid */
> resctrl_arch_config_cntr()
The call resctrl_arch_config_cntr() requires both RMID and CLOSID. So,
we will have to find the rdtgroup here (not just RMID, we need CLOSID also).
Yea. I think it can be done. Let me try.
> }
> }
> }
> }
>
>
> mbm_config_write_domain() {
>
> ...
> smp_call_function_any(&d->hdr.cpu_mask, resctrl_mon_event_config_set, ...)
> ...
>
> }
>
> By removing reset of arch state from resctrl_arch_config_cntr() this works well with the
> resctrl_arch_reset_rmid_all() that is done from mbm_config_write_domain().
> Even though resctrl_arch_config_cntr() contains a smp_call_function_any() it should
> already be running on CPU in mask and thus should just run on local CPU.
Ok.
--
- Babu Moger
Powered by blists - more mailing lists