Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200514060243.106976-1-zong.li@sifive.com>
In-Reply-To: <20200514060243.106976-1-zong.li@sifive.com>
From:   Zong Li <zong.li@sifive.com>
Date:   Thu, 14 May 2020 17:29:33 +0800
Message-ID: <CANXhq0q6xb1DaVp8skvvC+G9MDMXpXrv0dxmceTrerc6noBH8A@mail.gmail.com>
Subject: Re: [PATCH] riscv: perf: fix build error for dependency issue
To:     Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>,
        Kefeng Wang <wangkefeng.wang@huawei.com>
Cc:     Greentime Hu <greentime.hu@sifive.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 14, 2020 at 2:03 PM Zong Li <zong.li@sifive.com> wrote:
>
> CONFIG_RISCV_BASE_PMU can be selected or unselected, but in fact,
> CONFIG_RISCV_BASE_PMU must be always selected when selecting
> CONFIG_PERF_EVENTS on current perf implementation, otherwise, it
> would cause the build error when only selecting CONFIG_PERF_EVENTS.
> The build case is applied randconfig which generated by kbuild test.
>
> This patch removes the unnecessary configuration and implementations.
> Eventually, the number of counters should be determinated at runtime,
> such as DTB, so we don't need to re-build kernel for various platform
> which has got different number of hpmcounters.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> Signed-off-by: Greentime Hu <greentime.hu@sifive.com>
> ---
>  arch/riscv/Kconfig                  | 13 -------------
>  arch/riscv/include/asm/perf_event.h | 16 +++-------------
>  2 files changed, 3 insertions(+), 26 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 74f82cf4f781..7d5123576953 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -283,19 +283,6 @@ config RISCV_ISA_C
>
>            If you don't know what to do here, say Y.
>
> -menu "supported PMU type"
> -       depends on PERF_EVENTS
> -
> -config RISCV_BASE_PMU
> -       bool "Base Performance Monitoring Unit"
> -       def_bool y
> -       help
> -         A base PMU that serves as a reference implementation and has limited
> -         feature of perf.  It can run on any RISC-V machines so serves as the
> -         fallback, but this option can also be disable to reduce kernel size.
> -
> -endmenu
> -
>  config FPU
>         bool "FPU support"
>         default y
> diff --git a/arch/riscv/include/asm/perf_event.h b/arch/riscv/include/asm/perf_event.h
> index 0234048b12bc..8e5b1d81112c 100644
> --- a/arch/riscv/include/asm/perf_event.h
> +++ b/arch/riscv/include/asm/perf_event.h
> @@ -16,15 +16,11 @@
>
>  /*
>   * The RISCV_MAX_COUNTERS parameter should be specified.
> + * Currently, we only support base PMU, so just make
> + * RISCV_MAX_COUNTERS be equal to RISCV_BASE_COUNTERS.
>   */
>
> -#ifdef CONFIG_RISCV_BASE_PMU
> -#define RISCV_MAX_COUNTERS     2
> -#endif
> -
> -#ifndef RISCV_MAX_COUNTERS
> -#error "Please provide a valid RISCV_MAX_COUNTERS for the PMU."
> -#endif
> +#define RISCV_MAX_COUNTERS RISCV_BASE_COUNTERS
>
>  /*
>   * These are the indexes of bits in counteren register *minus* 1,
> @@ -38,12 +34,6 @@
>   */
>  #define RISCV_PMU_CYCLE                0
>  #define RISCV_PMU_INSTRET      1
> -#define RISCV_PMU_MHPMCOUNTER3 2
> -#define RISCV_PMU_MHPMCOUNTER4 3
> -#define RISCV_PMU_MHPMCOUNTER5 4
> -#define RISCV_PMU_MHPMCOUNTER6 5
> -#define RISCV_PMU_MHPMCOUNTER7 6
> -#define RISCV_PMU_MHPMCOUNTER8 7
>
>  #define RISCV_OP_UNSUPP                (-EOPNOTSUPP)
>
> --
> 2.26.2
>

Hi all,

I don't notice that there was a patch already to fix this issue by
Kefeng, (https://lore.kernel.org/linux-riscv/mhng-58148e77-03b6-4c56-98ea-0d0cbf99d522@palmerdabbelt-glaptop1/T/#u).
Even though it was a different way, but I also think it was good for
the present. As my comment in this patch, eventually, the number of
counters should be decided at runtime, such as DTB, then we don't need
to re-build kernel for various platform
which has got a different number of hpmcounters.