Can somebody tell me if this ASM is ok ?

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Can somebody tell me if this ASM is ok ?

Post 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 !
User avatar
bluemoon
Member
Member
Posts: 1761
Joined: Wed Dec 01, 2010 3:41 am
Location: Hong Kong

Re: Can somebody tell me if this ASM is ok ?

Post 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.
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post 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
User avatar
neon
Member
Member
Posts: 1567
Joined: Sun Feb 18, 2007 7:28 pm
Contact:

Re: Can somebody tell me if this ASM is ok ?

Post 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) );
}
OS Development Series | Wiki | os | ncc
char c[2]={"\x90\xC3"};int main(){void(*f)()=(void(__cdecl*)(void))(void*)&c;f();}
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post 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
User avatar
neon
Member
Member
Posts: 1567
Joined: Sun Feb 18, 2007 7:28 pm
Contact:

Re: Can somebody tell me if this ASM is ok ?

Post 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.
OS Development Series | Wiki | os | ncc
char c[2]={"\x90\xC3"};int main(){void(*f)()=(void(__cdecl*)(void))(void*)&c;f();}
User avatar
bluemoon
Member
Member
Posts: 1761
Joined: Wed Dec 01, 2010 3:41 am
Location: Hong Kong

Re: Can somebody tell me if this ASM is ok ?

Post 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.
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post by Luca91 »

Thanks to both for the reply.

Neon, you have some pm :wink:
User avatar
sortie
Member
Member
Posts: 931
Joined: Wed Mar 21, 2012 3:01 pm
Libera.chat IRC: sortie

Re: Can somebody tell me if this ASM is ok ?

Post 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();
}
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post 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 ..
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: Can somebody tell me if this ASM is ok ?

Post 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.
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post 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 [-o<

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 :x :( 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 ?? :|
User avatar
neon
Member
Member
Posts: 1567
Joined: Sun Feb 18, 2007 7:28 pm
Contact:

Re: Can somebody tell me if this ASM is ok ?

Post 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
OS Development Series | Wiki | os | ncc
char c[2]={"\x90\xC3"};int main(){void(*f)()=(void(__cdecl*)(void))(void*)&c;f();}
Luca91
Member
Member
Posts: 35
Joined: Wed Oct 09, 2013 12:16 pm

Re: Can somebody tell me if this ASM is ok ?

Post by Luca91 »

I've used that code but it still give me default handler exception #-o
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;

}
User avatar
neon
Member
Member
Posts: 1567
Joined: Sun Feb 18, 2007 7:28 pm
Contact:

Re: Can somebody tell me if this ASM is ok ?

Post 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.
OS Development Series | Wiki | os | ncc
char c[2]={"\x90\xC3"};int main(){void(*f)()=(void(__cdecl*)(void))(void*)&c;f();}
Post Reply