Minor changes

1 files changed, 23 insertions, 29 deletions

diff --git a/arch/x86/machine/pmu_amd.c b/arch/x86/machine/pmu_amd.c
index 4d2a4f8..105ba50 100644
--- a/arch/x86/machine/pmu_amd.c
+++ b/arch/x86/machine/pmu_amd.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/>.
- *
- * AMD 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>
@@ -40,7 +38,7 @@
 #define PMU_AMD_RE_DCACHE_REF       6
 #define PMU_AMD_RE_DCACHE_MISS      7
 #define PMU_AMD_RE_IFETCH_STALL     8
-
+#define PMU_AMD_RE_INVALID          ((unsigned int)-1)
 
 /*
  * PMU MSR addresses
@@ -57,23 +55,29 @@
 #define PMU_AMD_EVTSEL_EN   0x00400000
 
 /*
- * AMD PMU properties seem to be identical across all processors despite
- * many of them being implementation-specific.
+ * XXX These properties have the minimum values required by the architecture.
+ * TODO Per-family/model event availability database.
  */
 #define PMU_AMD_NR_PMCS     4
 #define PMU_AMD_PMC_WIDTH   48
 
+/*
+ * Global PMU properties.
+ *
+ * The bitmap is used to implement counter allocation, where each bit denotes
+ * whether a counter is available or not.
+ */
 struct pmu_amd {
     unsigned int pmc_bm;
 };
 
+static struct pmu_amd pmu_amd;
+
 struct pmu_amd_event_code {
     unsigned short event_select;
     unsigned short umask;
 };
 
-static struct pmu_amd pmu_amd;
-
 /*
  * TODO Per-family/model event availability database.
  */
@@ -89,8 +93,6 @@ static const struct pmu_amd_event_code pmu_amd_event_codes[] = {
     [PMU_AMD_RE_IFETCH_STALL]   = { 0x87, 0x00 },
 };
 
-#define PMU_AMD_RE_INVALID ((unsigned int)-1)
-
 static const unsigned int pmu_amd_generic_events[] = {
     [PERFMON_EV_CYCLE]          = PMU_AMD_RE_CYCLE,
     [PERFMON_EV_REF_CYCLE]      = PMU_AMD_RE_INVALID,
@@ -120,7 +122,6 @@ pmu_amd_translate(unsigned int *raw_event_idp, unsigned int event_id)
     assert(event_id < ARRAY_SIZE(pmu_amd_generic_events));
 
     *raw_event_idp = pmu_amd_generic_events[event_id];
-
     return 0;
 }
 
@@ -130,7 +131,7 @@ pmu_amd_alloc(unsigned int *pmc_idp, unsigned int raw_event_id)
     struct pmu_amd *pmu;
     unsigned int pmc_id;
 
-    /* TODO Check raw event availability */
+    /* TODO Per-family/model event availability database */
     (void)raw_event_id;
 
     pmu = pmu_amd_get();
@@ -156,9 +157,7 @@ pmu_amd_free(unsigned int pmc_id)
 
     pmu = pmu_amd_get();
     mask = (1U << pmc_id);
-
     assert(!(pmu->pmc_bm & mask));
-
     pmu->pmc_bm |= mask;
 }
 
@@ -200,37 +199,35 @@ pmu_amd_read(unsigned int pmc_id)
     return cpu_get_msr64(PMU_AMD_MSR_PERCTR0 + pmc_id);
 }
 
-#ifdef CONFIG_PERFMON_TEST
-
 static void
 pmu_amd_write(unsigned int pmc_id, uint64_t value)
 {
     cpu_set_msr64(PMU_AMD_MSR_PERCTR0 + pmc_id, value);
 }
 
-#endif /* CONFIG_PERFMON_TEST */
-
+/*
+ * TODO Make the perfmon module handle basic overflow handling by polling
+ * counters.
+ */
 static void
-pmu_amd_handle_of_intr_v1(struct trap_frame *frame)
+pmu_amd_handle_of_intr_v1(void)
 {
     struct pmu_amd *pmu;
+    uint64_t value, prev;
     unsigned int mask;
-    uint64_t value;
-    uint64_t prev;
-
-    (void)frame;
 
     pmu = pmu_amd_get();
 
     for (unsigned int pmc_id = 0; pmc_id != PMU_AMD_NR_PMCS; pmc_id++) {
         mask = (1U << pmc_id);
+
         if (pmu->pmc_bm & mask) {
-            /* counter not enabled: can't overflow. */
             continue;
         }
 
         value = pmu_amd_read(pmc_id);
         prev = perfmon_cpu_pmc_get_prev(pmc_id);
+
         if (prev > value) {
             /* Overflow */
             perfmon_cpu_pmc_inc_of(pmc_id);
@@ -274,11 +271,9 @@ pmu_amd_setup(void)
     pmu_driver.start = pmu_amd_start;
     pmu_driver.stop = pmu_amd_stop;
     pmu_driver.read = pmu_amd_read;
+    pmu_driver.write = pmu_amd_write;
     pmu_driver.get_pmc_width = pmu_amd_get_pmc_width;
     pmu_driver.handle_of_intr = pmu_amd_handle_of_intr_v1;
-#ifdef CONFIG_PERFMON_TEST
-    pmu_driver.write = pmu_amd_write;
-#endif
 
     return perfmon_pmu_register(&pmu_driver);
 }
@@ -287,4 +282,3 @@ INIT_OP_DEFINE(pmu_amd_setup,
                INIT_OP_DEP(perfmon_bootstrap, true),
                INIT_OP_DEP(cpu_setup, true),
                INIT_OP_DEP(log_setup, true));
-