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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04574C433F5 for ; Wed, 1 Dec 2021 16:10:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351400AbhLAQNh (ORCPT ); Wed, 1 Dec 2021 11:13:37 -0500 Received: from foss.arm.com ([217.140.110.172]:40994 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351420AbhLAQMi (ORCPT ); Wed, 1 Dec 2021 11:12:38 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EA79F143B; Wed, 1 Dec 2021 08:09:16 -0800 (PST) Received: from monolith.localdoman (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 58FC13F766; Wed, 1 Dec 2021 08:09:15 -0800 (PST) Date: Wed, 1 Dec 2021 16:09:09 +0000 From: Alexandru Elisei To: Reiji Watanabe Cc: Eric Auger , Marc Zyngier , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Will Deacon , Peter Shier , Paolo Bonzini , linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH v3 09/29] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest Message-ID: References: <20211117064359.2362060-1-reijiw@google.com> <20211117064359.2362060-10-reijiw@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Reiji, On Wed, Dec 01, 2021 at 03:53:18PM +0000, Alexandru Elisei wrote: > Hi Reiji, > > On Mon, Nov 29, 2021 at 09:32:02PM -0800, Reiji Watanabe wrote: > > Hi Eric, > > > > On Thu, Nov 25, 2021 at 12:30 PM Eric Auger wrote: > > > > > > Hi Reiji, > > > > > > On 11/17/21 7:43 AM, Reiji Watanabe wrote: > > > > When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which > > > > means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally > > > > expose the value for the guest as it is. Since KVM doesn't support > > > > IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should > > > > exopse 0x0 (PMU is not implemented) instead. > > > s/exopse/expose > > > > > > > > Change cpuid_feature_cap_perfmon_field() to update the field value > > > > to 0x0 when it is 0xf. > > > is it wrong to expose the guest with a Perfmon value of 0xF? Then the > > > guest should not use it as a PMUv3? > > > > > is it wrong to expose the guest with a Perfmon value of 0xF? Then the > > > guest should not use it as a PMUv3? > > > > For the value 0xf in ID_AA64DFR0_EL1.PMUVER and ID_DFR0_EL1.PERFMON, > > Arm ARM says: > > "IMPLEMENTATION DEFINED form of performance monitors supported, > > PMUv3 not supported." > > > > Since the PMU that KVM supports for guests is PMUv3, 0xf shouldn't > > be exposed to guests (And this patch series doesn't allow userspace > > to set the fields to 0xf for guests). > > While it's true that a value of 0xf means that PMUv3 is not present (both > KVM and the PMU driver handle it this way) this is an userspace visible > change. > > Are you sure there isn't software in the wild that relies on this value > being 0xf to detect that some non-Arm architected hardware is present? > > Since both 0 and 0xf are valid values that mean that PMUv3 is not present, > I think it's best that both are kept. Sorry, somehow I managed to get myself confused and didn't realize that this is only used by KVM. What I said above about the possibility of software existing that pokes IMP DEF registers when PMUVer = 0xf is in fact a good argument for this patch, because KVM injects an undefined exception when a guest tries to access such registers. Thanks, Alex > > Thanks, > Alex > > > > > Thanks, > > Reiji > > > > > > > > Eric > > > > > > > > Fixes: 8e35aa642ee4 ("arm64: cpufeature: Extract capped perfmon fields") > > > > Signed-off-by: Reiji Watanabe > > > > --- > > > > arch/arm64/include/asm/cpufeature.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > > > > index ef6be92b1921..fd7ad8193827 100644 > > > > --- a/arch/arm64/include/asm/cpufeature.h > > > > +++ b/arch/arm64/include/asm/cpufeature.h > > > > @@ -553,7 +553,7 @@ cpuid_feature_cap_perfmon_field(u64 features, int field, u64 cap) > > > > > > > > /* Treat IMPLEMENTATION DEFINED functionality as unimplemented */ > > > > if (val == ID_AA64DFR0_PMUVER_IMP_DEF) > > > > - val = 0; > > > > + return (features & ~mask); > > > > > > > > if (val > cap) { > > > > features &= ~mask; > > > > > > >