First cut at hwpmc support on MIPS

[Joseph Koshy]

> Please review and let me know if you have comments.  I'd like to commit this
> in a week or so.

> http://people.freebsd.org/~gnn/mipshwpmc_1.diff

Nice work!

Review comments, adding to those already commented on:

1) There appears to be unnecessary gunk (UTF8?) in the manual page.

+.%B "MIPS32 24K Processor Core Family Software User<E2><80><99>s Manual"
...
+to a D-cache miss. The LSU can signal a <E2><80><9C>long stall<E2><80><9D> on a D-cache

etc.

You could use .Bq, .Sq.  Plain ASCII quote marks would also work here.

2) The manual page is missing a short description of the capabilities
   of these PMCs.

3) The code doesn't appear to support sampling; the manual page should
   mention this.

4) The debug printf() in mips_intr should be removed:

+static int
+mips_intr(int cpu, struct trapframe *tf)
+{
+       printf("intr\n");
+       return 0;
+}

5) It would be help to split "hwpmc_mips.c" into two files:

   - One for code specific to this PMC family.  Say "hwpmc_mips24k.c".
   - One for "generic" MIPS related code, e.g.,
     code to walk stacks.

   You should expect that every manufacturer will have their own kind
   of PMCs, with differing capabilities and programming models.

   Catering for the variability upfront would be prudent.  As
   commented on by earlier reviewers, you may want to consider
   changing symbol naming to suit.

56 These definitions will cause trouble on 64 bit MIPS systems:

+#define        PMCLOG_READADDR         PMCLOG_READ32
+#define        PMCLOG_EMITADDR         PMCLOG_EMIT32


7) From the definitions in the header file, these PMCs appear to
   support the concept of sampling based on processor mode:

+#define MIPS_PMC_USER_ENABLE           0x08    /* Count in USER mode */
+#define MIPS_PMC_SUPER_ENABLE          0x04    /* Count in SUPERVISOR mode */
+#define MIPS_PMC_KERNEL_ENABLE         0x02    /* Count in KERNEL mode */

   If that is the case, then you should support those modifiers in
   libpmc's event parsing.  The libpmc code in the patch appears to be
   a stub:

+static int
+mips_allocate_pmc(enum pmc_event pe, char *ctrspec __unused,
+                 struct pmc_op_pmcallocate *pmc_config __unused)
+{
+       switch (pe) {
+       default:
+               break;
+       }
+       
+       return (0);
+}


8) You can reduce the size of the following table in "hwpmc_mips.c",
   by treating the pe_counter field as a set of flags.

+struct mips_event_code_map {
+       enum pmc_event  pe_ev;      /* enum value */
+       uint8_t         pe_counter; /* Which counter this can be counted in. */
+       uint8_t         pe_code;    /* numeric code */
+};

+const struct mips_event_code_map mips_event_codes[] = {
+       { PMC_EV_MIPS_CYCLE, 0, 0},
+       { PMC_EV_MIPS_CYCLE, 1, 0},  <<<--- repeated information 


9) You'd want to support flags that control counting based on
   processor modes.  For this, you would want to pass down flags
   from userland and change the `pm_mips_evsel' field to suit:

+static int
+mips_allocate_pmc(int cpu, int ri, struct pmc *pm,
+  const struct pmc_op_pmcallocate *a)
+{
...
+       pm->pm_md.pm_mips.pm_mips_evsel = config;


10) If the number and width of these PMCs are fixed, you should
    document that in the manual page:

pmc_md_initialize()
{
+       mips_npmcs = 2;
...
+       pcd->pcd_width = 32; /* XXX: Fix for 64 bit MIPS */


Regards,
Koshy