linux-kernel - Re: [PATCH v1 13/20] x86/recstrl: Allow per-rmid arch private storage to be reset

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: <aef96d1b-0ffd-0f51-bade-9e7b7684d26e@arm.com>
Date:   Fri, 1 Oct 2021 17:01:22 +0100
From:   James Morse <james.morse@....com>
To:     "tan.shaopeng@...itsu.com" <tan.shaopeng@...itsu.com>,
        "'x86@...nel.org'" <x86@...nel.org>,
        "'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
Cc:     'Fenghua Yu' <fenghua.yu@...el.com>,
        'Reinette Chatre' <reinette.chatre@...el.com>,
        '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'" 
        <shameerali.kolothum.thodi@...wei.com>,
        'Jamie Iles' <jamie@...iainc.com>,
        'D Scott Phillips OS' <scott@...amperecomputing.com>,
        "'lcherian@...vell.com'" <lcherian@...vell.com>,
        "'bobo.shaobowang@...wei.com'" <bobo.shaobowang@...wei.com>
Subject: Re: [PATCH v1 13/20] x86/recstrl: Allow per-rmid arch private storage
 to be reset

Hi Shaopeng Tan,

On 24/09/2021 07:34, tan.shaopeng@...itsu.com wrote:
>> To abstract the rmid counters into a helper that returns the number of bytes
>> counted, architecture specific per-rmid state is needed.
>>
>> It needs to be possible to reset this hidden state, as the values may outlive the
>> life of an rmid, or the mount time of the filesystem.
>>
>> mon_event_read() is called with first = true when an rmid is first allocated in
>> mkdir_mondata_subdir(). Add resctrl_arch_reset_rmid() and call it from
>> __mon_event_count()'s rr->first check.


>> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c
>> b/arch/x86/kernel/cpu/resctrl/monitor.c
>> index af60e154f0ed..3b8b29470a5c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
>> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
>> @@ -137,7 +137,34 @@ static inline struct rmid_entry *__rmid_entry(u32 rmid)
>>  	return entry;
>>  }
>>
>> -static u64 __rmid_read(u32 rmid, u32 eventid)
>> +static struct arch_mbm_state *get_arch_mbm_state(struct rdt_hw_domain
>> *hw_dom,
>> +						 u32 rmid,
>> +						 enum resctrl_event_id
>> eventid)
>> +{
>> +	switch (eventid) {
>> +	case QOS_L3_OCCUP_EVENT_ID:
>> +		return NULL;
>> +	case QOS_L3_MBM_TOTAL_EVENT_ID:
>> +		return &hw_dom->arch_mbm_total[rmid];
>> +	case QOS_L3_MBM_LOCAL_EVENT_ID:
>> +		return &hw_dom->arch_mbm_local[rmid];
>> +	}
>> +
> 
> Since it is unexpected to come here,
> it might be better to add WARN_ON.

Sure. (it'll be the 'once' version to avoid spamming the console)
I'm relying on the compiler generating a warning a built-time here if a new enum entry is
ever added, but it can't hurt to warning if someone passes something totally crazy to it.


> In addition, I have tested these patches on Intel(R) Xeon(R) Gold 6254 CPU with
> resctrl selftest. It is no problem.

Good to know, thanks!


Thanks,

James