Re: [PATCH v4 14/19] x86/resctrl: Add the interface to assign ABMC counter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Reinette,

On 6/20/2024 5:50 PM, Reinette Chatre wrote:
Hi Babu,

On 6/19/24 3:38 PM, Moger, Babu wrote:
On 6/13/2024 8:48 PM, Reinette Chatre wrote:
On 5/24/24 5:23 AM, Babu Moger wrote:

+int resctrl_arch_assign(struct rdt_domain *d, u32 evtid, u32 rmid,
+            u32 ctr_id, u32 closid, bool enable)
+{
+    struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
+    union l3_qos_abmc_cfg abmc_cfg = { 0 };
+    struct arch_mbm_state *arch_mbm;
+
+    abmc_cfg.split.cfg_en = 1;
+    abmc_cfg.split.ctr_en = enable ? 1 : 0;
+    abmc_cfg.split.ctr_id = ctr_id;
+    abmc_cfg.split.bw_src = rmid;
+
+    /*
+     * Read the event configuration from the domain and pass it as
+     * bw_type.
+     */
+    if (evtid == QOS_L3_MBM_TOTAL_EVENT_ID) {
+        abmc_cfg.split.bw_type = hw_dom->mbm_total_cfg;
+        arch_mbm = &hw_dom->arch_mbm_total[rmid];
+    } else {
+        abmc_cfg.split.bw_type = hw_dom->mbm_local_cfg;
+        arch_mbm = &hw_dom->arch_mbm_local[rmid];
+    }
+
+    smp_call_function_any(&d->cpu_mask, rdtgroup_abmc_cfg, &abmc_cfg, 1);
+
+    /* Reset the internal counters */

"internal counters"? This needs a definition ... but since this is not
a new data structure the comment can be more specific about what is done
and why.

How about?
/* Reset internal mbm event counters. */

I think it is potentially confusing to introduce a new concept of
"internal counters". This is just the architectural state helping to
detect overflows. This commit has no mention of this but I assume that
the re-configuration of the counter will reset it, which explains
why it is required to reset the state maintained by software.
A similar comment to what can be found in mbm_config_write_domain()
would be useful. This does not have to be so elaborate, maybe just something
like (please feel free to improve):

     Configuring a hardware counter resets it. Reset architectural
     state to match.


Sure. Thanks
- Babu Moger




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux