From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1lly6j-0006xC-LF for mharc-grub-devel@gnu.org; Wed, 26 May 2021 14:18:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58888) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lly6i-0006vn-6F for grub-devel@gnu.org; Wed, 26 May 2021 14:18:16 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:42104) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lly6g-0000T2-PI; Wed, 26 May 2021 14:18:14 -0400 Received: from [2001:980:1b4f:1:42d2:832d:bb59:862] (port=55134 helo=dundal.janneke.lilypond.org) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lly6f-0003wB-Fd; Wed, 26 May 2021 14:18:14 -0400 From: Jan Nieuwenhuizen To: Daniel Kiper Cc: grub-devel@gnu.org Subject: Re: [PATCH v2] grub-core: Build fixes for i386 Organization: AvatarAcademy.nl References: <20210518104733.23391-1-janneke@gnu.org> <20210526150448.2xqk6k2vpmxovgi6@tomti.i.net-space.pl> X-Url: http://AvatarAcademy.nl Date: Wed, 26 May 2021 20:18:11 +0200 In-Reply-To: <20210526150448.2xqk6k2vpmxovgi6@tomti.i.net-space.pl> (Daniel Kiper's message of "Wed, 26 May 2021 17:04:48 +0200") Message-ID: <87v9751hr0.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 May 2021 18:18:16 -0000 Daniel Kiper writes: Hello, > Mostly nits... Please take a look below... Great! > On Tue, May 18, 2021 at 12:47:33PM +0200, Jan (janneke) Nieuwenhuizen wro= te: >> To reproduce, update the Grub source description in your local Guix > > s/Grub/GRUB/ Ok. >> or install an x86 cross-build environment on x86-linux (32bit!) and > > s/32bit/32-bit/ Ok. >> configure to cross build and make, e.g., do something like >> >> ./configure \ >> CC_FOR_BUILD=3Dgcc \ >> --build=3Di686-unknown-linux-gnu > > Missing "\" at the end of the line... Fixed. >> * grub-core/lib/i386/relocator64.S: Avoid x86_64 instructions on i386. > > Hmmm... What is this? The "gitlog-to-changelog" scripts needs entries like this in order to generate a GNU-compliant ChangeLog, see Emacs, LilyPond, Guile, Guix, etc. GRUB is the first GNU project that I encounter that has a different take on this; sorry for missing that! I have changed it to --8<---------------cut here---------------start------------->8--- This fixes cross-compiling to x86 (e.g., the Hurd) from x86-linux of grub-core/lib/i386/relocator64.S This file has six sections that only build with a 64-bit assembler, yet only the first two sections had support for a 32-bit assembler; this patch completes this for the remaining sections. --8<---------------cut here---------------end--------------->8--- > And you should add your SOB at the end of commit message: > > Signed-off-by: Jan (janneke) Nieuwenhuizen Done. And sorry; you asked that before. I failed to do so because I only ever encountered this use (e.g., quoting the Guix manual) When pushing a commit on behalf of somebody else, please add a =E2=80=98Signed-off-by=E2=80=99 line at the end of the commit log messa= ge=E2=80=94e.g., with =E2=80=98git am --signoff=E2=80=99. This improves tracking of who did = what. and as I am the author, that would be redundant. >> --- >> grub-core/lib/i386/relocator64.S | 27 +++++++++++++++++++++++++-- >> 1 file changed, 25 insertions(+), 2 deletions(-) [..] >> + .byte 0x48 >> + .byte 0x83 >> + .byte 0xe4 >> + .byte 0xf0 > > Formatting is broken here... Oops! Apparently GRUB uses TAB characters. Fixed! (A .dir-locals.el file at the project root is often to avoid such mistakes.) >> +#ifdef __x86_64__ >> movq %rax, %rsi >> - > > This and... > >> +#else >> + /* movq %rax, %rsi */ >> + .byte 0x48 >> + .byte 0x89 >> + .byte 0xc6 >> +#endif >> + > > ... this change look strange. Could you fix it? Or explain in the > commit message you are doing a cleanup by the way... Hmm...what is it that looks strange here? Obviously the MOV %RAX.. statement must be guarded and get a 32-bit alternative, and the empty line after the move instruction now moves to the end of the block? Greetings, Janneke --=20 Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar=C2=AE http://AvatarAcademy.com