[coreboot] inline asm peculiarities in v3
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri Sep 5 03:50:12 CEST 2008
On 04.09.2008 17:46, ron minnich wrote:
> On Thu, Sep 4, 2008 at 8:32 AM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>> Hi,
>>
>> I decided to look at all cases of inline asm in our v3 code and I found
>> a few bits which either work by accident or need better documentation.
>>
>> We use __asm__ and asm, __volatile__ and volatile. Can we please decide
>> which one we want, then I'll switch the tree over.
>>
>
> somebody who knows better than me the implications can comment.
>
The gcc info page states that __asm__ and asm are equivalent, but if you
use -ansi or -fno-asm or -fno-gnu-keywords in your CFLAGS only __asm__
will be recognized. asm is shorter, so I'd prefer asm.
__volatile__ vs. volatile is not explained in the gcc docs at all, but
various places on the net state that they're identical.
Segher confirmed these findings. I'll send a patch to move everything to
the short syntax.
>> northbridge/amd/geodelx/vsmsetup.c:143
>>
>>> __asm__(".text\n" "real_mode_switch_end:\n");
>>> extern char real_mode_switch_end[];
>>>
>> AFAICS the compiler and linker are free to place the resulting code
>> anywhere in the binary independent of each other.
>>
>
> Not a problem. The "extern" is simply making the label available to C.
> They don't depend on any trickiness.
>
My point was about the label. Sorry for being unclear. That label can be
placed anywhere, so I can't see it serving any purpose.
>> include/arch/x86/amd/k8/k8.h:746
>>
>>> static void disable_cache_as_ram_bsp(void)
>>> {
>>> __asm__ volatile (
>>> // "pushl %eax\n\t"
>>> "pushl %edx\n\t"
>>> "pushl %ecx\n\t"
>>> );
>>>
>>> disable_cache_as_ram();
>>> __asm__ volatile (
>>> "popl %ecx\n\t"
>>> "popl %edx\n\t"
>>> // "popl %eax\n\t"
>>> );
>>> }
>>>
>> The pushl and popl instructions seem to serve no real purpose. Kill them?
>>
>
> My rule with the Clever v2 code is not to modify it until I understand
> it. So I did not touch these because I don't know what function they
> serve. That's one reason that I am bringing code over and leaving it
> ugly to start. Let's leave this alone until K8 is working.
>
But can I add a comment that the current code seems nonsensical?
>> The K8 CAR disabling has inline asm depending on good compiler behaviour
>> and luck. Patch will be sent separately.
>>
>
> Thank you.
>
You're welcome. See the mail with subject "[PATCH] v3: correct K8 stack
preservation asm".
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list