Received-SPF: pass (google.com: domain of linux-kernel+bounces-136178-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <e85e7786-7995-42d5-a5ac-1e08a84492fe@intel.com>
Date: Mon, 8 Apr 2024 20:14:47 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 03/31] x86/resctrl: Move ctrlval string parsing policy
 away from the arch code
To: James Morse <james.morse@arm.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: Fenghua Yu <fenghua.yu@intel.com>, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, H Peter Anvin
	<hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	<shameerali.kolothum.thodi@huawei.com>, D Scott Phillips OS
	<scott@os.amperecomputing.com>, <carl@os.amperecomputing.com>,
	<lcherian@marvell.com>, <bobo.shaobowang@huawei.com>,
	<tan.shaopeng@fujitsu.com>, <baolin.wang@linux.alibaba.com>, Jamie Iles
	<quic_jiles@quicinc.com>, Xin Hao <xhao@linux.alibaba.com>,
	<peternewman@google.com>, <dfustini@baylibre.com>, <amitsinght@marvell.com>,
	David Hildenbrand <david@redhat.com>, Rex Nie <rex.nie@jaguarmicro.com>,
	"Dave Martin" <dave.martin@arm.com>
References: <20240321165106.31602-1-james.morse@arm.com>
 <20240321165106.31602-4-james.morse@arm.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20240321165106.31602-4-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TytZRDB4UEVSWGZvTEhSYW1LR1d4UGx5aU1YVWNmSDlwQVhXYmpqSVQxanZL?=
 =?utf-8?B?ajhPdFp0MjdPaDIzSXVJcTc0anEyaCtoVUVPbHJjTkwrdG5wRnk1cGtiTmR1?=
 =?utf-8?B?ZWlOeDhCczgxQlZJMTVHblN0c04zK2xzaXdhRjlyVzIxOG5pZnVHNWROaDM1?=
 =?utf-8?B?TWhMWXRSSVpmOWNQS2JuTlJWeThQVlhTMm11WURRS1lneUJtYUNySm51WVhM?=
 =?utf-8?B?VmJpYkxVYkFmSmQzNTJBU0drTWNhdUZxdkh0ZEk0aUEzZzBLOVp2MjErY0dy?=
 =?utf-8?B?am1wb1VEekw2OXVBdzFZVlhqamlDNklyYVp4ajVKNk9Ib2VDSjZhbnM4Yllq?=
 =?utf-8?B?ZmEwSE84aTFRVXVPVWxKOTUxeEhxQ1ZmT2VlUWdqWkV6cStKc2xFOEJxRXdS?=
 =?utf-8?B?VzlYRVFUYllsenFLODBxaFNEZmFJZFFCcnJlSTYyODFJNzVveWdCWG1kU3Mw?=
 =?utf-8?B?K2NHQTJEMUVIa2I4MDg5YjZLMXNabVVqRGs2Y3RueUZobEhJT0dWSjVnU25H?=
 =?utf-8?B?dU1VWTF0MUF0M1BXNDBuRVhJUXJRSTZkcVJXNUMybXJEQytBWWNpT25uYS9p?=
 =?utf-8?B?eGRjSTdWTW1nbFRJV004cG1qWHZzUmtpajlrSzlDUGxsUGgwcTh3WE9OelVr?=
 =?utf-8?B?WDJFL1p0MWlwK2FOQ2FvRFNxUEVzaDN2STNERm5MMDNVcm5Ra1JQb3dNWWJP?=
 =?utf-8?B?cjlGYnNhYzlRRkh2c1IyVW9pOGNqTXdtTDR0ZnRDeWJpRW9oKzhjRGFOWUcx?=
 =?utf-8?B?RVcwemtTYWRlcUJ2SnFJTlVQZlBTZHExS1Y1R1psUHhMNXJENTdQMC9UUzJD?=
 =?utf-8?B?NUJicVBPTnFETVIxb1RTVXRKdUtXd25VUGxsMHlhaS9mclJ3Tzd1eW95N0JI?=
 =?utf-8?B?dExOMStpOWcycEFneldDSG83aGhTSVRuWVRiWGtHbFpnWnBiUnIwaHZSZnFJ?=
 =?utf-8?B?MlN4a0FMQkZBQTF2cTRyaXJhV1ArNVdyQ0tkbEU2UEt2OCs4R0g5M3ZuWkxR?=
 =?utf-8?B?cm9kbld2QTVzSlpPWGNjMzVxV0kxeWtjNUlqQTdadDN6UzdDd3ptbUZ1OUhF?=
 =?utf-8?B?K0ZPSlg5QUZyTFZTb0V4M1Y5YVlPMXZ0bDZ3dmMvbjJ1Mml6S1Z3Q25lR2Mr?=
 =?utf-8?B?aEhkd0t5eVR0ZUJTRXpwaFF1VFQzOEZjcWxHTFpxbEZRdk5vbS9FUmd5YTlu?=
 =?utf-8?B?dlZSSnZGb2lrYWZvMlI4L1ZPb2VSeTBYdkJDUU0reXFCcjBpZENwOUpFUTB1?=
 =?utf-8?B?eVVyL3d3UjFyMDhuQjdtVk40emVUU2lVS3psMXR5OVQ0ODlNQkpTZ2RKMXlH?=
 =?utf-8?B?WDZVOWE1akVhRjFNTkpiaHZMb3c4RVBTM2FicDVsckozZXQvQ2s0aGJYa2tQ?=
 =?utf-8?B?UHA1OEoraEE2OE4wNDFnc0xRSGV0bU9qbWpPeVJTSmEwRFhCY2F2T0pTemU2?=
 =?utf-8?B?bm5TbmlTUGJ6dFZRd1ZYanBHQkNSMC92bE5NQ0lWY1JuMmVEeXVzOUs2ZHJO?=
 =?utf-8?B?T3F4MEhFNjM1V1pVM2QxSENNUWc1L2hGKzFKV3NlZkhuSUFPeGltSWt6M0xV?=
 =?utf-8?B?RWx3K0RSSVpZeDhxTStDTzdYMUVTL3JITThtY2xiWjVOQis2c2xhMTY2T0tq?=
 =?utf-8?B?dnZGTE00SGkvdVpRUHBQdThWaVZBODBpb3VJTnQ1YUhKU01nZ1cvamh1Skp2?=
 =?utf-8?B?M2VldlVYTmdyQzVVTkNULzYwYTdzNlZERlVuMjJPVE01Q1d3TythNEdjQXR1?=
 =?utf-8?B?VWFCaE0xTEhQWGJZNUV1UkRwQWtBRlI3bHFoaS9YNnZ6S1JVQytuSWFJcFNI?=
 =?utf-8?B?R2hMcnp2Qjd3VUxzaTMxRk02K0pweXZYYkdnOWdOTUVybEtRQndPUWVoVU10?=
 =?utf-8?B?UE9HaHFFRDN1MG90RjZTOWRPN2UwOHNjVmZQL29ReFhWOEQ0QWJOc0doRk1D?=
 =?utf-8?B?cFEwVXlmeFNPdS96eFZEQUlrWExMaW41ZVRLTUdmV0VXM1M1cUp2UXRTVFVF?=
 =?utf-8?B?QWZicmQzd2xOckVWMGlPZ3N2SHFocjlFQWVQdjEzdFc3TjVvZDJEK2JEdVJx?=
 =?utf-8?B?MTAyUE10Z1hDeFFwTk5aNm9HclNxMHI4TFFDdnRXR1JjK2hqdVdET0FmQjY4?=
 =?utf-8?B?QVcvaUhhaFp2REhXWEdTc2d3K1I4SG9zM01EYTBOZFNvSm9PelhPUkZ3MlBQ?=
 =?utf-8?B?K3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 53b92b6b-1048-4f4e-ecc2-08dc58433775
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Apr 2024 03:14:49.7458
 (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: cgqkFIAal2cuUU8XuPC5swcNvgjnCMRL8A5KnehtjNIy3xeWS/QrSWbrVTdugznBpblE5vdzuLA25nAYnsw8V5jkA4SRSTaJ8lgim/lnZq8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4783
X-OriginatorOrg: intel.com

Hi James,

On 3/21/2024 9:50 AM, James Morse wrote:
> The policy for parsing the configuration values as a string from
> user-space is specified by a function pointer the arch code specifies.
> 
> These strings are part of resctrl's ABI, and the functions and their
> caller both live in the same file. Exporting the parsing functions and
> allowing the architecture to choose how a schema is parsed allows an
> architecture to get this wrong.
> 
> Keep this all in the flesystem parts of resctrl. This should prevent any

flesystem -> filesystem

> architecture's string-parsing behaviour from varying without core code
> changes. Use the fflags to spot caches and bandwidth resources, and use
> the appropriate helper.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> ---

..

> @@ -195,6 +204,14 @@ int parse_cbm(struct rdt_parse_data *data, struct resctrl_schema *s,
>  	return 0;
>  }
>  
> +static ctrlval_parser_t *get_parser(struct rdt_resource *res)
> +{
> +	if (res->fflags & RFTYPE_RES_CACHE)
> +		return &parse_cbm;
> +	else
> +		return &parse_bw;
> +}

This is borderline ... at minimum it expands what fflags means and how it
is intended to be used and that needs to be documented because it reads:

	* @fflags:		flags to choose base and info files

I am curious why you picked fflags instead of an explicit check against
rid?

Reinette