Page 1 of 2
Can somebody tell me if this ASM is ok ?
Posted: Wed Oct 16, 2013 2:28 pm
by Luca91
I'm trying to implement the hardware interrupts (software interrupts works ok), but I get a double fault just after boot.. I'm starting to think that I've messed up something when trying to convert ASM from intel to AT&T (the guide that I'm using use intel asm and MSVC++, I'm using GCC instead).
-1st CODE-
original:
Code: Select all
unsigned char _cdecl inportb (unsigned short portid) {
#ifdef _MSC_VER
_asm {
mov dx, word ptr [portid]
in al, dx
mov byte ptr [portid], al
}
#endif
return (unsigned char)portid;
}
my attempt:
Code: Select all
unsigned char inportb(unsigned short portid){
__asm__(
"movw %[portid],%%dx \n"
"in %%dx,%%al \n"
"movb %%al, %[portid] \n"
: // no output
: [portid] "m" (portid) // input
: "dx","al" //clobbers
);
return (unsigned char) portid;
}
2nd CODE:
original:
Code: Select all
void _cdecl outportb (unsigned short portid, unsigned char value) {
#ifdef _MSC_VER
_asm {
mov al, byte ptr [value]
mov dx, word ptr [portid]
out dx, al
}
#endif
}
my attempt:
Code: Select all
void outportb (unsigned short portid, unsigned char value){
__asm__(
"movb %[value], %%al \n"
"movw %[portid], %%dx \n"
"out %%al,%%dx \n"
: // no output
:[value] "m" (value), [portid] "m" (portid)
: "al","dx"
);
}
3rd CODE:
original:
Code: Select all
void _cdecl i86_pit_irq () {
_asm add esp, 12
_asm pushad
//! increment tick count
_pit_ticks++;
//! tell hal we are done
interruptdone(0);
_asm popad
_asm iretd
}
my attempt:
Code: Select all
void i86_pit_irq(){
__asm__(
"addl $12,%esp \n"
"pusha \n"
);
_pit_ticks++;
interruptdone(0);
__asm__(
"popa \n"
"iret \n"
);
}
4nd CODE: (ATTENTION: my attempt doesn't compile- luckily this is not a required function)
original:
Code: Select all
char* i86_cpu_get_vender () {
static char vender[32] = {0};
#ifdef _MSC_VER
_asm {
mov eax, 0
cpuid
mov dword ptr [vender], ebx
mov dword ptr [vender+4], edx
mov dword ptr [vender+8], ecx
}
#endif
return vender;
}
my attempt:
Code: Select all
char* i86_cpu_get_vender(){
static char vendername[50] = {0};
__asm__(
"movl $0,%%eax \n"
"cpuid \n"
"movl %%ebx, %[vendername] \n"
"movl %%edx, %[vendername+$4] \n"
"movl %%ecx, %[vendername+$8] \n"
:"=r"(vendername) //user vendername as output
:[vendername]"m"(vendername) //use vendername as input
:"eax","ebx","edx","ecx" // clobbers those regs
);
return vendername;
}
Thank you very much for the support !
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Oct 17, 2013 6:25 am
by bluemoon
A quick scan spot two issue
1. in the function i86_pit_irq(), you have 2 separated block of assembly without side effect, the compiler is free to re-order them or even eliminate them. Furthermore, you should use the attribute hack to tell gcc that you are using iret in inline assembly.
2. vender should be spelled vendor.
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Oct 17, 2013 6:50 am
by Luca91
Code: Select all
in the function i86_pit_irq(), you have 2 separated block of assembly without side effect, the compiler is free to re-order them or even eliminate them. Furthermore, you should use the attribute hack to tell gcc that you are using iret in inline assembly.
Thanks for this hint, can you tell me how can I do that ? or at least put me on the right direction ? Many thanks
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Oct 17, 2013 2:44 pm
by neon
Hello,
Please note that the MSVC implementation for
i86_pit_irq is hackish and guaranteed to break on GCC. It is highly recommended to implement interrupt handlers in assembly language rather then C. Please reference
this article for an example of this using GCC. It also includes example code that implements support for the PIT.
If you must only use GCC, perhaps
this article might help. It introduces the GCC extensions
__attribute__ ( ( signal, naked ) ). For example,
Code: Select all
/* your interrupt handler */
void i86_pit_irq (void) __attribute__ ( ( signal, naked ) );
void i86_pit_irq (void) {
_pit_ticks++;
interruptdone(0);
asm("reti");
}
/* inportb and outportb */
unsigned char _cdecl inportb (unsigned short portid) {
unsigned char ret;
asm volatile( "inb %1, %0" : "=a"(ret) : "Nd"(portid) );
return ret;
}
void _cdecl outportb (unsigned short portid, unsigned char value) {
asm volatile( "outb %0, %1" : : "a"(value), "Nd"(portid) );
}
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Oct 17, 2013 5:17 pm
by Luca91
Thanks Mike for the reply.
Uhmmm it looks like that __attribute__ ( ( signal, naked ) ) is not supported by x86 GCC. Are you sure ?
Many thanks
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Oct 17, 2013 5:42 pm
by neon
Hello,
It would appear that you are correct. That is unfortunate; they should look into implementing support for it on x86. Unfortunately this means we have two options for ISR's:
1.
Repair the stack. In the C function, you would need to view the disassembly and see how much is pushed on the stack in the generated epilog code of that function and pop them off before returning (this is what the "add esp, 12" does in the MSVC version of the routine; it pops 12 bytes off the stack that was pushed by the epilog code. For GCC, the number will be different. This is what results in triple fault.)
2.
Assembly language. Most portable method and the recommended approach in the long run.
This might be an interesting article. It provides an alternative for writing ISR's in GCC. We do still recommend option (2) above though.
Perhaps someone here knows of an alternative way to write ISR's in C without using those attributes.
Re: Can somebody tell me if this ASM is ok ?
Posted: Fri Oct 18, 2013 2:08 am
by bluemoon
Check the wiki
Interrupt_Service_Routines.
Yes I double check that attribute(IRQ) and interrupt handler related stuff for gcc does not support x86.
You either doing hack as inspired by the wiki, or do proper assembly.
In any case, gcc attribute extension, if it were supported, would not provide any better performance over "asm by hand" since there must be "glue code" anyway.
Re: Can somebody tell me if this ASM is ok ?
Posted: Fri Oct 18, 2013 5:17 am
by Luca91
Thanks to both for the reply.
Neon, you have some pm
Re: Can somebody tell me if this ASM is ok ?
Posted: Sat Oct 19, 2013 10:37 am
by sortie
Don't write interrupt handlers in C. It's that simple. You are begging for trouble otherwise - indeed the reason naked isn't supported on x86 is that a lot of people would abuse it. Indeed, if you wanted to emit assembly from C, why not just use a top-level assembly statement?
Code: Select all
#include <stdfoo.h>
asm (".global foo\n"
".type foo, @function\n"
"foo:\n"
" ret\n"
".size foo, . - foo");
#if defined(__cplusplus)
extern "C"
#endif
void foo(void);
void bar()
{
foo();
}
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Nov 07, 2013 12:27 pm
by Luca91
Sorry mates if I'm resurecting this old post, but I still haven't fixed this issue
According to Sortie's example I've written this code:
Code: Select all
asm (".global i86_pit_irq\n"
".type i86_pit_irq, @function\n"
"i86_pit_irq:\n"
" pusha\n"
"call i86_pit_irq_from_c\n"
"popa\n"
"iret\n"
".size i86_pit_irq, . - i86_pit_irq");
void i86_pit_irq();
void i86_pit_irq_from_c(){
_pit_ticks++;
interruptdone(0);
}
Okay, now when an interrupt occurs, i86_pit_irq is called (the asm function), then this asm function, call the i86_pit_irq_from_c() function, and the it do popa and iret.
I've still the same error.. when I boot the Os I still receive the default handler exception..
Maybe I misunderstood Sortie's example..
Any help please ? I'm stuck ..
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Nov 07, 2013 5:12 pm
by Combuster
This is one of the reasons I avoid AT&T syntax - I can't see if PUSHA and IRET are actually encoded as PUSHADs and IRETDs, rather than their 16-bit variants. In particular because the latter case would break pretty much everything.
That said, the exact error which you're getting is not clear. Which exception (number) occurs at which exact instruction? What is the error code? Does Bochs print any error message (get it if you don't have it yet)? It might be just as well that the error is not actually caused by your code.
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Nov 07, 2013 5:18 pm
by Luca91
I dunno
I'm really confused at this point...
I've also tried to write the asm in another file like suggested by the wiki, but I still get the default handler exception..
I feel a bit lost ..
I think that I've to try to add a debug string..
EDIT: this is the pure asm file that I've written:
Code: Select all
global i86_pit_irq
align 4
extern i86_pit_irq_from_c
i86_pit_irq:
pusha
call i86_pit_irq_from_c
popa
iret
compiled with: nasm -f elf ./Hal/i86_pit_irq.asm -o $build/i86_pit_irq.o
Can anybody confirm at least that this is right ?? thanks
EDIT2: Tomorrow I'll print mike's (Neon) kernel example from his guide and I'll check 1:1 with my code, in the hope that I can figure out what's wrong
Also I found that alot of guides use to add a number to esp to fix the stack even if the use asm for irq handlers, why ??
Re: Can somebody tell me if this ASM is ok ?
Posted: Thu Nov 07, 2013 9:56 pm
by neon
Hello,
The assembly file should be like this,
Code: Select all
global _i86_pit_irq
align 4
extern _i86_pit_irq_from_c
_i86_pit_irq:
pusha
call _i86_pit_irq_from_c
popa
iret
Then just link the resulting
i86_pit_irq.o using your linker in the same way the object files produced by the C compiler are linked. The driver source would look something like this,
Code: Select all
#ifdef __cplusplus
extern "C" { /* don't need if using C. */
#endif
#define CDECL __cdecl
/* define as uint32_t to discourage direct calling. */
extern uint32_t i86_pit_irq;
void PitInitialize (void) {
/* register interrupt handler. */
IdtRegisterHandler (i86_pit_irq, 0);
}
void CDECL i86_pit_irq_from_c (void) {
/* real interrupt handler. */
}
#ifdef __cplusplus
}
#endif
Re: Can somebody tell me if this ASM is ok ?
Posted: Fri Nov 08, 2013 9:26 am
by Luca91
I've used that code but it still give me default handler exception
Any other clue ?
also accourding to your tutorial, this code:
Code: Select all
void PitInitialize (void) {
/* register interrupt handler. */
IdtRegisterHandler (i86_pit_irq, 0);
}
is:
Code: Select all
void i86_pit_initialize(){
setvect(32,i86_pit_irq);
_pit_isinit = 1;
}
Re: Can somebody tell me if this ASM is ok ?
Posted: Fri Nov 08, 2013 12:34 pm
by neon
Hello,
Sorry, the provided code was not tested. You are correct, for the series use the provided i86_pit_initialize. Also make sure it is stored at IDT entry 32 like it is in i86_pit_initialize. After this call, enabling hardware interrupts cannot result in a call to the default handler unless the IDT is not being properly updated or an invalid interrupt is actually being triggered.
We would typically use the Bochs debugger here. That is, you could just run the debugger with the c command until the handler is called; ctrl+c to break out and use the info idt command to validate that the IDT was properly updated and the entry is correct.
If you run it in Bochs, do you get any errors or warnings in the output log? How you are linking i86_pit_irq.o? Also, make sure to use the provided asm file - that is, keeping those underscore prefixes. Make sure the project is being built without errors or warnings.