Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Tue, 12 Sep 2023 08:53:19 +0200
From:   Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>
To:     Reinette Chatre <reinette.chatre@intel.com>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] x86/resctrl: Enable non-contiguous bits in Intel CAT
Message-ID: <ob4e7sb4pfitjn7arlt7f6be3hvdoyp5yecb4o5ub3i36u2qhv@4s6vzu5nk342>
References: <cover.1693557919.git.maciej.wieczor-retman@intel.com>
 <1bd3aab725a4e2948530095eb48b11bcc4028f8e.1693557919.git.maciej.wieczor-retman@intel.com>
 <4c89d09f-c5a9-892a-4c7d-bdef0204547a@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <4c89d09f-c5a9-892a-4c7d-bdef0204547a@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?YTs+WL5EKrZWUpQxwaFi2/OBqURH/Xn5ac1VrsQOFqAprowZfaL7tLFh6X?=
 =?iso-8859-1?Q?T1yoh/J3VeaDYYvP1/cH8JWHn3CWIx9NEs353ysWE7WN5v9H3ogeoeZGfE?=
 =?iso-8859-1?Q?7hHUiq9y68K7bRL6LvdAzcml+aMHBl37HDd1Kh5//wSNxgaAcAFKoQ6u/R?=
 =?iso-8859-1?Q?PxDgkWiaPZS3KKP8hu31+OC7z4zkjtqqvn6KE7H5i47TL8+H2+1yPQ5In3?=
 =?iso-8859-1?Q?LgV0F7MWaItkixKIuiMAhyvfhKrkYedrwX3z1CaDv2UpR8sZ9Bq7LmqKa8?=
 =?iso-8859-1?Q?H9zJeVMCMmGG/67b3/Id0IWmIoxT2qgUfo9bbwkVVEd7J5yz8pGBK81Pdo?=
 =?iso-8859-1?Q?+pwZ72oYjGYGk0gwFJPNvRXLgaEhCyOyNPo/896P685jiT8IdQZtz/ZS19?=
 =?iso-8859-1?Q?2wju9yJCHovbBRduvpKpEbgXR9V5FdMQOenwGiYLS497BZPoDDR6J8S8Oi?=
 =?iso-8859-1?Q?PKfRMhYxtdJ/axhnhVMmi/JDdyN9mzZ8gJ/9u8TGTIQr2FeVa16imdHFO8?=
 =?iso-8859-1?Q?lPtfY/eBf9MBHW6naNigbqRFi3wf0t7KtRV8lxeYCabzSrveIVVBWbv+Gl?=
 =?iso-8859-1?Q?fyZaVbZ1Rx+ISL2OwNxEgLD/GXVbqeKVJBvYfWgamK1qQjlOUaiBew+pIG?=
 =?iso-8859-1?Q?SskRQG08iwa7sLh/gJcWU4NNZTuGRlylzK8QpnCE8xPuNGYpaktey0TofT?=
 =?iso-8859-1?Q?meyvg1wUGAFhxMHKSVgpUy9s9iIY9qOvqQTqkokcUpJ18WnCg2RIkNUkBo?=
 =?iso-8859-1?Q?S1GhZpK1jdCxO7eSQbnd8j22bRwkD8yrbVB5UmM7bKKAPbPE88KNkLmXJX?=
 =?iso-8859-1?Q?1HsfRjKKpjR834sduiHDtPxfcbKl4WI35bFh9K5hTq3vZFRiw6BpOxVng3?=
 =?iso-8859-1?Q?AH7FDXz4TYMZGM5QcITEXFuclWdckRF1yVBDSVKVW1NYTwoWRTYBog3Ik0?=
 =?iso-8859-1?Q?H0DccinPMMEL+n5lfUwvCXzV5rDQvD7kU87JTdGZDtuRcyJVnEgzHRs93y?=
 =?iso-8859-1?Q?NKkk80pdh4I4R6b7PaP5HoT6sF8TMF1VONSbS+uLO8zGvNlE3/PZEOuuwX?=
 =?iso-8859-1?Q?f8B71nVAH9mpfEvpGyddUmeS8WbKTtOx+Qlc4M64kGHeylYrSYwIkMHr64?=
 =?iso-8859-1?Q?JspSiSUfHNcHxnDFjRf7+HtaJ5GHCt8tRNegd09EtLASh5QPYAkP6ckmlR?=
 =?iso-8859-1?Q?/NC3nucBCWCfMaYJTB54tJMKL1XxoUVBLoG/fEoJJhCW9YoyGupVwS7g9i?=
 =?iso-8859-1?Q?qvLlO29a3AXTYYYhZE/Bo4gQfIKBhz5o1PNVrGA9a8oV+Zd/X/SJY415Ab?=
 =?iso-8859-1?Q?shCCNF01DvEFHdC2cEhoIecOpV26mrvTo1kqq2B9vRwA9uxgPW1Qu9DPOn?=
 =?iso-8859-1?Q?oauiyvW051ZPHUZ7BKFzUZGIUeAn5pwNKAIoPu4ls7lIofcKsYRETpO9R8?=
 =?iso-8859-1?Q?a/5xXFJyCPG972BHKnL3wEffAPLDp81iSb2psukJXCNGRT/reRGYu0QWqV?=
 =?iso-8859-1?Q?hG3a+ij+INUDANKDUsFoU5P7jBdbhmCdNCkCAiqtAuy2qb5od/pqZmlQZf?=
 =?iso-8859-1?Q?QjrPCOvg1ME+CEoKCFICyx3omTwJdpGOq2VrjmlKSh5GgtylrasmgwUOA2?=
 =?iso-8859-1?Q?mqgQOXarfpX9ZyvDtYdFaUop7e7pIKDVgjIplTgg6PbpTcMCY9MRwaiuvV?=
 =?iso-8859-1?Q?fRe9OCytnRuewqhFkjY=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 249fb8c9-7033-4ac7-e28e-08dbb35cf5e7
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Sep 2023 06:53:24.8966
 (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: 2Au83uRw4kx2vf72ImLu0qZowx8L52n3Ed6f1S17kX8prmQYk1kJ2kZGOL+A5gpPFP5iHMzK6WcXIrFYR63CHUhT1YDZtwiGV8IykmIeP9U=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6698
X-OriginatorOrg: intel.com
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 (morse.vger.email [0.0.0.0]); Mon, 11 Sep 2023 23:53:38 -0700 (PDT)
X-Spam-Status: No, score=-2.0 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,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email

Hello,

On 2023-09-11 at 13:05:16 -0700, Reinette Chatre wrote:
>Hi Maciej,
>> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> index b44c487727d4..782e2700290b 100644
>> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
>> @@ -87,10 +87,13 @@ int parse_bw(struct rdt_parse_data *data, struct resctrl_schema *s,
>>  
>>  /*
>>   * Check whether a cache bit mask is valid.
>> - * For Intel the SDM says:
>> - *	Please note that all (and only) contiguous '1' combinations
>> - *	are allowed (e.g. FFFFH, 0FF0H, 003CH, etc.).
>> - * Additionally Haswell requires at least two bits set.
>> + * On Intel CPUs, non-contiguous 1s value support is indicated by CPUID:
>> + *   - CPUID.0x10.1:ECX[3]: L3 non-contiguous 1s value supported if 1
>> + *   - CPUID.0x10.2:ECX[3]: L2 non-contiguous 1s value supported if 1
>> + *
>> + * Additionally Haswell requires at least two bits set. Since it does not
>> + * have CPUID enumeration support for Cache allocation the non-contiguous 1s
>> + * value support is disabled.
>
>The second sentence can be misinterpreted. non-contiguous 1s value support is
>disabled on Haswell because Haswell does not support non-contiguous 1s value,
>not because it does not have CPUID enumeration for RDT.

Would phrasing it like this:

+ * Additionally Haswell requires at least two bits set. Since it does not
+ * allow the non-contiguous 1s for cache allocation the support is
+ * hardcoded to disabled.

be more sensible?

>>   * AMD allows non-contiguous bitmasks.
>
>(connecting to comments in following patch on this topic ... notice how the above
>comments use the term "bit mask" and "bitmasks")

I'll change it so the naming is unified, thanks.

>>   */
>>  static bool cbm_validate(char *buf, u32 *data, struct rdt_resource *r)
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 85ceaf9a31ac..c47ef2f13e8e 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -492,6 +492,15 @@ union cpuid_0x10_3_eax {
>>  	unsigned int full;
>>  };
>>  
>> +/* CPUID.(EAX=10H, ECX=ResID).ECX */
>> +union cpuid_0x10_x_ecx {
>> +	struct {
>> +		unsigned int reserved:3;
>> +		unsigned int noncont:1;
>> +	} split;
>> +	unsigned int full;
>> +};
>> +
>>  /* CPUID.(EAX=10H, ECX=ResID).EDX */
>>  union cpuid_0x10_x_edx {
>>  	struct {
>
>The core of the change looks good to me.
>
>Reinette

Thanks for reviewing!

-- 
Kind regards
Maciej Wiecz?r-Retman