Date:   Wed, 08 Feb 2023 11:24:53 +0000
Message-ID: <86lel8z9fe.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Zaid Al-Bassam <zalbassam@google.com>
Cc:     Jesus Sanchez-Palencia <jesussanp@google.com>,
        Russell King <linux@armlinux.org.uk>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Oliver Upton <oliver.upton@linux.dev>,
        Zenghui Yu <yuzenghui@huawei.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, kvmarm@lists.linux.dev,
        kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH 4/8] perf: pmuv3: Add wrappers for KVM accesses
In-Reply-To: <20230126204444.2204061-5-zalbassam@google.com>
References: <20230126204444.2204061-1-zalbassam@google.com>
        <20230126204444.2204061-5-zalbassam@google.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Thu, 26 Jan 2023 20:44:40 +0000,
Zaid Al-Bassam <zalbassam@google.com> wrote:
> 
> KVM host support is available only on arm64. This patch adds wrappers
> to the KVM host function references in the arm_pmuv3.c, so that it is
> up to architecture to populate these wrappers if supported.
> 
> Signed-off-by: Zaid Al-Bassam <zalbassam@google.com>
> ---
>  arch/arm64/include/asm/arm_pmuv3.h | 16 ++++++++++++++++
>  drivers/perf/arm_pmuv3.c           | 11 +++++------
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/arm_pmuv3.h b/arch/arm64/include/asm/arm_pmuv3.h
> index f41a354d1022..9f2a48f5340c 100644
> --- a/arch/arm64/include/asm/arm_pmuv3.h
> +++ b/arch/arm64/include/asm/arm_pmuv3.h
> @@ -20,6 +20,7 @@
>  
>  #include <asm/cpufeature.h>
>  #include <asm/sysreg.h>
> +#include <linux/kvm_host.h>

"linux" includes must be placed before the "asm" ones.

>  
>  /*
>   * This code is really good
> @@ -191,4 +192,19 @@ static inline u32 read_pmceid1(void)
>  	return read_sysreg(pmceid1_el0);
>  }
>  
> +static inline void armv8pmu_kvm_set_events(u32 set, struct perf_event_attr *attr)
> +{
> +	kvm_set_pmu_events(set, attr);
> +}
> +
> +static inline void armv8pmu_kvm_clr_events(u32 clr)
> +{
> +	kvm_clr_pmu_events(clr);
> +}
> +
> +static inline bool armv8pmu_kvm_counter_deferred(struct perf_event_attr *attr)
> +{
> +	return kvm_pmu_counter_deferred(attr);
> +}
> +
>  #endif
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 505f0758260c..d7063dd52827 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -16,7 +16,6 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/clocksource.h>
> -#include <linux/kvm_host.h>
>  #include <linux/of.h>
>  #include <linux/perf/arm_pmu.h>
>  #include <linux/perf/arm_pmuv3.h>
> @@ -607,10 +606,10 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
>  	struct perf_event_attr *attr = &event->attr;
>  	u32 mask = armv8pmu_event_cnten_mask(event);
>  
> -	kvm_set_pmu_events(mask, attr);
> +	armv8pmu_kvm_set_events(mask, attr);

Why the change of name? given that you will implement empty helpers in
the AArch32 code, you might as well keep the names and reduce the churn.

At the end of the day, the only thing you need to do in this patch is
to move the "#include <linux/kvm_host.h>" from this file into the
arch-specific file. Nothing else.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.