Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <673b861a-5c87-e066-09cc-d17e2d16ef68@intel.com>
Date:   Tue, 23 Aug 2022 13:56:10 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.12.0
Subject: Re: [PATCH v3 01/10] x86/resctrl: Fix min_cbm_bits for AMD
Content-Language: en-US
To:     Babu Moger <babu.moger@amd.com>, <fenghua.yu@intel.com>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>
CC:     <eranian@google.com>, <dave.hansen@linux.intel.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <corbet@lwn.net>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <bagasdotme@gmail.com>
References: <166117559756.6695.16047463526634290701.stgit@bmoger-ubuntu>
 <166117577023.6695.4825793379162387650.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <166117577023.6695.4825793379162387650.stgit@bmoger-ubuntu>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bUJabVd2K2JDYVUxdllGMlg2eGNuL1lpUnJZMFltRXlCZCtYZ2hwdno2L1hJ?=
 =?utf-8?B?S2crUVZiaXRpeHFSZ1dCYUZHUkI5ZHNHYkxmdnROL2ZaQ3RwVjYzcUZGR0tK?=
 =?utf-8?B?RityeVpEcXZLWHBwY2g4bGMwVWlGMXc2a2JYK1ViQ0lUQUR5aUJneGxYSnFW?=
 =?utf-8?B?S01ITWhxS1pPU1FmOHlCV21NeTlJOThMZGhtUE9zY0dFS3pGMEdzeEtOKzRG?=
 =?utf-8?B?WVlUeGR4eGljTm00bjFEZGRSVE5xOGdQWUlXT245Y0JuSXNkVHJZZlRNb3VT?=
 =?utf-8?B?bTFwZXpHRForaE1RWFV0WmtPRkloNTRwU01HZnI2RE93eUlEWldDR3ZURncz?=
 =?utf-8?B?NWlKUWEwWllGbDhPNElDRUo0Q0lrRWRPK1dIMUlEa3BpSXhuTWU1TXIrRy9q?=
 =?utf-8?B?NzRIcWNCV0dCVGUzcVVabUg1ajJ0dmx3UkJRN29RMWVrVkRiaEkrM2lzcitn?=
 =?utf-8?B?NmJUUmxyVE0wQi9JNDYreTBJRmR4czBqRGJ2ampCejNxcG9oZE1qQ2hvY1h3?=
 =?utf-8?B?Q0VJNUtBMnhZVnhCR2VuQUcraFFEV2ZQQklzYzRwZDUrUHUyd0FaUjd3UTdo?=
 =?utf-8?B?dXRiSnlxdEVzU3FuR2kxSE1QZkdVUm1jWk9kTTByQ2xDbHpMWnczRHdnZDdK?=
 =?utf-8?B?Q3VlelMwNHhwdk82dFNqUTRjcUF1SXVsWGNFVENUNW1kTmdMQ3lUUjROdVNG?=
 =?utf-8?B?VCtiN1BxM3NZRGozTEVjemkzV05mTVh5WlpjT0NEdU9TbGJBM3VBcmViSnhT?=
 =?utf-8?B?dnluYkNnR2RPSjh3c3F2ZDVNZWFGYU04YUsxOFhwNEpYZENEM0NYQ0hVNEU0?=
 =?utf-8?B?TElWNHFqWFc0ZThPaVhubmp3RTkvdVg1MW01ME4rNjR4bFJ2NEQrZllSL3o1?=
 =?utf-8?B?ZW1tMC94VkVGSU8yR3lqeWFpbXkvalQvb1hNdHpCT2dOVGxJK0VlS0t1b3FY?=
 =?utf-8?B?NEFZc2ZqZmhQdkovTjc3TGxsSmRPZkg2ZUR6dTExM1RLT0xDbGsvTDJHc3FM?=
 =?utf-8?B?bnltRENQWXVkZzh0VVM3c09XbTRDRzExL29TR3ZaVll0RTFxam5MNkgyUFNQ?=
 =?utf-8?B?bGhST3hLdmxXcFlWOVV2V2RQMnplekdhbDBHcFFuKzhoTmZZcXF6VnhYQnhU?=
 =?utf-8?B?M1craUpIS20rMmM4T0JJaGdTT0ttSU5pSTZmMHppR1RNR3dHdm9OYVBlVm94?=
 =?utf-8?B?WU1rYjlTSWl2emovVzZtVnR5dnhxTmoxOHN3UDYzbDcyRXVuWHNrSTIydFpT?=
 =?utf-8?B?N0RpVFFOSHN5a0Y0SC9sdXo1VDFWZTRkZEZId0RiMHNONVNkRlVsZXZtNFZC?=
 =?utf-8?B?YlNIQVdMTGlJSDFPNzhSanlxVGpOZzMweEVEcjBzVjlCcnc4bEJ1Tk9OaG9p?=
 =?utf-8?B?NG0rSDdMVGpQZFZxeE5sdHNubWFLYVhOdkZtR2Fma0N3cUYxcHFUOEorYTZ5?=
 =?utf-8?B?MWRGNkducldWc3hOcDBBdmZjNC9tUmZyVm4wVHNsNm5kN1FRS0hpNURaR3E5?=
 =?utf-8?B?Z2o0anptMFN5NE05RlZEb3VTcXZHaTRpaGplcFN4TzUrZFpJY3FQdGZNMlNJ?=
 =?utf-8?B?TmVXR0RMVGFvQkJ1VldWWnQyMTAwcXdWekJzYmgxMks0aWwreWp5SWtac3hD?=
 =?utf-8?B?UFBMZFNKWFNXckVhRTliU1UvYS9LbVRqYjJ4RDgxVmxnVENrRkFsdUpkY2d4?=
 =?utf-8?B?d2hkMW9ObGFBL0R4VllNYUZnU3F1UWZXOXdKZEl6SFlCMVRHZEdtbGU4QjRP?=
 =?utf-8?B?alhQNXN6eFNwN0RpM0lPVlhPa3dKVzVsSG1DVzB2aE5SNGJPS3pqOEpLVHNq?=
 =?utf-8?B?M1oyK0dadEtmYms1Y242SitmTjk2QUgzWXVva0dlZVBaOFJnWE9CcEtxakg5?=
 =?utf-8?B?RlNGbnFHNkNTVDhNNUd5QXpWY3dXd2FxVUxudEc0STNqdXJ1b05uOWJ0Zm02?=
 =?utf-8?B?THV4dFZ6MFVOOGlQSTBma0V3VCtpUTdmcDVCSFFWQW9HamVJdm5hMHNGbyt5?=
 =?utf-8?B?U2xFWmRaU1VDU2J5eEFvRmUzd2lzNFpweXBuZzVZbFB3OWJzVGNLUDJJRDRB?=
 =?utf-8?B?VUF5VHpLYitUT05KUTdEWnJVTGNaWUlzTUhqaUU5VFNCTUlPUzFOMzlmMnp2?=
 =?utf-8?B?d2Z3VTJSaEpSVElZUlVqOFNVWDVQV1ZTcXppNXN3UmVBZ2ZZSnd5bmZzNFJB?=
 =?utf-8?B?TXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 32f939b6-b44e-4339-c5bd-08da8549e9fc
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Aug 2022 20:56:13.2672
 (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: 0RKlwjl3ad0czons9+KvA1aDxQH3fYb06KQaAptb89HC8PvB1LaUZYv5e5Sjr8X8T490Pfr8CIdmLkmHfG7RQh/0U1wUAX3J6XGmRV4oRUg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4925
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Babu,

On 8/22/2022 6:42 AM, Babu Moger wrote:
> AMD systems support zero CBM (capacity bit mask) for L3 allocation.
> That is reflected in rdt_init_res_defs_amd() by:
> 
> 	r->cache.arch_has_empty_bitmaps = true;
> 
> However given the unified code in cbm_validate(), checking for:
> 	val == 0 && !arch_has_empty_bitmaps
> 
> is not enough because of another check in cbm_validate():
> 
> 	if ((zero_bit - first_bit) < r->cache.min_cbm_bits)
> 
> The default value of r->cache.min_cbm_bits = 1.
> 
> Leading to:
> 
> 	$ cd /sys/fs/resctrl
> 	$ mkdir foo
> 	$ cd foo
> 	$ echo L3:0=0 > schemata
>           -bash: echo: write error: Invalid argument
> 	$ cat /sys/fs/resctrl/info/last_cmd_status
> 	  Need at least 1 bits in the mask
> 
> Fix the issue by initializing the min_cbm_bits to 0 for AMD. Also remove
> the default setting of min_cbm_bits and initialize it separately.
> 
> After the fix
> 	$ cd /sys/fs/resctrl
> 	$ mkdir foo
> 	$ cd foo
> 	$ echo L3:0=0 > schemata
> 	$ cat /sys/fs/resctrl/info/last_cmd_status
> 	  ok
> 
> Link: https://lore.kernel.org/lkml/20220517001234.3137157-1-eranian@google.com/
> Fixes: 316e7f901f5a ("x86/resctrl: Add struct rdt_cache::arch_has_{sparse, empty}_bitmaps")
> Signed-off-by: Stephane Eranian <eranian@google.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/kernel/cpu/resctrl/core.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index bb1c3f5f60c8..a5c51a14fbce 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -66,9 +66,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
>  			.rid			= RDT_RESOURCE_L3,
>  			.name			= "L3",
>  			.cache_level		= 3,
> -			.cache = {
> -				.min_cbm_bits	= 1,
> -			},
>  			.domains		= domain_init(RDT_RESOURCE_L3),
>  			.parse_ctrlval		= parse_cbm,
>  			.format_str		= "%d=%0*x",
> @@ -83,9 +80,6 @@ struct rdt_hw_resource rdt_resources_all[] = {
>  			.rid			= RDT_RESOURCE_L2,
>  			.name			= "L2",
>  			.cache_level		= 2,
> -			.cache = {
> -				.min_cbm_bits	= 1,
> -			},
>  			.domains		= domain_init(RDT_RESOURCE_L2),
>  			.parse_ctrlval		= parse_cbm,
>  			.format_str		= "%d=%0*x",
> @@ -877,6 +871,7 @@ static __init void rdt_init_res_defs_intel(void)
>  			r->cache.arch_has_sparse_bitmaps = false;
>  			r->cache.arch_has_empty_bitmaps = false;
>  			r->cache.arch_has_per_cpu_cfg = false;
> +			r->cache.min_cbm_bits = 1;
>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>  			hw_res->msr_base = MSR_IA32_MBA_THRTL_BASE;
>  			hw_res->msr_update = mba_wrmsr_intel;
> @@ -897,6 +892,7 @@ static __init void rdt_init_res_defs_amd(void)
>  			r->cache.arch_has_sparse_bitmaps = true;
>  			r->cache.arch_has_empty_bitmaps = true;
>  			r->cache.arch_has_per_cpu_cfg = true;
> +			r->cache.min_cbm_bits = 0;
>  		} else if (r->rid == RDT_RESOURCE_MBA) {
>  			hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
>  			hw_res->msr_update = mba_wrmsr_amd;
> 
> 

Thank you for putting this together.

This change makes arch_has_empty_bitmaps redundant. Can it be removed?

Reinette