[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