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 49AFAC10F1B for ; Sat, 10 Dec 2022 11:03:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229538AbiLJLDm (ORCPT ); Sat, 10 Dec 2022 06:03:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51992 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229529AbiLJLDl (ORCPT ); Sat, 10 Dec 2022 06:03:41 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DD646165BB for ; Sat, 10 Dec 2022 03:03:39 -0800 (PST) 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 93E9CB80ABF for ; Sat, 10 Dec 2022 11:03:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1E593C433D2; Sat, 10 Dec 2022 11:03:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670670217; bh=Oc6bgUBpCXSzLCNc1FFwEecQeeVQ0ev/WIiBfYnZUmE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=d4YOgDkygnkwA+JkltLVbXRWGG7Y/ekq494XE1Hafajd2M5iiSwasVGLwuJTMPENK 0lMsBntsjFoDfVcg7gO6tvMTTfEU6jTjbajTzgHXq3/Q7lhGVAC6D0Z9uTvqaVvHJ8 +6+hae5B2UEZKEmPgYZq9802LC0rvg3kfmMaaOMl3PsyQpL8Q4NgMvDC6TzegVmuAp gVdjucCD4CCZSfdFjPB4DtrAfxqcZ7gvo/p4pWsqnnecWbm/TcbBS8Ify17/Ot8efd wvZW/Khq/DvRtlYdcHxTG8BQtaZg1y0GTzTXtgdcDwZj0M0VNEhuKR/A6OHqEzNSZT 5kGmHaVb/Zqmg== Received: from sofa.misterjones.org ([185.219.108.64] 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 1p3xdm-00BlH2-QC; Sat, 10 Dec 2022 11:03:34 +0000 Date: Sat, 10 Dec 2022 11:01:53 +0000 Message-ID: <87fsdnfroe.wl-maz@kernel.org> From: Marc Zyngier To: Alexandru Elisei Cc: Ricardo Koller , kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, andrew.jones@linux.dev, eric.auger@redhat.com, oliver.upton@linux.dev, reijiw@google.com Subject: Re: [kvm-unit-tests PATCH 1/3] arm: pmu: Fix overflow checks for PMUv3p5 long counters In-Reply-To: References: <20221202045527.3646838-1-ricarkol@google.com> <20221202045527.3646838-2-ricarkol@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/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: alexandru.elisei@arm.com, ricarkol@google.com, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, andrew.jones@linux.dev, eric.auger@redhat.com, oliver.upton@linux.dev, reijiw@google.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, 09 Dec 2022 17:47:14 +0000, Alexandru Elisei wrote: > > Hi, > > On Fri, Dec 02, 2022 at 04:55:25AM +0000, Ricardo Koller wrote: > > PMUv3p5 uses 64-bit counters irrespective of whether the PMU is configured > > for overflowing at 32 or 64-bits. The consequence is that tests that check > > the counter values after overflowing should not assume that values will be > > wrapped around 32-bits: they overflow into the other half of the 64-bit > > counters on PMUv3p5. > > > > Fix tests by correctly checking overflowing-counters against the expected > > 64-bit value. > > > > Signed-off-by: Ricardo Koller > > --- > > arm/pmu.c | 29 ++++++++++++++++++----------- > > 1 file changed, 18 insertions(+), 11 deletions(-) > > > > diff --git a/arm/pmu.c b/arm/pmu.c > > index cd47b14..eeac984 100644 > > --- a/arm/pmu.c > > +++ b/arm/pmu.c > > @@ -54,10 +54,10 @@ > > #define EXT_COMMON_EVENTS_LOW 0x4000 > > #define EXT_COMMON_EVENTS_HIGH 0x403F > > > > -#define ALL_SET 0xFFFFFFFF > > -#define ALL_CLEAR 0x0 > > -#define PRE_OVERFLOW 0xFFFFFFF0 > > -#define PRE_OVERFLOW2 0xFFFFFFDC > > +#define ALL_SET 0x00000000FFFFFFFFULL > > +#define ALL_CLEAR 0x0000000000000000ULL > > +#define PRE_OVERFLOW 0x00000000FFFFFFF0ULL > > +#define PRE_OVERFLOW2 0x00000000FFFFFFDCULL > > > > #define PMU_PPI 23 > > > > @@ -538,6 +538,7 @@ static void test_mem_access(void) > > static void test_sw_incr(void) > > { > > uint32_t events[] = {SW_INCR, SW_INCR}; > > + uint64_t cntr0; > > int i; > > > > if (!satisfy_prerequisites(events, ARRAY_SIZE(events))) > > @@ -572,9 +573,9 @@ static void test_sw_incr(void) > > write_sysreg(0x3, pmswinc_el0); > > > > isb(); > > - report(read_regn_el0(pmevcntr, 0) == 84, "counter #1 after + 100 SW_INCR"); > > - report(read_regn_el0(pmevcntr, 1) == 100, > > - "counter #0 after + 100 SW_INCR"); > > + cntr0 = (pmu.version < ID_DFR0_PMU_V3_8_5) ? 84 : PRE_OVERFLOW + 100; > > Hm... in the Arm ARM it says that counters are 64-bit if PMUv3p5 is > implemented. But it doesn't say anywhere that versions newer than p5 are > required to implement PMUv3p5. And I don't think it needs to say it, because there is otherwise no way for SW to discover whether 64bit counters are implemented or not. > > For example, for PMUv3p7, it says that the feature is mandatory in Arm8.7 > implementations. My interpretation of that is that it is not forbidden for > an implementer to cherry-pick this version on older versions of the > architecture where PMUv3p5 is not implemented. I'm sorry to have to say that, but I find your suggestion that PMUv3p7 could be implemented without supporting the full gamut of PMUv3p5 ludicrous. Please look back at the ARM ARM, specially at the tiny section titled "Alternative ID scheme used for the Performance Monitors Extension version" (DDI0487I.a, D17.1.3, page 5553), and the snipped of C code that performs exactly this check: if (value != 0xF and value >= number) { // do something that relies on version 'number' of the feature } Replace 'value' with 7 (PMUv3p7), 'number' with 6 (PMUv3p5), and you get the exact property that you pretend doesn't exist, allowing you to rely on PMUv3p5 to be implemented when the HW has PMUv3p7. > Maybe the check should be pmu.version == ID_DFR0_PMU_V3_8_5, to match the > counter definitions in the architecture? No, that'd be totally wrong. You need to check your understanding of how the ID registers work. M. -- Without deviation from the norm, progress is not possible.