Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Fri, 22 Sep 2023 16:14:41 +0200
In-Reply-To: <918e147601697b1d4b8f8589f5751e05d6ceccdf.1695371055.git.maciej.wieczor-retman@intel.com>
Mime-Version: 1.0
References: <918e147601697b1d4b8f8589f5751e05d6ceccdf.1695371055.git.maciej.wieczor-retman@intel.com>
Message-ID: <20230922141441.3353077-1-peternewman@google.com>
Subject: Re: [PATCH v2 1/4] x86/resctrl: Enable non-contiguous bits in Intel CAT
From:   Peter Newman <peternewman@google.com>
To:     maciej.wieczor-retman@intel.com
Cc:     bp@alien8.de, dave.hansen@linux.intel.com, fenghua.yu@intel.com,
        hpa@zytor.com, linux-kernel@vger.kernel.org, mingo@redhat.com,
        reinette.chatre@intel.com, tglx@linutronix.de, eranian@google.com,
        x86@kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Maciej,

On Fri, Sep 22, 2023 at 10:48:23AM +0200, Maciej Wieczor-Retman wrote:
> The setting for non-contiguous 1s support in Intel CAT is
> hardcoded to false. On these systems, writing non-contiguous
> 1s into the schemata file will fail before resctrl passes
> the value to the hardware.
> 
> In Intel CAT CPUID.0x10.1:ECX[3] and CPUID.0x10.2:ECX[3] stopped
> being reserved and now carry information about non-contiguous 1s
> value support for L3 and L2 cache respectively. The CAT
> capacity bitmask (CBM) supports a non-contiguous 1s value if
> the bit is set.

How new of an SDM do I need? The June 2023 revision I downloaded today didn't
list it.

> 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;
>  	r->alloc_capable = true;
>  
>  	rdt_alloc_capable = true;
> @@ -267,15 +268,18 @@ static void rdt_get_cache_alloc_cfg(int idx, struct rdt_resource *r)
>  {
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
>  	union cpuid_0x10_1_eax eax;
> +	union cpuid_0x10_x_ecx ecx;
>  	union cpuid_0x10_x_edx edx;
> -	u32 ebx, ecx;
> +	u32 ebx;
>  
> -	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?

Thanks!
-Peter