1 files changed, 41 insertions, 35 deletions

diff --git a/arch/x86/machine/pmu_intel.c b/arch/x86/machine/pmu_intel.c
index 0833a71..f8168bb 100644
--- a/arch/x86/machine/pmu_intel.c
+++ b/arch/x86/machine/pmu_intel.c
@@ -14,14 +14,12 @@
  *
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * INTEL PMU driver module.
  */
 
+#include <assert.h>
+#include <errno.h>
 #include <stdint.h>
 
-#include <include/assert.h>
-#include <kern/error.h>
 #include <kern/init.h>
 #include <kern/log.h>
 #include <kern/perfmon.h>
@@ -76,6 +74,12 @@
 #define PMU_INTEL_ID_EVLEN_OFFSET       24
 #define PMU_INTEL_ID_EVLEN_MAX          7
 
+/*
+ * Global PMU properties.
+ *
+ * The bitmap is used to implement counter allocation, where each bit denotes
+ * whether a counter is available or not.
+ */
 struct pmu_intel {
     unsigned int version;
     unsigned int nr_pmcs;
@@ -84,12 +88,6 @@ struct pmu_intel {
     unsigned int events;
 };
 
-struct pmu_intel_event_code {
-    unsigned int hw_event_id;
-    unsigned short event_select;
-    unsigned short umask;
-};
-
 static struct pmu_intel pmu_intel;
 
 /*
@@ -103,6 +101,12 @@ static struct pmu_intel pmu_intel;
 #define PMU_INTEL_EVENT_BRANCH         0x20
 #define PMU_INTEL_EVENT_BRANCH_MISS    0x40
 
+struct pmu_intel_event_code {
+    unsigned int hw_event_id;
+    unsigned short event_select;
+    unsigned short umask;
+};
+
 static const unsigned int pmu_intel_raw_events[] = {
     [PERFMON_EV_CYCLE]          = PMU_INTEL_RE_CYCLE,
     [PERFMON_EV_REF_CYCLE]      = PMU_INTEL_RE_REF_CYCLE,
@@ -141,20 +145,24 @@ pmu_intel_ack_status(uint64_t status)
     return cpu_set_msr64(PMU_INTEL_MSR_GLOBAL_OVF_CTRL, status);
 }
 
+/*
+ * TODO use the compiler built-in once libgcc is linked again.
+ */
 static unsigned int
 pmu_popcount(unsigned int bits)
 {
-    unsigned int count = 0;
+    unsigned int count;
+
+    count = 0;
 
-    /* XXX: Dummy version of popcount. We should implement a faster one if it
-     * gets needed somewhere else.
-     */
     while (bits) {
         if (bits & 1) {
             count++;
         }
+
         bits >>= 1;
     }
+
     return count;
 }
 
@@ -167,8 +175,8 @@ pmu_intel_info(void)
     pmu = pmu_intel_get();
     nr_events = pmu_popcount(pmu->events);
     log_info("pmu: driver: intel, architectural v1\n"
-            "pmu: nr_pmcs: %u, pmc_width: %u, events: %#x, nr_events: %u\n",
-            pmu->nr_pmcs, pmu->pmc_width, pmu->events, nr_events);
+             "pmu: nr_pmcs: %u, pmc_width: %u, events: %#x, nr_events: %u\n",
+             pmu->nr_pmcs, pmu->pmc_width, pmu->events, nr_events);
 }
 
 static int
@@ -179,7 +187,6 @@ pmu_intel_translate(unsigned int *raw_event_idp, unsigned event_id)
     }
 
     *raw_event_idp = pmu_intel_raw_events[event_id];
-
     return 0;
 }
 
@@ -217,9 +224,7 @@ pmu_intel_free(unsigned int pmc_id)
 
     pmu = pmu_intel_get();
     mask = (1U << pmc_id);
-
     assert(!(pmu->pmc_bm & mask));
-
     pmu->pmc_bm |= mask;
 }
 
@@ -255,26 +260,24 @@ pmu_intel_read(unsigned int pmc_id)
     return cpu_get_msr64(PMU_INTEL_MSR_PMC0 + pmc_id);
 }
 
-#ifdef CONFIG_PERFMON_TEST
-
 static void
 pmu_intel_write(unsigned int pmc_id, uint64_t value)
 {
     cpu_set_msr64(PMU_INTEL_MSR_PMC0 + pmc_id, value);
 }
 
-#endif /* CONFIG_PERFMON_TEST */
-
+/*
+ * TODO Make the perfmon module handle basic overflow handling by polling
+ * counters.
+ */
 static void
-pmu_intel_handle_of_intr_v1(struct trap_frame *frame)
+pmu_intel_handle_of_intr_v1(void)
 {
     struct pmu_intel *pmu;
     unsigned int mask;
     uint64_t value;
     uint64_t prev;
 
-    (void)frame;
-
     pmu = pmu_intel_get();
 
     for (unsigned int pmc_id = 0; pmc_id != pmu->nr_pmcs; pmc_id++) {
@@ -301,37 +304,40 @@ pmu_intel_consume_bits(uint64_t *bits)
     int bit;
 
     bit = __builtin_ffsll(*bits) - 1;
+
     if (bit < 0) {
         return bit;
     }
-    *bits &= ~(1U << bit);
 
+    *bits &= ~(1U << bit);
     return bit;
 }
 
 static void
-pmu_intel_handle_of_intr_v2(struct trap_frame *frame)
+pmu_intel_handle_of_intr_v2(void)
 {
     struct pmu_intel *pmu;
     uint64_t status;
     int pmc_id;
 
-    (void)frame;
-
     status = pmu_intel_get_status();
+
     if (status == 0) {
         return;
     }
+
     pmu_intel_ack_status(status);
     pmu = pmu_intel_get();
 
-    /* XXX: Mask on all PMCs (we do not check FIXED counters status */
     status &= ((1U << pmu->pmc_width) - 1);
-    for(;;) {
+
+    for (;;) {
         pmc_id = pmu_intel_consume_bits(&status);
+
         if (pmc_id < 0) {
             break;
         }
+
         perfmon_cpu_pmc_inc_of(pmc_id);
     }
 }
@@ -369,6 +375,7 @@ pmu_intel_setup(void)
     cpu_cpuid(&eax, &ebx, &ecx, &edx);
     pmu->version = eax & PMU_INTEL_ID_VERSION_MASK;
 
+    /* TODO Check this */
     if ((pmu->version == 0) || (pmu->version > 3)) {
         return ENODEV;
     }
@@ -391,15 +398,14 @@ pmu_intel_setup(void)
     pmu_driver.start = pmu_intel_start;
     pmu_driver.stop = pmu_intel_stop;
     pmu_driver.read = pmu_intel_read;
+    pmu_driver.write = pmu_intel_write;
     pmu_driver.get_pmc_width = pmu_intel_get_pmc_width;
+
     if (pmu->version > 2) {
         pmu_driver.handle_of_intr = pmu_intel_handle_of_intr_v1;
     } else {
         pmu_driver.handle_of_intr = pmu_intel_handle_of_intr_v2;
     }
-#ifdef CONFIG_PERFMON_TEST
-    pmu_driver.write = pmu_intel_write;
-#endif
 
     return perfmon_pmu_register(&pmu_driver);
 }