Hi Reinette, Already responded to this but i don't see my response in archives yet. On 10/3/22 10:36, Reinette Chatre wrote: > Hi Babu, > > On 10/3/2022 7:28 AM, Moger, Babu wrote: >> Hi Reinette, >> >> On 9/29/22 17:10, Reinette Chatre wrote: >>> Hi Babu, >>> >>> In subject: resctrl_ui.rst -> resctrl.rst >>> >>> On 9/27/2022 1:27 PM, Babu Moger wrote: >>>> Update the documentation for the new features: >>>> 1. Slow Memory Bandwidth allocation (SMBA). >>>> With this feature, the QOS enforcement policies can be applied >>>> to the external slow memory connected to the host. QOS enforcement >>>> is accomplished by assigning a Class Of Service (COS) to a processor >>>> and specifying allocations or limits for that COS for each resource >>>> to be allocated. >>>> >>>> 2. Bandwidth Monitoring Event Configuration (BMEC). >>>> The bandwidth monitoring events mbm_total_bytes and mbm_local_bytes >>>> are set to count all the total and local reads/writes respectively. >>>> With the introduction of slow memory, the two counters are not >>>> enough to count all the different types are memory events. With the >>> types are memory events -> types of memory events? >> Ok Sure >>>> feature BMEC, the users have the option to configure mbm_total_bytes >>>> and mbm_local_bytes to count the specific type of events. >>>> >>>> Also add configuration instructions with examples. >>>> >>>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx> >>>> --- >>> ... >>> >>>> + >>>> +"mbm_total_config", "mbm_local_config": >>>> + These files contain the current event configuration for the events >>>> + mbm_total_bytes and mbm_local_bytes, respectively, when the >>>> + Bandwidth Monitoring Event Configuration (BMEC) feature is supported. >>>> + The event configuration settings are domain specific. Changing the >>>> + configuration on one CPU in a domain would affect the whole domain. >>> This contradicts the implementation done in this series where the >>> configuration is changed on every CPU in the domain. >> How about this? >> >> The event configuration settings are domain specific and will affect all the CPUs in the domain. > There remains a disconnect between this and the implementation that writes the > configuration to every CPU. > > You could make this change to the documentation but then the > implementation needs more than "Update MSR_IA32_EVT_CFG_BASE MSR on all > the CPUs in cpu_mask" - that comment needs to highlight that the > implementation does not follow the architecture and scope rules nor how > configuration changes are made in the rest of the driver and why. Previously [1] > you indicated that this is based on guidance from hardware team so perhaps you > could document it as a hardware quirk related to this feature? At the minimum > it should acknowledge the disconnect. ok. I could document this in the code patch 9([PATCH v5 09/12] x86/resctrl: Add sysfs interface to write mbm_total_bytes event configuration. Something like this. /* + * Update MSR_IA32_EVT_CFG_BASE MSR on all the CPUs in cpu_mask. + * The MSR MSR_IA32_EVT_CFG_BASE is domain specific. Writing the + * MSR on one CPU will affect all the CPUs in the domain. + * However, the hardware team recommends to update the MSR on + * all the CPU threads. It is not clear in the document yet. * * Doc will be updated in the next revision. + */ + on_each_cpu_mask(cpu_mask, mon_event_config_write, &mon_info, 1); + Thanks Babu
