[Top][All Lists]

[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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]