On Mon, Jul 31, 2023 at 12:15 PM Alexandre Ghiti [off-list ref] wrote:

Hi Ian,

On Fri, Jul 28, 2023 at 7:53 PM Ian Rogers [off-list ref] wrote:

quoted

On Thu, Jul 27, 2023 at 7:28 AM Alexandre Ghiti [off-list ref] wrote:

quoted

riscv now supports mmaping hardware counters so add what's needed to
take advantage of that in libperf.

Signed-off-by: Alexandre Ghiti <redacted>
Reviewed-by: Andrew Jones <redacted>
Reviewed-by: Atish Patra <redacted>
---
 tools/lib/perf/mmap.c | 65 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index 0d1634cedf44..378a163f0554 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c

@@ -392,6 +392,71 @@ static u64 read_perf_counter(unsigned int counter)

 static u64 read_timestamp(void) { return read_sysreg(cntvct_el0); }

+#elif __riscv_xlen == 64

This is something of an odd guard, perhaps:
#elif defined(__riscv) && __riscv_xlen == 64

That way it is more intention revealing that this is riscv code. Could
you add a comment relating to the __riscv_xlen ?

I guess Andrew answered that already.

quoted

quoted

+
+/* TODO: implement rv32 support */
+
+#define CSR_CYCLE      0xc00
+#define CSR_TIME       0xc01
+
+#define csr_read(csr)                                          \
+({                                                             \
+       register unsigned long __v;                             \
+               __asm__ __volatile__ ("csrr %0, " #csr          \
+                : "=r" (__v) :                                 \
+                : "memory");                                   \

To avoid the macro pasting that could potentially go weird, could this be:

__asm__ __volatile__ ("csrr %0, %1",
  : "=r"(__v) /* outputs */
  : "i"(csr) /* inputs */
  : "memory" /* clobbers */)

Forgot to answer this one: it compiles, but I have to admit that I
don't understand the difference and if that's correct (all macros in
arch/riscv/include/asm/csr.h use # to do this) and what benefits it
brings. Can you elaborate more on things that could "go weird"?

Thanks again,

Alex

quoted

Also, why is this clobbering memory? Worth adding a comment.

No idea, I see that it is also done this way in
arch/riscv/include/asm/csr.h. @Atish Kumar Patra , @Palmer Dabbelt ?

Thanks for your comments!

Alex

quoted

Thanks,
Ian

quoted

+                __v;                                           \
+})
+
+static unsigned long csr_read_num(int csr_num)
+{
+#define switchcase_csr_read(__csr_num, __val)           {\
+       case __csr_num:                                 \
+               __val = csr_read(__csr_num);            \
+               break; }
+#define switchcase_csr_read_2(__csr_num, __val)         {\
+       switchcase_csr_read(__csr_num + 0, __val)        \
+       switchcase_csr_read(__csr_num + 1, __val)}
+#define switchcase_csr_read_4(__csr_num, __val)         {\
+       switchcase_csr_read_2(__csr_num + 0, __val)      \
+       switchcase_csr_read_2(__csr_num + 2, __val)}
+#define switchcase_csr_read_8(__csr_num, __val)         {\
+       switchcase_csr_read_4(__csr_num + 0, __val)      \
+       switchcase_csr_read_4(__csr_num + 4, __val)}
+#define switchcase_csr_read_16(__csr_num, __val)        {\
+       switchcase_csr_read_8(__csr_num + 0, __val)      \
+       switchcase_csr_read_8(__csr_num + 8, __val)}
+#define switchcase_csr_read_32(__csr_num, __val)        {\
+       switchcase_csr_read_16(__csr_num + 0, __val)     \
+       switchcase_csr_read_16(__csr_num + 16, __val)}
+
+       unsigned long ret = 0;
+
+       switch (csr_num) {
+       switchcase_csr_read_32(CSR_CYCLE, ret)
+       default:
+               break;
+       }
+
+       return ret;
+#undef switchcase_csr_read_32
+#undef switchcase_csr_read_16
+#undef switchcase_csr_read_8
+#undef switchcase_csr_read_4
+#undef switchcase_csr_read_2
+#undef switchcase_csr_read
+}
+
+static u64 read_perf_counter(unsigned int counter)
+{
+       return csr_read_num(CSR_CYCLE + counter);
+}
+
+static u64 read_timestamp(void)
+{
+       return csr_read_num(CSR_TIME);
+}
+
 #else
 static u64 read_perf_counter(unsigned int counter __maybe_unused) { return 0; }
 static u64 read_timestamp(void) { return 0; }
--
2.39.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel