From d51166f15e1c08fdaa3df0992553ad5127da220c Mon Sep 17 00:00:00 2001 From: jhb Date: Fri, 14 Aug 2009 21:05:08 +0000 Subject: [PATCH] Adjust the handling of the local APIC PMC interrupt vector: - Provide lapic_disable_pmc(), lapic_enable_pmc(), and lapic_reenable_pmc() routines in the local APIC code that the hwpmc(4) driver can use to manage the local APIC PMC interrupt vector. - Do not enable the local APIC PMC interrupt vector by default when HWPMC_HOOKS is enabled. Instead, the hwpmc(4) driver explicitly enables the interrupt when it is succesfully initialized and disables the interrupt when it is unloaded. This avoids enabling the interrupt on unsupported CPUs which may result in spurious NMIs. Reported by: rnoland Reviewed by: jkoshy Approved by: re (kib) MFC after: 2 weeks --- sys/amd64/amd64/local_apic.c | 86 ++++++++++++++++++++++++++++++++++-- sys/amd64/include/apicvar.h | 3 ++ sys/amd64/include/pmc_mdep.h | 1 - sys/dev/hwpmc/hwpmc_core.c | 7 ++- sys/dev/hwpmc/hwpmc_piv.c | 5 ++- sys/dev/hwpmc/hwpmc_ppro.c | 5 ++- sys/dev/hwpmc/hwpmc_x86.c | 22 +++------ sys/i386/i386/local_apic.c | 86 ++++++++++++++++++++++++++++++++++-- sys/i386/include/apicvar.h | 3 ++ sys/i386/include/pmc_mdep.h | 1 - 10 files changed, 191 insertions(+), 28 deletions(-) diff --git a/sys/amd64/amd64/local_apic.c b/sys/amd64/amd64/local_apic.c index cd3073caf994..13bd7743cb93 100644 --- a/sys/amd64/amd64/local_apic.c +++ b/sys/amd64/amd64/local_apic.c @@ -123,7 +123,7 @@ static struct lvt lvts[LVT_MAX + 1] = { { 1, 1, 0, 1, APIC_LVT_DM_NMI, 0 }, /* LINT1: NMI */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_TIMER_INT }, /* Timer */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_ERROR_INT }, /* Error */ - { 1, 1, 0, 1, APIC_LVT_DM_NMI, 0 }, /* PMC */ + { 1, 1, 1, 1, APIC_LVT_DM_NMI, 0 }, /* PMC */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_THERMAL_INT }, /* Thermal */ }; @@ -305,11 +305,9 @@ lapic_setup(int boot) lapic->lvt_lint0 = lvt_mode(la, LVT_LINT0, lapic->lvt_lint0); lapic->lvt_lint1 = lvt_mode(la, LVT_LINT1, lapic->lvt_lint1); -#ifdef HWPMC_HOOKS /* Program the PMC LVT entry if present. */ if (maxlvt >= LVT_PMC) lapic->lvt_pcint = lvt_mode(la, LVT_PMC, lapic->lvt_pcint); -#endif /* Program timer LVT and setup handler. */ lapic->lvt_timer = lvt_mode(la, LVT_TIMER, lapic->lvt_timer); @@ -332,6 +330,88 @@ lapic_setup(int boot) intr_restore(eflags); } +void +lapic_reenable_pmc(void) +{ +#ifdef HWPMC_HOOKS + uint32_t value; + + value = lapic->lvt_pcint; + value &= ~APIC_LVT_M; + lapic->lvt_pcint = value; +#endif +} + +#ifdef HWPMC_HOOKS +static void +lapic_update_pmc(void *dummy) +{ + struct lapic *la; + + la = &lapics[lapic_id()]; + lapic->lvt_pcint = lvt_mode(la, LVT_PMC, lapic->lvt_pcint); +} +#endif + +int +lapic_enable_pmc(void) +{ +#ifdef HWPMC_HOOKS + u_int32_t maxlvt; + + /* Fail if the local APIC is not present. */ + if (lapic == NULL) + return (0); + + /* Fail if the PMC LVT is not present. */ + maxlvt = (lapic->version & APIC_VER_MAXLVT) >> MAXLVTSHIFT; + if (maxlvt < LVT_PMC) + return (0); + + lvts[LVT_PMC].lvt_masked = 0; + +#ifdef SMP + /* + * If hwpmc was loaded at boot time then the APs may not be + * started yet. In that case, don't forward the request to + * them as they will program the lvt when they start. + */ + if (smp_started) + smp_rendezvous(NULL, lapic_update_pmc, NULL, NULL); + else +#endif + lapic_update_pmc(NULL); + return (1); +#else + return (0); +#endif +} + +void +lapic_disable_pmc(void) +{ +#ifdef HWPMC_HOOKS + u_int32_t maxlvt; + + /* Fail if the local APIC is not present. */ + if (lapic == NULL) + return; + + /* Fail if the PMC LVT is not present. */ + maxlvt = (lapic->version & APIC_VER_MAXLVT) >> MAXLVTSHIFT; + if (maxlvt < LVT_PMC) + return; + + lvts[LVT_PMC].lvt_masked = 1; + +#ifdef SMP + /* The APs should always be started when hwpmc is unloaded. */ + KASSERT(mp_ncpus == 1 || smp_started, ("hwpmc unloaded too early")); +#endif + smp_rendezvous(NULL, lapic_update_pmc, NULL, NULL); +#endif +} + /* * Called by cpu_initclocks() on the BSP to setup the local APIC timer so * that it can drive hardclock, statclock, and profclock. This function diff --git a/sys/amd64/include/apicvar.h b/sys/amd64/include/apicvar.h index 73fff6c006f5..9d6d538de1d8 100644 --- a/sys/amd64/include/apicvar.h +++ b/sys/amd64/include/apicvar.h @@ -201,7 +201,9 @@ int ioapic_set_triggermode(void *cookie, u_int pin, int ioapic_set_smi(void *cookie, u_int pin); void lapic_create(u_int apic_id, int boot_cpu); void lapic_disable(void); +void lapic_disable_pmc(void); void lapic_dump(const char *str); +int lapic_enable_pmc(void); void lapic_eoi(void); u_int lapic_error(void); int lapic_id(void); @@ -212,6 +214,7 @@ void lapic_ipi_vectored(u_int vector, int dest); int lapic_ipi_wait(int delay); void lapic_handle_intr(int vector, struct trapframe *frame); void lapic_handle_timer(struct trapframe *frame); +void lapic_reenable_pmc(void); void lapic_set_logical_id(u_int apic_id, u_int cluster, u_int cluster_id); int lapic_set_lvt_mask(u_int apic_id, u_int lvt, u_char masked); int lapic_set_lvt_mode(u_int apic_id, u_int lvt, u_int32_t mode); diff --git a/sys/amd64/include/pmc_mdep.h b/sys/amd64/include/pmc_mdep.h index f8c26f2ec902..f233a510d3cc 100644 --- a/sys/amd64/include/pmc_mdep.h +++ b/sys/amd64/include/pmc_mdep.h @@ -115,7 +115,6 @@ union pmc_md_pmc { */ void start_exceptions(void), end_exceptions(void); -void pmc_x86_lapic_enable_pmc_interrupt(void); struct pmc_mdep *pmc_amd_initialize(void); void pmc_amd_finalize(struct pmc_mdep *_md); diff --git a/sys/dev/hwpmc/hwpmc_core.c b/sys/dev/hwpmc/hwpmc_core.c index 214e42cb4b06..f84e0f1548cb 100644 --- a/sys/dev/hwpmc/hwpmc_core.c +++ b/sys/dev/hwpmc/hwpmc_core.c @@ -32,10 +32,13 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include +#include +#include #include #include #include @@ -1771,7 +1774,7 @@ core_intr(int cpu, struct trapframe *tf) } if (found_interrupt) - pmc_x86_lapic_enable_pmc_interrupt(); + lapic_reenable_pmc(); atomic_add_int(found_interrupt ? &pmc_stats.pm_intr_processed : &pmc_stats.pm_intr_ignored, 1); @@ -1895,7 +1898,7 @@ core2_intr(int cpu, struct trapframe *tf) (uintmax_t) rdmsr(IA_GLOBAL_OVF_CTRL)); if (found_interrupt) - pmc_x86_lapic_enable_pmc_interrupt(); + lapic_reenable_pmc(); atomic_add_int(found_interrupt ? &pmc_stats.pm_intr_processed : &pmc_stats.pm_intr_ignored, 1); diff --git a/sys/dev/hwpmc/hwpmc_piv.c b/sys/dev/hwpmc/hwpmc_piv.c index 565be88cfeef..8ee851828b1c 100644 --- a/sys/dev/hwpmc/hwpmc_piv.c +++ b/sys/dev/hwpmc/hwpmc_piv.c @@ -32,6 +32,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -39,6 +40,8 @@ __FBSDID("$FreeBSD$"); #include #include +#include +#include #include #include #include @@ -1537,7 +1540,7 @@ p4_intr(int cpu, struct trapframe *tf) */ if (did_interrupt) - pmc_x86_lapic_enable_pmc_interrupt(); + lapic_reenable_pmc(); atomic_add_int(did_interrupt ? &pmc_stats.pm_intr_processed : &pmc_stats.pm_intr_ignored, 1); diff --git a/sys/dev/hwpmc/hwpmc_ppro.c b/sys/dev/hwpmc/hwpmc_ppro.c index 909bfe248979..8da185bf6abb 100644 --- a/sys/dev/hwpmc/hwpmc_ppro.c +++ b/sys/dev/hwpmc/hwpmc_ppro.c @@ -32,6 +32,7 @@ __FBSDID("$FreeBSD$"); #include +#include #include #include #include @@ -39,6 +40,8 @@ __FBSDID("$FreeBSD$"); #include #include +#include +#include #include #include #include @@ -718,7 +721,7 @@ p6_intr(int cpu, struct trapframe *tf) * unmasked after a PMC interrupt. */ if (retval) - pmc_x86_lapic_enable_pmc_interrupt(); + lapic_reenable_pmc(); atomic_add_int(retval ? &pmc_stats.pm_intr_processed : &pmc_stats.pm_intr_ignored, 1); diff --git a/sys/dev/hwpmc/hwpmc_x86.c b/sys/dev/hwpmc/hwpmc_x86.c index b48a6b064240..09d04bb713ee 100644 --- a/sys/dev/hwpmc/hwpmc_x86.c +++ b/sys/dev/hwpmc/hwpmc_x86.c @@ -39,7 +39,8 @@ __FBSDID("$FreeBSD$"); #include #include -#include +#include +#include #include #include @@ -47,18 +48,6 @@ __FBSDID("$FreeBSD$"); #include #include -extern volatile lapic_t *lapic; - -void -pmc_x86_lapic_enable_pmc_interrupt(void) -{ - uint32_t value; - - value = lapic->lvt_pcint; - value &= ~APIC_LVT_M; - lapic->lvt_pcint = value; -} - /* * Attempt to walk a user call stack using a too-simple algorithm. * In the general case we need unwind information associated with @@ -252,16 +241,15 @@ pmc_md_initialize() struct pmc_mdep *md; /* determine the CPU kind */ - md = NULL; if (cpu_vendor_id == CPU_VENDOR_AMD) md = pmc_amd_initialize(); else if (cpu_vendor_id == CPU_VENDOR_INTEL) md = pmc_intel_initialize(); else - KASSERT(0, ("[x86,%d] Unknown vendor", __LINE__)); + return (NULL); /* disallow sampling if we do not have an LAPIC */ - if (md != NULL && lapic == NULL) + if (!lapic_enable_pmc()) for (i = 1; i < md->pmd_nclass; i++) md->pmd_classdep[i].pcd_caps &= ~PMC_CAP_INTERRUPT; @@ -271,6 +259,8 @@ pmc_md_initialize() void pmc_md_finalize(struct pmc_mdep *md) { + + lapic_disable_pmc(); if (cpu_vendor_id == CPU_VENDOR_AMD) pmc_amd_finalize(md); else if (cpu_vendor_id == CPU_VENDOR_INTEL) diff --git a/sys/i386/i386/local_apic.c b/sys/i386/i386/local_apic.c index 2cc6a4504824..5e79aa805c5f 100644 --- a/sys/i386/i386/local_apic.c +++ b/sys/i386/i386/local_apic.c @@ -123,7 +123,7 @@ static struct lvt lvts[LVT_MAX + 1] = { { 1, 1, 0, 1, APIC_LVT_DM_NMI, 0 }, /* LINT1: NMI */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_TIMER_INT }, /* Timer */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_ERROR_INT }, /* Error */ - { 1, 1, 0, 1, APIC_LVT_DM_NMI, 0 }, /* PMC */ + { 1, 1, 1, 1, APIC_LVT_DM_NMI, 0 }, /* PMC */ { 1, 1, 1, 1, APIC_LVT_DM_FIXED, APIC_THERMAL_INT }, /* Thermal */ }; @@ -307,11 +307,9 @@ lapic_setup(int boot) lapic->lvt_lint0 = lvt_mode(la, LVT_LINT0, lapic->lvt_lint0); lapic->lvt_lint1 = lvt_mode(la, LVT_LINT1, lapic->lvt_lint1); -#ifdef HWPMC_HOOKS /* Program the PMC LVT entry if present. */ if (maxlvt >= LVT_PMC) lapic->lvt_pcint = lvt_mode(la, LVT_PMC, lapic->lvt_pcint); -#endif /* Program timer LVT and setup handler. */ lapic->lvt_timer = lvt_mode(la, LVT_TIMER, lapic->lvt_timer); @@ -334,6 +332,88 @@ lapic_setup(int boot) intr_restore(eflags); } +void +lapic_reenable_pmc(void) +{ +#ifdef HWPMC_HOOKS + uint32_t value; + + value = lapic->lvt_pcint; + value &= ~APIC_LVT_M; + lapic->lvt_pcint = value; +#endif +} + +#ifdef HWPMC_HOOKS +static void +lapic_update_pmc(void *dummy) +{ + struct lapic *la; + + la = &lapics[lapic_id()]; + lapic->lvt_pcint = lvt_mode(la, LVT_PMC, lapic->lvt_pcint); +} +#endif + +int +lapic_enable_pmc(void) +{ +#ifdef HWPMC_HOOKS + u_int32_t maxlvt; + + /* Fail if the local APIC is not present. */ + if (lapic == NULL) + return (0); + + /* Fail if the PMC LVT is not present. */ + maxlvt = (lapic->version & APIC_VER_MAXLVT) >> MAXLVTSHIFT; + if (maxlvt < LVT_PMC) + return (0); + + lvts[LVT_PMC].lvt_masked = 0; + +#ifdef SMP + /* + * If hwpmc was loaded at boot time then the APs may not be + * started yet. In that case, don't forward the request to + * them as they will program the lvt when they start. + */ + if (smp_started) + smp_rendezvous(NULL, lapic_update_pmc, NULL, NULL); + else +#endif + lapic_update_pmc(NULL); + return (1); +#else + return (0); +#endif +} + +void +lapic_disable_pmc(void) +{ +#ifdef HWPMC_HOOKS + u_int32_t maxlvt; + + /* Fail if the local APIC is not present. */ + if (lapic == NULL) + return; + + /* Fail if the PMC LVT is not present. */ + maxlvt = (lapic->version & APIC_VER_MAXLVT) >> MAXLVTSHIFT; + if (maxlvt < LVT_PMC) + return; + + lvts[LVT_PMC].lvt_masked = 1; + +#ifdef SMP + /* The APs should always be started when hwpmc is unloaded. */ + KASSERT(mp_ncpus == 1 || smp_started, ("hwpmc unloaded too early")); +#endif + smp_rendezvous(NULL, lapic_update_pmc, NULL, NULL); +#endif +} + /* * Called by cpu_initclocks() on the BSP to setup the local APIC timer so * that it can drive hardclock, statclock, and profclock. This function diff --git a/sys/i386/include/apicvar.h b/sys/i386/include/apicvar.h index a13766f95f97..b15452b1d81c 100644 --- a/sys/i386/include/apicvar.h +++ b/sys/i386/include/apicvar.h @@ -230,7 +230,9 @@ int ioapic_set_triggermode(void *cookie, u_int pin, int ioapic_set_smi(void *cookie, u_int pin); void lapic_create(u_int apic_id, int boot_cpu); void lapic_disable(void); +void lapic_disable_pmc(void); void lapic_dump(const char *str); +int lapic_enable_pmc(void); void lapic_eoi(void); u_int lapic_error(void); int lapic_id(void); @@ -241,6 +243,7 @@ void lapic_ipi_vectored(u_int vector, int dest); int lapic_ipi_wait(int delay); void lapic_handle_intr(int vector, struct trapframe *frame); void lapic_handle_timer(struct trapframe *frame); +void lapic_reenable_pmc(void); void lapic_set_logical_id(u_int apic_id, u_int cluster, u_int cluster_id); int lapic_set_lvt_mask(u_int apic_id, u_int lvt, u_char masked); int lapic_set_lvt_mode(u_int apic_id, u_int lvt, u_int32_t mode); diff --git a/sys/i386/include/pmc_mdep.h b/sys/i386/include/pmc_mdep.h index 63d5f8bf0b67..4389a20c2036 100644 --- a/sys/i386/include/pmc_mdep.h +++ b/sys/i386/include/pmc_mdep.h @@ -150,7 +150,6 @@ struct pmc_mdep; */ void start_exceptions(void), end_exceptions(void); -void pmc_x86_lapic_enable_pmc_interrupt(void); struct pmc_mdep *pmc_amd_initialize(void); void pmc_amd_finalize(struct pmc_mdep *_md);