On Wed, Jan 15, 2025 at 08:09:45PM +0530, Vaibhav Jain wrote:
> Introduce a new PMU named 'kvm-hv' to report Book3s kvm-hv specific
> performance counters. This will expose KVM-HV specific performance
> attributes to user-space via kernel's PMU infrastructure and would enable
> users to monitor active kvm-hv based guests.
>
> The patch creates necessary scaffolding to for the new PMU callbacks and
> introduces two new exports kvmppc_{,un}register_pmu() that are called from
> kvm-hv init and exit function to perform initialize and cleanup for the
> 'kvm-hv' PMU. The patch doesn't introduce any perf-events yet, which will
> be introduced in later patches
>
> Signed-off-by: Vaibhav Jain <vaib...@linux.ibm.com>
>
> ---
> Changelog
>
> v1->v2:
> * Fixed an issue of kvm-hv not loading on baremetal kvm [Gautam]
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 12 +++
> arch/powerpc/kvm/Makefile | 6 ++
> arch/powerpc/kvm/book3s_hv.c | 9 ++
> arch/powerpc/kvm/book3s_hv_pmu.c | 133 ++++++++++++++++++++++++++
> 4 files changed, 160 insertions(+)
> create mode 100644 arch/powerpc/kvm/book3s_hv_pmu.c
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
> b/arch/powerpc/include/asm/kvm_book3s.h
> index e1ff291ba891..cf91a1493159 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -334,6 +334,9 @@ static inline bool kvmhv_is_nestedv1(void)
> return !static_branch_likely(&__kvmhv_is_nestedv2);
> }
>
> +int kvmppc_register_pmu(void);
> +void kvmppc_unregister_pmu(void);
> +
> #else
>
> static inline bool kvmhv_is_nestedv2(void)
> @@ -346,6 +349,15 @@ static inline bool kvmhv_is_nestedv1(void)
> return false;
> }
>
> +static int kvmppc_register_pmu(void)
> +{
> + return 0;
> +}
> +
> +static void kvmppc_unregister_pmu(void)
> +{
> +}
> +
> #endif
>
> int __kvmhv_nestedv2_reload_ptregs(struct kvm_vcpu *vcpu, struct pt_regs
> *regs);
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index 4bd9d1230869..094c3916d9d0 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -92,6 +92,12 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER)
> += \
> $(kvm-book3s_64-builtin-tm-objs-y) \
> $(kvm-book3s_64-builtin-xics-objs-y)
>
> +# enable kvm_hv perf events
> +ifdef CONFIG_HAVE_PERF_EVENTS
> +kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
> + book3s_hv_pmu.o
> +endif
> +
> obj-$(CONFIG_GUEST_STATE_BUFFER_TEST) += test-guest-state-buffer.o
> endif
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 25429905ae90..6365b8126574 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -6662,6 +6662,14 @@ static int kvmppc_book3s_init_hv(void)
> return r;
> }
>
> + r = kvmppc_register_pmu();
> + if (r == -EOPNOTSUPP) {
> + pr_info("KVM-HV: PMU not supported %d\n", r);
> + } else if (r) {
> + pr_err("KVM-HV: Unable to register PMUs %d\n", r);
> + goto err;
> + }
> +
I believe we can simplify this part as follows:
r = kvmppc_register_pmu();
if (r) {
pr_err("KVM-HV: Unable to register PMUs %d\n", r);
goto err;
}
This would also require a minor change in kvmppc_register_pmu(), see below
> kvm_ops_hv.owner = THIS_MODULE;
> kvmppc_hv_ops = &kvm_ops_hv;
>
> @@ -6676,6 +6684,7 @@ static int kvmppc_book3s_init_hv(void)
>
> static void kvmppc_book3s_exit_hv(void)
> {
> + kvmppc_unregister_pmu();
> kvmppc_uvmem_free();
> kvmppc_free_host_rm_ops();
> if (kvmppc_radix_possible())
> diff --git a/arch/powerpc/kvm/book3s_hv_pmu.c
> b/arch/powerpc/kvm/book3s_hv_pmu.c
> new file mode 100644
> index 000000000000..8c6ed30b7654
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_hv_pmu.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Description: PMUs specific to running nested KVM-HV guests
> + * on Book3S processors (specifically POWER9 and later).
> + */
> +
> +#define pr_fmt(fmt) "kvmppc-pmu: " fmt
> +
> +#include "asm-generic/local64.h"
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/ratelimit.h>
> +#include <linux/kvm_host.h>
> +#include <linux/gfp_types.h>
> +#include <linux/pgtable.h>
> +#include <linux/perf_event.h>
> +#include <linux/spinlock_types.h>
> +#include <linux/spinlock.h>
> +
> +#include <asm/types.h>
> +#include <asm/kvm_ppc.h>
> +#include <asm/kvm_book3s.h>
> +#include <asm/mmu.h>
> +#include <asm/pgalloc.h>
> +#include <asm/pte-walk.h>
> +#include <asm/reg.h>
> +#include <asm/plpar_wrappers.h>
> +#include <asm/firmware.h>
> +
> +enum kvmppc_pmu_eventid {
> + KVMPPC_EVENT_MAX,
> +};
> +
> +static struct attribute *kvmppc_pmu_events_attr[] = {
> + NULL,
> +};
> +
> +static const struct attribute_group kvmppc_pmu_events_group = {
> + .name = "events",
> + .attrs = kvmppc_pmu_events_attr,
> +};
> +
> +PMU_FORMAT_ATTR(event, "config:0");
> +static struct attribute *kvmppc_pmu_format_attr[] = {
> + &format_attr_event.attr,
> + NULL,
> +};
> +
> +static struct attribute_group kvmppc_pmu_format_group = {
> + .name = "format",
> + .attrs = kvmppc_pmu_format_attr,
> +};
> +
> +static const struct attribute_group *kvmppc_pmu_attr_groups[] = {
> + &kvmppc_pmu_events_group,
> + &kvmppc_pmu_format_group,
> + NULL,
> +};
> +
> +static int kvmppc_pmu_event_init(struct perf_event *event)
> +{
> + unsigned int config = event->attr.config;
> +
> + pr_debug("%s: Event(%p) id=%llu cpu=%x on_cpu=%x config=%u",
> + __func__, event, event->id, event->cpu,
> + event->oncpu, config);
> +
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + if (config >= KVMPPC_EVENT_MAX)
> + return -EINVAL;
> +
> + local64_set(&event->hw.prev_count, 0);
> + local64_set(&event->count, 0);
> +
> + return 0;
> +}
> +
> +static void kvmppc_pmu_del(struct perf_event *event, int flags)
> +{
> +}
> +
> +static int kvmppc_pmu_add(struct perf_event *event, int flags)
> +{
> + return 0;
> +}
> +
> +static void kvmppc_pmu_read(struct perf_event *event)
> +{
> +}
> +
> +/* L1 wide counters PMU */
> +static struct pmu kvmppc_pmu = {
> + .task_ctx_nr = perf_sw_context,
> + .name = "kvm-hv",
> + .event_init = kvmppc_pmu_event_init,
> + .add = kvmppc_pmu_add,
> + .del = kvmppc_pmu_del,
> + .read = kvmppc_pmu_read,
> + .attr_groups = kvmppc_pmu_attr_groups,
> + .type = -1,
> +};
> +
> +int kvmppc_register_pmu(void)
> +{
> + int rc = -EOPNOTSUPP;
> +
> + /* only support events for nestedv2 right now */
> + if (kvmhv_is_nestedv2()) {
> + /* Setup done now register the PMU */
> + pr_info("Registering kvm-hv pmu");
> +
> + /* Register only if we arent already registered */
> + rc = (kvmppc_pmu.type == -1) ?
> + perf_pmu_register(&kvmppc_pmu, kvmppc_pmu.name,
> + -1) : 0;
> + }
> +
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_register_pmu);
We can change the return code to support change suggested above
int kvmppc_register_pmu(void)
{
int rc;
/* only support events for nestedv2 right now */
if (kvmhv_is_nestedv2()) {
/* Setup done now register the PMU */
pr_info("Registering kvm-hv pmu");
/* Register only if we arent already registered */
rc = (kvmppc_pmu.type == -1) ?
perf_pmu_register(&kvmppc_pmu, kvmppc_pmu.name,
-1) : 0;
} else {
pr_info("KVM-HV: PMU not supported %d\n", r);
}
return rc;
}
> +
> +void kvmppc_unregister_pmu(void)
> +{
> + if (kvmhv_is_nestedv2()) {
> + if (kvmppc_pmu.type != -1)
> + perf_pmu_unregister(&kvmppc_pmu);
> +
> + pr_info("kvmhv_pmu unregistered.\n");
> + }
> +}
We can get rid of nested ifs. That way, the pr_info will also be emitted
only when we actually call perf_pmu_unregister()
if (kvmhv_is_nestedv2() && kvmppc_pmu.type != -1) {
perf_pmu_unregister(&kvmppc_pmu);
pr_info("kvmhv_pmu unregistered.\n");
}
> +EXPORT_SYMBOL_GPL(kvmppc_unregister_pmu);
> --
> 2.47.1
>
