Robert, Vince,
My main issue which this patch is that I wonder if the K7
support does qualify as a AMD64 PMU which is how this
patch would implement it.
Shouldn't we rather introduce an AMD_K7 PMU model
for libpfm. We could reuse most of the code but it would
logically be separate.
Opinions?
On 5/9/08, Robert Richter <[EMAIL PROTECTED]> wrote:
> Vince,
>
> thanks for your contibution. Please see my comments below.
>
> In general, please run checkpatch.pl that is part of the kernel
> scripts to keep the coding style. I found these errors:
>
> ERROR: code indent should use tabs where possible
> ERROR: spaces required around that '=' (ctx:VxV)
>
> -Robert
>
>
> On 02.05.08 21:40:18, Vince Weaver wrote:
> > Hello
> >
> > so here is code that adds "proper" support for the 32-bit Athlon systems
> > to libpfm.
> >
> > The event masks are a subset of the 64-bit versions with slight
> > differences. I split off a separate file insteald of using #defines
> > to separate it out.
> >
> > The values are based on the document
> > "AMD Athlon Processor x86 Code Optimization Guide". I've tested it on a
> > 32-bit Athlon system and it seems to work properly.
> >
> > I'm open to any comments people might have on this...
> >
> > Thanks,
> >
> > Vince
> >
> >
> > diff -u -r -N libpfm-3.4/lib/amd64_events.h
> libpfm-3.4-k7/lib/amd64_events.h
> > --- libpfm-3.4/lib/amd64_events.h 2008-04-25 09:46:04.000000000 -0400
> > +++ libpfm-3.4-k7/lib/amd64_events.h 2008-05-02 21:25:57.000000000 -0400
> > @@ -24,6 +24,7 @@
> > * applications on Linux.
> > */
> >
> > +#include "amd64_events_k7.h"
> > #include "amd64_events_k8.h"
> > #include "amd64_events_fam10h.h"
> >
> > @@ -34,6 +35,13 @@
> > unsigned int ret_inst;
> > };
> >
> > +static struct pme_amd64_table amd64_k7_table = {
> > + .num = PME_AMD64_K7_EVENT_COUNT,
> > + .events = amd64_k7_pe,
> > + .cpu_clks = PME_AMD64_K7_CPU_CLK_UNHALTED,
> > + .ret_inst = PME_AMD64_K7_RETIRED_INSTRUCTIONS,
> > +};
> > +
> > static struct pme_amd64_table amd64_k8_table = {
> > .num = PME_AMD64_K8_EVENT_COUNT,
> > .events = amd64_k8_pe,
> > diff -u -r -N libpfm-3.4/lib/amd64_events_k7.h
> libpfm-3.4-k7/lib/amd64_events_k7.h
> > --- libpfm-3.4/lib/amd64_events_k7.h 1969-12-31 19:00:00.000000000 -0500
> > +++ libpfm-3.4-k7/lib/amd64_events_k7.h 2008-05-02
> 21:26:26.000000000 -0400
> > @@ -0,0 +1,219 @@
> > +/*
> > + * Copyright (c) 2006, 2007 Advanced Micro Devices, Inc.
> > + * Contributed by Ray Bryant <[EMAIL PROTECTED]>
> > + * Contributed by Robert Richter <[EMAIL PROTECTED]>
> > + * Modified for K7 by Vince Weaver <vince _at_ csl.cornell.edu>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> a copy
> > + * of this software and associated documentation files (the "Software"),
> to deal
> > + * in the Software without restriction, including without limitation the
> rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
> sell copies
> > + * of the Software, and to permit persons to whom the Software is
> furnished to do so,
> > + * subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be
> included in all
> > + * copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR IMPLIED,
> > + * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> FITNESS FOR A
> > + * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS
> OR COPYRIGHT
> > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
> IN AN ACTION OF
> > + * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
> WITH THE SOFTWARE
> > + * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> > + *
> > + * This file is part of libpfm, a performance monitoring support library
> for
> > + * applications on Linux.
> > + */
> > +
> > +/*
> > + * Definitions taken from "AMD Athlon Processor x86 Code Optimization
> Guide"
> > + * Table 11 February 2002
> > + */
> > +
> > +static pme_amd64_entry_t amd64_k7_pe[]={
> > +/* 0 */{.pme_name = "DATA_CACHE_ACCESSES",
> > + .pme_code = 0x40,
> > + .pme_desc = "Data Cache Accesses",
> > + },
> > +/* 1 */{.pme_name = "DATA_CACHE_MISSES",
> > + .pme_code = 0x41,
> > + .pme_desc = "Data Cache Misses",
> > + },
> > +/* 2 */{.pme_name = "DATA_CACHE_REFILLS",
> > + .pme_code = 0x42,
> > + .pme_desc = "Data Cache Refills from L2",
> > + .pme_flags = PFMLIB_AMD64_UMASK_COMBO,
> > + .pme_numasks = 6,
> > + .pme_umasks = {
> > + { .pme_uname = "L2_INVALID",
> > + .pme_udesc = "Invalid line from L2",
> > + .pme_ucode = 0x01,
> > + },
> > + { .pme_uname = "L2_SHARED",
> > + .pme_udesc = "Shared-state line from L2",
> > + .pme_ucode = 0x02,
> > + },
> > + { .pme_uname = "L2_EXCLUSIVE",
> > + .pme_udesc = "Exclusive-state line from L2",
> > + .pme_ucode = 0x04,
> > + },
> > + { .pme_uname = "L2_OWNED",
> > + .pme_udesc = "Owned-state line from L2",
> > + .pme_ucode = 0x08,
> > + },
> > + { .pme_uname = "L2_MODIFIED",
> > + .pme_udesc = "Modified-state line from L2",
> > + .pme_ucode = 0x10,
> > + },
> > + { .pme_uname = "ALL",
> > + .pme_udesc = "Shared, Exclusive, Owned, Modified State
> Refills",
> > + .pme_ucode = 0x1F,
> > + },
> > + },
> > + },
> > +/* 3 */{.pme_name = "DATA_CACHE_REFILLS_FROM_SYSTEM",
> > + .pme_code = 0x43,
> > + .pme_desc = "Data Cache Refills from System",
> > + .pme_flags = PFMLIB_AMD64_UMASK_COMBO,
> > + .pme_numasks = 6,
> > + .pme_umasks = {
> > + { .pme_uname = "INVALID",
> > + .pme_udesc = "Invalid",
> > + .pme_ucode = 0x01,
> > + },
> > + { .pme_uname = "SHARED",
> > + .pme_udesc = "Shared",
> > + .pme_ucode = 0x02,
> > + },
> > + { .pme_uname = "EXCLUSIVE",
> > + .pme_udesc = "Exclusive",
> > + .pme_ucode = 0x04,
> > + },
> > + { .pme_uname = "OWNED",
> > + .pme_udesc = "Owned",
> > + .pme_ucode = 0x08,
> > + },
> > + { .pme_uname = "MODIFIED",
> > + .pme_udesc = "Modified",
> > + .pme_ucode = 0x10,
> > + },
> > + { .pme_uname = "ALL",
> > + .pme_udesc = "Invalid, Shared, Exclusive, Owned, Modified",
> > + .pme_ucode = 0x1F,
> > + },
> > + },
> > + },
> > +/* 4 */{.pme_name = "DATA_CACHE_LINES_EVICTED",
> > + .pme_code = 0x44,
> > + .pme_desc = "Data Cache Lines Evicted",
>
>
> The spec description here is "Data cache writebacks". Need to figure
> out if the K8 event is backward compatible to this one. Otherwise we
> should rename it according to the spec.
>
>
> > + .pme_flags = PFMLIB_AMD64_UMASK_COMBO,
> > + .pme_numasks = 6,
> > + .pme_umasks = {
> > + { .pme_uname = "INVALID",
> > + .pme_udesc = "Invalid",
> > + .pme_ucode = 0x01,
> > + },
> > + { .pme_uname = "SHARED",
> > + .pme_udesc = "Shared",
> > + .pme_ucode = 0x02,
> > + },
> > + { .pme_uname = "EXCLUSIVE",
> > + .pme_udesc = "Exclusive",
> > + .pme_ucode = 0x04,
> > + },
> > + { .pme_uname = "OWNED",
> > + .pme_udesc = "Owned",
> > + .pme_ucode = 0x08,
> > + },
> > + { .pme_uname = "MODIFIED",
> > + .pme_udesc = "Modified",
> > + .pme_ucode = 0x10,
> > + },
> > + { .pme_uname = "ALL",
> > + .pme_udesc = "Invalid, Shared, Exclusive, Owned, Modified",
> > + .pme_ucode = 0x1F,
> > + },
> > + },
> > + },
> > +/* 5 */{.pme_name = "L1_DTLB_MISS_AND_L2_DTLB_HIT",
> > + .pme_code = 0x45,
> > + .pme_desc = "L1 DTLB Miss and L2 DTLB Hit",
> > + },
> > +/* 6 */{.pme_name = "L1_DTLB_AND_L2_DTLB_MISS",
> > + .pme_code = 0x46,
> > + .pme_desc = "L1 DTLB and L2 DTLB Miss",
> > + },
> > +/* 7 */{.pme_name = "MISALIGNED_ACCESSES",
> > + .pme_code = 0x47,
> > + .pme_desc = "Misaligned Accesses",
> > + },
> > + /* CPU_CLK_UNHALTED is undocumented in the Athlon Guide? */
> > +/* 8 */{.pme_name = "CPU_CLK_UNHALTED",
> > + .pme_code = 0x76,
> > + .pme_desc = "CPU Clocks not Halted",
> > + },
>
>
> Will check if the event exists and is the same as for K8.
>
>
> > +/* 9 */{.pme_name = "INSTRUCTION_CACHE_FETCHES",
> > + .pme_code = 0x80,
> > + .pme_desc = "Instruction Cache Fetches",
> > + },
> > +/* 10 */{.pme_name = "INSTRUCTION_CACHE_MISSES",
> > + .pme_code = 0x81,
> > + .pme_desc = "Instruction Cache Misses",
> > + },
> > +/* 11 */{.pme_name = "L1_ITLB_MISS_AND_L2_ITLB_HIT",
> > + .pme_code = 0x84,
> > + .pme_desc = "L1 ITLB Miss and L2 ITLB Hit",
> > + },
> > +/* 12 */{.pme_name = "L1_ITLB_MISS_AND_L2_ITLB_MISS",
> > + .pme_code = 0x85,
> > + .pme_desc = "L1 ITLB Miss and L2 ITLB Miss",
> > + },
> > +/* 13 */{.pme_name = "RETIRED_INSTRUCTIONS",
> > + .pme_code = 0xC0,
> > + .pme_desc = "Retired Instructions (includes exceptions, interrupts,
> resyncs)",
> > + },
> > +/* 14 */{.pme_name = "RETIRED_UOPS",
> > + .pme_code = 0xC1,
> > + .pme_desc = "Retired uops",
> > + },
> > +/* 15 */{.pme_name = "RETIRED_BRANCH_INSTRUCTIONS",
> > + .pme_code = 0xC2,
> > + .pme_desc = "Retired Branch Instructions",
> > + },
> > +/* 16 */{.pme_name = "RETIRED_MISPREDICTED_BRANCH_INSTRUCTIONS",
> > + .pme_code = 0xC3,
> > + .pme_desc = "Retired Mispredicted Branch Instructions",
> > + },
> > +/* 17 */{.pme_name = "RETIRED_TAKEN_BRANCH_INSTRUCTIONS",
> > + .pme_code = 0xC4,
> > + .pme_desc = "Retired Taken Branch Instructions",
> > + },
> > +/* 18 */{.pme_name = "RETIRED_TAKEN_BRANCH_INSTRUCTIONS_MISPREDICTED",
> > + .pme_code = 0xC5,
> > + .pme_desc = "Retired Taken Branch Instructions Mispredicted",
> > + },
> > +/* 19 */{.pme_name = "RETIRED_FAR_CONTROL_TRANSFERS",
> > + .pme_code = 0xC6,
> > + .pme_desc = "Retired Far Control Transfers",
> > + },
> > +/* 20 */{.pme_name = "RETIRED_BRANCH_RESYNCS",
> > + .pme_code = 0xC7,
> > + .pme_desc = "Retired Branch Resyncs (only non-control transfer
> branches)",
> > + },
> > +/* 21 */{.pme_name = "INTERRUPTS_MASKED_CYCLES",
> > + .pme_code = 0xCD,
> > + .pme_desc = "Interrupts-Masked Cycles",
> > + },
> > +/* 22 */{.pme_name = "INTERRUPTS_MASKED_CYCLES_WITH_INTERRUPT_PENDING",
> > + .pme_code = 0xCE,
> > + .pme_desc = "Interrupts-Masked Cycles with Interrupt Pending",
> > + },
> > +/* 23 */{.pme_name = "INTERRUPTS_TAKEN",
> > + .pme_code = 0xCF,
> > + .pme_desc = "Interrupts Taken",
> > + },
> > +};
> > +
> > +#define PME_AMD64_K7_EVENT_COUNT
> (sizeof(amd64_k7_pe)/sizeof(pme_amd64_entry_t))
> > +#define PME_AMD64_K7_CPU_CLK_UNHALTED 8
> > +#define PME_AMD64_K7_RETIRED_INSTRUCTIONS 13
> > diff -u -r -N libpfm-3.4/lib/pfmlib_amd64.c
> libpfm-3.4-k7/lib/pfmlib_amd64.c
> > --- libpfm-3.4/lib/pfmlib_amd64.c 2008-04-25 09:46:04.000000000 -0400
> > +++ libpfm-3.4-k7/lib/pfmlib_amd64.c 2008-05-02 21:30:14.000000000 -0400
> > @@ -75,7 +75,8 @@
> > #define AMD64_FAM10H AMD64_FAM10H_REV_B
> >
> > typedef enum {
> > - AMD64_CPU_UN,
> > + AMD64_CPU_UN,
> > + AMD64_K7,
>
>
> Fix whitspaces please.
>
>
> > AMD64_K8_REV_B,
> > AMD64_K8_REV_C,
> > AMD64_K8_REV_D,
> > @@ -92,6 +93,7 @@
> >
> > static const char *amd64_cpu_strs[]= {
> > "unknown model",
> > + "K7",
>
>
> Dito.
>
>
> > "K8 RevB",
> > "K8 RevC",
> > "K8 RevD",
> > @@ -125,7 +127,10 @@
> > static amd64_rev_t
> > amd64_get_revision(int family, int model, int stepping)
> > {
> > - if (family == 15) {
> > + if (family == 6) {
> > + return AMD64_K7;
> > + }
> > + else if (family == 15) {
>
>
> use:
>
> } else if ...
>
>
> > switch (model >> 4) {
> > case 0:
> > if (model == 5 && stepping < 2)
> > @@ -185,16 +190,28 @@
> > amd64_cpu_strs[revision]);
> > amd64_support.pmu_name = amd64_pmu.name;
> >
> > - /* defaults (K8) */
> > - amd64_pmu.events = amd64_k8_table.events;
> > - amd64_support.pme_count = amd64_k8_table.num;
> > - amd64_pmu.cpu_clks = amd64_k8_table.cpu_clks;
> > - amd64_pmu.ret_inst = amd64_k8_table.ret_inst;
> > - amd64_support.pmu_type = PFMLIB_AMD64_PMU;
> > - amd64_support.num_cnt = PMU_AMD64_NUM_COUNTERS;
> > - amd64_support.pmc_count = PMU_AMD64_NUM_COUNTERS;
> > - amd64_support.pmd_count = PMU_AMD64_NUM_COUNTERS;
> > -
> > + if (amd64_pmu.revision ==AMD64_K7) {
>
> " == "
>
> > + amd64_pmu.events = amd64_k7_table.events;
> > + amd64_support.pme_count = amd64_k7_table.num;
> > + amd64_pmu.cpu_clks = amd64_k7_table.cpu_clks;
> > + amd64_pmu.ret_inst = amd64_k7_table.ret_inst;
> > + amd64_support.pmu_type = PFMLIB_AMD64_PMU;
> > + amd64_support.num_cnt = PMU_AMD64_NUM_COUNTERS;
> > + amd64_support.pmc_count = PMU_AMD64_NUM_COUNTERS;
> > + amd64_support.pmd_count = PMU_AMD64_NUM_COUNTERS;
>
>
> Better return from function here and leave the rest as is.
>
> When changing indention for '=', please change it in the whole
> function.
>
>
> > + }
> > + else {
> > + /* defaults (K8) */
> > + amd64_pmu.events = amd64_k8_table.events;
> > + amd64_support.pme_count = amd64_k8_table.num;
> > + amd64_pmu.cpu_clks = amd64_k8_table.cpu_clks;
> > + amd64_pmu.ret_inst = amd64_k8_table.ret_inst;
> > + amd64_support.pmu_type = PFMLIB_AMD64_PMU;
> > + amd64_support.num_cnt = PMU_AMD64_NUM_COUNTERS;
> > + amd64_support.pmc_count = PMU_AMD64_NUM_COUNTERS;
> > + amd64_support.pmd_count = PMU_AMD64_NUM_COUNTERS;
> > + }
> > +
>
>
> Use tabs only for indention.
>
>
> > /* additional features */
> > if (IS_FAMILY_10H()) {
> > amd64_pmu.events = amd64_fam10h_table.events;
> >
> >
> > -------------------------------------------------------------------------
> > This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
> > Don't miss this year's exciting event. There's still time to save $100.
> > Use priority code J8TL2D2.
> >
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> > _______________________________________________
> > perfmon2-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
> >
>
>
> --
> Advanced Micro Devices, Inc.
> Operating System Research Center
> email: [EMAIL PROTECTED]
>
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
> Don't miss this year's exciting event. There's still time to save $100.
> Use priority code J8TL2D2.
>
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> perfmon2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
>
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
perfmon2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
