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 D546BC433F5 for ; Mon, 17 Jan 2022 18:09:36 +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:In-Reply-To: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=XOsrh4FHUrrH1VaKqIcSaXwjT9j/0qAwpfr2W/+Ppwo=; b=pZpU+1dbrIouM5 M4MxOyFKDcAhNUAtLA4ke6LRv9A3syZnJCyZL8WsJrbwfiYKy8c03K7Gy92U874vlj5/8M+G8X54A HwIDBvUaH6SQKs6fIflVCSnNlw1PG+2gJcZdflpFlac9HAJX8PKg8Ca7e9uvWm4fvUgwv7XvFEpN8 RuTPniSwB/GkG0FSf2gSxFNRlHdrCsCo9O23rO5xiSuu4Di3IO9w9tyyIAFyxAJEpue6BADILVi6h uADSi+UWyQ1ZUTsXoL7wk4b9p6KqWquEUtdSHs5pqugcaT8DqARo8lKaOMJQw46hsWRBbXtJXksjG hO52fchu7ye/wTPCX8UQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1n9WQZ-00G3EP-NQ; Mon, 17 Jan 2022 18:08:23 +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 1n9WQW-00G3Du-NK for linux-arm-kernel@lists.infradead.org; Mon, 17 Jan 2022 18:08:22 +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 3FF496D; Mon, 17 Jan 2022 10:08:18 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.38.30]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7C51B3F774; Mon, 17 Jan 2022 10:08:16 -0800 (PST) Date: Mon, 17 Jan 2022 18:08:13 +0000 From: Mark Rutland To: Andre Przywara Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com, robin.murphy@arm.com, vladimir.murzin@arm.com, Wei.Chen@arm.com Subject: Re: [bootwrapper PATCH v2 09/13] aarch64: move the bulk of EL3 initialization to C Message-ID: <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> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220117143104.28db5001@donnerap.cambridge.arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220117_100820_879829_96748316 X-CRM114-Status: GOOD ( 27.10 ) 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, 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. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel