From nobody Tue Aug 30 21:51:26 2022 X-Original-To: dev-commits-src-all@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 4MHLc638b6z4bPjf; Tue, 30 Aug 2022 21:51:34 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4MHLc62fWsz3ctD; Tue, 30 Aug 2022 21:51:34 +0000 (UTC) (envelope-from bz@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661896294; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JkHz6KT7ncJKUVCR7fGejR+l0It/TlSgUUvXu92g9xc=; b=O6fZHK4lvhGKHVrMSsxFZLprS6iT66rWsUZYOeZ7T6tqvptc6ZY9jD7LAn2XCygRhMy3hs r5g5Q1gYGXbtmefQlbApWVyn4/NvUBgzB3gNrqYK37MkkhikPssiLS8dWGltPqcyWq3KRG 0sYKfk/hhWXF+xG1QK9oWbjsySCQW9bjzZZNfXISEs/iOk2LVKoAjBeMj5uMknQcFigcih shvfAV+e/8XjV5u+t5leKeLlrghN2X+uZwEYuCi0p7iWZ9oIMafZ5uhaDd51JPGKhfvKHA GyIDKUT+LFvoqGOgMx/4hFd/5yqtArTvsnw22XxCYsBkyXagF1AsWzQgyp+ECw== Received: from mx1.sbone.de (cross.sbone.de [195.201.62.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mx1.sbone.de", Issuer "SBone.DE" (not verified)) (Authenticated sender: bz/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 4MHLc615vXz1PkK; Tue, 30 Aug 2022 21:51:34 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:1025]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 9D2978D4A15D; Tue, 30 Aug 2022 21:51:32 +0000 (UTC) Received: from content-filter.t4-02.sbone.de (content-filter.t4-02.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:2742]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id C63595C3A831; Tue, 30 Aug 2022 21:51:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:4902:0:7404:2:1025]) by content-filter.t4-02.sbone.de (content-filter.t4-02.sbone.de [IPv6:fde9:577b:c1a9:4902:0:7404:2:2742]) (amavisd-new, port 10024) with ESMTP id fggVRBis7pNJ; Tue, 30 Aug 2022 21:51:26 +0000 (UTC) Received: from strong-iwl0.sbone.de (strong-iwl0.sbone.de [IPv6:fde9:577b:c1a9:4902:b66b:fcff:fef3:e3d2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPSA id C381B5C3A82F; Tue, 30 Aug 2022 21:51:26 +0000 (UTC) Date: Tue, 30 Aug 2022 21:51:26 +0000 (UTC) From: "Bjoern A. Zeeb" To: Jessica Clarke cc: Toomas Soome , "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" 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. In-Reply-To: <1E37449E-B6C8-47A5-AD79-34F24138CC64@freebsd.org> Message-ID: References: <202206262217.25QMHOuH076130@gitrepo.freebsd.org> <1E37449E-B6C8-47A5-AD79-34F24138CC64@freebsd.org> X-OpenPGP-Key-Id: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="1098556516-278802634-1661896286=:94196" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1661896294; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JkHz6KT7ncJKUVCR7fGejR+l0It/TlSgUUvXu92g9xc=; b=Uz6LjgJgNE/ms/YH34k887//TPOOI9WUINFvZ7F2NkzlVHP9/00jGobbCYVeBbN0Q1+q0d kR3lJCVKXeA+EjCyZhvxk0nGrmfCuIhZ6nDiXQwqLNuGaWuniYX/uBdozyhRZ/5jNNy70X zlhPPu7ReNgAZ57vMFWEtjLhmHSEPxVOV1G/NRxeApzjs25cnfmkNd7dJsZvgIgxmCRaGw Js0xWDj/xQxOmN9d0sTVwq27Ae16O4iDYl2hJ3Myz2HaL9ryG9kUd+Ryf4DOAF47VzzZWx 5ol4CwGhAmrg4rU0T5+EQywgoaQ/XzUwP7UF1336zR8u8ob5ixyNkAWXkj/8sw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1661896294; a=rsa-sha256; cv=none; b=EVwvsPSPdiRrhjy5XolCBWFYCe4RJcsIQXtg6xR90XR5QXSr+/lemtldzrX5hgO6gIyqSe Gfw9ELSTMV/mgTFBOtLaU6bt42n2qGM4EVkQLI/80pdAB06mubSCkiYCo9fHdtW2yZktKH WN1uxBHTP38lcTW+/rK0IlPIrh4gtyqFxX6rWNokd79UBK66XDY80IJJImMEJXARBfgprh Jh0GTkqhCday6JgjrQga4klTtzW23DV5O7DW6RXrTvdz0c5Q/n68CVQvFkDKrFmPmZEwy+ 9X1gC9Xjyqb5mLjR+uAB6I5RHXghPsbD+OPh9PauJOQcS8DjdQPkSMiFL2OFKw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --1098556516-278802634-1661896286=:94196 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT On Tue, 30 Aug 2022, Jessica Clarke wrote: Hi Jessica, > On 27 Jun 2022, at 01:58, Bjoern A. Zeeb wrote: >> >> On Mon, 27 Jun 2022, Jessica Clarke wrote: >> >> Hi, >> >>> On 27 Jun 2022, at 01:26, Bjoern A. Zeeb wrote: >>>> >>>> On Mon, 27 Jun 2022, Jessica Clarke wrote: >>>> >>>>> On 26 Jun 2022, at 23:17, Toomas Soome wrote: >>>>>> >>>>>> The branch main has been updated by tsoome: >>>>>> >>>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=e3572eb654733a94e1e765fe9e95e0579981d851 >>>>>> >>>>>> commit e3572eb654733a94e1e765fe9e95e0579981d851 >>>>>> Author: Aleksandr Rybalko >>>>>> AuthorDate: 2022-02-16 00:19:19 +0000 >>>>>> Commit: Toomas Soome >>>>>> CommitDate: 2022-06-26 18:52:26 +0000 >>>>>> >>>>>> Allocate event for DMC-620 and CMN-600 controllers PMU. Add events supported by DMC-620 and CMN-600 controllers PMU. >>>>>> >>>>>> Allocate event for DMC-620 and CMN-600 controllers PMU. >>>>>> Add events supported by DMC-620 and CMN-600 controllers PMU. >>>>>> >>>>>> Reviewed by: bz >>>>>> Sponsored By: ARM >>>>>> Sponsored By: Ampere Computing >>>>>> Differential Revision: https://reviews.freebsd.org/D35609 >>>>> >>>>> This includes the following (skipped due to lines) diff: >>>>> >>>>>> * 0x14100 0x0100 ARMv8 events >>>>>> + * 0x14200 0x0020 ARM DMC-620 clkdiv2 events >>>>>> + * 0x14220 0x0080 ARM DMC-620 clk events >>>>>> + * 0x14300 0x0100 ARM CMN-600 events >>>>> >>>>> >>>>> 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’s events. >>>>> Please relocate these new events well past the end of the existing >>>>> Armv8 events so the space can remain contiguous. >>>> >>>> Should this be 0x3ff then as well btw? >>>> https://github.com/CTSRD-CHERI/cheribsd/commit/4ea869cd8b717ca0b07672eb7acc99bf949249de >>> >>> 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). >>> >>>> 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? >>> >>> Yes, if you want to cover all the v8.1 space then you need to go that >>> high too, but it’ll get quite sparse in that range so it’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. >> >> 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. >> >> 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’s a rather more glaring problem: > >> @@ -1313,6 +1475,10 @@ pmc_init(void) >> >> /* Fill soft events information. */ >> pmc_class_table[n++] = &soft_class_table_descr; >> + >> + pmc_class_table[n++] = &cmn600_pmu_class_table_descr; >> + pmc_class_table[n++] = &dmc620_pmu_cd2_class_table_descr; >> + pmc_class_table[n++] = &dmc620_pmu_c_class_table_descr; > > This doesn’t 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. I am just replying really given I am on Cc: hoping that Toomas will get to this. pmc_init() is libpmc, right? Does a simple #if 0 around these avert all issues for now or do the kernel bits also need backing out? I only have vague memories of multiple commit to unbreak this one from that night (which tried to fix a previous different breakage). Backing out everything might be more tedious than just reverting the commit hence asking if "disabling" it does fix the problems. > Given > this has broken libpmc on large swathes of AArch64 hardware (maybe That has taken a lot of time for anyone to notice :( > 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 > > -- Bjoern A. Zeeb r15:7 --1098556516-278802634-1661896286=:94196--