[PATCH v3 4/6] target/arm: Ensure icount is enabled when emulating INST_RETIRED

Philippe Mathieu-Daudé posted 6 patches 11 months, 3 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Peter Maydell <peter.maydell@linaro.org>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>
[PATCH v3 4/6] target/arm: Ensure icount is enabled when emulating INST_RETIRED
Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago
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.

PMU events are TCG-only, hardware accelerators handle them
directly. Unfortunately we register the events in non-TCG builds,
leading to linking error such:

  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)

As a kludge, give a hint to the compiler by asserting 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é <philmd@linaro.org>
---
As discussed in
https://lore.kernel.org/qemu-devel/CAFEAcA-HVf8vWLzmdStEo2NrSKQdZV612rBjiaj-gLW4vXyvpA@mail.gmail.com/
better would be to restrict the PMU events to TCG, but this is
out of the scope of this series.
---
 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
-- 
2.41.0


Re: [PATCH v3 4/6] target/arm: Ensure icount is enabled when emulating INST_RETIRED
Posted by Richard Henderson 11 months, 3 weeks ago
On 12/8/23 03:35, 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.
> 
> PMU events are TCG-only, hardware accelerators handle them
> directly. Unfortunately we register the events in non-TCG builds,
> leading to linking error such:
> 
>    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)
> 
> As a kludge, give a hint to the compiler by asserting 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é <philmd@linaro.org>
> ---
> As discussed in
> https://lore.kernel.org/qemu-devel/CAFEAcA-HVf8vWLzmdStEo2NrSKQdZV612rBjiaj-gLW4vXyvpA@mail.gmail.com/
> better would be to restrict the PMU events to TCG, but this is
> out of the scope of this series.


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~