linux-kernel - Re: [PATCH v1 22/31] x86/resctrl: Make resctrl_arch_pseudo_lock

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]

Date: Thu, 11 Apr 2024 10:40:37 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Dave Martin <Dave.Martin@....com>
CC: James Morse <james.morse@....com>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>, Fenghua Yu <fenghua.yu@...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>, "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>
Subject: Re: [PATCH v1 22/31] x86/resctrl: Make resctrl_arch_pseudo_lock_fn()
 take a plr

Hi Dave,

On 4/11/2024 7:38 AM, Dave Martin wrote:
> On Mon, Apr 08, 2024 at 08:24:35PM -0700, Reinette Chatre wrote:
>> Hi James,
>>
>> On 3/21/2024 9:50 AM, James Morse wrote:
>>> resctrl_arch_pseudo_lock_fn() has architecture specific behaviour,
>>> and takes a struct rdtgroup as an argument.
>>>
>>> After the filesystem code moves to /fs/, the definition of struct
>>> rdtgroup will not be available to the architecture code.
>>>
>>> The only reason resctrl_arch_pseudo_lock_fn() wants the rdtgroup is
>>> for the CLOSID. Embed that in the pseudo_lock_region as a hw_closid,
>>
>> Above creates expectation that the new member will be named hw_closid,
>> but that is not what the code does.
> 
> I'll flag this for review, but I'd guess that this can probably just be
> "closid".  I'll make a note to consider what needs to change to make
> things consistent between the patch and commit message.
> 
> James might have had other ideas, connected with the remapping done for
> CDP emulation causing the resctrl closid being different from the actual
> value used by the hardware, at least for MPAM (see my response on
> patch 24).  I don't fully understand how this works for x86 though.
> 
> So long as functionality is unaffected, and this patch is introducing no
> new confusion that wasn't there beforehand, the exact name may not
> matter too much(?)

closid sounds good. It may be a good match for what is expected to be in
general/fs code. 

> 
> Did you have other concerns here?

No.

Reinette