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: <06ca0226-687e-c123-4132-b8aa2ae76439@amd.com>
Date:   Wed, 23 Nov 2022 12:23:53 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Reply-To: babu.moger@amd.com
Subject: Re: [PATCH v8 07/13] x86/resctrl: Introduce data structure to support
 monitor configuration
Content-Language: en-US
To:     Reinette Chatre <reinette.chatre@intel.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>
 <74fc09e7-3a75-2c62-d505-244a12b6190f@intel.com>
From:   "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <74fc09e7-3a75-2c62-d505-244a12b6190f@intel.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?bWdPKzNuakgyK3J3R2Jzb05KZlI2REJlNHZDSDZLdndsSklHVDgzbCtxeXlP?=
 =?utf-8?B?MzFVcFJPY1Uwck91bXg0NXhmK3VEVFhwU0lCRWxJUnI2Q25rUXpkRkQvM1pp?=
 =?utf-8?B?NWFlaWphU0VzcWNLRC81K2xqaDNIKzZ2Lzd6RXlQekVmb1VWMURTanBaTlBC?=
 =?utf-8?B?eVIyUXd3bkhINUNZRVhwYldjUkcrWm9iMy81R1lvaEhlMnFKcTdyazVEL1E2?=
 =?utf-8?B?V2VrbDROU3l4bFBjVHVqNGlGZGZSK0RlYThiSjJBWG1NeVVqeVI0VlIyakxI?=
 =?utf-8?B?bGoyLzI0ZlpBU25LMjlBNWNrbUFyMXlobytZRXk4dit0WkNGTnk5dGJNZVBm?=
 =?utf-8?B?TDV4MTl4a3V6MUoydXFVU0tWRzl6UEZtZUJHRTM1SmJ4d1crZlhyWHNJQWd2?=
 =?utf-8?B?cWRiZXNKaDZLNVdlS1Q1UXJJMGhzYTNnWDAwRi9CKzVCeVBMNkhHaXJwWGVG?=
 =?utf-8?B?dTVnSUJpcVFhamx3VHJZeW4xeHNsVkxuN2lZSkUxdEswRlFLVFh5a3ZHOU51?=
 =?utf-8?B?VWJsa1kzZW5qeS9vaW1UTU5ZUGVpcWdJaW9BSVloUjhkOHBRSDZmRWszeWlN?=
 =?utf-8?B?VHY4cmlaa3c1b3NTVHAwa29TNXloTitIVW03Szd6ZVcvR0x2QzhOU3Q3Ujd3?=
 =?utf-8?B?ZENBZXdXQmRIM0FuWTBPTndLanpHMndJUnVPcVY5RDUwYTU0U1ZQRDlxVnE4?=
 =?utf-8?B?T01VUmJPcnEza3ZYb1pwT2tNaGo1c2xtOE5aTzJVWTBwL2EraHBYNXZXUG4y?=
 =?utf-8?B?OVJDWVVINE1FRzRqYnRkSFFKcDNTY2tjcytUNkRYZmt4elZiZzk0aWMzYmNR?=
 =?utf-8?B?Z2VqTExEK2J6STYxU2xUem9iNVlWZmFyNUFacDVyUExCcnBtYUh4VURBM1JG?=
 =?utf-8?B?eDBncDNGdll1eEl3bTZlSGMxazdCQTNERVlnWUtjUXJTU0ZUcFdWZVFPWUN1?=
 =?utf-8?B?RmcyY2R2cXhwYzlxa0l1Smxac3dpMWNwSXQ2ZEJHaFZ0L2V0alBnd01vNFUz?=
 =?utf-8?B?dVVBeWFHTG1EZi9ybk9COUVFQlI0Nm1WWVZBdkswTThNbmpiTnB0c0w2TFVj?=
 =?utf-8?B?dFA1ZWNVUm1HNkJiVVFPU25NWkFMQ1p4NmNBNGZ2NzFIcEZiL3A3VnI2b3Rp?=
 =?utf-8?B?ZGt5d1BqZjJiQURBMVRXQzFIdTdScEFBL0tZTmJaeFd2Sk4wNGtscUFQYmVN?=
 =?utf-8?B?R1V2OGRmUHNNdnplR0p4cVZTNGdrN2VEeXEvVlcxT0dONWFCS01NWlNQRlRK?=
 =?utf-8?B?MEdsdlJZMXJ1cGdMa2tJb21PSjlqRkIxdTF5WWl2MGZSbmtWd3lDK3hXb2xH?=
 =?utf-8?B?bWtEOGRoVTgzcmx1QlhjdGQzMnIwNGZMMTM1WjlvUVBpRDQ5cTBQNjlFQ2tS?=
 =?utf-8?B?UnovVmp4U3BFNzYvYU51aFh1RmhKN0Q1b09jMW5mQVRxUGZaaWNwU2RnZVFs?=
 =?utf-8?B?QW5pRVZOeXlMNFVuNUcyL2lINUtZc0RNMldob0JXMnNLMjR1bGFJaW5yQUV5?=
 =?utf-8?B?UzVuR3N2cDByc2ZhQ3V4ekRDVTRrazFLR3c0VDhIYjdoMHdRM205SDlHZ0xt?=
 =?utf-8?B?SDJoRWVzMXdsbW1OT096M0Q3Zlcvc25XMkhpTk9OMVd2Q1JtdXNpZUFyMDZX?=
 =?utf-8?B?WVdZVkJZelZQcFBta1A1UXlESmZlL1ZpdzgrMENxOGFPVHNqelhONDg1enZQ?=
 =?utf-8?B?SHZJOEM0RTZrTittUytMWllWWWluMVpCSE1ZUDIrM2IrMVVTTVN0VGlqNks4?=
 =?utf-8?B?Sk80M25idFEzeTI4eHdQTU1Vakw1VlpOVGVOcXhZYzR0eSt3RHFtWHJMRUk3?=
 =?utf-8?B?Q0xpYWU3eVVuc0IyWmMzblNNa3ovSDBaU3ROZ1VrcFMvQm5keWFDTDc2eGUy?=
 =?utf-8?B?am1mdzhvZkxWTEc4ekJSQVIzbXpwMldsUDZtWXk0aDRRMXpPSzVzSlZwTmpL?=
 =?utf-8?B?a0JqdjVpWGdJN0o0SmhwTDN0Wm9teWtiZWVCMldLbUwxNW52cmVUVnUybUZW?=
 =?utf-8?B?NkdyWlI3WTNUUWJNTGJCa1pXU2dDbURPRnJ5MWY5NkxOV1BaaHFLazY3eEVR?=
 =?utf-8?B?OGlkQXI3YkFibDBZTVVxdFRFOGdPb0hPK3M0Vm9hRlc4NHdSb1A3UlUrbHM2?=
 =?utf-8?Q?nOWc=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c5b49eae-b6ed-4324-0b1d-08dacd7fe3ce
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2022 18:23:59.5786
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: s/VuRT3eRMXRncZP7ehVAfneuQlmPkU/ievDKb93UBpnLrIbhcHDcazymnOdFIGK
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB7792
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS 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 Reinette,

On 11/22/22 18:14, Reinette Chatre wrote:
> 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 ?
Sure.
>
>> 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.

Will change it to:

The sysfs -> resctrl file "mon_features" will display the supported events
and files that can be used to configure those events if monitor
configuration is supported.


>
>> 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))


Local variable not requited. Will change it.

Thanks

Babu

>
>
> Reinette

-- 
Thanks
Babu Moger