linux-kernel - Re: [PATCH v5 12/12] Documentation/x86: Update resctrl

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d60a73cd-3228-84bc-dd49-52ccb9fd0015@amd.com>
Date:   Tue, 4 Oct 2022 09:00:40 -0500
From:   "Moger, Babu" <babu.moger@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de
Cc:     fenghua.yu@...el.com, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, paulmck@...nel.org, akpm@...ux-foundation.org,
        quic_neeraju@...cinc.com, rdunlap@...radead.org,
        damien.lemoal@...nsource.wdc.com, songmuchun@...edance.com,
        peterz@...radead.org, jpoimboe@...nel.org, pbonzini@...hat.com,
        chang.seok.bae@...el.com, pawan.kumar.gupta@...ux.intel.com,
        jmattson@...gle.com, daniel.sneddon@...ux.intel.com,
        sandipan.das@....com, tony.luck@...el.com, james.morse@....com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        bagasdotme@...il.com, eranian@...gle.com
Subject: Re: [PATCH v5 12/12] Documentation/x86: Update resctrl_ui.rst for new
 features

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@....com>
>>>> ---
>>> ...
>>>
>>>> +
>>>> +"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