[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 03/18] x86/boot: use %ecx instead of %eax
From: |
Daniel Kiper |
Subject: |
Re: [PATCH 03/18] x86/boot: use %ecx instead of %eax |
Date: |
Tue, 3 Feb 2015 18:43:18 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Feb 03, 2015 at 10:02:09AM +0000, Jan Beulich wrote:
> >>> On 30.01.15 at 18:54, <address@hidden> 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().
Yep, this is confusing. I should change reloc.c:_start() in this patch too.
> 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
So, I think that we should add another patch which fixes this issue
and put all arguments on the stack according to the cdecl calling
convention on x86.
> 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?
If we do thing(s) mentioned above then this issue will disappear too.
Additionally, I think that we should not use another register if
it is not really required.
Daniel