[coreboot] Multiboot

Jordan Crouse jordan.crouse at amd.com
Tue Sep 23 16:06:31 CEST 2008


On 23/09/08 15:19 +0200, Robert Millan wrote:
> On Tue, Sep 23, 2008 at 10:10:31AM +0200, Mathias Krause wrote:
> > Hi,
> > 
> > Robert Millan wrote:
> > >   - Add "edx" to clobber list.  Together with eax and ecx which are already
> > >     in input list, this makes the call able to return for any payload that
> > >     complies with "cdecl" calling convention.
> > 
> > ...
> > 
> > > +
> > > +static int run_address_multiboot(void *f)
> > > +{
> > > +	int ret;
> > > +	__asm__ __volatile__ ("call *%3" : "=a" (ret) : "a" (MB_MAGIC2), "b" (0xf0000), "c" (f) : "edx");
> > > +	return ret;
> > > +}
> > > +
> > 
> > this assembler inline is not quite correct to be able to let the call
> > return because the compiler assumes that after this assembler statement
> > %ecx still holds the value f which hasn't to be the case. It assumes the
> > same for %ebx but this should be no problem since the calling function
> > must preserve it's value if it follows the C calling convention. But
> > %ecx isn't one of the registers that needs to be preserved, so the
> > assumption the compiler makes here is wrong.
> > 
> > How about this one:
> > 
> > | static int run_address_multiboot(void *f)
> > | {
> > |     int ret, dummy;
> > |     __asm__ __volatile__ ("call *%4" : "=a" (ret), "=c" (dummy) : "a"
> > (MB_MAGIC2), "b" (0xf0000), "c" (f) : "edx", "memory");
> > |     return ret;
> > | }
> 
> I see.  But why not just add "ecx" to clobber list instead?  Then a dummy
> variable isn't needed.
> 
> > The memory clobber is needed since you cannot know what the called
> > function will actually do with the memory and to ensure all cached
> > values are actually written back to memory before calling f().
> 
> Is this really a problem?  If the payload expects to return, it isn't
> supposed to modify coreboot's memory at all.  If it does, I'd say it's
> normal that things break.

Thats exactly right - if the payload is expected to return, then the
responsiblity is on it not to destroy memory.   All coreboot can do is
call the function and hope for the best.

Jordan





More information about the coreboot mailing list