| 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: <f082bba6-6efa-5d3c-5670-d4a24d4fc5da@amd.com> Date: Fri, 23 Aug 2024 17:05:52 -0500 From: "Moger, Babu" <bmoger@....com> To: Reinette Chatre <reinette.chatre@...el.com>, 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 v6 16/22] x86/resctrl: Add the interface to unassign a MBM counter Hi Reinette, On 8/23/2024 3:18 PM, Reinette Chatre wrote: > Hi Babu, > > On 8/21/24 9:01 AM, Moger, Babu wrote: >> Hi Reinette, >> >> On 8/16/24 16:41, Reinette Chatre wrote: >>> Hi Babu, >>> >>> On 8/6/24 3:00 PM, Babu Moger wrote: >>>> The ABMC feature provides an option to the user to assign a hardware >>> >>> This is about resctrl fs so "The ABMC feature" -> "mbm_cntr_assign mode" >> >> Sure. >> >>> (please check whole series). >> >> Sure. >> >>> >>>> counter to an RMID and monitor the bandwidth as long as it is assigned. >>>> The assigned RMID will be tracked by the hardware until the user >>>> unassigns >>>> it manually. >>>> >>>> Hardware provides only limited number of counters. If the system >>>> runs out >>>> of assignable counters, kernel will display an error when a new >>>> assignment >>>> is requested. Users need to unassign a already assigned counter to make >>>> space for new assignment. >>>> >>>> Provide the interface to unassign the counter ids from the group. >>>> Free the >>>> counter if it is not assigned in any of the domains. >>>> >>>> The feature details are documented in the APM listed below [1]. >>>> [1] AMD64 Architecture Programmer's Manual Volume 2: System Programming >>>> Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable >>>> Bandwidth >>>> Monitoring (ABMC). >>>> >>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 >>>> Signed-off-by: Babu Moger <babu.moger@....com> >>>> --- >>>> v6: Removed mbm_cntr_free from this patch. >>>> Added counter test in all the domains and free if it is not >>>> assigned to >>>> any domains. >>>> >>>> v5: Few name changes to match cntr_id. >>>> Changed the function names to >>>> rdtgroup_unassign_cntr >>>> More comments on commit log. >>>> >>>> v4: Added domain specific unassign feature. >>>> Few name changes. >>>> >>>> v3: Removed the static from the prototype of rdtgroup_unassign_abmc. >>>> The function is not called directly from user anymore. These >>>> changes are related to global assignment interface. >>>> >>>> v2: No changes. >>>> --- >>>> arch/x86/kernel/cpu/resctrl/internal.h | 2 + >>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 52 >>>> ++++++++++++++++++++++++++ >>>> 2 files changed, 54 insertions(+) >>>> >>>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h >>>> b/arch/x86/kernel/cpu/resctrl/internal.h >>>> index 4e8109dee174..cc832955b787 100644 >>>> --- a/arch/x86/kernel/cpu/resctrl/internal.h >>>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >>>> @@ -689,6 +689,8 @@ int resctrl_arch_assign_cntr(struct rdt_mon_domain >>>> *d, enum resctrl_event_id evt >>>> u32 rmid, u32 cntr_id, u32 closid, bool assign); >>>> int rdtgroup_assign_cntr(struct rdtgroup *rdtgrp, enum >>>> resctrl_event_id evtid); >>>> int rdtgroup_alloc_cntr(struct rdtgroup *rdtgrp, int index); >>>> +int rdtgroup_unassign_cntr(struct rdtgroup *rdtgrp, enum >>>> resctrl_event_id evtid); >>>> +void rdtgroup_free_cntr(struct rdt_resource *r, struct rdtgroup >>>> *rdtgrp, int index); >>>> 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 1ee91a7293a8..0c2215dbd497 100644 >>>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>>> @@ -1961,6 +1961,58 @@ int rdtgroup_assign_cntr(struct rdtgroup >>>> *rdtgrp, >>>> enum resctrl_event_id evtid) >>>> return 0; >>>> } >>>> +static int rdtgroup_mbm_cntr_test(struct rdt_resource *r, u32 >>>> cntr_id) >>> >>> Could "test" be replaced with something more specific about what is >>> tested? >>> for example, "rdtgroup_mbm_cntr_is_assigned()" or something better? The >> >> Yes. We can do that. >> >>> function >>> looks like a good candidate for returning a bool. >> >> Sure. >>> >>> Is this function needed though? (more below) >> >> Yes. It is required. It is called from two places >> (rdtgroup_unassign_update and rdtgroup_unassign_cntr). >> >> We can open code in rdtgroup_unassign_cntr. But we can't do that in >> rdtgroup_unassign_update. But, I will check again for sure. > > Similar to rdtgroup_assign_cntr() and rdtgroup_assign_update() discussed > in previous patch, it also looks like rdtgroup_unassign_cntr() and > rdtgroup_unassign_update() can be merged. Yes. We can do that. Thanks -- - Babu Moger
Powered by blists - more mailing lists