linux-kernel - Re: [PATCH v2 02/24] x86/resctrl: Split struct rdt

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 8 Apr 2021 13:13:01 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     James Morse <james.morse@....com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Cc:     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,
        Jamie Iles <jamie@...iainc.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>
Subject: Re: [PATCH v2 02/24] x86/resctrl: Split struct rdt_domain

Hi James,

On 4/8/2021 10:20 AM, James Morse wrote:
> On 31/03/2021 22:36, Reinette Chatre wrote:
>> On 3/12/2021 9:58 AM, James Morse wrote:
>>> resctrl is the defacto Linux ABI for SoC resource partitioning features.
>>> To support it on another architecture, it needs to be abstracted from
>>> the features provided by Intel RDT and AMD PQoS, and moved to /fs/.
>>>
>>> Split struct rdt_domain up too. Move everything that that is particular
>>
>> s/that that/that/
>>
>>> to resctrl into a new header file. resctrl code paths touching a 'hw'
>>> struct indicates where an abstraction is needed.
>>
>> Similar to previous patch it would help to explain how this split was chosen. For example,
>> why are the CPUs to which a resource is associated not considered a hardware property?
> 
> Similarly, because the meaning of those CPUs doesn't differ or change between architectures.

Got it. This seems to be a repeat of the discussion about patch 1. 
Please note that the description of rdt_hw_domain in this patch reads 
"hw attributes of a group of CPUs that share a resource". This can be 
understood to mean that the struct contains attributes discovered from 
hardware.


> I've expanded the middle paragraph in the commit message to explain why the arch specific
> things are arch specific:
> | Continue by splitting struct rdt_domain, into an architecture private
> | 'hw' struct, which contains the common resctrl structure that would be
> | used by any architecture.
> |
> | The hardware values in ctrl_val and mbps_val need to be accessed via
> | helpers to allow another architecture to convert these into a different
> | format if necessary.
> |
> | After this split, filesystem code code paths touching a 'hw' struct
> | indicates where an abstraction is needed.
> 
> and similarly changed the kernel doc comment.

Thank you. I assume this includes changing the "hw attributes of a group 
of CPUs that share a resource" I mention above.

> 
> Let me know if you prefer some other struct name.
> 

I am ok with current naming. Other folks may have better ideas.

Thank you

Reinette