[PATCH 0/4] KVM: arm64: PMU: Fix PMUVer handling on heterogeneous PMU systems

[Reiji Watanabe]

> > > Also, didn't you have patches for the EL0 side of the PMU? I've been
> > > trying to look for a new version, but couldn't find it...
> > 
> > While I'm working on fixing the series based on the recent comment from
> > Oliver (https://lore.kernel.org/all/ZG%2Fw95pYjWnMJB62@linux.dev/),
> > I have a new PMU EL0 issue, which blocked my testing of the series.
> > So, I am debugging the new PMU EL0 issue.
> > 
> > It appears that arch_perf_update_userpage() defined in
> > drivers/perf/arm_pmuv3.c isn't used, and instead, the weak one in
> > kernel/events/core.c is used.
> 
> Wut??? How comes? /me disassembles the kernel:
> 
> ffff8000082a1ab0 <arch_perf_update_userpage>:
> ffff8000082a1ab0:       d503201f        nop
> ffff8000082a1ab4:       d503201f        nop
> ffff8000082a1ab8:       d65f03c0        ret
> ffff8000082a1abc:       d503201f        nop
> ffff8000082a1ac0:       d503201f        nop
> ffff8000082a1ac4:       d503201f        nop
> 
> What the hell is happening here???
> 
> > This prevents cap_user_rdpmc (, etc)
> > from being set (This prevented my test program from directly
> > accessing counters).  This seems to be caused by the commit 7755cec63ade
> > ("arm64: perf: Move PMUv3 driver to drivers/perf").
> 
> It is becoming more puzzling by the minute.
> 
> > 
> > I have not yet figured out why the one in arm_pmuv3.c isn't used
> > though (The weak one in core.c seems to take precedence over strong
> > ones under drivers/ somehow...).
> > 
> > Anyway, I worked around the new issue for now, and ran the test for
> > my series though. I will post the new version of the EL0 series
> > tomorrow hopefully.
> 
> I have a "fix" for this. It doesn't make any sense, but it seems to
> work here (GCC 10.2.1 from Debian). Can you please give it a shot?
> 
> Thanks,
> 
> 	M.
> 
> From 236ac26bd0e03bf2ca3b40471b61a35b02272662 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz at kernel.org>
> Date: Fri, 2 Jun 2023 09:52:25 +0100
> Subject: [PATCH] perf/core: Drop __weak attribute on arch-specific prototypes
> 
> Reiji reports that the arm64 implementation of arch_perf_update_userpage()
> is now ignored and replaced by the dummy stub in core code.
> This seems to happen since the PMUv3 driver was moved to driver/perf.
> 
> As it turns out, dropping the __weak attribute from the *prototype*
> of the function solves the problem. You're right, this doesn't seem
> to make much sense. And yet...
> 
> With this, arm64 is able to enjoy arch_perf_update_userpage() again.

Oh, that's interesting... But, it worked, thank you!
(With the patch, the disassembles of the kernel for
arch_perf_update_userpage look right, and my EL0 test works fine)


> And while we're at it, drop the same __weak attribute from the
> arch_perf_get_page_size() prototype.

The arch_perf_get_page_size() prototype seems to be unnecessary now
(after the commit 8af26be06272 "erf/core: Fix arch_perf_get_page_size()").
So, it appears that we could drop the prototype itself.

Thank you,
Reiji


> 
> Reported-by: Reiji Watanabe <reijiw at google.com>
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> ---
>  include/linux/perf_event.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index d5628a7b5eaa..1509aea69a16 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1845,12 +1845,12 @@ int perf_event_exit_cpu(unsigned int cpu);
>  #define perf_event_exit_cpu	NULL
>  #endif
>  
> -extern void __weak arch_perf_update_userpage(struct perf_event *event,
> -					     struct perf_event_mmap_page *userpg,
> -					     u64 now);
> +extern void arch_perf_update_userpage(struct perf_event *event,
> +				      struct perf_event_mmap_page *userpg,
> +				      u64 now);
>  
>  #ifdef CONFIG_MMU
> -extern __weak u64 arch_perf_get_page_size(struct mm_struct *mm, unsigned long addr);
> +extern u64 arch_perf_get_page_size(struct mm_struct *mm, unsigned long addr);
>  #endif
>  
>  /*
> -- 
> 2.39.2
> 
> 
> -- 
> Without deviation from the norm, progress is not possible.