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

[Maciej Wieczór-Retman]

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@xxxxxxxxx> 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@xxxxxxxxxx>
>
>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@xxxxxxxxxx>
>
>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