On Wed, 2022-05-11 at 14:37 -0400, William Cohen wrote: > On 5/10/22 16:52, will schmidt wrote: > > On Wed, 2022-05-04 at 10:51 -0400, William Cohen wrote: > > > On 2/23/22 14:42, will schmidt wrote: > > > > On Wed, 2022-02-23 at 10:33 -0500, William Cohen wrote: > > > > > On 2/18/22 17:17, will schmidt wrote: > > > > > > Hi, > > > > > > I've created a fork of perfmon2/libpfm and pushed > > > > > > the powerpc/power10 support into that fork. > > > > > > > > > > > > > > > > <snippage> > > > > > Also should there be some tests for the power10 PMU events > > > > > added > > > > > to > > > > > tests/validate_power.c? > > > > > > > > > > -Will Cohen > > > > > > > > > > > > > Hi, > > > > > > > > I've spun up the changes to validate_power.c for power10, > > > > tested, > > > > and pushed > > > > them to my fork. > > > > > > > > [ I'm not familiar with the incantations, but the info > > > > below should be sufficient to get at the patch :-) ] > > > > > > > > https://sourceforge.net/u/willschm/perfmon2/ci/a3fd768a5879c93d43377b89450568c9fe9cb0d6/ > > > > > > > > git clone https://git.code.sf.net/u/willschm/perfmon2 willschm- > > > > perfmon2 > > > > > > > > commit a3fd768a5879c93d43377b89450568c9fe9cb0d6 (HEAD -> > > > > master, > > > > origin/master, origin/HEAD, > > > > refs/patches/master/pushed.perfmon_libpfm_update_verification_t > > > > est. > > > > diff > > > > ) > > > > Author: Will Schmidt <will_schm...@vnet.ibm.com> > > > > Date: Wed Feb 23 12:34:30 2022 -0600 > > > > > > > > Add power10 to tests/validate_power.c > > > > > > > > Update perfmon/tests/validate_power.c to add entries > > > > for power10. > > > > Tested successfully in a Power10 environment. > > > > > > > > Thanks > > > > -Will > > > > > > > > > > Hi, > > > > > > What is the status of these patches for power10? They are still > > > not > > > in the upstream libpfm master git repository. > > > > Hi, > > > > A few minutes ago I pushed this patch to my perfmon fork. > > > > This particular commit is at > > https://sourceforge.net/u/willschm/perfmon2/ci/60918a39237a0502fdd3de99a37d39a3d78100bb/ > > > > with the tree itself available via > > git clone https://git.code.sf.net/u/willschm/perfmon2 willschm- > > perfmon2 > > > > The relevant detail within is: > > > > /* > > * Documentation on the PMU events for Power10 can be found > > * in Appendix E of the Power10 Users Manual. > > * The Power10 manual is at > > * https://ibm.ent.box.com/v/power10usermanual > > * This and other PowerPC related documents can be found at > > * https://www-50.ibm.com/systems/power/openpower/ > > */ > > > > > > Thanks > > -Will (Schmidt) > > > > Hi Will Schmidt, > Thanks for the review. I have a few comments and follow-up questions below. I'm predicting the answers, and will start spinning up some fixes in the meantine.
Thanks, > > Thanks for the update with the pointer to the power10 event > documentation. I was looking though the patches and noticed that > following patch has PFM_PMU_POWER10 below the "/* MUST ADD NEW PMU > MODELS HERE */" in pfmlib.h. That comment should be adjacent to the > PFM_PMU_MAX. Good catch. I'll fix. > > https://sourceforge.net/u/willschm/perfmon2/ci/61697515bbc09d7568bd50665f3934bebc0c0485/ > > What is the utility of the enum power10_events? They are being used > to put the events in specific spots in the power10_pe[], but it looks > like the way things are generated > that the entries would end up at those locations in the array > anyway. Eliminating those would reduce > https://sourceforge.net/u/willschm/perfmon2/ci/3d783a6db770885d5eddafb390faf146a64d0acd/ > by 950+ lines. Yes.. For previous processors, instead of the enum we had a raw list of #defines, one for each of the event entries. My change here, wrt previous content was to try to simplify the text a bit, dropping the chain of defines for an enum was an improvement at least, allowing better isolated changes. It had not occurred to me to strip those out entirely. (I thought those values were also used for the papi event mapping, but upon further review there I see that is not the case..) I'l l give that a respin and confirm I'm not missing something more. > > The POWER9 has descriptions (.pme_sort_desc and .pme_long_desc) for > virtually all the events. For POWER10 some of the events don't have > descriptions. The _ALTx event don't have a descriptions, but the > first event has a description. Like: Yes. This was by design to try to save space. Early internal versions of this patch set were gargantuan. I will regenerate to get the currently blank descriptions filled in. Thanks -Will (Schmidt) > > > +[ POWER10_PME_PM_DATA_FROM_DL2L3_MOD ] = { > + .pme_name = "PM_DATA_FROM_DL2L3_MOD", > + .pme_code = 0x0E4240000001C040, > + .pme_short_desc = "Data Source;The processor's L1 data cache > was reloaded with a line in the M (exclusive) state from another > core's L2 or L3 from a distant chip due to a demand miss.", > + .pme_long_desc = "Data Source;The processor's L1 data cache was > reloaded with a line in the M (exclusive) state from another core's > L2 or L3 from a distant chip due to a demand miss.", > +}, > +[ POWER10_PME_PM_DATA_FROM_DL2L3_MOD_PMC2 ] = { > + .pme_name = "PM_DATA_FROM_DL2L3_MOD_ALT2", > + .pme_code = 0x0E4240000002C040, > + .pme_short_desc = "", > + .pme_long_desc = "", > +}, > +[ POWER10_PME_PM_DATA_FROM_DL2L3_MOD_PMC3 ] = { > + .pme_name = "PM_DATA_FROM_DL2L3_MOD_ALT3", > + .pme_code = 0x0E4240000003C040, > + .pme_short_desc = "", > + .pme_long_desc = "", > +}, > +[ POWER10_PME_PM_DATA_FROM_DL2L3_MOD_PMC4 ] = { > + .pme_name = "PM_DATA_FROM_DL2L3_MOD_ALT4", > + .pme_code = 0x0E4240000004C040, > + .pme_short_desc = "", > + .pme_long_desc = "", > +}, > > > -Will Cohen > _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel