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
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
