From owner-freebsd-riscv@freebsd.org Fri Oct 30 15:00:57 2020 Return-Path: Delivered-To: freebsd-riscv@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 8AD314541A0 for ; Fri, 30 Oct 2020 15:00:57 +0000 (UTC) (envelope-from ah@melesmeles.cz) Received: from mx-9.mail.web4u.cz (smtp10.web4u.cz [81.91.87.90]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4CN5944TkLz44w0; Fri, 30 Oct 2020 15:00:55 +0000 (UTC) (envelope-from ah@melesmeles.cz) Received: from mx-9.mail.web4u.cz (localhost [127.0.0.1]) by mx-9.mail.web4u.cz (Postfix) with ESMTP id 2E40E201B01; Fri, 30 Oct 2020 16:00:43 +0100 (CET) Received: from antos (unknown [85.207.122.76]) (Authenticated sender: ah@melesmeles.cz) by mx-9.mail.web4u.cz (Postfix) with ESMTPA id 0BE75201ACA; Fri, 30 Oct 2020 16:00:43 +0100 (CET) From: Antonin Houska To: "Kristof Provost" cc: freebsd-riscv@freebsd.org Subject: Re: locore.S - two cosmetic changes In-reply-to: References: <65290.1603693402@antos> Comments: In-reply-to "Kristof Provost" message dated "Tue, 27 Oct 2020 13:45:11 +0100." X-Mailer: MH-E 8.6+git; nmh 1.7; GNU Emacs 26.3.50 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 30 Oct 2020 17:02:50 +0100 Message-ID: <78054.1604073770@antos> X-W4U-Auth: cd4dd8aa31fe0cd96ef38b5808e9eefe42a340d5 X-Rspamd-Queue-Id: 4CN5944TkLz44w0 X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of ah@melesmeles.cz has no SPF policy when checking 81.91.87.90) smtp.mailfrom=ah@melesmeles.cz X-Spamd-Result: default: False [0.42 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.59)[-0.595]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-0.52)[-0.520]; MIME_GOOD(-0.10)[text/plain]; MID_RHS_NOT_FQDN(0.50)[]; DMARC_NA(0.00)[melesmeles.cz]; AUTH_NA(1.00)[]; NEURAL_SPAM_SHORT(0.13)[0.130]; RCVD_COUNT_THREE(0.00)[3]; RCPT_COUNT_TWO(0.00)[2]; R_SPF_NA(0.00)[no SPF record]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:39790, ipnet:81.91.80.0/20, country:CZ]; RCVD_TLS_LAST(0.00)[]; MAILMAN_DEST(0.00)[freebsd-riscv] X-BeenThere: freebsd-riscv@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: FreeBSD on the RISC-V instruction set architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Oct 2020 15:00:57 -0000 Kristof Provost wrote: > On 26 Oct 2020, at 7:23, Antonin Houska wrote: > > Hello, > > > > while trying to understand the code, I've spotted two things that might= need > > to be improved - please see the attachments. > > > > remove_useless_assignment.patch removes setting of register value which= is > > not > > used until the next value is set. use_constant_not_literal.patch probab= ly > > needs no explanation. > > > That=E2=80=99s gone in as in r367078. Thanks! Thanks, this is my first contribution to the FreeBSD code :-) Actually Mitchell Horne replied to my patch too but probably forgot to CC t= he mailing list. He suggested that some more replacements of literals by preprocessor constants can be included in the patch. While thinking about that, I came across something that I don't understand. On line 156, the position of the L1 entry for particular virtual address is computed: 151: /* Create an L1 page for early devmap */ 152: lla s1, pagetable_l1 153: lla s2, pagetable_l2_devmap /* Link to next level PN */ 154: srli s2, s2, PAGE_SHIFT 155: 156: li a5, (VM_MAX_KERNEL_ADDRESS - L2_SIZE) However the corresponding L2 entry is at position 510: 183: /* Store PTE entry to position */ 184: li a6, PTE_SIZE 185: li a5, 510 I'd expect 511 since the virtual addres is (1 * L2_SIZE) below VM_MAX_KERNEL_ADDRESS. On the other hand, the (obviously related) virtual address passed to initriscv() is 249: li t0, (VM_EARLY_DTB_ADDRESS) defined in vmparam.h as #define VM_EARLY_DTB_ADDRESS (VM_MAX_KERNEL_ADDRESS - (2 * L2_SIZE)) So I wonder if the assignment on line 156 should rather be 156: li a5, (VM_EARLY_DTB_ADDRESS) I suspect that the (VM_MAX_KERNEL_ADDRESS - L2_SIZE) expression is a thinko which does not break anything because (VM_MAX_KERNEL_ADDRESS - L2_SIZE) >> L1_SHIFT appears to be equal to VM_EARLY_DTB_ADDRESS >> L1_SHIFT Do I miss anything? --=20 Anton=C3=ADn Houska www.melesmeles.cz