On Mon, 09 Apr 2018 00:07:11 PDT (-0700), alan...@andestech.com wrote:
On Thu, Apr 05, 2018 at 09:47:50AM -0700, Palmer Dabbelt wrote:
On Mon, 26 Mar 2018 00:57:54 PDT (-0700), alan...@andestech.com wrote:
>This patch provide a basic PMU, riscv_base_pmu, which supports two
>general hardware event, instructions and cycles.  Furthermore, this
>PMU serves as a reference implementation to ease the portings in
>the future.
>riscv_base_pmu should be able to run on any RISC-V machine that
>conforms to the Priv-Spec.  Note that the latest qemu model hasn't
>fully support a proper behavior of Priv-Spec 1.10 yet, but work
>around should be easy with very small fixes.  Please check
>https://github.com/riscv/riscv-qemu/pull/115 for future updates.
>Cc: Nick Hu <nic...@andestech.com>
>Cc: Greentime Hu <greent...@andestech.com>
>Signed-off-by: Alan Kao <alan...@andestech.com>

We should really be able to detect PMU types at runtime (via a device tree
entry) rather than requiring that a single PMU is built in to the kernel.
This will require a handful of modifications to how this patch works, which
I'll try to list below.

>+menu "PMU type"
>+   depends on PERF_EVENTS
>+   bool "Base Performance Monitoring Unit"
>+   def_bool y
>+   help
>+     A base PMU that serves as a reference implementation and has limited
>+     feature of perf.

Rather than a menu where a single PMU can be selected, there should be
options to enable or disable support for each PMU type -- this is just like
how all our other drivers work.

I see.  Sure.  The descriptions and implementation will be refined in v3.

>+struct pmu * __weak __init riscv_init_platform_pmu(void)
>+   riscv_pmu = &riscv_base_pmu;
>+   return riscv_pmu->pmu;

Rather than relying on a weak symbol that gets overridden by other PMU
types, this should look through the device tree for a compatible PMU (in the
case of just the base PMU it could be any RISC-V hart) and install a PMU
handler for it.  There'd probably be some sort of priority scheme here, like
there are for other driver subsystems, where we'd pick the best PMU driver
that's compatible with the PMUs on every hart.

>+int __init init_hw_perf_events(void)
>+   struct pmu *pmu = riscv_init_platform_pmu();
>+   perf_irq = NULL;
>+   perf_pmu_register(pmu, "cpu", PERF_TYPE_RAW);
>+   return 0;

Since we only have a single PMU type right now this isn't critical to handle
right away, but we will have to refactor this before adding another PMU.

I see.  My rough plan is to do the device tree parsing here, and if no specific
PMU string is found then just register the base PMU proposed in this patch.
How about this idea?

Sounds good. We know the generic PMU will work on all RISC-V harts, so there's no need to add an explicit device tree entry for it. Then we can figure out how to add device tree entries for custom performance monitors later :)
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to