From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CA95DC7EE2C for ; Fri, 2 Jun 2023 09:05:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=o1gb1jI5xf40LQ97a/jX4CFTqZWtEvNojg1VeWMsb2o=; b=C45oImd7cOwX4+ VV+jnMQyoFVPTFIiEqZ3zCyuuBqBu8UUdJAsHxY5nUtiGvu13rLcqd2XhIyJPAm+R9ZZpWhVieQR7 cd6t1NmbmAvZo9A0qpcYbXzrA1u0XXgP183N/bz9vmEx35F0C2IQwWfI4Sk4sbaJbrdgM1G+Vjult 7f2ztJnemm7xWnYMujr28DHjjG055/S+c101tI0/xLtnMBAdSxaBh27eTKN2fsJyyqTU8ldPdYi/N Hg3y6QE5rq1cfLFSYGOVTQkSa4Vxxcbp89+GQub+x6vVmfTA1h2xZjCkPzD1KcVLBoPGhyRoGJYcI WEbaaUzSLxX1vitsgcZA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q50ir-006J7u-1b; Fri, 02 Jun 2023 09:05:25 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q50in-006J5y-1A for linux-arm-kernel@lists.infradead.org; Fri, 02 Jun 2023 09:05:23 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9B0B864DCA; Fri, 2 Jun 2023 09:05:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BFE24C43443; Fri, 2 Jun 2023 09:05:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685696720; bh=gV19WUb11hoo0vbk5f8hro1RpEfA3/kK52O1V4qHOEI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=QP9MiC0C+kAhfkwc0YMZLL5lrq2HgWy1MmkYXdNnHSyjOdLU0z6UgHz3a8h75p3J8 sNm/RvU8z8z5kC68Cv/LrgetZPdZ4zlzbjBp8HwjdxAfy9wlChD+m2q85jVc/3RCCA bNDYC+rq4WMuBs73DbTg8kNv8zHUR8Wnw3oFmo9Psilv1jSViOyuBU0b+Q8LvGTM1O 4w5LU/EY05ouFmQ+y3iZSxrRz1DddZBvEkHMAWb4pZawX5mQ1NmG1MvrECZBGKJRV4 BY9EUG4hfgfNKU14DxPdFwTgnA8P7pRFg2fjYeTNU1ISJiPKb48cFjMRZg6UM+jqef 20ajp+aigv3RA== Received: from 90.4.23.109.rev.sfr.net ([109.23.4.90] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1q50ii-002I6P-PL; Fri, 02 Jun 2023 10:05:17 +0100 Date: Fri, 02 Jun 2023 10:05:15 +0100 Message-ID: <874jnqp73o.wl-maz@kernel.org> From: Marc Zyngier To: Reiji Watanabe Cc: Oliver Upton , kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, James Morse , Alexandru Elisei , Zenghui Yu , Suzuki K Poulose , Paolo Bonzini , Ricardo Koller , Jing Zhang , Raghavendra Rao Anata , Will Deacon Subject: Re: [PATCH 0/4] KVM: arm64: PMU: Fix PMUVer handling on heterogeneous PMU systems In-Reply-To: <20230602052323.shjn3q2rslbuwcmc@google.com> References: <20230527040236.1875860-1-reijiw@google.com> <87zg5njlyn.wl-maz@kernel.org> <20230530125324.ijrwrvoll2detpus@google.com> <87mt1jkc5q.wl-maz@kernel.org> <20230602052323.shjn3q2rslbuwcmc@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 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 109.23.4.90 X-SA-Exim-Rcpt-To: reijiw@google.com, oliver.upton@linux.dev, kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, james.morse@arm.com, alexandru.elisei@arm.com, yuzenghui@huawei.com, suzuki.poulose@arm.com, pbonzini@redhat.com, ricarkol@google.com, jingzhangos@google.com, rananta@google.com, will@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230602_020521_480573_630E8C83 X-CRM114-Status: GOOD ( 67.56 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 02 Jun 2023 06:23:23 +0100, Reiji Watanabe wrote: > > Hi Marc, > > On Thu, Jun 01, 2023 at 06:02:41AM +0100, Marc Zyngier wrote: > > Hey Reiji, > > > > On Tue, 30 May 2023 13:53:24 +0100, > > Reiji Watanabe wrote: > > > > > > Hi Marc, > > > > > > On Mon, May 29, 2023 at 02:39:28PM +0100, Marc Zyngier wrote: > > > > On Sat, 27 May 2023 05:02:32 +0100, > > > > Reiji Watanabe wrote: > > > > > > > > > > This series fixes issues with PMUVer handling for a guest with > > > > > PMU configured on heterogeneous PMU systems. > > > > > Specifically, it addresses the following two issues. > > > > > > > > > > [A] The default value of ID_AA64DFR0_EL1.PMUVer of the vCPU is set > > > > > to its sanitized value. This could be inappropriate on > > > > > heterogeneous PMU systems, as arm64_ftr_bits for PMUVer is defined > > > > > as FTR_EXACT with safe_val == 0 (when ID_AA64DFR0_EL1.PMUVer of all > > > > > PEs on the host is not uniform, the sanitized value will be 0). > > > > > > > > Why is this a problem? The CPUs don't implement the same version of > > > > the architecture, we don't get a PMU. Why should we try to do anything > > > > better? I really don't think we should go out or out way and make the > > > > code more complicated for something that doesn't really exist. > > > > > > Even when the CPUs don't implement the same version of the architecture, > > > if one of them implement PMUv3, KVM advertises KVM_CAP_ARM_PMU_V3, > > > and allows userspace to configure PMU (KVM_ARM_VCPU_PMU_V3) for vCPUs. > > > > Ah, I see it now. The kernel will register the PMU even if it decides > > that advertising it is wrong, and then we pick it up. Great :-/. > > > > > In this case, although KVM provides PMU emulations for the guest, > > > the guest's ID_AA64DFR0_EL1.PMUVer will be zero. Also, > > > KVM_SET_ONE_REG for ID_AA64DFR0_EL1 will never work for vCPUs > > > with PMU configured on such systems (since KVM also doesn't allow > > > userspace to set the PMUVer to 0 for the vCPUs with PMU configured). > > > > > > I would think either ID_AA64DFR0_EL1.PMUVer for the guest should > > > indicate PMUv3, or KVM should not allow userspace to configure PMU, > > > in this case. > > > > My vote is on the latter. Even if a PMU is available, we should rely > > on the feature exposed by the kernel to decide whether exposing a PMU > > or not. > > > > To be honest, this will affect almost nobody (I only know of a single > > one, an obscure ARMv8.0+ARMv8.2 system which is very unlikely to ever > > use KVM). I'm happy to take the responsibility to actively break those. > > Thank you for the information! Just curious, how about a mix of > cores with and without PMU ? (with the same ARMv8.x version) > I'm guessing there are very few if any though :) I don't know of any. Similar things for IMPDEF PMUs. And to be honest, I'd be very tempted to nuke that in KVM as well, because this is one of the worse decision I ever made. > > > This series is a fix for the former, mainly to keep the current > > > behavior of KVM_CAP_ARM_PMU_V3 and KVM_ARM_VCPU_PMU_V3 on such > > > systems, since I wasn't sure if such systems don't really exist :) > > > (Also, I plan to implement a similar fix for PMCR_EL0.N on top of > > > those changes) > > > > > > I could make a fix for the latter instead though. What do you think ? > > > > I think this would be valuable. > > Thank you for the comment! I will go with the latter. Thanks. > > 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 : 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 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. And while we're at it, drop the same __weak attribute from the arch_perf_get_page_size() prototype. Reported-by: Reiji Watanabe Signed-off-by: Marc Zyngier --- 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel