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: <b5c6c0c2-6ec7-a01e-75cb-113324298324@intel.com>
Date:   Thu, 15 Dec 2022 09:11:27 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.6.0
Subject: Re: [PATCH v9 03/13] x86/cpufeatures: Add Bandwidth Monitoring Event
 Configuration feature flag
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>, <christophe.leroy@csgroup.eu>,
        <jarkko@kernel.org>, <adrian.hunter@intel.com>,
        <quic_jiles@quicinc.com>, <peternewman@google.com>
References: <166990882621.17806.16780480657453071426.stgit@bmoger-ubuntu>
 <166990897968.17806.16187773270520553795.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <166990897968.17806.16187773270520553795.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?azhoSzI0TWZ4THBKSlNZWW9aV2w1WWZWVXhMckpBWnY1ek44VzV0ejFTYkVK?=
 =?utf-8?B?VUFLYnl4cTVvQUlqWFRmWnFIMGlOWndya3RSOXB6c3gzVEJHNDR2QXNuV2xm?=
 =?utf-8?B?Y2U4bnJJTm1rbHlrdnBjcjR6SE9mbUpLcjMwM2dTS3dLS01ySEFnOTY5VTJN?=
 =?utf-8?B?TzRKeXpSUmxGN1R1Uks0cHREOTBZbUhUUVJFTTcvOFhZSzFQTlhrdmJkclo0?=
 =?utf-8?B?L08zMytuTmRWaWZibjFMZlFmeDM5UmVZNWo0TzRiSlIzMFEzS2htQUgxdThI?=
 =?utf-8?B?T2I1OUxVcnI4czlEek1BVU0ycVVlMkdNZ3ZBTlVCSXBGSjNsOWFmZWY2RDYr?=
 =?utf-8?B?ckMwWXpnNG5CUUFnM01NLy80M0xYb2xkMlZzNktVZkR3Tm94SmxZQzdXM1gx?=
 =?utf-8?B?OUhkLzhhelJzUXRxK1lFVzFiS1FHMDNwanBGWDZMZHl3cW1pUU9Pc2llV1FC?=
 =?utf-8?B?UW53RFp5cHJOaHQxa2l0UXdzQTBCSzNZMWtZbHUzai95NUNaalowSXlMVnVB?=
 =?utf-8?B?M2R5ZDVTOE5GelRCZVpiRHc4OWJpclJ6WGtjV2tNSUhKOFE0a0xnVGw1Z1hz?=
 =?utf-8?B?QzZGU2JIY09aM3BPdU90dkt2d2o2Z2lqTTlHTUF4L1Z2bXRJT3AzK0RNaUR6?=
 =?utf-8?B?NTEvNGFvU2U5YnBBZTgyVXI1Q3ZHZnNidk9hRFluUmVodm5qYnBtelB0akt2?=
 =?utf-8?B?L1g0S0VTYW1sdm1ZT2J4SmJaMnZvN2xWbDgzMTBqWmRxU2oxcjBncTZleHI4?=
 =?utf-8?B?dnVnRzRDWUk2OXdRMUdOem50UFd5V1lBY2w3cWpyN0NzM0dkcjkxbjVvS2ds?=
 =?utf-8?B?eGErOGFuTEdFVTRXSjE4S1h2djd3UUc3VU5OdWYvbjF6MkNISWNTSFpwd2Va?=
 =?utf-8?B?Q0k5M1JNS1pqSkFKd3g4eVo0aUgxWFo1WDl3UExJTDZWSzdhRGQ2TGloY0s3?=
 =?utf-8?B?SHNKcWROYlNwWVpNZjNwTU9BRDJtMnRtRmxqYTBGdzBvd1NZVWhJZ3FjTG5i?=
 =?utf-8?B?WWdtRzBacG50T2JtalBFNHJzcjRoclFud3RyeEdYR01La2JqMWtpQW1JLy84?=
 =?utf-8?B?bjBpbDF6MU16cFhKWlBkMWptSEY5OHdmVU9UQndoc0I3c0tHQlo3aDVPbTRJ?=
 =?utf-8?B?UEhXbUJZYTZLYjBsRk5YU295dTAvVWVkUTFtWVlYTGxpVmJURytpeVBoWG81?=
 =?utf-8?B?aWRsWVhUWWhIZGhQeGs2TDZWK1gzbnRWeHRteCtDT3E1RHNpdk9tSndrZkFr?=
 =?utf-8?B?N21kdUdmbHh0TWp3OHFTdzNIaDE2UERnYjJFazN6bnBwRElBc0YwelQ5N1hy?=
 =?utf-8?B?dFdIa004U3BVK2g5eFlEREE1VGxVa0RRbThpY1JoZEZHUTNOMERvcHpCN0lu?=
 =?utf-8?B?ekxnTm1NZ05RQUJKRUF3MTduY0p2aVM5U09ocVFIZzFXVzUyZE11MStlbDZZ?=
 =?utf-8?B?ODVlcXRWSzl3Qkw5WmRpbUN2WHVsUWhkd0Z4VFFSR0VYNVY3aWlLV1BHbzM4?=
 =?utf-8?B?bEVxKzJFMENkQmJQSDJQMWprWjZsUVNzSStkZ1JqbzhRME54RmZteE9YWWRu?=
 =?utf-8?B?SkpQT0J3N1RDS0xwZS9lSW1WK0x6WmFoOFhYNEpOenFqM0JDM01ZUmRDK3Bq?=
 =?utf-8?B?RTRqMlliQmZISGpUaXlyZGw4Mzc4Q0U1amltbXd6U1dsY05aTmN6M2NJN0hM?=
 =?utf-8?B?M3QvTmdJZGFvazdLVDRFa0x3TGptY2ZvbHhOV3RaWXN1ZUdkOHZwd1V2c2Rq?=
 =?utf-8?B?VzYzbnRLSWE5SjMyeG5UcHdOUDRTRC9KY2k3K0ZZN1NWZVJKNmxJSVB1c2F5?=
 =?utf-8?B?bWdxa3N3WDg4S2lnVWF0SHR4WWZkYU51dVRlSGE1a3VNUnc1UmUrNUxVOVVR?=
 =?utf-8?B?VWNySEIyY2dEKzdFSFlDUnJkdzNCb01BQWRoL2I2RFFjVnhpQ29TbmliSkgz?=
 =?utf-8?B?TVlrVlJ5c08rVVhtQlhpb0UzYzNISGcxRU1aaHk0dUMwSkhCVXFDN2ZPYjZq?=
 =?utf-8?B?SDhqNVFlK2RPZjhXNVE1QTJpQWhxVHBpYnlvNDJ2NStTLzhYWEkrVUFvVlhs?=
 =?utf-8?B?MHNUYTNyQTliQnNPMW9sYW5INzUvVDIxcDdqWDhmWm9iWUxMWVBWakkwS2Yv?=
 =?utf-8?B?Y1dZLzZQRDZyYmV0WWtIOGczcDVqdmgvRjZSb0dOaGlVQktISDZUR294NCtS?=
 =?utf-8?B?Zmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a9de2ae-013c-46ca-9fd7-08dadebf6949
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2022 17:11:31.5867
 (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: wysy2hr7WXz9/aL/X0bty8thvFdGVDFLbgFWgxkUrU+ovvi3WCGI1OU3/W2NjT3giT+MsAg1Wboual5HwpeTSb1a1+mVsnUSJipxTkMqh98=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5329
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 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_HI,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 12/1/2022 7:36 AM, Babu Moger wrote:
> Newer AMD processors support the new feature Bandwidth Monitoring Event
> Configuration (BMEC).
> 
> The feature support is identified via CPUID Fn8000_0020_EBX_x0 (ECX=0).
> Bits    Field Name    Description
> 3       EVT_CFG       Bandwidth Monitoring Event Configuration (BMEC)
> 
> Currently, the bandwidth monitoring events mbm_total_bytes and

Please drop "Currently,".

> mbm_local_bytes are set to count all the total and local reads/writes
> respectively. With the introduction of slow memory, the two counters
> are not enough to count all the different types of memory events. With
> the feature BMEC, the users have the option to configure
> mbm_total_bytes and mbm_local_bytes to count the specific type of
> events.
> 
> Each BMEC event has a configuration MSR, which contains one field for
> each bandwidth type that can be used to configure the bandwidth event
> to track any combination of supported bandwidth types. The event will
> count requests from every bandwidth type bit that is set in the
> corresponding configuration register.
> 
> Following are the types of events supported:
> 
> ====    ========================================================
> Bits    Description
> ====    ========================================================
> 6       Dirty Victims from the QOS domain to all types of memory
> 5       Reads to slow memory in the non-local NUMA domain
> 4       Reads to slow memory in the local NUMA domain
> 3       Non-temporal writes to non-local NUMA domain
> 2       Non-temporal writes to local NUMA domain
> 1       Reads to memory in the non-local NUMA domain
> 0       Reads to memory in the local NUMA domain
> ====    ========================================================
> 
> By default, the mbm_total_bytes configuration is set to 0x7F to count
> all the event types and the mbm_local_bytes configuration is set to
> 0x15 to count all the local memory events.
> 
> Feature description is available in the specification, "AMD64
> Technology Platform Quality of Service Extensions, Revision: 1.03
> Publication

Missing end quote above.

> 
> Link: https://www.amd.com/en/support/tech-docs/amd64-technology-platform-quality-service-extensions
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> Signed-off-by: Babu Moger <babu.moger@amd.com>

Same comment about "Link:" ordering as for patch 1.

> ---
>  arch/x86/include/asm/cpufeatures.h |    1 +
>  arch/x86/kernel/cpu/cpuid-deps.c   |    2 ++
>  arch/x86/kernel/cpu/scattered.c    |    1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index b6a45e56cd0c..415796d7b309 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -308,6 +308,7 @@
>  #define X86_FEATURE_CALL_DEPTH		(11*32+19) /* "" Call depth tracking for RSB stuffing */
>  #define X86_FEATURE_MSR_TSX_CTRL	(11*32+20) /* "" MSR IA32_TSX_CTRL (Intel) implemented */
>  #define X86_FEATURE_SMBA		(11*32+21) /* Slow Memory Bandwidth Allocation */
> +#define X86_FEATURE_BMEC		(11*32+22) /* Bandwidth Monitoring Event Configuration */
>  
>  /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
>  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index d95221117129..f6748c8bd647 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -68,6 +68,8 @@ static const struct cpuid_dep cpuid_deps[] = {
>  	{ X86_FEATURE_CQM_OCCUP_LLC,		X86_FEATURE_CQM_LLC   },
>  	{ X86_FEATURE_CQM_MBM_TOTAL,		X86_FEATURE_CQM_LLC   },
>  	{ X86_FEATURE_CQM_MBM_LOCAL,		X86_FEATURE_CQM_LLC   },
> +	{ X86_FEATURE_BMEC,			X86_FEATURE_CQM_MBM_TOTAL   },
> +	{ X86_FEATURE_BMEC,			X86_FEATURE_CQM_MBM_LOCAL   },
>  	{ X86_FEATURE_AVX512_BF16,		X86_FEATURE_AVX512VL  },
>  	{ X86_FEATURE_AVX512_FP16,		X86_FEATURE_AVX512BW  },
>  	{ X86_FEATURE_ENQCMD,			X86_FEATURE_XSAVES    },
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index d925753084fb..0dad49a09b7a 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -46,6 +46,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>  	{ X86_FEATURE_PROC_FEEDBACK,    CPUID_EDX, 11, 0x80000007, 0 },
>  	{ X86_FEATURE_MBA,		CPUID_EBX,  6, 0x80000008, 0 },
>  	{ X86_FEATURE_SMBA,		CPUID_EBX,  2, 0x80000020, 0 },
> +	{ X86_FEATURE_BMEC,		CPUID_EBX,  3, 0x80000020, 0 },
>  	{ X86_FEATURE_PERFMON_V2,	CPUID_EAX,  0, 0x80000022, 0 },
>  	{ X86_FEATURE_AMD_LBR_V2,	CPUID_EAX,  1, 0x80000022, 0 },
>  	{ 0, 0, 0, 0, 0 }
> 
> 

With changelog comments addressed:

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette