linux-kernel - Re: [PATCH v11 00/23] x86/resctrl : Support AMD Assignable Bandwidth Monitoring Counters (ABMC)

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: <be6d23c8-3e93-4bdc-8b33-d3af7df7cc94@amd.com>
Date: Thu, 13 Mar 2025 15:13:32 -0500
From: "Moger, Babu" <babu.moger@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, "Moger, Babu"
 <bmoger@....com>, "Luck, Tony" <tony.luck@...el.com>
Cc: Peter Newman <peternewman@...gle.com>, Dave Martin <Dave.Martin@....com>,
 corbet@....net, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 paulmck@...nel.org, akpm@...ux-foundation.org, thuth@...hat.com,
 rostedt@...dmis.org, xiongwei.song@...driver.com,
 pawan.kumar.gupta@...ux.intel.com, daniel.sneddon@...ux.intel.com,
 jpoimboe@...nel.org, perry.yuan@....com, sandipan.das@....com,
 kai.huang@...el.com, xiaoyao.li@...el.com, seanjc@...gle.com,
 xin3.li@...el.com, andrew.cooper3@...rix.com, ebiggers@...gle.com,
 mario.limonciello@....com, james.morse@....com, tan.shaopeng@...itsu.com,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 maciej.wieczor-retman@...el.com, eranian@...gle.com
Subject: Re: [PATCH v11 00/23] x86/resctrl : Support AMD Assignable Bandwidth
 Monitoring Counters (ABMC)

Hi Reinette,

On 3/13/25 11:08, Reinette Chatre wrote:
> Hi Babu,
> 
> On 3/12/25 11:14 AM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 3/12/25 12:14, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 3/12/25 9:03 AM, Moger, Babu wrote:
>>>> Hi Reinette,
>>>>
>>>> On 3/12/25 10:07, Reinette Chatre wrote:
>>>>> Hi Babu,
>>>>>
..

>>>>>> We can add the mkdir support later. That way we can provide basic ABMC
>>>>>> support without too much code complexity with mkdir support.
>>>>>
>>>>> This is not clear to me how you envision the "first phase". Is it what you
>>>>> proposed above, for example:
>>>>>       #echo "LclFill, LclNTWr, RmtFill" >
>>>>>          /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes
>>>>>
>>>>> In above the counter configuration name is a file. 
>>>>
>>>> Yes. That is correct.
>>>>
>>>> There will be two configuration files by default when resctrl is mounted
>>>> when ABMC is enabled.
>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes
>>>> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_local_bytes
>>>>
>>>>>
>>>>> How could mkdir support be added to this later if there are already files present?
>>>>
>>>> We already have these directories when resctrl is mounted.
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_total_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_00/mbm_local_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_total_bytes
>>>> /sys/fs/resctrl/test/mon_data/mon_L3_01/mbm_local_bytes
>>>>
>>>> We dont need "mkdir" support for default  configurations.
>>>
>>> I was referring to the "mkdir" support for additional configurations that
>>> I understood you are thinking about adding later. For example,
>>> (copied from Peter's message
>>> https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@mail.gmail.com/):
>>>
>>>
>>>  # mkdir info/L3_MON/counter_configs/mbm_local_bytes
>>>  # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>>  # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>>  # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>>  # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter
>>> LclFill
>>> LclNTWr
>>> LclSlowFill
>>>
>>> Any "later" work needs to be backward compatible with the first phase.
>>
>> Actually, we dont need extra file "event_filter".
>> This was discussed here.
>> https://lore.kernel.org/lkml/CALPaoChLL8p49eANYgQ0dJiFs7G=223fGae+LJyx3DwEhNeR8A@mail.gmail.com/
> 
> I undestand from that exchange that it is possible to read/write from
> an *existing* kernfs file but it is not obvious to me how that file is
> planned to be created.

My bad.. I misspoke here. We need "event_filter" file under each
configuration.


> 
> My understanding of the motivation behind support for "mkdir" is to enable
> user space to create custom counter configurations.
> 

That is correct.

> I understand that ABMC support aims to start with existing mbm_total_bytes/mbm_local_bytes
> configurations but I believe the consensus is that custom configurations need
> to be supported in the future.
> If resctrl starts with support where counter configuration as
> managed with a *file*, for example:
> /sys/fs/resctrl/info/L3_MON/counter_configs/mbm_total_bytes
> how will user space create future custom configurations?
> As I understand that is only possible with mkdir.
> 
>>
>> # echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes
>> # echo LclNTWr > info/L3_MON/counter_configs/mbm_local_bytes
>> # echo LclSlowFill > info/L3_MON/counter_configs/mbm_local_bytes
>> # cat info/L3_MON/counter_configs/mbm_local_bytes
>>  LclFill
>>  LclNTWr
>>  LclSlowFill
>>
>> In the future, we can add mkdir support.
>>
>> # mkdir info/L3_MON/counter_configs/mbm_read_only
> 
> This is exactly my concern. resctrl should not start with a user space where
> a counter configuration is a file (mbm_local_bytes/mbm_total_bytes) and then
> switch user space interface to have counter configuration be done with
> directories.
> 
>> # echo LclFill > info/L3_MON/counter_configs/mbm_read_only
>> # cat info/L3_MON/counter_configs/mbm_read_only
>>   LclFill
> 
> ... wait ... user space writes to the directory?
> 

My bad. This is wrong. Let me rewrite the steps below.

> 
> 
>>
>> #echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive
>>
>> Which would result in the creation of test/mon_data/mon_L3_*/mbm_read_only
>>
>> So, there is not breakage of backword compatibility.
> 
> The way I understand it I am seeing many incompatibilities. Perhaps I am missing
> something. Could you please provide detailed steps of how first phase and
> second phase would look?

No. You didn't miss anything. I misspoke on few steps.

Here are the steps. Just copying steps from Peters proposal.
https://lore.kernel.org/lkml/CALPaoCiii0vXOF06mfV=kVLBzhfNo0SFqt4kQGwGSGVUqvr2Dg@mail.gmail.com/


1. Mount the resctrl
   mount -t resctrl resctrl /sys/fs/resctrl

2. When ABMC is supported two default configurations will be created.

  a. info/L3_MON/counter_configs/mbm_total_bytes/event_filter
  b. info/L3_MON/counter_configs/mbm_local_bytes/event_filter

  These files will be populated with default total and local events
  # cat info/L3_MON/counter_configs/mbm_total_bytes/event_filter
    VictimBW
    RmtSlowFill
    RmtNTWr
    RmtFill
    LclFill
    LclNTWr
    LclSlowFill

  # cat info/L3_MON/counter_configs/mbm_local_bytes/event_filter
   LclFill,
   LclNTWr
   LclSlowFill

3. Users will have options to update the event configuration.
   echo LclFill > info/L3_MON/counter_configs/mbm_local_bytes/event_filter

4. As usual the events can be read from the mon_data directories.
   #mkdir /sys/fs/resctrl/test
   #cd   /sys/fs/resctr/test
   #cat  test/mon_data/mon_data/mon_L3_00/mbm_tota_bytes
   101010
   #cat   test/mon_data/mon_data/mon_L3_00/mbm_local_bytes
   32323

5. There will be 3 files created in each group's mon_data directory when
ABMC is supported.

   a. test/mon_data/mon_L3_00/assign_exclusive
   b. test/mon_data/mon_L3_00/assign_shared
   c. test/mon_data/mon_L3_00/unassign


6. Events can be assigned/unassigned by these commands

 # echo mbm_total_bytes > test/mon_data/mon_L3_00/assign_exclusive
 # echo mbm_local_bytes > test/mon_data/mon_L3_01/assign_exclusive
 # echo mbm_local_bytes > test/mon_data/mon_L3_01/unassign


Note:
I feel 3 files are excessive here. We can probably achieve everything in
just one file.

Not sure about mbm_assign_control interface as there are concerns with
group listing holding the lock for long.

-----------------------------------------------------------------------
Second phase, we can add support for "mkdir"

1. mkdir info/L3_MON/counter_configs/mbm_read_only

2. mkdir option will create "event_filter" file.
   info/L3_MON/counter_configs/mbm_read_only/event_filter

3. Users can modify event configuration.
   echo LclFill > info/L3_MON/counter_configs/mbm_read_only/event_filter

4. Users can assign the events

  echo mbm_read_only > test/mon_data/mon_L3_00/assign_exclusive

5. Events can be read in

   test/mon_data/mon_data/mon_L3_00/mbm_read_only

-- 
Thanks
Babu Moger