Re: [PATCH v2] KVM: arm64: pmu: Resync EL0 state on counter rotation

From: Alexander Stein <alexander.stein@ew.tq-group.com>
To: Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Huang Shijie <shijie@os.amperecomputing.com>,
	Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2] KVM: arm64: pmu: Resync EL0 state on counter rotation
Date: Wed, 23 Aug 2023 13:15:47 +0200	[thread overview]
Message-ID: <4055305.aeNJFYEL58@steina-w> (raw)
In-Reply-To: <86h6oqf712.wl-maz@kernel.org>

Hi Marc,

Am Mittwoch, 23. August 2023, 11:21:45 CEST schrieb Marc Zyngier:
> On Wed, 23 Aug 2023 08:15:38 +0100,
> 
> Alexander Stein <alexander.stein@ew.tq-group.com> wrote:
> > Hi,
> > 
> > > diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> > > index 08b3a1bf0ef6..6a3d8176f54a 100644
> > > --- a/drivers/perf/arm_pmuv3.c
> > > +++ b/drivers/perf/arm_pmuv3.c
> > > @@ -772,6 +772,8 @@ static void armv8pmu_start(struct arm_pmu *cpu_pmu)
> > > 
> > >  	/* Enable all counters */
> > >  	armv8pmu_pmcr_write(armv8pmu_pmcr_read() | ARMV8_PMU_PMCR_E);
> > > 
> > > +
> > > +	kvm_vcpu_pmu_resync_el0();
> > 
> > This breaks if CONFIG_HW_PERF_EVENTS is enabled but CONFIG_KVM is not. As
> > in this case arch/arm64/kvm/pmu.c is never compiled, but the dummy inline
> > in include/kvm/arm_pmu.h for kvm_vcpu_pmu_resync_el0() only depends on
> > CONFIG_HW_PERF_EVENTS.
> > This results in this error:
> > 
> > aarch64-v8a-linux-gnu-ld: Unexpected GOT/PLT entries detected!
> > aarch64-v8a-linux-gnu-ld: Unexpected run-time procedure linkages detected!
> > aarch64-v8a-linux-gnu-ld: drivers/perf/arm_pmuv3.o: in function
> > `armv8pmu_start':
> > drivers/perf/arm_pmuv3.c:753: undefined reference to
> > `kvm_vcpu_pmu_resync_el0'
> Huh, that's unexpected. Let's fix it with an adequately sized hammer.
> Does the patch below work for you?

Yes, the patch below does work for me. Thanks.
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Best regards,
Alexander

> Thanks,
> 
> 	M.
> 
> From 2446f15d355ab7e766291eccf5b670a2060b4c3a Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Wed, 23 Aug 2023 10:14:48 +0100
> Subject: [PATCH] KVM: arm64: pmu: Guard PMU emulation definitions with
>  CONFIG_KVM
> 
> Most of the internal definitions for PMU emulation are guarded with
> CONFIG_HW_PERF_EVENTS. However, this isn't enough, and leads to
> these definitions leaking if CONFIG_KVM isn't enabled.
> 
> This leads to some compilation breakage in this exact configuration.
> Fix it by falling back to the dummy stubs if either perf or KVM
> isn't selected.
> 
> Reported-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  include/kvm/arm_pmu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 3a8a70a60794..31029f4f7be8 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -12,7 +12,7 @@
> 
>  #define ARMV8_PMU_CYCLE_IDX		(ARMV8_PMU_MAX_COUNTERS - 1)
> 
> -#ifdef CONFIG_HW_PERF_EVENTS
> +#if IS_ENABLED(CONFIG_HW_PERF_EVENTS) && IS_ENABLED(CONFIG_KVM)
> 
>  struct kvm_pmc {
>  	u8 idx;	/* index into the pmu->pmc array */

-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/