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: <b35c1b06-4236-15dc-6aed-ece8a0331c8c@intel.com>
Date:   Fri, 12 May 2023 09:57:00 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.10.1
Subject: Re: [RFC PATCH 0/7] Add driver registration i/f to resctrl
Content-Language: en-US
To:     "Luck, Tony" <tony.luck@intel.com>,
        "Yu, Fenghua" <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>,
        "H. Peter Anvin" <hpa@zytor.com>,
        Shaopeng Tan <tan.shaopeng@fujitsu.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        James Morse <james.morse@arm.com>,
        "Babu Moger" <babu.moger@amd.com>
CC:     "x86@kernel.org" <x86@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20230420220636.53527-1-tony.luck@intel.com>
 <c0755752-ce84-b5f9-6ac6-97812e98b8c3@intel.com>
 <SJ1PR11MB60832074F3140B38C1B08596FC719@SJ1PR11MB6083.namprd11.prod.outlook.com>
 <237603b3-f553-2ede-5725-239b65626c69@intel.com>
 <SJ1PR11MB6083CFFA8CBDD296E966EC85FC769@SJ1PR11MB6083.namprd11.prod.outlook.com>
 <SJ1PR11MB6083031582F2A61A7D24ABAFFC749@SJ1PR11MB6083.namprd11.prod.outlook.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <SJ1PR11MB6083031582F2A61A7D24ABAFFC749@SJ1PR11MB6083.namprd11.prod.outlook.com>
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?QjZ3K2NkYzI5SU1CeXNJQ1ZrT0ZlaVVIeHNZbzYxbm9yL0h4aFVxS1NveWFp?=
 =?utf-8?B?bVlLNkczWkxjV0xRMEltenVjL3RPUzhqQWY1SnJDU2s3M2xjRUZESUhXUG5I?=
 =?utf-8?B?RTlKNExJLzhmRmtEWC9kTlRFTlNvS0dnREpUVWhiaUV4NUdaOStWazlnc1l6?=
 =?utf-8?B?cnhpRFVoTXdERE1sM1VUVThtamg3emZ2WWcyb2w3Qi9mWm45ZkhzaXJYbmlS?=
 =?utf-8?B?UG8wU1Q3WllpTFlDWkltRGUweGNRUWFheENHMk81a3FSVzEvd3VwTnllZFp2?=
 =?utf-8?B?UVl1c2dLNVZaWlhQQm1MTUZZTnNmS0RIWWFPdEh5WmIvOUZQRURNSWptV2l0?=
 =?utf-8?B?MytyTDBlOXZOZ3FEbEowdjg2eTErdE5WT3UxcUFEK2krOVBNWEtPWTJQR0xR?=
 =?utf-8?B?RVNSb1E2MFV2UXlmdy94QzA1b09SVmNDeEdpcnNRMERHZDVFcjlmUVJTSGU2?=
 =?utf-8?B?VEcweFlSQmFPR3hRcEhzYjZiSWhzRXNGcWtGOERUMFdmNEd1WjhnUUZNMlZo?=
 =?utf-8?B?MFdHekYvZC9Gd1VVUXN4dnJUb1FYMnVWbHp6R2ljUVYvWE16Y3k4N0krRDJ5?=
 =?utf-8?B?MjVnK3l4TU5mNCsrakI3Vy9tTk5pUmt6KytvaFpHb04rNS9GNlU1N1dtY2pY?=
 =?utf-8?B?VmZaU1hTbzRuallldmZxYnNaWmNYYm5LeEU1aTdYTkh2UitQYmFKMk1kaHlX?=
 =?utf-8?B?QW43RGF2STBNZzlMWmIwK0ZUWWdiT3k4UkdYWlVKdmFRdkk2TnNjZ1dzeUpi?=
 =?utf-8?B?cUY1ajJYcWNyb3VlVi9tT203Y0drTGs3L201RkZUUzBGQmI4d2ZieVd2MWNz?=
 =?utf-8?B?d1dHeGEwNWlESCtOUHdvL3ZlNGVhVnpmNEYxaGpYRDM5S25GTXlqTUNYdUl1?=
 =?utf-8?B?VDJ4RWVJR09NVVl3MnFFRWRQQm1jRUtxNURZNVpTNkU4TWcwdTJqN0lWeXlp?=
 =?utf-8?B?M0NiSkdhLzhNVFA1YloyWjNqdUQ0V3FMNEw4RmhFSFlRZ3RrL29YS1AyTG5W?=
 =?utf-8?B?b0EweG8vQzN1emRzL2VTZG5HU1B0R0RPd0lUWjZYSDJvYTVEdC9mb0o3ZlZ5?=
 =?utf-8?B?dVo3Y1VhZWZNMEpRaFRib3VWem54R3kwUFh2dGRHbkpqR09lQ3NLbUxRVWJj?=
 =?utf-8?B?NmdYU1dFTVYwdUhMb1JYb0hxZ29aTWhHalhUWmN1alJGRXBzRFE2d3JnZWFr?=
 =?utf-8?B?b1NoUllhMWhTRXFFb3dzZzB0VlFYNG5FWFdZaDdWbURTNWYwd2REK2RKVWt3?=
 =?utf-8?B?VWFINUZpU0U3enBILzM4MTl1WHczeDhjYVcwbnRUM3hiQTFPZTkzaHcxVC9B?=
 =?utf-8?B?bEt1eCtmZ2lrRUxhOVhCR0hPUGE2aXM0OEpMY21taUhyRXAxYjF4MTBoSWJz?=
 =?utf-8?B?UkhJbmRXbXdyMUNqY09aeHZOMHpDL08yUG9qbFBzVThaZWRsZ1JkaTVHemdz?=
 =?utf-8?B?TXhxRkpIeTVjTWFsald1ZHd2UUkrU2tXSTV4a05hTXNDVHJ5SVNtQU5JdzZ1?=
 =?utf-8?B?bVhTQklTemNwOUsyUEwxSzNkZ0ROOXE1VnZuQjhPU3NOLzIycG1kSmdnZk80?=
 =?utf-8?B?M1hwQ3k1NU5FNk9YMVdGdWpPajhDczdybzlrOTcrODR4ckQva20vczhZNkww?=
 =?utf-8?B?K1RxMFhrRFhPTWtkRjhFL1FUVXFjRkNaNzlTNXZmNE05eGJMU0crM0tkU0hI?=
 =?utf-8?B?cjRhVjE2bVpvOHZXTkpLRmJ2U3FyZnZwNWRCdWU2dHQybmRHUi9MKzV0RDVU?=
 =?utf-8?B?ODFPR1hJdXpZRG8yVXAzdFEyKzJ5dUNLNjhBQk5VVm5VSDFPT3llOGQrOXky?=
 =?utf-8?B?U3RmQjc3ZmV2ZGNiTjRpYVBacWgrWG5PZkd3RS9zVU1nd3JKWkoveE9kVEVk?=
 =?utf-8?B?aC9abkM5QnRIaFUydmR3OUVrenJ0eTJUMHJULzBaa1RBUGhEbnAxQmQ2RmRu?=
 =?utf-8?B?LzI4aVd4OEdHTmhOYlpDa0xZZUxlNmZLbjdleHhESHZONWlBWklQUUdXc053?=
 =?utf-8?B?b2RnTy9DMGhMT1lsVHA2K21vSlh3Q1UyVEZ3T3NUQTlFQlFsUlF3bXRuUFdB?=
 =?utf-8?B?d2VPQU9kZ1MzOVk5QjNieW5iWXpFckNTRTVDbTZmQXhzdno4SEthRityQ3l0?=
 =?utf-8?B?bmx5UXN2NE8xRjBlWmJKUFBIak9aSUdRRGJrZWJXRVhpV1RUaUhrUXlISHBG?=
 =?utf-8?B?RlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ddad4ba0-7966-4147-5453-08db5309e89d
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 May 2023 16:57:02.7822
 (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: m49pRf5R95hWd1+VF9Hn+wXUXUhjVEYuaaZzz95hjLWKuoB51iOXoXkTtcAS4CJvp4EANnXANuNKbxl1sYsb7AMptZjTPyiBaONoabx2cXI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6346
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-6.3 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_H2,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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 Tony,

On 5/11/2023 1:35 PM, Luck, Tony wrote:
> Reinette,
> 
> You asked for examples of resources that don't fit well into the
> existing schemata model.
Primarily about me trying to understand what about the existing schemata
model makes these new features hard to support. My first confusion is
that it is not clear to me what you mean with "schemata
model". I think there are two parts to this: the schemata model as
the user interface and the schemata model as implemented within
resctrl.

It is not clear in your motivation whether you are talking about the
former or the latter and thus dug through the patches to figure this
out. At first it seemed as though the driver aims for a totally
different configuration mechanism with the support for driver specific
writable files in every group ... but then then the driver pivots and
hooks into schemata files so it then seems that the driver mechanism does
aim to maintain the schemata model as user interface (perhaps with
additional configuration files to give more meaning to the values
written to the schemata file).

So, it seems to me that the concern is not with the user interface, but
how the schemata parsing is done in resctrl. I just seem to keep guessing
here so I would appreciate more insight.

> Here's the details behind one of those cases.
> 
> Existing resources have a single value per domain (e.g. a cache-way
> bitmask, or a memory bandwidth percentage or target). One of my new
> resources has several parameters. At first glance this might be
> solved by just listing a comma separated list of these parameters
> for each domain. So the schemata entry for feature XYZ that has two
> domains might look like this:

 
> XYZ:0=param1,param2,param3;1=param1,param2,param3
> 
> But this feature has a second problem. The hardware supports a very
> limited set of variations. This could be handled by reporting
> num_closid for this resource to that low number. But then resctrl
> core code would limit all resources to that value. Instead the h/w
> allows programming a mapping feature from closid numbers to resource
> instances (as the saying goes "any computer science problem can be
> solved with one extra level of indirection").
> 
> So if the driver named these instances: A, B, C, D. Then a schemata
> file might look like:
> 
> XYZ:0=B;1=C
> 
> meaning that the driver will set up so CLOSID for this resctrl
> resource is mapped to instance "B" on domain 0 and to instance "C" on
> domain 1
Apologies but it is still not clear to me how this cannot be handled
in resctrl. For example, consider this hypothetical snippet that uses
a unique callback to parse a resource's schemata entry (similar to the
s->update() callback you introduce, but not have it be optional):

 	struct resctrl_schema *s;
 
 	list_for_each_entry(s, &resctrl_schema_all, list) {
		if (!strcmp(resname, s->name) && rdtgrp->closid < s->num_closid)
			return s->parse_line(tok, s, rdtgrp);
 	}
 	rdt_last_cmd_printf("Unknown or unsupported resource name '%s'\n", resname);

 
> The driver provides a way to set param1, param2, param3 for each of
> the A, B, C, D instances.

Seems like these could be some new RFTYPE_RES_XYZ files?

On the model specific motivation topic: If the goal of the driver
interface is to support model specific changes to resctrl there is a
publicly available use case for consideration:
https://lore.kernel.org/lkml/20230421141723.2405942-1-peternewman@google.com/

It is not obvious to me how that model specific problem could be solved
with this driver interface. Your insight here would be appreciated but it
seems to me that we will have to keep accommodating model specific code
in resctrl.

Reinette