Received-SPF: pass (google.com: domain of linux-kernel+bounces-167501-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <32b1b0e0-133f-4000-b4ab-9babaa2ae035@amd.com>
Date: Fri, 3 May 2024 15:37:01 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] perf/x86/rapl: Fix the energy-pkg event for AMD CPUs
To: Ingo Molnar <mingo@kernel.org>
Cc: tglx@linutronix.de, bp@alien8.de, dave.hansen@linux.intel.com,
 x86@kernel.org, peterz@infradead.org, mingo@redhat.com,
 ananth.narayan@amd.com, gautham.shenoy@amd.com,
 linux-perf-users@vger.kernel.org, linux-hardening@vger.kernel.org,
 linux-kernel@vger.kernel.org, sandipan.das@amd.com,
 alexander.shishkin@linux.intel.com, irogers@google.com,
 gustavoars@kernel.org, kprateek.nayak@amd.com, ravi.bangoria@amd.com
References: <20240502095115.177713-1-Dhananjay.Ugwekar@amd.com>
 <20240502095115.177713-3-Dhananjay.Ugwekar@amd.com>
 <ZjS0Uio+nvZLLmyb@gmail.com>
Content-Language: en-US
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
In-Reply-To: <ZjS0Uio+nvZLLmyb@gmail.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?dzdDZ3QyTmJ1S1VHY1g5d1ZoZTBUYmVzbHhaQ3F2NjYvZnhaeWQ1Zi9iUlY5?=
 =?utf-8?B?a1dWOWN5QWt2VkNDemRkdmtuUkkwa0h2cElxR1BaY3R5eWN2NlhuRzBOM0lO?=
 =?utf-8?B?YUFpVWlCU0Iwekw2endTS3M4Y05NRmVJVXkyVHVSQUczKzd0WFZlVmJmVnN6?=
 =?utf-8?B?VW8venNTNE53MGgrMGROMmxRck9NRXRjSDVmZTBhUFJxcTRQS1E0M1FrRGE4?=
 =?utf-8?B?aE5wUC8rVExnRFpDSkV6S0ZDL1lkeG5weng1bnlZUkc2b1Zkd0pLVjRlb0p5?=
 =?utf-8?B?WWZQL0xwb0o0V1dDRnJiS1h0RzRVVW0rcVdGVnVyWk1CVUY0UE13MVhpcEVw?=
 =?utf-8?B?VXh2eEtldmhURUdDNVhWMm13QXpLRE5qU1JqaWtrM0lzN3lQd1VGSVJHQU9k?=
 =?utf-8?B?L281RU9OczUyUStaQ0d1cnVONDdESGx0NlBZb1VHSm80UGFMeTRNUENiakNq?=
 =?utf-8?B?ODJ5YXpTME5ONmh1amVJdzczTmtoTXJzdFR5Tko5VTZuYTYyTGhrWWJpMlBh?=
 =?utf-8?B?VEhNRkd6eHdrQXRIRVoxaldxanBMWGJTT25KUExXTFFIMU1KUUV3ckVkeVVu?=
 =?utf-8?B?REVRSVZRUnVyc0pkOHVYSWQ1Y1NvbHQvTEVyTFNCSXRick1jaEcwb01oc2E0?=
 =?utf-8?B?bWx4YmVleUN4eVBEdnhTOXl2YTFkdXRUT3NsWEhscllyVWJvYmZHcTNEajRH?=
 =?utf-8?B?YVNsZE80Y2pBNGovV0d2Wk16MEptcjFPall3eFhHUjVKUlg3WjB6QXkvM1Zi?=
 =?utf-8?B?S3o1QWRwdFJDOWowL3hLV21GNStEL2xJWGQxQ1I5ejZWdHcyamJ5ZC9wcC9P?=
 =?utf-8?B?dVZSWDd3NTVwOHI0LzZYTnpWZlAya2ZFdk9XS0VMYWVwZjMzb3dyQUpzYzZC?=
 =?utf-8?B?d1VIV1ZJeStjMU9RUW5UdXJhdzFwU2N3dDcrbS8wOW5kdEdLTGZkQkhlckVD?=
 =?utf-8?B?NE81TDQ2YmZHbHJWcDU2Nk1YenI4bDhNbloyK2hGTHJTZlg1b2VLekpSYms3?=
 =?utf-8?B?czNLVkZvc250SmRYQkU2M2pEZ2Y2TTEwckZ4YzdtSmNsc1JxV3l5bURqSVAw?=
 =?utf-8?B?aStLNlRUbGp1ckF5UUl5VVZpdjQ4V2xaanVyTEJ1U0xpTnloOTAwV2ZaYlRI?=
 =?utf-8?B?ZUV1ZjJZMExsejY4cDY3dFh0cTU5bmVCdG0rVm9pWVg5VHUvM3RjWTg5ckl4?=
 =?utf-8?B?d1p5MFlNSCtGMXdoeFVTcjJRTGVnMm9VVW9mYWRsTnN6dUJYLzZqTkhjUnNY?=
 =?utf-8?B?RGhIN3ZUMXpBejhweEdJTU9POXlmOUtnVi9ROG9WQXIreDZIN01pNzg1Q3JR?=
 =?utf-8?B?SThBWEJYbTVMdlFVN1lqOVpkRE05SFUvSEFJQVJCbTQ0TGdkMnNGWWdpcHZx?=
 =?utf-8?B?RXliamU4azkzZDZ1VUJIb1pQdXl1N09IYzBENmp5T1pkcWhTWEk5SDM4M3hY?=
 =?utf-8?B?SzNqQldhR0lkRkIwLzVKR2VTK3NjMHF0dmw0RUtoOWdnbDUzT3hadENGa2ZW?=
 =?utf-8?B?Tm9sL3RkTWVwZ3JWc1ZzRXlzaFhDZU5sVmh0bkFSUkhIMjZZaUozL2ppNVpm?=
 =?utf-8?B?VXQ0c2lkaGdaWmFVTmJVNytvTE5Cb00yb3p2bnR6MGlyTHhnNUg3aDZ3ZWpl?=
 =?utf-8?B?cXJsaFpxQjFNUXlDZEdkZ1hHbjhQQ1NtaUZZcFJUbGd1Z0ZRTUpHUndKT0FY?=
 =?utf-8?B?MW1Rei9CMHVvbjA3V1g4WHkrbUUxbi8rY2MrQXcrWjhvd1kwdEJwREFEay9p?=
 =?utf-8?B?NjdTTWtGMXVNOHdMdy8rQ2Z2Y0J1aVA4Ky9YemZIandHeHRtRjZma3ZRUzY3?=
 =?utf-8?B?VWpHSEROOFB6Y1lvNVNBblptbnVMU0RXRnREOG1IQ04vTFB5b3l3OEQ3Z1Rm?=
 =?utf-8?B?ZkZOY1N2blFaTjVMNENRUUFEQ1lGU21XL040bUFFMGVWRExpN0prZFFSRFZO?=
 =?utf-8?B?WGpNR1VmbURmMGJyMDlqUFYreGJ5Y25nYUtkMlZqbHFpNlVSakJ0WjlBWEpV?=
 =?utf-8?B?WW9NWVBZeXhXTHU5R21aM1R1TEhXVTk1eG5JdDVKWS9jM0ZwS0pkVjBUYWdF?=
 =?utf-8?B?aTYwMlpGRUszZjBJVzNNQ1ozTzZiZGZmdjhLNnJ6T084WHZyZnlna1pGVjJk?=
 =?utf-8?Q?2cGWPchvXiZ5MveT0hl1JdoNx?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ba15695-8298-459f-7fb8-08dc6b58cce4
X-MS-Exchange-CrossTenant-AuthSource: LV8PR12MB9207.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 May 2024 10:07:12.1621
 (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: jKwd1s8xBdMD5wT68ioLd2Dojm3xJkxwiZlieGQL7/Wu/z2t4QHUbZ3UkQ6mVlnGfcKlJadvxlnVkIOGOhKkFA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4424

Hi Ingo,

On 5/3/2024 3:24 PM, Ingo Molnar wrote:
> 
> * Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> wrote:
> 
>> After commit ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf"),
>> on AMD processors that support extended CPUID leaf 0x80000026, the
>> topology_die_cpumask() and topology_logical_die_id() macros, no longer 
>> return the package cpumask and package id, instead they return the CCD 
>> (Core Complex Die) mask and id respectively. This leads to the energy-pkg 
>> event scope to be modified to CCD instead of package.
>>
>> Replacing these macros with their package counterparts fixes the
>> energy-pkg event for AMD CPUs.
>>
>> However due to the difference between the scope of energy-pkg event for 
>> Intel and AMD CPUs, we have to replace these macros conditionally only for
>> AMD CPUs.
>>
>> On a 12 CCD 1 Package AMD Zen4 Genoa machine:
>>
>> Before:
>> $ cat /sys/devices/power/cpumask
>> 0,8,16,24,32,40,48,56,64,72,80,88.
>>
>> The expected cpumask here is supposed to be just "0", as it is a package 
>> scope event, only one CPU will be collecting the event for all the CPUs in 
>> the package.
>>
>> After:
>> $ cat /sys/devices/power/cpumask
>> 0
>>
>> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
>> Fixes: 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf")
>> ---
>>  arch/x86/events/rapl.c | 30 ++++++++++++++++++++++++++----
>>  1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
>> index 46e673585560..d77bf7959a43 100644
>> --- a/arch/x86/events/rapl.c
>> +++ b/arch/x86/events/rapl.c
>> @@ -102,6 +102,10 @@ static struct perf_pmu_events_attr event_attr_##v = {				\
>>  	.event_str	= str,							\
>>  };
>>  
>> +#define rapl_pmu_is_pkg_scope()				\
>> +	(boot_cpu_data.x86_vendor == X86_VENDOR_AMD ||	\
>> +	 boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
>> +
>>  struct rapl_pmu {
>>  	raw_spinlock_t		lock;
>>  	int			n_active;
>> @@ -139,9 +143,21 @@ static unsigned int rapl_cntr_mask;
>>  static u64 rapl_timer_ms;
>>  static struct perf_msr *rapl_msrs;
>>  
>> +static inline unsigned int get_rapl_pmu_idx(int cpu)
>> +{
>> +	return rapl_pmu_is_pkg_scope() ? topology_logical_package_id(cpu) :
>> +					 topology_logical_die_id(cpu);
>> +}
>> +
>> +static inline cpumask_t *get_rapl_pmu_cpumask(int cpu)
>> +{
>> +	return rapl_pmu_is_pkg_scope() ? topology_core_cpumask(cpu) :
>> +					 topology_die_cpumask(cpu);
>> +}
> 
> Note that this breaks the build with this config:
> 
>   https://download.01.org/0day-ci/archive/20240503/202405030828.RgFuznL9-lkp@intel.com/config
> 
> I've removed the commit from perf/core for now.

Sure, will fix this in v2.

Regards,
Dhananjay

> 
> Thanks,
> 
> 	Ingo