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: <74fc09e7-3a75-2c62-d505-244a12b6190f@intel.com>
Date:   Tue, 22 Nov 2022 16:14:45 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.5.0
Subject: Re: [PATCH v8 07/13] x86/resctrl: Introduce data structure to support
 monitor configuration
Content-Language: en-US
To:     Babu Moger <babu.moger@amd.com>, <corbet@lwn.net>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>
CC:     <fenghua.yu@intel.com>, <dave.hansen@linux.intel.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>,
        <akpm@linux-foundation.org>, <quic_neeraju@quicinc.com>,
        <rdunlap@infradead.org>, <damien.lemoal@opensource.wdc.com>,
        <songmuchun@bytedance.com>, <peterz@infradead.org>,
        <jpoimboe@kernel.org>, <pbonzini@redhat.com>,
        <chang.seok.bae@intel.com>, <pawan.kumar.gupta@linux.intel.com>,
        <jmattson@google.com>, <daniel.sneddon@linux.intel.com>,
        <sandipan.das@amd.com>, <tony.luck@intel.com>,
        <james.morse@arm.com>, <linux-doc@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <bagasdotme@gmail.com>,
        <eranian@google.com>
References: <166759188265.3281208.11769277079826754455.stgit@bmoger-ubuntu>
 <166759204323.3281208.9744810874584175730.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <166759204323.3281208.9744810874584175730.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?UTl0T0h1M1FzN0lTRTliVnVqVVRXRlJKRzYrWUU3VmhnMGNmT1J1UjVDNGlM?=
 =?utf-8?B?T3ZTZnMxdUNsbmFVUkxxN3luUDJuWmlaaitVdDdGZlkwQlJQTTU3bWJKbWtv?=
 =?utf-8?B?SDRTbWdCNUZ2THk5aFFrVXJZT2lEK3pIQkV2YXh1RzZPRjNZRXlZdVBxSll1?=
 =?utf-8?B?YzN0a2hTUDF5T09iVlZ3N01MWldJTTRxZ1pscUtCNFNwRld6UUgzVUx1Wnhw?=
 =?utf-8?B?dTJDeG4xSVVMODgvQm9hWEU1TW05bXRHZ2gzK1JvOEJ6UDNSNVBSdVlFQnVN?=
 =?utf-8?B?VEFXdXJwRlgwNmY5VThpSlB6YkN1cVdLaUZnNlFwcUo3Ty9zYk9KbzJIOTIy?=
 =?utf-8?B?NkVMQ3hzcjJiMFhBQitZUWY2ZlZicU9YdGRaQ09QbFFQR002QkVPWE1WSUtQ?=
 =?utf-8?B?ZmxpOEtBNEI1RFpuT0M3YUhMRmZXMGdxNG1Yc0NRZXJMclRtREVhRWRzRDh4?=
 =?utf-8?B?c3JFQ0RndkY2ajFwWXIza0JBQitTN3lGS2ROejE0eXV0UWc3SjZYVWN4MUxn?=
 =?utf-8?B?UFRsY21mcTdtd1pFNzBadkFsZFZOMTZuWmNEVmdvN3dIb0FweVFhbW16czlT?=
 =?utf-8?B?UnkrMlV6MSs4WU9aV05DYTN5b0s5dlV1S2VNRDN0TXlxMGlQR2hhZUpUWDdy?=
 =?utf-8?B?MnNBN1Frb3FkdlVYRjl1bzlhSDFKUTRmM0puZDJjOHU4RWl3OUFsUmEvbVVJ?=
 =?utf-8?B?Nk5aQ3RSNEkvNVVIUVViaituRXRFTVhJYXRKMUtEZVNpT2xjbjQwV3RmcGNI?=
 =?utf-8?B?aWZGeitjOFA4dWVWdlpzSXVjL3lUQ3NlK05RbDdaME9SU3VYc1BuR0ZrSHFV?=
 =?utf-8?B?TWNCQlc1YkNBNDFqTEFyOTEvQ0xJVmlqTVcyV1BaMjNRSmV4MHF2c0phUkwv?=
 =?utf-8?B?UWNoNW5ZN2VYc3BZaGlFUkNPVUthWEljNE1qcytXY2p1V1F0R3RrUUtsZjAw?=
 =?utf-8?B?dlY4dHhWNWNhQ2tSV3NEL21UWFNFL2p1UHk5dXhDMG5WZWQ3ekMrYnFoVllY?=
 =?utf-8?B?RGpXamhkTTNPOW1XcThKY1MyMFJCWSsrbll2NnRFZjFoaFRmRzdFZHBJSVAr?=
 =?utf-8?B?TXdSb2UzekRrM2dISjRoMmZxZXNSTGFENXZPUGRZNlVqOXk5RGFxMmlHR0Y4?=
 =?utf-8?B?UW1NZFJLYXkrWFlhWWNOV08raG5pL3ZZOERlYUE3WnNKaVh3djcraWEvOU1h?=
 =?utf-8?B?YUtpT1E0Q1F6Zi8rSFBhVUtGOUFIN2M5dUlieVBpdHdBL3ZhTDg3KzFxc0Fj?=
 =?utf-8?B?bTZVbG1Rc0VQRDVPbUw0OWFydnBrV2Rkckx2Tm5sUmNOWHU5Y2svUDhqM2JJ?=
 =?utf-8?B?MmphbXczcERIS3dMVXpFT1c0ZTNha0JqRlZLejNWR1FLVUdZSHh0QmE1dEFU?=
 =?utf-8?B?dk5oRTNya0R2M1RKK29nRjYzWjdaaDJrSlYyWFhNcDdqaittandUQzFKMk1m?=
 =?utf-8?B?TkRBbVZRbGJ2Q25CdEtLSVpKUkVhL2dXWGhBMjEzQ1pnRnl6eWE1V2pDa1NS?=
 =?utf-8?B?b1RRdTkxYnd5bk5TRVEyc3U1U0NLTDZ0S3BPa0ZWUmhaZ0tiOTFqaGxnL1JZ?=
 =?utf-8?B?V2xhYnNKWDdldWZMOHlKZElML1phaHhHUzFFaEczSjMxZVlTblVDSmNnMW9m?=
 =?utf-8?B?TjdRV3NhendsNFBYRk1uWUozZW9TY1J3cjhXVnJXRGlPaDNOTmN2QnZ3YkN1?=
 =?utf-8?B?WjhGZmMrTGVyVGpuSXB4ZHJ0YjFaM3paNGhKd1pOZGhVTjNPY2h6emUzOHd6?=
 =?utf-8?B?c0xBdERmUXAraC9KczJlK0pLUlI3eHFIUkNINTZmNmpXbjhKUFZqcmF0U0RQ?=
 =?utf-8?B?eEVIVVNhcit6MDlUdy9aQ2xaMXlVY3g5NXQxU1RpQzY5UmhQWVU5cVpCT3BW?=
 =?utf-8?B?b1AxcVF4eDN0OXZXRkxPbWlYS0RMZE5saDZRM0NSNk02MlNMVVFsNjZRSUkv?=
 =?utf-8?B?bUFQNXZMcGRHRmZpdlBxeENFZndpRmt6eTlDenU1WUNiYXlBM0ZSeUxOWFk0?=
 =?utf-8?B?dGp0RUo1aEdaRHo2dkp2QUFjNzZ5cW5FNmN4OTNMR0lWZE0zTG1GOGIyOHFR?=
 =?utf-8?B?WTZvNlludUlBNEhGRGtxcGdJWmZqTDVvOWI2QmU2Y3ZITUZIMEVKR1dmTmNH?=
 =?utf-8?B?OWJUcmpQUlFPVXJyV2JGUzcrUWoyWmkrdFlwM2Eva2dORm4vVmdISEdaK2lp?=
 =?utf-8?B?MVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a9faf90b-af43-4931-e772-08dacce7bba8
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2022 00:14:48.6610
 (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: LVMlP96RqOA96DP7hdAUbHOZa783eY9GU/7s52R4/fLFiqH/poNLso4DyE0JpNwsAKsmp9wafoDCRHlCvF9XNPpHN3ZIc+rWAwYA13U8PXk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6949
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,SPF_HELO_NONE,SPF_NONE 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 11/4/2022 1:00 PM, Babu Moger wrote:
> Add a new field in mon_evt to support Bandwidth Monitoring Event
> Configuration(BMEC) and also update the "mon_features" display.
> 
> The sysfs file "mon_features" will display the monitor configuration

sysfs -> resctrl ?

> if supported.

This is not clear. "mon_features" does not display the monitor
configuration, it displays the name of the file that can be used to 
see the monitor configuration.

> 
> Before the change.
> 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> 	llc_occupancy
> 	mbm_total_bytes
> 	mbm_local_bytes
> 
> After the change when BMEC is supported.
> 	$cat /sys/fs/resctrl/info/L3_MON/mon_features
> 	llc_occupancy
> 	mbm_total_bytes
> 	mbm_total_bytes_config
> 	mbm_local_bytes
> 	mbm_local_bytes_config
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/internal.h |    2 ++
>  arch/x86/kernel/cpu/resctrl/monitor.c  |    6 ++++++
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |    5 ++++-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e30e8b23f6b5..5459b5022760 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -63,11 +63,13 @@ DECLARE_STATIC_KEY_FALSE(rdt_mon_enable_key);
>   * struct mon_evt - Entry in the event list of a resource
>   * @evtid:		event id
>   * @name:		name of the event
> + * @configurable:	true if the event is configurable
>   * @list:		entry in &rdt_resource->evt_list
>   */
>  struct mon_evt {
>  	enum resctrl_event_id	evtid;
>  	char			*name;
> +	bool			configurable;
>  	struct list_head	list;
>  };
>  
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index efe0c30d3a12..06c2dc980855 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -750,6 +750,7 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
>  {
>  	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
>  	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
> +	bool mon_configurable = rdt_cpu_has(X86_FEATURE_BMEC);
>  	unsigned int threshold;
>  	int ret;
>  
> @@ -783,6 +784,11 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
>  	if (ret)
>  		return ret;
>  
> +	if (mon_configurable) {
> +		mbm_total_event.configurable = true;
> +		mbm_local_event.configurable = true;
> +	}
> +

Is the local variable needed? Why not just:
	if (rdt_cpu_has(X86_FEATURE_BMEC))


Reinette