From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1YIaJK-00084p-Jx for mharc-grub-devel@gnu.org; Tue, 03 Feb 2015 05:02:22 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIaJH-00084V-Pb for grub-devel@gnu.org; Tue, 03 Feb 2015 05:02:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YIaJD-0007IY-0Z for grub-devel@gnu.org; Tue, 03 Feb 2015 05:02:19 -0500 Received: from mail.emea.novell.com ([130.57.118.101]:55008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YIaJC-0007I5-OF for grub-devel@gnu.org; Tue, 03 Feb 2015 05:02:14 -0500 Received: from EMEA1-MTA by mail.emea.novell.com with Novell_GroupWise; Tue, 03 Feb 2015 10:02:11 +0000 Message-Id: <54D0AAB1020000780005C491@mail.emea.novell.com> X-Mailer: Novell GroupWise Internet Agent 14.0.1 Date: Tue, 03 Feb 2015 10:02:09 +0000 From: "Jan Beulich" To: "Daniel Kiper" Subject: Re: [PATCH 03/18] x86/boot: use %ecx instead of %eax References: <1422640462-28103-1-git-send-email-daniel.kiper@oracle.com> <1422640462-28103-4-git-send-email-daniel.kiper@oracle.com> In-Reply-To: <1422640462-28103-4-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 130.57.118.101 Cc: Juergen Gross , grub-devel@gnu.org, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, phcoder@gmail.com, xen-devel@lists.xenproject.org, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2015 10:02:21 -0000 >>> On 30.01.15 at 18:54, wrote: > /* Save the Multiboot info struct (after relocation) for later = use. */ > mov $sym_phys(cpu0_stack)+1024,%esp > - push %ebx > - call reloc > + mov %ecx,%eax > + push %ebx /* Multiboot information address */ > + call reloc /* %eax contains trampoline address = */ This last part looks completely unrelated to the change made here (and contrary to the description, as here you clobber %eax while the description says reloc() needs it unclobbered); afaict it belongs in whatever patch add the consumption of this value in reloc(). That said - passing parameters to reloc() by two different means looks very odd too. I'm clearly of the opinion that parameter passing should follow an existing convention unless entirely unfeasible. Which then raises the question whether this patch is really needed: Rather than fiddling with a lot of code, can't you just copy the incoming %eax into some other register, making this a single line change that can again simply be done in the patch where you actually consume the new information? Jan