From nobody Mon Jan 10 14:31:09 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 10E2F1944E5E; Mon, 10 Jan 2022 14:31:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JXbq15jwQz3LK2; Mon, 10 Jan 2022 14:31:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A424426E80; Mon, 10 Jan 2022 14:31:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 20AEV9AW075285; Mon, 10 Jan 2022 14:31:09 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 20AEV9iZ075284; Mon, 10 Jan 2022 14:31:09 GMT (envelope-from git) Date: Mon, 10 Jan 2022 14:31:09 GMT Message-Id: <202201101431.20AEV9iZ075284@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Jessica Clarke Subject: git: e74c7ffcb11b - main - hwpmc: Fix amd/arm64/armv7/uncore sampling overflow race List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jrtc27 X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: e74c7ffcb11b6ac879167249adc23a1f9ee5aab6 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1641825069; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=QFfPnsNcDBCiIjWgUBtvwqGLsJhaRZyTpaxeo0ijcAc=; b=yYNcimfnkUOv7H7sUjmJK4TGgO/ISkQnT9P+yHt+WOWoY2pTgYrpLlRDxxqY/KnIVql4RF 6C193dxnR4l5IrghuSV3/vBZWW6Mw//A4ONFT3rV4978U2dCISVBCexRJzeJ/2DKtoDELm D7ijlK6vjv4PCBaqPEerCFTk5z679KcvQTaCHNiENOCYj/I4NxxfLWeBkTJIFxGpRwP0dS uzGgQ57Hc0uMLZyDdJIt6CGEpSjRGfUUzVClWsxNUESDFVONWsD7MugQ1RuPV7sMepmSSD zw45FlqTeaRM64KeC9mnJ9J1wSWLpvsDnjxXf5d2BAXVsbRTka0pBgOWauZGYQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1641825069; a=rsa-sha256; cv=none; b=VY9Xa8kqAVxsR6tgCDZYmBIcVl4EQX7FIhPfenNp+xR0NfT414eUYczC/OAg7NhXIFjdoN qiPZOMbzJZTT+9FSS7UKIiPRiBtEIehmhGokboDBqb+ANKiNHZjLDyuVzW3RQc3GJpZ6Gd uFnkmsefYloqj4gi3eM590OKFIsndwwAiErDZ2Bn48DSU1+4Lxap/OhnWXDi0vt7+/c78J RJ7TfIlMWIuVC2uKUQwGlhV9Alu7cfdPS4rgtlhm5TXsc8mDn8X3avDzUQzADPGAjYNf1s dZrjK+mGcSCQzkKKHpl/7j5Lv/R+lq8rfrDD/G2fQ/jTUhxn3L6pc+038GApfA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jrtc27: URL: https://cgit.FreeBSD.org/src/commit/?id=e74c7ffcb11b6ac879167249adc23a1f9ee5aab6 commit e74c7ffcb11b6ac879167249adc23a1f9ee5aab6 Author: Jessica Clarke AuthorDate: 2022-01-10 14:30:05 +0000 Commit: Jessica Clarke CommitDate: 2022-01-10 14:30:05 +0000 hwpmc: Fix amd/arm64/armv7/uncore sampling overflow race If a counter more than overflows just as we read it on switch out then, if using sampling mode, we will negate this small value to give a huge reload count, and if we later switch back in that context we will validate that value against pm_reloadcount and panic an INVARIANTS kernel with: panic: [pmc,1470] pmcval outside of expected range cpu=2 ri=16 pmcval=fffff292 pm_reloadcount=10000 or similar. Presumably in a non-INVARIANTS kernel we will instead just use the provided value as the reload count, which would lead to the overflow not happing for a very long time (e.g. 78 minutes for a 48-bit counter incrementing at an averate rate of 1GHz). Instead, clamp the reload count to 0 (which corresponds precisely to the value we would compute if it had just overflowed and no more), which will result in hwpmc using the full original reload count again. This is the approach used by core for Intel (for both fixed and programmable counters). As part of this, armv7 and arm64 are made conceptually simpler; rather than skipping modifying the overflow count for sampling mode counters so it's always kept as ~0, those special cases are removed so it's always applicable and the concatentation of it and the hardware counter can always be viewed as a 64-bit counter, which also makes them look more like other architectures. Whilst here, fix an instance of UB (shifting a 1 into the sign bit) for amd in its sign-extension code. Reviewed by: andrew, mhorne, kib MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D33654 --- sys/dev/hwpmc/hwpmc_amd.c | 15 ++++++++++++--- sys/dev/hwpmc/hwpmc_arm64.c | 25 ++++++++++++++++--------- sys/dev/hwpmc/hwpmc_armv7.c | 26 +++++++++++++++++--------- sys/dev/hwpmc/hwpmc_uncore.c | 4 ++++ 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/sys/dev/hwpmc/hwpmc_amd.c b/sys/dev/hwpmc/hwpmc_amd.c index b26e31cc1543..936974080fed 100644 --- a/sys/dev/hwpmc/hwpmc_amd.c +++ b/sys/dev/hwpmc/hwpmc_amd.c @@ -431,9 +431,18 @@ amd_read_pmc(int cpu, int ri, pmc_value_t *v) tmp = rdmsr(pd->pm_perfctr); /* RDMSR serializes */ PMCDBG2(MDP,REA,2,"amd-read (pre-munge) id=%d -> %jd", ri, tmp); if (PMC_IS_SAMPLING_MODE(mode)) { - /* Sign extend 48 bit value to 64 bits. */ - tmp = (pmc_value_t) (((int64_t) tmp << 16) >> 16); - tmp = AMD_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); + /* + * Clamp value to 0 if the counter just overflowed, + * otherwise the returned reload count would wrap to a + * huge value. + */ + if ((tmp & (1ULL << 47)) == 0) + tmp = 0; + else { + /* Sign extend 48 bit value to 64 bits. */ + tmp = (pmc_value_t) ((int64_t)(tmp << 16) >> 16); + tmp = AMD_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); + } } *v = tmp; diff --git a/sys/dev/hwpmc/hwpmc_arm64.c b/sys/dev/hwpmc/hwpmc_arm64.c index 14c176894c6a..988cd1744a07 100644 --- a/sys/dev/hwpmc/hwpmc_arm64.c +++ b/sys/dev/hwpmc/hwpmc_arm64.c @@ -218,8 +218,7 @@ arm64_read_pmc(int cpu, int ri, pmc_value_t *v) if ((READ_SPECIALREG(pmovsclr_el0) & reg) != 0) { /* Clear Overflow Flag */ WRITE_SPECIALREG(pmovsclr_el0, reg); - if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) - pm->pm_pcpu_state[cpu].pps_overflowcnt++; + pm->pm_pcpu_state[cpu].pps_overflowcnt++; /* Reread counter in case we raced. */ tmp = arm64_pmcn_read(ri); @@ -228,10 +227,18 @@ arm64_read_pmc(int cpu, int ri, pmc_value_t *v) intr_restore(s); PMCDBG2(MDP, REA, 2, "arm64-read id=%d -> %jd", ri, tmp); - if (PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) - *v = ARMV8_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); - else - *v = tmp; + if (PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) { + /* + * Clamp value to 0 if the counter just overflowed, + * otherwise the returned reload count would wrap to a + * huge value. + */ + if ((tmp & (1ull << 63)) == 0) + tmp = 0; + else + tmp = ARMV8_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); + } + *v = tmp; return (0); } @@ -379,10 +386,10 @@ arm64_intr(struct trapframe *tf) retval = 1; /* Found an interrupting PMC. */ - if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) { - pm->pm_pcpu_state[cpu].pps_overflowcnt += 1; + pm->pm_pcpu_state[cpu].pps_overflowcnt += 1; + + if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) continue; - } if (pm->pm_state != PMC_STATE_RUNNING) continue; diff --git a/sys/dev/hwpmc/hwpmc_armv7.c b/sys/dev/hwpmc/hwpmc_armv7.c index 12b3fff4d743..090e4523102d 100644 --- a/sys/dev/hwpmc/hwpmc_armv7.c +++ b/sys/dev/hwpmc/hwpmc_armv7.c @@ -189,8 +189,7 @@ armv7_read_pmc(int cpu, int ri, pmc_value_t *v) if ((cp15_pmovsr_get() & reg) != 0) { /* Clear Overflow Flag */ cp15_pmovsr_set(reg); - if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) - pm->pm_pcpu_state[cpu].pps_overflowcnt++; + pm->pm_pcpu_state[cpu].pps_overflowcnt++; /* Reread counter in case we raced. */ tmp = armv7_pmcn_read(ri, pm->pm_md.pm_armv7.pm_armv7_evsel); @@ -199,10 +198,18 @@ armv7_read_pmc(int cpu, int ri, pmc_value_t *v) intr_restore(s); PMCDBG2(MDP, REA, 2, "armv7-read id=%d -> %jd", ri, tmp); - if (PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) - *v = ARMV7_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); - else - *v = tmp; + if (PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) { + /* + * Clamp value to 0 if the counter just overflowed, + * otherwise the returned reload count would wrap to a + * huge value. + */ + if ((tmp & (1ull << 63)) == 0) + tmp = 0; + else + tmp = ARMV7_PERFCTR_VALUE_TO_RELOAD_COUNT(tmp); + } + *v = tmp; return 0; } @@ -360,10 +367,11 @@ armv7_intr(struct trapframe *tf) retval = 1; /* Found an interrupting PMC. */ - if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) { - pm->pm_pcpu_state[cpu].pps_overflowcnt += 1; + pm->pm_pcpu_state[cpu].pps_overflowcnt += 1; + + if (!PMC_IS_SAMPLING_MODE(PMC_TO_MODE(pm))) continue; - } + if (pm->pm_state != PMC_STATE_RUNNING) continue; diff --git a/sys/dev/hwpmc/hwpmc_uncore.c b/sys/dev/hwpmc/hwpmc_uncore.c index adb0e12c7454..7ccb93cb8a58 100644 --- a/sys/dev/hwpmc/hwpmc_uncore.c +++ b/sys/dev/hwpmc/hwpmc_uncore.c @@ -175,6 +175,10 @@ uncore_pcpu_fini(struct pmc_mdep *md, int cpu) static pmc_value_t ucf_perfctr_value_to_reload_count(pmc_value_t v) { + + /* If the PMC has overflowed, return a reload count of zero. */ + if ((v & (1ULL << (uncore_ucf_width - 1))) == 0) + return (0); v &= (1ULL << uncore_ucf_width) - 1; return (1ULL << uncore_ucf_width) - v; }