From nobody Tue Aug 30 21:16:35 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4MHKqr1n8Fz4bLZ6 for ; Tue, 30 Aug 2022 21:16:40 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4MHKqq2DHlz3Z6v for ; Tue, 30 Aug 2022 21:16:39 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: by mail-wr1-f53.google.com with SMTP id e13so14919800wrm.1 for ; Tue, 30 Aug 2022 14:16:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc; bh=xCrmaBIkM2uq59S6d2+NhtF1PaOQUerdDjwYQ0Esbfw=; b=GLYMEy7WshUllihdGZmCRxBpt97tQ66LLK565To1Zj7RvNpHpR1wr3eR3mF1YPcjI+ plb101mgmxw/QMFo5aSa+mDOsj3v2SD/Uy5hORzTAK/MlkX4b0vj/rQtf+Oo0+MoET7e z0BWHUd+158RMm8BLgzkITidcYMtznHVAh4w4zXfpTQH3i6BNedaQUqQa9RN/SAWZiDd BjiAECWdelvewZVAfvRijxZF8zOIsRZonu9ZdnRWbNhSqONb4dRFytZR/sdMOMzhoHPp +OlFOBVBslZGEpwbRZEh62T/TBN2UaSrsPGhn5iSgHfcx403Cn1Wqq4NKXveqSafYrXB NTtw== X-Gm-Message-State: ACgBeo0WPpsCc3ZjEoC+DpC9dQ1Dr8CPBZhgG5jK+q3+DaU4S+nR3yB0 IwUSIxt+HDs4nvYq9JhLyoNSnQ== X-Google-Smtp-Source: AA6agR7IKQDZ51J/dttrIVSC9BIyiTJWNOyWTQig7OTWNh+I9+btISlGIAqKXg66j08n2f2EISBaCw== X-Received: by 2002:a05:6000:2c4:b0:225:6782:5755 with SMTP id o4-20020a05600002c400b0022567825755mr10426590wry.299.1661894197696; Tue, 30 Aug 2022 14:16:37 -0700 (PDT) Received: from smtpclient.apple (global-5-141.n-2.net.cam.ac.uk. [131.111.5.141]) by smtp.gmail.com with ESMTPSA id j15-20020a5d464f000000b0022526db2363sm10478862wrs.30.2022.08.30.14.16.36 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 30 Aug 2022 14:16:36 -0700 (PDT) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.80.82.1.1\)) Subject: Re: git: e3572eb65473 - main - Allocate event for DMC-620 and CMN-600 controllers PMU. Add events supported by DMC-620 and CMN-600 controllers PMU. From: Jessica Clarke In-Reply-To: Date: Tue, 30 Aug 2022 22:16:35 +0100 Cc: Toomas Soome , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <1E37449E-B6C8-47A5-AD79-34F24138CC64@freebsd.org> References: <202206262217.25QMHOuH076130@gitrepo.freebsd.org> To: "Bjoern A. Zeeb" X-Mailer: Apple Mail (2.3696.80.82.1.1) X-Rspamd-Queue-Id: 4MHKqq2DHlz3Z6v X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of jrtc27@jrtc27.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=jrtc27@jrtc27.com X-Spamd-Result: default: False [-2.50 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MV_CASE(0.50)[]; FORGED_SENDER(0.30)[jrtc27@freebsd.org,jrtc27@jrtc27.com]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; MIME_GOOD(-0.10)[text/plain]; MIME_TRACE(0.00)[0:+]; FROM_HAS_DN(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DMARC_NA(0.00)[freebsd.org]; RCVD_IN_DNSWL_NONE(0.00)[209.85.221.53:from]; PREVIOUSLY_DELIVERED(0.00)[dev-commits-src-main@freebsd.org]; FREEFALL_USER(0.00)[jrtc27]; TO_DN_EQ_ADDR_SOME(0.00)[]; FROM_NEQ_ENVFROM(0.00)[jrtc27@freebsd.org,jrtc27@jrtc27.com]; MLMMJ_DEST(0.00)[dev-commits-src-main@freebsd.org]; RCPT_COUNT_FIVE(0.00)[5]; TO_DN_SOME(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; R_DKIM_NA(0.00)[]; RCVD_TLS_LAST(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; RWL_MAILSPIKE_POSSIBLE(0.00)[209.85.221.53:from] X-ThisMailContainsUnwantedMimeParts: N On 27 Jun 2022, at 01:58, Bjoern A. Zeeb wrote: >=20 > On Mon, 27 Jun 2022, Jessica Clarke wrote: >=20 > Hi, >=20 >> On 27 Jun 2022, at 01:26, Bjoern A. Zeeb wrote: >>>=20 >>> On Mon, 27 Jun 2022, Jessica Clarke wrote: >>>=20 >>>> On 26 Jun 2022, at 23:17, Toomas Soome wrote: >>>>>=20 >>>>> The branch main has been updated by tsoome: >>>>>=20 >>>>> URL: = https://cgit.FreeBSD.org/src/commit/?id=3De3572eb654733a94e1e765fe9e95e057= 9981d851 >>>>>=20 >>>>> commit e3572eb654733a94e1e765fe9e95e0579981d851 >>>>> Author: Aleksandr Rybalko >>>>> AuthorDate: 2022-02-16 00:19:19 +0000 >>>>> Commit: Toomas Soome >>>>> CommitDate: 2022-06-26 18:52:26 +0000 >>>>>=20 >>>>> Allocate event for DMC-620 and CMN-600 controllers PMU. Add events = supported by DMC-620 and CMN-600 controllers PMU. >>>>>=20 >>>>> Allocate event for DMC-620 and CMN-600 controllers PMU. >>>>> Add events supported by DMC-620 and CMN-600 controllers PMU. >>>>>=20 >>>>> Reviewed by: bz >>>>> Sponsored By: ARM >>>>> Sponsored By: Ampere Computing >>>>> Differential Revision: https://reviews.freebsd.org/D35609 >>>>=20 >>>> This includes the following (skipped due to lines) diff: >>>>=20 >>>>> * 0x14100 0x0100 ARMv8 events >>>>> + * 0x14200 0x0020 ARM DMC-620 clkdiv2 events >>>>> + * 0x14220 0x0080 ARM DMC-620 clk events >>>>> + * 0x14300 0x0100 ARM CMN-600 events >>>>=20 >>>>=20 >>>> Not enough space was allocated for Armv8 events as it goes up to = 0x3ff >>>> in Armv8 (and beyond in later versions of the architecture). = Downstream >>>> we extend this range in CheriBSD as required for Morello=E2=80=99s = events. >>>> Please relocate these new events well past the end of the existing >>>> Armv8 events so the space can remain contiguous. >>>=20 >>> Should this be 0x3ff then as well btw? >>> = https://github.com/CTSRD-CHERI/cheribsd/commit/4ea869cd8b717ca0b07672eb7ac= c99bf949249de >>=20 >> Well, 0x400 for count not max, but yes. We only extended as far as we >> needed, not to cover the entire range (but intended to eventually >> upstream it as the full v8 range). >>=20 >>> Looking more closely it seems from ARMv8.1 onwards it goes up to = 0xFFFF >>> if I read 'Table D8-7 Allocation of the PMU event number space' of = ARM >>> DDI 0487H.a correctly? >>=20 >> Yes, if you want to cover all the v8.1 space then you need to go that >> high too, but it=E2=80=99ll get quite sparse in that range so it=E2=80=99= s unclear if >> we want to go ahead and do that already or try and be smarter (the >> current EVENT_xH list would get a bit silly). We should probably >> reserve all of it though at least so we can if we want to in future. >=20 > I'll let you and Toomas sort that out. I am just trying to fix the > build breakage as I kind-of pushed him to get the remaining bits in > by accepting that review after scrolling through and it looking > reasonable and addressing all comments from the previous review. > That was all to unbreak an already earlier build breakage. >=20 > Given it wasn't too late for me I was trying to get through it > before falling asleep soon as well, especially now that the > thunderstorms seems to have mostly passed. Nobody ever got round to addressing this, and it is in fact causing us issues downstream now. However, there=E2=80=99s a rather more glaring = problem: > @@ -1313,6 +1475,10 @@ pmc_init(void) >=20 > /* Fill soft events information. */ > pmc_class_table[n++] =3D &soft_class_table_descr; > + > + pmc_class_table[n++] =3D &cmn600_pmu_class_table_descr; > + pmc_class_table[n++] =3D &dmc620_pmu_cd2_class_table_descr; > + pmc_class_table[n++] =3D &dmc620_pmu_c_class_table_descr; This doesn=E2=80=99t work (even if you ifdef it appropriately like now = exists upstream). If there is no CMN-600 etc then PMC_CLASS_TABLE_SIZE, i.e. cpu_info.pm_nclass, is not going to include those, so you cannot add them to pmc_class_table otherwise you have a buffer overflow. Given this has broken libpmc on large swathes of AArch64 hardware (maybe without CHERI the memory corruption happens to not trample over anything important for now, but who knows), can we please revert this patch until a fixed version exists, with both the event numbers reallocated and libpmc made suitably dynamic so as to not introduce buffer overflows? Note that cmn600 only has an ACPI attachment so FDT-based systems will definitely hit this case. Jess