linux-kernel - Re: [PATCH v2 1/4] x86/resctrl: Enable non-contiguous bits in Intel CAT

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, 28 Sep 2023 11:28:42 -0500
From:   "Moger, Babu" <babu.moger@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>,
        Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] x86/resctrl: Enable non-contiguous bits in Intel
 CAT

Hi Reinette,

On 9/28/23 10:53, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/28/2023 8:08 AM, Moger, Babu wrote:
>> On 9/28/23 02:06, Maciej Wieczór-Retman wrote:
>>> On 2023-09-27 at 17:34:27 -0500, Moger, Babu wrote:
>>>> On 9/22/2023 3:48 AM, Maciej Wieczor-Retman wrote:
> ...
> 
>>>>> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
>>>>> index 030d3b409768..c783a873147c 100644
>>>>> --- a/arch/x86/kernel/cpu/resctrl/core.c
>>>>> +++ b/arch/x86/kernel/cpu/resctrl/core.c
>>>>> @@ -152,6 +152,7 @@ static inline void cache_alloc_hsw_probe(void)
>>>>>   	r->cache.cbm_len = 20;
>>>>>   	r->cache.shareable_bits = 0xc0000;
>>>>>   	r->cache.min_cbm_bits = 2;
>>>>> +	r->cache.arch_has_sparse_bitmaps = false;
>>>>
>>>> Is this change required?
>>>>
>>>> This is always set to false in rdt_init_res_defs_intel().
>>>
>>> The logic behind moving this variable initialization from
>>> rdt_init_res_defs_intel() into both cache_alloc_hsw_probe() and
>>> rdt_get_cache_alloc_cfg() is that the variable doesn't really have a
>>> default value anymore. It used to when the CPUID.0x10.1:ECX[3] and
>>> CPUID.0x10.2:ECX[3] bits were reserved.
>>>
>>> Now for the general case the variable is dependent on CPUID output.
>>> And only for Haswell case it needs to be hardcoded to "false", so the
>>> assignment makes more sense in Haswell probe rather than in the default
>>> section.
>>
>> Here is the current sequence order with your change.
>>
>> 1.
>> resctrl_late_init -> check_quirks -> __check_quirks_intel ->
>> cache_alloc_hsw_probe
>>    r->cache.arch_has_sparse_bitmaps = false; (new code)
>>
>> 2. resctrl_late_init -> rdt_init_res_defs -> rdt_init_res_defs_intel
>>    r->cache.arch_has_sparse_bitmaps = false; (old code)
>>
>> 3. resctrl_late_init -> get_rdt_resources -> get_rdt_alloc_resources ->
>> rdt_get_cache_alloc_cfg
>>    r->cache.arch_has_sparse_bitmaps = ecx.split.noncont; (new code)
>>
>> The code in (3) is going to overwrite whatever is set in (1) or (2).
>>
>> I would say you can just remove initialization in both (1) and (2). That
>> makes the code clearer to me. I assume reserved bits in Intel is always 0.
>>
> 
> I believe Maciej already addressed this in his response to a similar question
> from Peter. Please see:
> https://lore.kernel.org/lkml/xnjmmsj5pjskbqeynor2ztha5dmkhxa44j764ohtjhtywy7idb@soobjiql4liy/

The rdt_alloc_capable part is kind of hidden. Now it makes sense.
Thanks
Babu Moger