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:   Wed, 27 Sep 2023 11:20:23 +0200
From:   Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>
To:     Peter Newman <peternewman@google.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>
Subject: Re: [PATCH v2 1/4] x86/resctrl: Enable non-contiguous bits in Intel
 CAT
Message-ID: <xnjmmsj5pjskbqeynor2ztha5dmkhxa44j764ohtjhtywy7idb@soobjiql4liy>
References: <918e147601697b1d4b8f8589f5751e05d6ceccdf.1695371055.git.maciej.wieczor-retman@intel.com>
 <20230922141441.3353077-1-peternewman@google.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230922141441.3353077-1-peternewman@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?Buyu9+ZUkjulSJkFTdt9WyUHbR+zYk7SppYrpO5fnel15BfScKmexRigYZ?=
 =?iso-8859-1?Q?7RytLqjVhg2O3xtw72Feqf4B7MxVR2Dyok8sP9GwGP6ilBDAsPJnEz8niX?=
 =?iso-8859-1?Q?t9KFLBZaiEhr5471Q85dj1Fl9UWIclO28L6W+j8BMMzGqd5DhH/kHCH8SW?=
 =?iso-8859-1?Q?Hk2ZsjcBiBFUTMykZFidsjGb1L1yYNLjmoAsI8nq9CXOJibCyHHpzX0Paz?=
 =?iso-8859-1?Q?p0AckBitnxH3TnHwzQPEZPEtgDllAfMfDHSvHp0WiQpQ8O/+PaaDYlYXTT?=
 =?iso-8859-1?Q?TrcX6UUH0C3RRwB8Dica9fLilp/QLRcqfFHZNwMf3ZYaVHP9AtV7/DzqRW?=
 =?iso-8859-1?Q?myMO9c14ZnVL3mKIRFXZZ358yGB8xa0iCXucXmjsv77M2EYvyXrGhONyW8?=
 =?iso-8859-1?Q?Tpt+cy8MdLoK+JjqXLt+ACpBJOIIf/tlyLPJ41Vzj9VOJEwJMhezy/md1i?=
 =?iso-8859-1?Q?/QmMRP2H/jlsoSjlT7VYi/pws3b3DzHbmSJ+ON0BCkjgb9JiaNk9DWXkZj?=
 =?iso-8859-1?Q?+EvsRABEts/ZlU1DxpbtIxL3h1eYFjtHdH1/mLrkFtalR47tgeEzFEG4+M?=
 =?iso-8859-1?Q?PggZMs1emeAsKNPIY8ppzJWJwQLOxeH6P0fbRF9pNAt4Of8YT4vNInXdui?=
 =?iso-8859-1?Q?TsjbUMDZO+t7N7/i3EUGkaWznfwYXHOK51eFpHLb754uUkiAUTn3+Lb9gh?=
 =?iso-8859-1?Q?MZTmxmsc5aYCMDWkdzVD/G4LxUfnnNmc2k0dkPjmDoeoYGsFRONiorHm6M?=
 =?iso-8859-1?Q?lFDxkwpVVhJvbCNzVnB73JUi3BjpSw0je9J7v8Tpo1DtfiER0x9abWvBwD?=
 =?iso-8859-1?Q?WKXoJzBxvtMIz8hFgYo4XdmxPnfqMIBb1HkULqHyZ5OJbN7Os0lv8kLBAv?=
 =?iso-8859-1?Q?nKut0UBvIyhcUVyZDN+Cxz6blaEdQS9veO1VR9foUuiCVI7uFijiIXshjj?=
 =?iso-8859-1?Q?5g90YyhU8vq5KygPYNFOq09tfQwzdKvU6lVEx4DE0LuHAFAl34myh//SsS?=
 =?iso-8859-1?Q?ErzGhy0gcKhZ1z130BsVVYrNvJORw7NFKctlRYOVc/WDzN8SFQcEQX8pr5?=
 =?iso-8859-1?Q?iDnceKA/QNBYxZegeh0AUKbEDBGThHsYsQJy4mLZwgvjFUMaANOxMzHsmp?=
 =?iso-8859-1?Q?fkmPcDEpFmZYkKQejWyKv0jezfxE8hTvHB/v0xeNMBajKRXMyM+RsAjnuA?=
 =?iso-8859-1?Q?ZT29fiBEjYuh4TvPd0+oQYHOMZV6abfZwJlLzL4/PtP8uhW9NVvQBiuEgS?=
 =?iso-8859-1?Q?qfzzjvVden5iegEEYzCYChsg+xvec4H2tp9zGCOlCUMK137o9DT2jv2xCA?=
 =?iso-8859-1?Q?xoHxbF7+7m3mhLQvswHUndr4fuiDni6MY1nyIaNQS1Dk0rRavcAQX3SRLC?=
 =?iso-8859-1?Q?AUZ8ncZSK8mHOte2PEDQkjTBrB7kbCDS3kLCNhItJmudQtS9UO8+CCL1wm?=
 =?iso-8859-1?Q?3tSLDhtTWxaiMQYKXbDQVkUCytQKBkzVd0zCEJnzdTyy0oeYofrRK7M7MS?=
 =?iso-8859-1?Q?tJmsBmU2OXPO2oI6Wc6MWXeCmEPbwGSdIWgoHwsQxEE7M3cKGQM0N/TVkC?=
 =?iso-8859-1?Q?lFd7DTugw/KyQgzXFFXOH+Od+EF1Yu1ih2Zh90JhSq1/yEzFf+rsOMCEwO?=
 =?iso-8859-1?Q?Gbjx6N43x0jFCDYPbi6/nBwmyeHinqBpBGosoqEHRQEek1z0UB8hct11HU?=
 =?iso-8859-1?Q?vd4AqG9crOIjey1XIvo=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1768f045-7a57-43c9-dbe3-08dbbf3afdbb
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Sep 2023 09:20:29.0276
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vAY7Vy8nkHgxorzaxFIo431bD254UJ/uHyPnUf2VslKs1Qu/ud2wTIg9w4AuB2w4QZ82FEan/EKHpZd7n398CCPatY5kaI01S8zBNFvei1g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7098
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Wed, 27 Sep 2023 02:20:47 -0700 (PDT)

Hi, and thanks for the review!

On 2023-09-22 at 16:14:41 +0200, Peter Newman wrote:
>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.

It's not currently in the SDM but in the Intel? Architecture
Instruction Set Extensions and Future Features (which I mentioned in the
second paragraph of the cover letter). My version of the ISA pdf was
from June 2023.

>> 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?

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.

-- 
Kind regards
Maciej Wiecz?r-Retman