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: <575bf1d7-8780-8c54-851c-e9849d1815f8@intel.com>
Date:   Wed, 24 Aug 2022 14:15:33 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.12.0
Subject: Re: [PATCH v3 06/10] x86/resctrl: Introduce mon_configurable to
 detect Bandwidth Monitoring Event Configuration
Content-Language: en-US
To:     Babu Moger <babu.moger@amd.com>, <fenghua.yu@intel.com>,
        <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>
CC:     <eranian@google.com>, <dave.hansen@linux.intel.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <corbet@lwn.net>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
        <bagasdotme@gmail.com>
References: <166117559756.6695.16047463526634290701.stgit@bmoger-ubuntu>
 <166117582698.6695.3458866624522799072.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <166117582698.6695.3458866624522799072.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?K28zNllldkNKM2xSRHdhbkQwQ1NWMTkzb3J2MTVoTHdXMXpCMUQvSi90VUd4?=
 =?utf-8?B?VW0vc1JZUGhINlNiY0tjNS9NcnZoQTBldWpkSHNZczBVWlQ4TVcyd0psN2t6?=
 =?utf-8?B?eGNrT0RzSHZGVTdIMkNUZ09DQStWUFZubnI4NmZTQlQvdjZSY3o3dXNNMWNh?=
 =?utf-8?B?Vk9sR0VkWldDMjNNcDMwYjdrVStuOTR3emdrOUdYWkxUdnNlaW1MMTZmRDRT?=
 =?utf-8?B?RnhRTTBKUzBzVmtaWkl6VGh2dHNqa1ZOWk1KdDJvTmxKZy9ETG8xVXNhQ3pG?=
 =?utf-8?B?NXRhS0R3dHZGODlST05jL2pxd1QzVTNFSkNoSkN0aGhKZWhZT3E4SzAya3ND?=
 =?utf-8?B?S0ZDZGhzUFQvanFMNWdqMW1HYS9vMFZ4bDlqUVBkQ1lQKy9OTzJJZFBnSVRB?=
 =?utf-8?B?dS9ocUozZkxjZ3BuYzJDdThZcWJRV2FHbXF5VkIyRnpjOXA2cEpkZnpsVVh5?=
 =?utf-8?B?T3p4WkVIZU9RcnlrRVdiUW9IRnlxSUJ2dGZkNTZuZUtxaVQ4U0FKQnlURXVV?=
 =?utf-8?B?dm4zWGhhT3NnM3c1ZDZpZGRhSU8wd3VKRUpEUHlaeTZyUFBPZkh3RU9jamtk?=
 =?utf-8?B?NWFlWTZmMlVITDB2by9uQkoxNU9COW9vOGNHdzZXRVREQ1ZZTHRVRXJHckFu?=
 =?utf-8?B?Uy96VThkenpVUUsvSEFLNWhYZDZielBHUzM1VW1FdlYybGhrMWltY2pycHRx?=
 =?utf-8?B?MFFyalhQOHZnZWZ4c1hrVHpTLzZubkJiMmw2aTM3WlFyN2I3TzFxK2dOM3Zj?=
 =?utf-8?B?STMzMWhEWk1ZMGtBL0xzeWpTSG1RL0FzSm5IZ0NIU3dtMjVCQ0RoSVAvL1Js?=
 =?utf-8?B?Z2U3NnVwb1NNTUdkM09udll5cGhVRUVNMTNybDAvS0RoVXNBcXZTRElYQ1lC?=
 =?utf-8?B?QTg5bVIzKzYrQ2RvK0srSG03eEcxMFo3VlVhYzlvSU43M2sxRjNsc2w0dzZS?=
 =?utf-8?B?elVGOUgranRLUm5JL1JRZWhjczd4em1PZkJKOFRNOHQvbWd1dGpKV2JpcFF1?=
 =?utf-8?B?dTFDQ2ZjaDZBeER5cStFTXJDUVlkdDlHU1JQeEc2akJjRUxUTFZFMm5jbFM1?=
 =?utf-8?B?T3Q5MnpjNXdneFNKd01SWG44bUttTGdZTW5oZEtSNGxlVHJQYVg5ZzB4bmQ2?=
 =?utf-8?B?UHJMOUJHMWNqZDNvQ0xDZnNvVDdYcGJMR3BiT1NqcGhZcDZvNnBjVXRXbmlI?=
 =?utf-8?B?QVY5d1lwcGh0bUwwdmhhQnQzUHJISG85YmdEN2tFUVpITnpYaG9VMHRWQlcr?=
 =?utf-8?B?dFQzb1ZDUmxYSzhyU3lBakEvNTFUMFJERmkxMjk0SnQ4VWNUTzEzMXdmVHFq?=
 =?utf-8?B?WFIvSzJ3TGpXOG9zV0FSZ3BxSFViYlFQSkI5V1c5WFB5NWNpVXhXUmwvZ0lC?=
 =?utf-8?B?Q2VCVjltcUIvaVpoTGxvallMUG10Q0dNREY4d2dYS1NrN0FuU2FyRk44TTRJ?=
 =?utf-8?B?ais3VlVKRG5KcWVwdW8zbEhiU2FSWkpCaGJTdEUreHBIeFpTZlhOcUhhQXZj?=
 =?utf-8?B?cW5VdC9sdng5NlhoQ0RFeUxHYnZlK3d4bm8vc3B2TGQ4RFZSeEtZNWdwb0JV?=
 =?utf-8?B?ckl6TmZGdUxqaXorblRLVCtHRnVGdHRRZlRHK2NJUVVIUEVHRUYwM29JT01h?=
 =?utf-8?B?MnFhVVpqZ2Q3bjI2bmR1cHFoZUo2QWxMUkZjbXJodzloK3NreXVjNFgzUStQ?=
 =?utf-8?B?Q0ZSY1RlT1p5WWxwZVNjeCtVaGtoOEMyOTJwdk5oRTdzRmdia1RlaGFOZzVY?=
 =?utf-8?B?M2lZNHZ5a2ptUUlxVjNuZW9scTB0a2ZaS25uZ3lKZHJBOXFJSDNTMmlMcGRa?=
 =?utf-8?B?cHhTbTlrWVg4bURmcUM0MXdEamVXZjBZRHYyTm0vYXNCejVCTjM0VzdsNTh0?=
 =?utf-8?B?dGtlVjN0bnJYZkVqVlpqWmF0WEZvVjlZc1p4NlgzNnNubEFWeXYydy9Qa2pu?=
 =?utf-8?B?NXdaMnU4TVRIMG9rV09FMVR4QmRPMER5K2tQS2ovL3lxOEpZMFVzYzdsRVV2?=
 =?utf-8?B?eWdtRGhtOXkrd2l5V2FnN1gwT0FseFpYTDNOMUluYnJ2WEJkcVVuRGtNdmYv?=
 =?utf-8?B?OVpGQnZzaUZaVXRFcDhuV3d3WW54VkNSTW1SSTRHQ0V5b1pGRmtGeUIwbkVD?=
 =?utf-8?B?RXNpVndQTjBVSTF1UXQrcWgzdW1JR1VHdENsRmxhdGEwQktubmVkNUF4V2Vl?=
 =?utf-8?B?bmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2b841ba1-d05b-4a7d-28e4-08da8615c9a7
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Aug 2022 21:15:36.4300
 (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: lXaIWe+U7oja3Xz1gHWX5OmE06OUkcyqatoRgu40Z3RtqgA/Evwwlz3UnQQp449wW2qAoIvx/BZOI9my8j8wdJynKxV9N3Vylmkvizs8Clw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB2713
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_PASS,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 Babu,

On 8/22/2022 6:43 AM, Babu Moger wrote:
> Newer AMD processors support the new feature Bandwidth Monitoring Event
> Configuration (BMEC). The events mbm_total_bytes and mbm_local_bytes
> are configurable when this feature is present.
> 
> Set mon_configurable if the feature is available.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/kernel/cpu/resctrl/monitor.c  |   14 ++++++++++++++
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   17 +++++++++++++++++
>  include/linux/resctrl.h                |    1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index eaf25a234ff5..b9de417dac1c 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -682,6 +682,16 @@ static void l3_mon_evt_init(struct rdt_resource *r)
>  		list_add_tail(&mbm_local_event.list, &r->evt_list);
>  }
>  
> +
> +void __rdt_get_mon_l3_config_amd(struct rdt_resource *r)
> +{
> +	/*
> +	 * Check if CPU supports the Bandwidth Monitoring Event Configuration
> +	 */
> +	if (boot_cpu_has(X86_FEATURE_BMEC))
> +		r->mon_configurable = true;
> +}

Could this rather use rdt_cpu_has() with the added support for disabling
the feature via kernel parameter?


> +
>  int rdt_get_mon_l3_config(struct rdt_resource *r)
>  {
>  	unsigned int mbm_offset = boot_cpu_data.x86_cache_mbm_width_offset;
> @@ -714,6 +724,10 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
>  	if (ret)
>  		return ret;
>  
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> +		__rdt_get_mon_l3_config_amd(r);
> +
> +

Why is this vendor check needed? Is X86_FEATURE_BMEC not sufficient?

>  	l3_mon_evt_init(r);
>  
>  	r->mon_capable = true;
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index fc5286067201..855483b297a8 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -995,6 +995,16 @@ static int rdt_num_rmids_show(struct kernfs_open_file *of,
>  	return 0;
>  }
>  
> +static int rdt_mon_configurable_show(struct kernfs_open_file *of,
> +				     struct seq_file *seq, void *v)
> +{
> +	struct rdt_resource *r = of->kn->parent->priv;
> +
> +	seq_printf(seq, "%d\n", r->mon_configurable);

Why is this file needed? It seems that the next patches also introduce
files in support of this new feature that will make the actual configuration
data accessible - those files are only created if this feature is supported.
Would those files not be sufficient for user space to learn about the feature
support?

Reinette