Use a goto to clarify the control flow when there is no process
descriptor. This wins back a level of indentation.
No functional change intended.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40518
Split out the functional logic from the syscall handler into a helper
function. This keeps it separate from the syscall control-flow logic,
resulting in better readability overall. It also wins back a level of
indentation.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40294
Split out the functional logic from the syscall handler into a helper
function. This keeps it separate from the syscall control-flow logic,
resulting in better readability overall. It also wins back a level of
indentation.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40293
Split out the large chunk of functional logic from the syscall handler
into a helper function. This keeps it separate from the syscall
control-flow logic, resulting in better readability overall. It also
wins back a level of indentation.
Flip the return values of the pmc_can_allocate_row() and
pmc_can_allocate_rowindex() functions to boolean types, like their
naming implies. We weren't actually using the error codes they were
returning.
While here, make some small style cleanups. No functional change
intended.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40292
Begin splitting out the large chunks of functional logic from the
syscall handler into separate helper functions. This keeps it separate
from the syscall control-flow logic, resulting in better readability
overall. It also wins back a level of indentation.
For this and the similar changes to follow, try to keep copyin() and
copyout() calls outside of the helper functions. The changes are
intended to have no functional impact, but do address style issues.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40291
Move the functionality into a separate helper function. All other
actions in pmc_hook_handler() already have this.
While here make some small style improvements. Restructure one for loop.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40333
Everything else I found worthy of adjustment.
- Order local variable declarations per style(9)
- Make use of __unused annotations rather than cast to void
- Remove unnecessary casts
- Add (void) casts to PMC class methods where the return value is
ignored
- A couple instances of reordering statements for clarity
- Prefer bool type where applicable
- unsigned int -> u_int
- Use uintmax_t/%j in printf calls
- Formatting of comments
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40517
- Explicitly check the value in conditional statements, per style(9)
- Add braces around more loops and conditionals, wherever it appears
(to me) to aid legibility
- Expand some assignments within if statements
- Fix formatting around list-type FOREACH macros
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40515
Handle a few things related to spacing:
- Remove redundant/superfluous blank lines (and add a couple where
helpful)
- Add spacing around binary operators
- Remove spacing after casts and before goto labels
- Adjustments for line width of 80 chars
- Tab/space character issues
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D40514
Currently the PROCEXEC event only reports a single address, entryaddr,
which is the entry point of the interpreter in the typical dynamic case,
and used solely to calculate the base address of the interpreter. For
PDEs this is fine, since the base address is known from the program
headers, but for PIEs the base address varies at run time based on where
the kernel chooses to load it, and so pmcstat has no way of knowing the
real address ranges for the executable. This was less of an issue in the
past since PIEs were rare, but now they're on by default on 64-bit
architectures it's more of a problem.
To solve this, pass through what was picked for et_dyn_addr by the
kernel, and use that as the offset for the executable's start address
just as is done for everything in the kernel. Since we're changing this
interface, sanitise the way we determine the interpreter's base address
by passing it through directly rather than indirectly via the entry
point and having to subtract off whatever the ELF header's e_entry is
(and anything that wants the entry point in future can still add that
back on as needed; this merely changes the interface to directly provide
the underlying variables involved).
This will be followed up by a bump to the pmc major version.
Reviewed by: jhb
Differential Revision: https://reviews.freebsd.org/D39595
This unifies the user object and kernel module paths in libpmcstat,
allows modules loaded from non-standard locations (e.g. from a user's
home directory when testing) to be found and, since buffer is what all
the warnings here use (they were never updated when buffer_modules were
added to pick based on where the file was found) has the side-effect of
ensuring the messages are correct.
This includes obsoleting the now-superfluous -k option in pmcstat.
This change breaks the hwpmc ABI and will be followed by a bump to the
pmc major version.
Reviewed by: jhb, jkoshy, mhorne
Differential Revision: https://reviews.freebsd.org/D40048
Whilst the former is not breaking, the latter is, and so this will be
followed by a bump to the pmc major version. This will allow the flags
to actually be usable in future, as otherwise we cannot distinguish
uninitialised stack junk from a deliberately-initialised value.
Reviewed by: jhb, mhorne
Differential Revision: https://reviews.freebsd.org/D40049
The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.
Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix
In many cases this avoids an extra lookup, since the callers always have
pm at hand. We can also eliminate several assertions, mostly for pm !=
NULL. The class methods are an internal interface, and the callers
already handle such a scenario. No functional change intended.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39915
These are preferred over casts to void. No functional change.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39914
Most platforms (non-x86) don't require these methods and implement stub
versions. If we initialize the pmc_mdep structure to always point to the
generic versions, then we can purge the duplicate stubs.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39913
These are unused on all platforms.
Reviewed by: jkoshy, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39912
Provide the log type names in their entirely, rather than relying on the
macro to prepend the prefix. This improves their searchability; for
example, if I see PMCLOG_TYPE_PMCALLOCATE in libpmc I will now be able
to find where that is emitted in the kernel with a simple grep.
Reviewed by: jkoshy, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39911
It is just wrapper around strlcpy(), but results in more complicated
code. Clean this up to use strlcpy() or snprintf() as appropriate.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39910
This existing helper function is preferable to the hand-rolled
calculation of the kstack bounds.
Make some small style improvements while here. Notably, rename every
instance of "r", the return address, to "ra". Tidy the includes in the
affected files.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39909
Use the unwind_frame() function, which properly validates the frame
pointer and uses ADDR_MAKE_CANONICAL() for the pc, required when PAC is
enabled.
Reviewed by: andrew, markj, jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39934
Improve the legibility of the list. Bump overall indentation, fix some
whitespace, and sort the IAF block.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39903
This comment is no longer in sync with the contents of __PMC_EVENTS().
Update to reflect the removal of various Intel event definitions from
this list; these event definitions now come from Linux and live in
lib/libpmc/pmu-events/.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39902
Although this block has remained in __PMC_EVENTS(), there is no handling
of UCP in libpmc/libpmc.c, so it is not possible to select one of these
events. It should therefore be impossible to trigger the code removed
from ucp_start_pmc(). Note that the GQ_SNOOP_MSF MSR exists only for
Nehalem and Westmere architectures, and the related events do not exist
for later generations.
The Uncore support in hwpmc has severely atrophied in general. We have
uncore event definitions in pmu-events, but the kernel support was
written against Intel Performance Measurement Architecture version 2,
and is disabled for processor generations later than Westmere. Nehalem
and Westmere lack uncore event definitions in pmu-events. I'd be
surprised if Uncore support is usable on any machine in its current
state.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39901
These are maintained elsewhere. No functional change.
Reviewed by: jkoshy
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D39900
A subsequent commit will instead use existing infrastructure to
exclude the files from hwpmc.ko for non-ACPI builds. Note that the
original commit left the files as optional in sys/conf/files.arm64.
This reverts commit 751d88119f.
Sponsored by: DARPA
Differential Revision: https://reviews.freebsd.org/D38736
The SPDX-License-Identifier was wrong in the Arm CoreLink DMC-620
driver files. It used the incorrect FreeBSD variant of the BSD-2-Clause
identifier. According to [1] all files should use BSD-2-Clause.
[1] https://tools.spdx.org/app/check_license/
Reported by: emaste
Sponsored by: Arm Ltd
The SPDX-License-Identifier was wrong in the Arm CoreLink CMN-600
driver files. It used the incorrect FreeBSD variant of the BSD-2-Clause
identifier. According to [1] all files should use BSD-2-Clause.
[1] https://tools.spdx.org/app/check_license/
Reported by: emaste
Sponsored by: Arm Ltd
In 0a9a4d2cd6 a check for OPT_ACPI was added to the hwpmc Makefile
to fix loading the module in a kernel where ACPI has been disabled.
This broke loading the module when ACPI was enabled in the build as
OPT_ACPI isn't a Makefile macro so was always disabled.
Move this check to the C files where the DEV_ACPI macro does exist.
Reviewed by: gnn
Sponsored by: Arm Ltd
Differential Revision: https://reviews.freebsd.org/D37773
Like many of the other encodings here, none of these are actually used
by our tables. However, defining the EVENT_xH names allows them to be
used by the user (e.g. when trying to use an implementation-defined
event that they know about from their core's documentation but we don't)
and allows us to define PMC_EV_ARMV8_LAST appropriately.
Some of these are also used downstream in CheriBSD on Morello.
Reviewed by: andrew, tsoome
Obtained from: CheriBSD
Differential Revision: https://reviews.freebsd.org/D36926
The documented encoding space for Armv8 was only 8 bits, but v8.0 has
always had a 10-bit encoding space for its events, and downstream in
CheriBSD we relied on this full space. This worked until the DMC-620 and
CMN-600 events were added, trampling on what should have been reserved
for Armv8.0 right from the start. Thus, renumber the DMC-620 and CMN-600
events to not do this before they make it into a stable release,
allowing for the full Armv8.0 encoding space to be used without having
to split it across two different regions.
Note that Armv8.1 grows the encoding space to 16 bits, which doesn't fit
well with our current approach. No attempt is made to allow for these
events in this change, only the ones that have always been valid (according to
the hardware) from the first commit of Armv8 support to hwpmc.
Reviewed by: arichardson, tsoome
Differential Revision: https://reviews.freebsd.org/D36925
At least KVM in Proxmox seems not happy about it. Just to be safe
block it for all VMs, since it should just improve profile accuracy.
MFC after: 1 week
With clang 15, the following -Werror warning is produced:
sys/dev/hwpmc/hwpmc_arm64.c:530:21: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
pmc_arm64_initialize()
^
void
This is because many of the functions are declared with a (void)
argument list, but defined with an empty argument list. Make the
definition match the declaration.
Sponsored by: The FreeBSD Foundation
Make most AST handlers dynamically registered. This allows to have
subsystem-specific handler source located in the subsystem files,
instead of making subr_trap.c aware of it. For instance, signal
delivery code on return to userspace is now moved to kern_sig.c.
Also, it allows to have some handlers designated as the cleanup (kclear)
type, which are called both at AST and on thread/process exit. For
instance, ast(), exit1(), and NFS server no longer need to be aware
about UFS softdep processing.
The dynamic registration also allows third-party modules to register AST
handlers if needed. There is one caveat with loadable modules: the
code does not make any effort to ensure that the module is not unloaded
before all threads processed through AST handler in it. In fact, this
is already present behavior for hwpmc.ko and ufs.ko. I do not think it
is worth the efforts and the runtime overhead to try to fix it.
Reviewed by: markj
Tested by: emaste (arm64), pho
Discussed with: jhb
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Differential revision: https://reviews.freebsd.org/D35888
With clang 15, the following -Werror warnings are produced:
sys/dev/hwpmc/hwpmc_mod.c:4805:6: error: variable 'nfree' set but not used [-Werror,-Wunused-but-set-variable]
int nfree;
^
sys/dev/hwpmc/hwpmc_mod.c:4804:6: error: variable 'ncallchains' set but not used [-Werror,-Wunused-but-set-variable]
int ncallchains;
^
The 'nfree' and 'ncallchains' variables were used in KASSERTs, but these
were removed due to refactoring in d9f1b8dbf2. Remove the variables
since they no longer serve any purpose.
MFC after: 3 days
With clang 15, the following -Werror warning is produced:
sys/dev/hwpmc/hwpmc_mod.c:2462:33: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
pmc_thread_descriptor_pool_drain()
^
void
This is because pmc_thread_descriptor_pool_drain() is declared with a
(void) argument list, but defined with an empty argument list. Make the
definition match the declaration.
MFC after: 3 days
With clang 15, the following -Werror warning is produced:
sys/dev/hwpmc/hwpmc_x86.c:245:18: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
pcm_md_initialize()
^
void
This is because pcm_md_initialize() is declared with a (void) argument
list, but defined with an empty argument list. Make the definition match
the declaration.
MFC after: 3 days
With clang 15, the following -Werror warnings are produced:
sys/dev/hwpmc/hwpmc_logging.c:1228:18: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
pmclog_initialize()
^
void
sys/dev/hwpmc/hwpmc_logging.c:1277:16: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
pmclog_shutdown()
^
void
This is because pcmlog_{initialize,shutdown}() are declared with (void)
argument lists, but defined with empty argument lists. Make the
definitions match the declarations.
MFC after: 3 days
Fix builds after 1459a22787 and
59191f3573 by using __diagused
for variables only used in KASSERT().
In addition remove two debug lines that look like a copy and paste error
from dmc620 to cmn600.
Further add the newly introduced files to sys/confg/files.arm64 as well
so that LINT compiles without missing symbols.
Fix build after e3572eb654 as
struct pmc_md_dmc620_pmu_op_pmcallocate is needed when building
libpmc/pmclog.c as it is partof the public API via machine/pmc_mdep.h.
Add support of ARM CMN-600 controller, PMU access functions only.
Add support of PMU counters of ARM CMN-600 controller.
Reviewed by: mhorne
Sponsored By: ARM
Differential Revision: https://reviews.freebsd.org/D32321
