linux-kernel - Re: [PATCH v6 29/42] x86/resctrl: Move get_config

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: <44cf4fb5-fa3f-42bc-ba16-46645495d669@arm.com>
Date: Fri, 28 Feb 2025 19:51:06 +0000
From: James Morse <james.morse@....com>
To: Reinette Chatre <reinette.chatre@...el.com>, x86@...nel.org,
 linux-kernel@...r.kernel.org
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, H Peter Anvin <hpa@...or.com>,
 Babu Moger <Babu.Moger@....com>, shameerali.kolothum.thodi@...wei.com,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
 Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>,
 Shanker Donthineni <sdonthineni@...dia.com>,
 Shaopeng Tan <tan.shaopeng@...fujitsu.com>, Tony Luck <tony.luck@...el.com>
Subject: Re: [PATCH v6 29/42] x86/resctrl: Move get_config_index() to a header

Hi Reinette,

On 20/02/2025 01:27, Reinette Chatre wrote:
> On 2/7/25 10:18 AM, James Morse wrote:
>> get_config_index() is used by the architecture specific code to map a
>> CLOSID+type pair to an index in the configuration arrays.
>>
>> MPAM needs to do this too to preserve the ABI to user-space, there is
>> no reason to do it differently.
>>
>> Move the helper to a header file.

>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -384,6 +384,21 @@ void resctrl_arch_mon_event_config_write(void *config_info);
>>   */
>>  void resctrl_arch_mon_event_config_read(void *config_info);
>>  
>> +/* For use by arch code to remap resctrl's smaller CDP CLOSID range */
>> +static inline u32 resctrl_get_config_index(u32 closid,
>> +					   enum resctrl_conf_type type)
>> +{
>> +	switch (type) {
>> +	default:
>> +	case CDP_NONE:
>> +		return closid;
>> +	case CDP_CODE:
>> +		return closid * 2 + 1;
>> +	case CDP_DATA:
>> +		return closid * 2;
>> +	}
>> +}
>> +

> Could you please add the motivation for the use of an inline function?

Putting this in the header file means it isn't duplicated, so its behaviour can't become
different. If its in a header file, it has to be marked inline otherwise every C file that
includes it gets a copy that probably isn't used, and upsets the linker.

Calling from the arch code into the filesystem prevents the arch code from being
standalone. This is a useful direction of travel because it allows fs/resctrl to one
day become a module

Today, the compiler is choosing to inline this:
| x86_64-linux-objdump -d ctrlmondata.o | grep resctrl_get_config_index | wc -l
| 0

This kind of arithmetic for an array lookup is the kind of thing its good to give the
compiler full visibility of as its good fodder for constant folding.

For so few call sites, I don't think this is really worth thinking about.
Forcing this call out of line makes the kernel text bigger, but only by 32 bytes.

I've expanded the last paragraph of the commit message to read:
| Move the helper to a header file to allow all architectures that either
| use or emulate CDP to use the same pattern of CLOSID values. Moving
| this to a header file means it must be marked inline, which matches
| the existing compiler choice for this static function.

Thanks,

James