From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755831Ab0C3NmJ (ORCPT ); Tue, 30 Mar 2010 09:42:09 -0400 Received: from tx2ehsobe001.messaging.microsoft.com ([65.55.88.11]:48591 "EHLO TX2EHSOBE001.bigfish.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752653Ab0C3NmH convert rfc822-to-8bit (ORCPT ); Tue, 30 Mar 2010 09:42:07 -0400 X-SpamScore: -28 X-BigFish: VPS-28(zba6lz1432R98dN936eM62a3L9371Pzz1202hzzz32i6bh2a8h61h) X-Spam-TCS-SCL: 0:0 X-FB-SS: 5, X-WSS-ID: 0L03KPM-02-5XH-02 X-M-MSG: Date: Tue, 30 Mar 2010 15:41:45 +0200 From: Robert Richter To: Stephane Eranian CC: Peter Zijlstra , Ingo Molnar , LKML Subject: Re: [PATCH 0/3] perf/core, x86: unify perfctr bitmasks Message-ID: <20100330134145.GI11907@erda.amd.com> References: <1269880612-25800-1-git-send-email-robert.richter@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Content-Transfer-Encoding: 8BIT X-OriginalArrivalTime: 30 Mar 2010 13:41:45.0855 (UTC) FILETIME=[BD5C68F0:01CAD00E] X-Reverse-DNS: ausb3extmailp02.amd.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30.03.10 12:11:46, Stephane Eranian wrote: > On Mon, Mar 29, 2010 at 6:36 PM, Robert Richter wrote: > > This patch set unifies performance counter bit masks for x86. All mask > > are almost the same for all x86 models and thus can use the same macro > > definitions in arch/x86/include/asm/perf_event.h. It removes duplicate > > code. There is also a patch that reverts some changes of the big > > perf_counter -> perf_event rename. > > > > But there are still fields which are unique to each vendor: > - GUEST vs. HOST on AMD > - ANY_THREAD on Intel. > > For instance, I noticed that in > > arch/x86/kernel/cpu/perf_event.c:__hw_perf_event_init(): > > if (attr->type == PERF_TYPE_RAW) { > hwc->config |= x86_pmu.raw_event(attr->config); > if ((hwc->config & ARCH_PERFMON_EVENTSEL_ANY) && > perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN)) > return -EACCES; > return 0; > } > > Assumes ANY also exists on AMD processors. That is not the case. > This check needs to be moved into an Intel specific function. Generally, ARCH_PERFMON_EVENTSEL_* refers to: Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3B: System Programming Guide, Part 2 30.2 ARCHITECTURAL PERFORMANCE MONITORING Appendix A: Performance-Monitoring Events Appendix B: Model-Specific Registers (MSRs) and AMD64_EVENTSEL_* to: AMD64 Architecture Programmer's Manual Volume 2: System Programming 13.3.1 Performance Counters X86_* is generic. If a feature is available from both vendors, the names shouln't be changed. Instead the first introduced mask should be used (at least this is my suggestion). So, there are some ARCH_PERFMON_EVENTSEL_* masks that are Intel only, which is true for ARCH_PERFMON_EVENTSEL_ANY. And indead, the code should be checked for this. ARCH_PERFMON_EVENTSEL_ANY is always cleared on AMD cpus, so this code is ok. Actually the bit is cleared for *all* cpus in x86_pmu_raw_event(), the code was and is broken for this. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com