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 C5FBEC433FE for ; Wed, 26 Oct 2022 14:44:40 +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=+dnJ8sxAxV4qtUMpZZ4YE9bC8jSE3fHfZpjp9colVkI=; b=sW4A0lVlLuPEDb wu6TP9B1Fwaa2QO4JzMpFCohxrpOOAs5M71xBidL6U9zXMKf0MaVPpla2PGIxcjYD+BCtWmnJXRMI QJ/01nNfhOqYwNR5Dt2MkdhA/T2e4INdxhDBkvDZxn9vWPe+daHiyddPgx/05ZAuMGuvbe9fFKFhu ANlxg0lAW5djOzElMb32wppxtgGihVdIZ4EHHhq2DscmnIQ86UkmHFi2wpkTsRsDulxLDlIVYAK7K JxItqMmzWDUobdigti7u0xI1uhsGfNzBRjMGbPiJwwijrcmaN8YUecbetDTnPZHATysuFYAPzvMq2 /ol1X7FE9208jGWN1ZUA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1onhco-009iRQ-UQ; Wed, 26 Oct 2022 14:43:23 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1onhck-009iNa-W4 for linux-arm-kernel@lists.infradead.org; Wed, 26 Oct 2022 14:43:21 +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 ams.source.kernel.org (Postfix) with ESMTPS id E8B3AB82033; Wed, 26 Oct 2022 14:43:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7D8DCC433C1; Wed, 26 Oct 2022 14:43:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666795390; bh=PlHGblO8nWBuAWZ/hs9qmYpkuPsuKRusd0TWKBsswZc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=jHHmhbk64uDLsOfWMpU46x1Yn587T0nY1tXg2r5ZB59auBlLJrCILBuUvKdlDb99D i3EUV3HLI5mz3MPI9Iw1Vdj0Dgrdm0o6OH37d97FUwjNEpfOxck4jOqAWHj62BZWln 2zV0X22SYL1gzAl/Jv2QUN39UWCMH4l2eILnpm3lWxHzhEIm13FoRYapUD5XH4GajG i5YAK+D8qSFPZQUld2ynbkRCgJFaNmE3K4OohFlp0TXUu3T9hZWG7IYHUlI2mDC4Sh 6rjOb+Ivwp7HQIH5b7HiBa9MQZdPQ3QK+b4ljsRvErtuNcFhO4/XEk04yr0gng2C3d LMZPtY13zK0Cw== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1onhca-001lyk-8C; Wed, 26 Oct 2022 15:43:08 +0100 Date: Wed, 26 Oct 2022 15:43:07 +0100 Message-ID: <86k04mejd0.wl-maz@kernel.org> From: Marc Zyngier To: Reiji Watanabe Cc: Linux ARM , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, kernel-team@android.com Subject: Re: [PATCH 6/9] KVM: arm64: PMU: Move the ID_AA64DFR0_EL1.PMUver limit to VM creation In-Reply-To: References: <20220805135813.2102034-1-maz@kernel.org> <20220805135813.2102034-7-maz@kernel.org> 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/27.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: reijiw@google.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, kernel-team@android.com 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-20221026_074319_350471_DF7F0B15 X-CRM114-Status: GOOD ( 40.39 ) 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 Hi Reiji, Sorry it took so long to get back to this. On Fri, 26 Aug 2022 07:02:21 +0100, Reiji Watanabe wrote: > > Hi Marc, > > On Thu, Aug 25, 2022 at 9:34 PM Reiji Watanabe wrote: > > > > Hi Marc, > > > > On Fri, Aug 5, 2022 at 6:58 AM Marc Zyngier wrote: > > > > > > As further patches will enable the selection of a PMU revision > > > from userspace, sample the supported PMU revision at VM creation > > > time, rather than building each time the ID_AA64DFR0_EL1 register > > > is accessed. > > > > > > This shouldn't result in any change in behaviour. > > > > > > Signed-off-by: Marc Zyngier > > > --- > > > arch/arm64/include/asm/kvm_host.h | 1 + > > > arch/arm64/kvm/arm.c | 6 ++++++ > > > arch/arm64/kvm/pmu-emul.c | 11 +++++++++++ > > > arch/arm64/kvm/sys_regs.c | 26 +++++++++++++++++++++----- > > > include/kvm/arm_pmu.h | 6 ++++++ > > > 5 files changed, 45 insertions(+), 5 deletions(-) > > > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > > index f38ef299f13b..411114510634 100644 > > > --- a/arch/arm64/include/asm/kvm_host.h > > > +++ b/arch/arm64/include/asm/kvm_host.h > > > @@ -163,6 +163,7 @@ struct kvm_arch { > > > > > > u8 pfr0_csv2; > > > u8 pfr0_csv3; > > > + u8 dfr0_pmuver; > > > > > > /* Hypercall features firmware registers' descriptor */ > > > struct kvm_smccc_features smccc_feat; > > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > > > index 8fe73ee5fa84..e4f80f0c1e97 100644 > > > --- a/arch/arm64/kvm/arm.c > > > +++ b/arch/arm64/kvm/arm.c > > > @@ -164,6 +164,12 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) > > > set_default_spectre(kvm); > > > kvm_arm_init_hypercalls(kvm); > > > > > > + /* > > > + * Initialise the default PMUver before there is a chance to > > > + * create an actual PMU. > > > + */ > > > + kvm->arch.dfr0_pmuver = kvm_arm_pmu_get_host_pmuver(); > > > + > > > return ret; > > > out_free_stage2_pgd: > > > kvm_free_stage2_pgd(&kvm->arch.mmu); > > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > > > index ddd79b64b38a..33a88ca7b7fd 100644 > > > --- a/arch/arm64/kvm/pmu-emul.c > > > +++ b/arch/arm64/kvm/pmu-emul.c > > > @@ -1021,3 +1021,14 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) > > > > > > return -ENXIO; > > > } > > > + > > > +u8 kvm_arm_pmu_get_host_pmuver(void) > > > > Nit: Since this function doesn't simply return the host's pmuver, but the > > pmuver limit for guests, perhaps "kvm_arm_pmu_get_guest_pmuver_limit" > > might be more clear (closer to what it does) ? Maybe a bit verbose, but I'll work something out. > > > > > +{ > > > + u64 tmp; > > > + > > > + tmp = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1); > > > + tmp = cpuid_feature_cap_perfmon_field(tmp, > > > + ID_AA64DFR0_PMUVER_SHIFT, > > > + ID_AA64DFR0_PMUVER_8_4); > > > + return FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER), tmp); > > > +} > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > > index 333efddb1e27..55451f49017c 100644 > > > --- a/arch/arm64/kvm/sys_regs.c > > > +++ b/arch/arm64/kvm/sys_regs.c > > > @@ -1062,6 +1062,22 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, > > > return true; > > > } > > > > > > +static u8 pmuver_to_perfmon(const struct kvm_vcpu *vcpu) > > > +{ > > > + if (!kvm_vcpu_has_pmu(vcpu)) > > > + return 0; > > > + > > > + switch (vcpu->kvm->arch.dfr0_pmuver) { > > > + case ID_AA64DFR0_PMUVER_8_0: > > > + return ID_DFR0_PERFMON_8_0; > > > + case ID_AA64DFR0_PMUVER_IMP_DEF: > > > + return 0; > > > + default: > > > + /* Anything ARMv8.4+ has the same value. For now. */ > > > + return vcpu->kvm->arch.dfr0_pmuver; > > > + } > > > +} > > > + > > > /* Read a sanitised cpufeature ID register by sys_reg_desc */ > > > static u64 read_id_reg(const struct kvm_vcpu *vcpu, > > > struct sys_reg_desc const *r, bool raz) > > > @@ -1112,10 +1128,10 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, > > > /* Limit debug to ARMv8.0 */ > > > val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER); > > > val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_DEBUGVER), 6); > > > - /* Limit guests to PMUv3 for ARMv8.4 */ > > > - val = cpuid_feature_cap_perfmon_field(val, > > > - ID_AA64DFR0_PMUVER_SHIFT, > > > - kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_4 : 0); > > > + /* Set PMUver to the required version */ > > > + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER); > > > + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_PMUVER), > > > + kvm_vcpu_has_pmu(vcpu) ? vcpu->kvm->arch.dfr0_pmuver : 0); > > I've just noticed one issue in this patch while I'm reviewing patch-7. > > I would think that this patch makes PMUVER and PERFMON inconsistent > when PMU is not enabled for the vCPU, and the host's sanitised PMUVER > is IMP_DEF. > > Previously, when PMU is not enabled for the vCPU and the host's > sanitized value of PMUVER is IMP_DEF(0xf), the vCPU's PMUVER and PERFMON > are set to IMP_DEF due to a bug of cpuid_feature_cap_perfmon_field(). > (https://lore.kernel.org/all/20220214065746.1230608-11-reijiw@google.com/) > > With this patch, the vCPU's PMUVER will be 0 for the same case, > while the vCPU's PERFMON will stay the same (IMP_DEF). > I guess you unintentionally corrected only the PMUVER value of the VCPU. I think that with this patch both PMUVer and Perfmon values get set to 0 (pmuver_to_perfmon() returns 0 for both ID_AA64DFR0_PMUVER_IMP_DEF and no PMU at all). Am I missing anything here? However, you definitely have a point that we should handle a guest being restored with an IMPDEF PMU. Which means I need to revisit this patch and the userspace accessors. Oh well... Thanks, M. -- 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