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 8F509C2BD09 for ; Tue, 9 Jul 2024 15:06:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=G3tmbPCmTOO7NzDl++c0CzweFr+xYw1iJLQTHC46ai8=; b=xe5tcDlqAnXxIMq2a/alc7D20J 5qtbc2yhlmBXAwX0P+c/cg0oTfeGXAW3s6QLZ97SxucY2lupcRsmMZ3zGH8J1QG2DANuqDb15PXIE g01ysAQSbUyoiU8uk1CE2yOQ3e7JOfiDbXGmfJMEZg+Y4OeUhHkQfW1viGTKW3ju4S+EiOUR9tk7a i6bmZuPJ72+i7CTc8lHcacdMckRXWaSHEaQhFj7fzUmwnEK+pQLXNZD0w0aNHoPJql/b6OgdZGjmd hR6+eRIYJh194BW/B7HxfNbnhlN0OCFk/JZY1ZqxEBu4tnRIExfMOdfWQ1CATwyb2FQv/31zgnLEU gEkPuRPA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRCPk-00000007fo9-2lUd; Tue, 09 Jul 2024 15:05:56 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sRCPT-00000007fie-3otb for linux-arm-kernel@lists.infradead.org; Tue, 09 Jul 2024 15:05:41 +0000 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 4B02312FC; Tue, 9 Jul 2024 08:06:03 -0700 (PDT) Received: from arm.com (e121798.manchester.arm.com [10.32.101.22]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 77DF53F766; Tue, 9 Jul 2024 08:05:35 -0700 (PDT) Date: Tue, 9 Jul 2024 16:05:32 +0100 From: Alexandru Elisei To: Alex =?utf-8?Q?Benn=C3=A9e?= Cc: Peter Maydell , pbonzini@redhat.com, drjones@redhat.com, thuth@redhat.com, kvm@vger.kernel.org, qemu-arm@nongnu.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, christoffer.dall@arm.com, maz@kernel.org, Anders Roxell , Andrew Jones , Eric Auger , "open list:ARM" Subject: Re: [kvm-unit-tests PATCH v1 1/2] arm/pmu: skip the PMU introspection test if missing Message-ID: References: <20240702163515.1964784-1-alex.bennee@linaro.org> <20240702163515.1964784-2-alex.bennee@linaro.org> <87ed82slt8.fsf@draig.linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87ed82slt8.fsf@draig.linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240709_080540_080155_D8F1BE99 X-CRM114-Status: GOOD ( 36.20 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On Tue, Jul 09, 2024 at 03:05:07PM +0100, Alex Bennée wrote: > Peter Maydell writes: > > > On Tue, 9 Jul 2024 at 09:58, Alexandru Elisei wrote: > >> > >> Hi, > >> > >> On Tue, Jul 02, 2024 at 05:35:14PM +0100, Alex Bennée wrote: > >> > The test for number of events is not a substitute for properly > >> > checking the feature register. Fix the define and skip if PMUv3 is not > >> > available on the system. This includes emulator such as QEMU which > >> > don't implement PMU counters as a matter of policy. > >> > > >> > Signed-off-by: Alex Bennée > >> > Cc: Anders Roxell > >> > --- > >> > arm/pmu.c | 7 ++++++- > >> > 1 file changed, 6 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/arm/pmu.c b/arm/pmu.c > >> > index 9ff7a301..66163a40 100644 > >> > --- a/arm/pmu.c > >> > +++ b/arm/pmu.c > >> > @@ -200,7 +200,7 @@ static void test_overflow_interrupt(bool overflow_at_64bits) {} > >> > #define ID_AA64DFR0_PERFMON_MASK 0xf > >> > > >> > #define ID_DFR0_PMU_NOTIMPL 0b0000 > >> > -#define ID_DFR0_PMU_V3 0b0001 > >> > +#define ID_DFR0_PMU_V3 0b0011 > >> > #define ID_DFR0_PMU_V3_8_1 0b0100 > >> > #define ID_DFR0_PMU_V3_8_4 0b0101 > >> > #define ID_DFR0_PMU_V3_8_5 0b0110 > >> > @@ -286,6 +286,11 @@ static void test_event_introspection(void) > >> > return; > >> > } > >> > > >> > + if (pmu.version < ID_DFR0_PMU_V3) { > >> > + report_skip("PMUv3 extensions not supported, skip ..."); > >> > + return; > >> > + } > >> > + > >> > >> I don't get this patch - test_event_introspection() is only run on 64bit. On > >> arm64, if there is a PMU present, that PMU is a PMUv3. A prerequisite to > >> running any PMU tests is for pmu_probe() to succeed, and pmu_probe() fails if > >> there is no PMU implemented (PMUVer is either 0, or 0b1111). As a result, if > >> test_event_introspection() is executed, then a PMUv3 is present. > >> > >> When does QEMU advertise FEAT_PMUv3*, but no event counters (other than the cycle > >> counter)? > > The other option I have is this: > > --8<---------------cut here---------------start------------->8--- > arm/pmu: event-introspection needs icount for TCG > > The TCG accelerator will report a PMU (unless explicitly disabled with > -cpu foo,pmu=off) however not all events are available unless you run > under icount. Fix this by splitting the test into a kvm and tcg > version. As far as I can tell, if test_event_introspection() fails under TCG without icount then there are two possible explanations for that: 1. Not all the events whose presence is checked by test_event_introspection() are actually required by the architecture. 2. TCG without icount is not implementing all the events required by the architecture. If 1, then test_event_introspection() should be fixed. I had a look and the function looked correct to me (except that the event name is not INST_PREC, it's INST_SPEC in the Arm DDI0487J.A and K.a, but that's not relevant for correctness). >From what I can tell from what Peter and you have said, explanation 2 is the correct one, because TCG cannot implement all the required events when icount is not specified. As far as test_event_introspection() is concerned, I consider this to be the expected behaviour: it fails because the required events are not implemented. I don't think the function should be changed to work around how QEMU was invoked. Do you agree? If you know that the test will fail without special command line parameters when accel is TCG, then I think what you are suggesting looks correct to me: the original test is skipped if KVM is not present, and when run under TCG, the correct parameters are passed to QEMU. Thanks, Alex > > Signed-off-by: Alex Bennée > > 1 file changed, 8 insertions(+) > arm/unittests.cfg | 8 ++++++++ > > modified arm/unittests.cfg > @@ -52,8 +52,16 @@ extra_params = -append 'cycle-counter 0' > file = pmu.flat > groups = pmu > arch = arm64 > +accel = kvm > extra_params = -append 'pmu-event-introspection' > > +[pmu-event-introspection-icount] > +file = pmu.flat > +groups = pmu > +arch = arm64 > +accel = tcg > +extra_params = -icount shift=1 -append 'pmu-event-introspection' > + > [pmu-event-counter-config] > file = pmu.flat > groups = pmu > --8<---------------cut here---------------end--------------->8--- > > which just punts icount on TCG to its own test (note there are commented > out versions further down the unitests.cfg file) > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro