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 4EC6EC433EF for ; Mon, 17 Jan 2022 18:32:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: 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=7wHTNmEgj/OZj23lAJ1kvhYJk/HzFy9ibaTCu6e02to=; b=ZL797Md325QzS9 cBgvT4v3kI4VeSv5B941GPulIi1GD4fmeB9+cuEKUpPUljzcOl8BsPgJI8P7ofdAH5L0hrCZktNLW Bj2NR/rKDR5n+awSYIVJPJ13Bl8NZOuShlL9h/GYXyq5clkoPPcmHMt2b0bBpA6qW+rXgdJdq+Scy MsxKUEfstsF2azZNhYKIvZSH2KPk+9S5t44wmxwAmhPQa+93+N0VHgB5BDRSbdx8j0x3TSApLi09C zFj9IELCeI0/dODEdXIZqWvgyQEKsCiFbmeYVGd6/4yO6K2/NX9WPtTm6H0RxbkFzX2I9hVvfj1j0 xLiNfImMh3hOq8qYlRsg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9Wms-00G6Pd-HJ; Mon, 17 Jan 2022 18:31:26 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9Wmo-00G6Ot-LX for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 18:31:24 +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 06D116D; Mon, 17 Jan 2022 10:31:21 -0800 (PST) Received: from donnerap.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F39083F774; Mon, 17 Jan 2022 10:31:19 -0800 (PST) Date: Mon, 17 Jan 2022 18:31:17 +0000 From: Andre Przywara To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com, robin.murphy@arm.com, vladimir.murzin@arm.com, Wei.Chen@arm.com, Mark Brown Subject: Re: [bootwrapper PATCH v2 09/13] aarch64: move the bulk of EL3 initialization to C Message-ID: <20220117183117.7f29dc66@donnerap.cambridge.arm.com> In-Reply-To: <20220117180813.GD94025@C02TD0UTHF1T.local> References: <20220114105653.3003399-1-mark.rutland@arm.com> <20220114105653.3003399-10-mark.rutland@arm.com> <20220117143104.28db5001@donnerap.cambridge.arm.com> <20220117180813.GD94025@C02TD0UTHF1T.local> Organization: ARM X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; aarch64-unknown-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_103122_834646_010372AD X-CRM114-Status: GOOD ( 36.29 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 17 Jan 2022 18:08:13 +0000 Mark Rutland wrote: Hi, > On Mon, Jan 17, 2022 at 02:31:04PM +0000, Andre Przywara wrote: > > On Fri, 14 Jan 2022 10:56:49 +0000 > > Mark Rutland wrote: > > > > Hi Mark, > > > > > The majority of state that we initialize at EL3 is necessary for code at > > > lower ELs to function, but isnt' necessary for the boot-wrapper itself. > > > Given that, it would be better to write this in C where it can be > > > written mode clearly, and where it will be possible to add logging/debug > > > logic. > > > > Ah, thanks, that looks much nicer and easier to read now, also is more > > robust, as keeping register values alive for more than a few assembly > > lines always scares me. > > > > > This patch migrates the AArch64 EL3 initialization to C. > > > > > > There should be no functional change as a result of this patch. > > > > I compared the removed assembly code against to added C code, and also > > checked the register bits against the ARM ARM. > > Two (and a half) things stood out, see below: > > Thanks for this! I've fixed those as noted below. > > [...] > > > > -#define HCR_EL2_RES1 (BIT(1)) > > > +#define ZCR_EL3 s3_6_c1_c2_0 > > > +#define ZCR_EL3_LEN BITS(3, 1) > > > > The (current) actual length field should be BITS(3, 0), no? > > Yes, it should. I've corrected that to BITS(3, 0) now. > > [...] > > > > +void cpu_init_el3(void) > > > +{ > > > + unsigned long scr = SCR_EL3_RES1 | SCR_EL3_NS | SCR_EL3_HCE; > > > + unsigned long mdcr = 0; > > > + unsigned long cptr = 0; > > > + > > > + if (cpu_has_pauth()) > > > + scr |= SCR_EL3_APK | SCR_EL3_API; > > > + > > > + if (mrs_field(ID_AA64ISAR0_EL1, TME)) > > > + scr |= SCR_EL3_TME; > > > + > > > + if (mrs_field(ID_AA64MMFR0_EL1, FGT)) > > > + scr |= SCR_EL3_FGTEN; > > > + > > > + if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2) > > > + scr |= SCR_EL3_ECVEN; > > > + > > > + if (mrs_field(ID_AA64PFR1_EL1, MTE)) > > > > The assembly code checked for >=2, which seems correct to me, as > > SCR_EL3_ATA is about MTE2? > > Yes; I botched that when converting to C. SCR_EL3.ATA is RES0 in the > absence of MTE2. I've added `>= 2` to the condition to match that. > > > > + scr |= SCR_EL3_ATA; > > [...] > > > > + if (mrs_field(ID_AA64PFR0_EL1, SVE)) { > > > + cptr |= CPTR_EL3_EZ; > > > + msr(CPTR_EL3, cptr); > > > + isb(); > > > + msr(ZCR_EL3, ZCR_EL3_LEN); > > > > So when comparing this to the other uses of XXX_EL3_YYY, they typically > > describe a mask, but here we seems to abuse this as a value? > > True; I'll add a separate defintion for the value. > > > And apart from bit 0 missing from it (as noted above), the existing > > code writes 0x1ff into that register, presumable to cover future > > vector length extensions beyond 2048 bits (which those RAZ/WI fields > > in bits[8:4] seem to suggest). > > Hmm... I went and found the SVE supplement and I can't see any rationale > for what SW *should* do, nor can I find a description of the register > (that seems to have been factored into some XML files I can't convince > anything to load on my machine). > > > So shall we define ZCR_EL3_MAX_VEC_LEN to 0x1ff above, and then use that? > > Or ignore the crystal ball, and just stick with 2048 bits, by writing 0xf? > > TBH, I'm not sure. In the absence of some documented guidance I'd prefer > to go with 0xf, but given we already use 0x1ff, I want to dig into this > a bit more. My impression was that this "[8:4] = RAZ/WI" compared to the "[63:9] = RES0" fields suggests this is for a potential extension, but I guess there would be more changes needed if SVE ever goes beyond 2048. So chances are high we need to adopt the code then anyway, and fixing the number then is the least of our problems. So I feel we should stick to what's explicitly documented, and put 0xf in there. Cheers, Andre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel