Received-SPF: pass (google.com: domain of linux-kernel+bounces-172145-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <753844b6-b68d-4b05-89b0-3ac36e1c8564@intel.com>
Date: Tue, 7 May 2024 13:27:00 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v3 03/17] x86/resctrl: Detect Assignable Bandwidth
 Monitoring feature details
To: <babu.moger@amd.com>, <corbet@lwn.net>, <fenghua.yu@intel.com>,
	<tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
	<dave.hansen@linux.intel.com>
CC: <x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>,
	<rdunlap@infradead.org>, <tj@kernel.org>, <peterz@infradead.org>,
	<yanjiewtw@gmail.com>, <kim.phillips@amd.com>, <lukas.bulwahn@gmail.com>,
	<seanjc@google.com>, <jmattson@google.com>, <leitao@debian.org>,
	<jpoimboe@kernel.org>, <rick.p.edgecombe@intel.com>,
	<kirill.shutemov@linux.intel.com>, <jithu.joseph@intel.com>,
	<kai.huang@intel.com>, <kan.liang@linux.intel.com>,
	<daniel.sneddon@linux.intel.com>, <pbonzini@redhat.com>,
	<sandipan.das@amd.com>, <ilpo.jarvinen@linux.intel.com>,
	<peternewman@google.com>, <maciej.wieczor-retman@intel.com>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<eranian@google.com>, <james.morse@arm.com>
References: <cover.1711674410.git.babu.moger@amd.com>
 <63134ddf18208bbd11bcec0ec75c33ef6e5635b2.1711674410.git.babu.moger@amd.com>
 <ac22b96d-c4bb-42d9-bac8-22b9e032b59e@intel.com>
 <80b33bc6-7b70-4853-92a0-718c2d8728d3@amd.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <80b33bc6-7b70-4853-92a0-718c2d8728d3@amd.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?bXJZZEN4SDZHUkhGYjhHWjZKT3RYMGUzb3lrdHphd0lOV0Y3TDZGak5MY3Iw?=
 =?utf-8?B?a2ZCUnlwdUFqWnVDVWpkdTdsS3NRU21hdHd5azVOZ1hSNitYWFgyeFcxbzcx?=
 =?utf-8?B?RjduK1RRaURLNVV6RTRrcTJEa0U1aHpST0lMK1FYcTZDcmVBZDlNdk5FR2FL?=
 =?utf-8?B?YU1mTXhKTDByM3NiWmQwMjR5OGI3RFl1M1BwanRtRHZsdjJSamVCbnk2bEd1?=
 =?utf-8?B?ZjhkeEpkUDhKOXQ2NGx6TTN2ZVFDMXcvdmxGZ2IwbjR4T0o5Vkk2azhVbkJT?=
 =?utf-8?B?NWR3S29pUjNRV0tGb0NDT0h2VmtGaWtQeTZqaFRhbmN0SHU1ZzlEenlDV0FT?=
 =?utf-8?B?MDEraVNjeHVvelRickxVQkZ5dkovSUVhWThVbVBab0VFMEZtTXppNWFETDVB?=
 =?utf-8?B?YU04VW10WXZ3L1Y2TnRYSlNlaE5VWDM5dVlFYTNJMS8vdGZSTG02RDZienhT?=
 =?utf-8?B?N2Z1VTVKU29KRytzVm1HeldEU0x1S2t2cXA1U1lKTXpSaG9UT3E2Zlc0NHpY?=
 =?utf-8?B?OElLS1pKWU1YRjhoYWsvUDNUWmZwR0pGMXU4QlcwWUMvKy9MVGFpSzh3b1Ur?=
 =?utf-8?B?Z3ZCZXVaQzhMOHhSS2U0RFJKMHJ0N0cvYWwzNjFFWHVnZUNCNzIrUjRtRHpr?=
 =?utf-8?B?UVE4YmdpMnhkRVhXbEswRStFUmNEeTZ0TnVFeHVEVXFkR2dRL1lkWWZRMnZs?=
 =?utf-8?B?WDV2VVJmMDZ1Q0FuY0lGbk01eHhJSlRCN3poeTYrcnNTNS9yRlJQa3NaZVhL?=
 =?utf-8?B?d3d2anVCTEJUVktGbitiU3RZZGdabUdkU0R0QjBGMDRqM1JBem9NenN6Nk80?=
 =?utf-8?B?Wml0Nm04eUVFdGMzNkVWUHdITUFMQS9RVFdHVklZTjdxRU5Xc2lYclVBUUhV?=
 =?utf-8?B?VC9mMlhEMlFlTk85dUNhRk1EYTY5UXMvR2lrTWpjOVlFbzFuK0ZDNFlHUGov?=
 =?utf-8?B?QTdjNkhXTGoxQ1RiOC9ZZ2luUTNLN1d6Qjh3MjdOanJtRTJPS1dxaFUya0dp?=
 =?utf-8?B?cEZvSTJoVWtZeExnRDdGK0M5MHRrb003ZE04L2NpZmRVVHZsL2t1NHJCbHJh?=
 =?utf-8?B?cTBXaW5HYWZWZ0xSMVFRRkJJK05vNnNERDhaSHZqdW90ZkFVWCt4YjBTMmty?=
 =?utf-8?B?WUdIUlpDY1NhTlB2WTlXNmlPVzFxRzhEVnNDUlVqZllrQVh0NXliYUJiLzlL?=
 =?utf-8?B?ZmwzWkNJWFdmS0E5ZGFtR2MzRi9leStkdVNlQ1NjSjZtNktzOFZUTkhmVjdL?=
 =?utf-8?B?SXVXcXZSSEMrY3ZOVTlTdW1JUGowdVJGRGhHRGRqa2RCeWIzM1g2Q3VDMHJq?=
 =?utf-8?B?bGh6b1JDVnVpRGR5cXRZNWYxZ3U3TTRqVjVUVCswRW1hMjBha3hNbkM2OG96?=
 =?utf-8?B?UFlNZG83eSs5RCtFR3ZYTUJyL0hUbGxJVWxLRDVxQmorazlWVmt3RzBTdE5z?=
 =?utf-8?B?aS9Vckd6bkxURVpvZlBidmxsUXZCQk4zb1RyNGdyVnU4azNpQjhpOVY3VHRD?=
 =?utf-8?B?Wmt1VWdMM0cxdCtydEw1eXgvSzVpclRqTHFub0ZOVFZwd3JHeStyUnYwRTNk?=
 =?utf-8?B?RVl0TlVmYkhpZEVtQWtNcmJRV1NUb1QrUVdZVlU3UGdxbkVzK1dhM2xFZ284?=
 =?utf-8?B?cFZTNjd5ek44Tm9wbnpONUdZWHh2NHNsMndqUlBYcEhLbjllWTJENTc5RGl3?=
 =?utf-8?B?bXZHU2lIL2xnSDhZTTdRYlMrMTJRWWRIblhIeWUvUEVsTTZPaHR2Q0ZhVnhr?=
 =?utf-8?B?VXVWbHZtbXZTMTJrNEptUWp5bmFYdmNVaGV0ZGpRU1g1RGlxK0tQTW9NMHBB?=
 =?utf-8?B?a0JKOXZwNk9PbTRUS2hMcVJUcFVSV0VQUG1FVmNOT2pXUkhKS2VrRTJaU003?=
 =?utf-8?B?Y2FPeTJQeHE1dG03RU05a3d2dUJIT3RMNnNzbml5Rnl4dldwd1NsYzc0UWdR?=
 =?utf-8?B?TEZmRlFLbGhuaFNHeEEwbmVKUVJtcFd4ZXh1bUpYWHd4QmhLUDRuWDZ1QlFO?=
 =?utf-8?B?YXQxWUc4bUoxcEZZVXNIOHF4RTJKWHNIbXBIMkdFZ25jZ3lqbVJ5UkJYSTg2?=
 =?utf-8?B?a3hKWmgvcXJOM1pIeld6ODZBRjVZWUNaS2FEWFVROGJqaUxmbXJkSHNmaUdD?=
 =?utf-8?B?QjFab2JxYlFCRkxlQWkyd1dGdnc5c3RmR2ZqbjBuM3dlMXhoMlBtZFBmK25u?=
 =?utf-8?B?MUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 04b968bd-7002-4b85-7c3b-08dc6ed40eaa
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 20:27:03.9728
 (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: jOrqG/3Tzew2QJvO84EsNQfwAvChEq3tOALBv2awZB/8/UUVIhN1/KbDGCeGmHqNX90EC09VOe24ynBhKmsA8qTGlrtSKeVgQdv7kHDeWaE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5902
X-OriginatorOrg: intel.com

Hi Babu,

On 5/6/2024 12:09 PM, Moger, Babu wrote:
> On 5/3/24 18:26, Reinette Chatre wrote:
>> On 3/28/2024 6:06 PM, Babu Moger wrote:

..

>>> + * @mbm_assign_cntrs:	Maximum number of assignable counters
>>>   */
>>>  struct rdt_resource {
>>>  	int			rid;
>>> @@ -188,6 +198,8 @@ struct rdt_resource {
>>>  	struct list_head	evt_list;
>>>  	unsigned long		fflags;
>>>  	bool			cdp_capable;
>>> +	bool			mbm_assign_capable;
>>> +	u32                     mbm_assign_cntrs;
>>>  };
>>
>> Please check tabs vs spaces (in this whole series please).
> 
> Sure. Will do.
> 
>>
>> I'm thinking that a new "MBM specific" struct within
>> struct rdt_resource will be helpful to clearly separate the MBM related
>> data. This will be similar to struct resctrl_cache for 
>> cache allocation and struct resctrl_membw for memory bandwidth
>> allocation.
> 
> Did you mean to move all the fields for monitoring to move to new struct?
> 
> Struct resctrl_mbm {
>           int                     num_rmid;
>           bool                    mbm_assign_capable;
>           u32                     mbm_assign_cntrs;
> }:
> 

Indeed, so not actually MBM specific but monitoring specific as you state (with
appropriate naming?). This is purely to help organize data within struct rdt_resource
and (similar to struct resctrl_cache and struct resctrl_membw) not a new
structure expected to be passed around. I think evt_list can also be a member.

Reinette