On Fri, 8 Dec 2023 at 10:36, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > On 7/12/23 23:12, Richard Henderson wrote: > > On 12/7/23 07:45, Philippe Mathieu-Daudé wrote: > >> pmu_init() register its event checking the pm_event::supported() > >> handler. For INST_RETIRED, the event is only registered and the > >> bit enabled in the PMU Common Event Identification register when > >> icount is enabled as ICOUNT_PRECISE. > >> > >> Assert the pm_event::get_count() and pm_event::ns_per_count() > >> handler will only be called under this icount mode. > >> > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> > >> --- > >> target/arm/helper.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/target/arm/helper.c b/target/arm/helper.c > >> index adb0960bba..333fd5f4bf 100644 > >> --- a/target/arm/helper.c > >> +++ b/target/arm/helper.c > >> @@ -940,11 +940,13 @@ static bool instructions_supported(CPUARMState > >> *env) > >> static uint64_t instructions_get_count(CPUARMState *env) > >> { > >> + assert(icount_enabled() == ICOUNT_PRECISE); > >> return (uint64_t)icount_get_raw(); > >> } > >> static int64_t instructions_ns_per(uint64_t icount) > >> { > >> + assert(icount_enabled() == ICOUNT_PRECISE); > >> return icount_to_ns((int64_t)icount); > >> } > >> #endif > > > > I don't think an assert is required -- that's exactly what the > > .supported field is for. If you think this needs additional > > clarification, a comment is sufficient. > > Without this I'm getting this link failure with TCG disabled: > > ld: Undefined symbols: > _icount_to_ns, referenced from: > _instructions_ns_per in target_arm_helper.c.o > clang: error: linker command failed with exit code 1 (use -v to see > invocation)
I think we should fix this earlier by not trying to enable these TCG-only PMU event types in a non-TCG config. -- PMM