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:   Wed, 27 Sep 2023 12:44:39 +0200
From:   Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>
To:     Peter Newman <peternewman@...gle.com>
CC:     <bp@...en8.de>, <dave.hansen@...ux.intel.com>,
        <fenghua.yu@...el.com>, <hpa@...or.com>,
        <linux-kernel@...r.kernel.org>, <mingo@...hat.com>,
        <reinette.chatre@...el.com>, <tglx@...utronix.de>,
        <eranian@...gle.com>, <x86@...nel.org>
Subject: Re: [PATCH v2 1/4] x86/resctrl: Enable non-contiguous bits in Intel
 CAT

On 2023-09-27 at 12:08:33 +0200, Peter Newman wrote:
>On Wed, Sep 27, 2023 at 11:20 AM Maciej Wieczór-Retman
><maciej.wieczor-retman@...el.com> wrote:
>> On 2023-09-22 at 16:14:41 +0200, Peter Newman wrote:
>> >On Fri, Sep 22, 2023 at 10:48:23AM +0200, Maciej Wieczor-Retman wrote:
>> >> -    cpuid_count(0x00000010, idx, &eax.full, &ebx, &ecx, &edx.full);
>> >> +    cpuid_count(0x00000010, idx, &eax.full, &ebx, &ecx.full, &edx.full);
>> >>      hw_res->num_closid = edx.split.cos_max + 1;
>> >>      r->cache.cbm_len = eax.split.cbm_len + 1;
>> >>      r->default_ctrl = BIT_MASK(eax.split.cbm_len + 1) - 1;
>> >>      r->cache.shareable_bits = ebx & r->default_ctrl;
>> >>      r->data_width = (r->cache.cbm_len + 3) / 4;
>> >> +    if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
>> >> +            r->cache.arch_has_sparse_bitmaps = ecx.split.noncont;
>> >
>> >This seems to be called after the clearing of arch_has_sparse_bitmaps in
>> >cache_alloc_hsw_probe(). If we can't make use of the CPUID bit on Haswell,
>> >is it safe to use its value here?
>>
>> I believe the calls go like this for a haswell system:
>> resctrl_late_init() -> check_quirks() -> __check_quirks_intel() ->
>> -> cache_alloc_hsw_probe()
>>
>> There this line is executed:
>>         rdt_alloc_capable = true;
>> where rdt_alloc_capable is global in the file scope.
>>
>> Then later in:
>> resctrl_late_init() -> get_rdt_resources() -> get_rdt_alloc_resources()
>>
>> this is executed at the function beginning:
>>         if (rdt_alloc_capable)
>>                 return true;
>>
>> So the rest of the get_rdt_alloc_resources() is skipped and calls to
>> rdt_get_cache_alloc_cfg() never get executed.
>
>Yuck. But it works I guess.
>
>The series looks fine to me.
>
>Reviewed-by: Peter Newman <peternewman@...gle.com>
>
>I applied the series and was able to confirm the behavior was still
>correct for contiguous-bitmap Intel hardware and that sprase_bitmaps
>is true on AMD and continues to work as expected.
>
>Tested-by: Peter Newman <peternewman@...gle.com>
>
>I'm not sure if I have access to any Intel hardware with
>non-contiguous bitmaps right now. Are you able to say where that would
>be implemented?

Thanks for testing!

Writing non-contiguous bitmasks is supported starting from the upcoming
GNR microarchitecture forward.

That's also why the new CPUID bit meaning is in the ISA pdf and not in
the SDM one currently.

-- 
Kind regards
Maciej Wieczór-Retman

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ