A gcc patch for x86 ISRs

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.
Post Reply
didier
Posts: 3
Joined: Fri Jun 24, 2016 3:22 pm

A gcc patch for x86 ISRs

Post by didier »

A patch of gcc has been published introducing the attribute interrupt for both i386 and amd64.

https://gcc.gnu.org/git/?p=gcc.git;a=co ... 65be8223be

Enjoy
May the wiki chapter dedicated to ISR be able to be uptaded soon.
User avatar
Neroku
Posts: 24
Joined: Tue Dec 01, 2015 4:53 am

Re: A gcc patch for x86 ISRs

Post by Neroku »

Cool 8)
Since the direction flag in the FLAGS register in interrupt (exception)
handlers is undetermined, cld instruction must be emitted in function
prologue if rep string instructions are used in interrupt (exception)
handler or interrupt (exception) handler isn't a leaf function.
No need for cld anymore, gcc will do it for you.
currently working on kboot86, the Genesis of my kernel
glauxosdever
Member
Member
Posts: 501
Joined: Wed Jun 17, 2015 9:40 am
Libera.chat IRC: glauxosdever
Location: Athens, Greece

Re: A gcc patch for x86 ISRs

Post by glauxosdever »

Hi,


I don't think using the interrupt attribute is a good idea. This might work now, but if this behaviour is changed in later versions, this will break very badly. Additionally, you want fine grained control of how registers are saved and restored. Just use a separate assembly file, so you know it works.

Interrupt handlers require stuff like iret at the end and lots of pushes/pops, changes to segment registers, etc. There is no actual reason to do this in C/C++ anyway.

P.S.: I attribute this knowledge to sortie, who unfortunately does not frequent the forums anymore...


Regards,
glauxosdever
User avatar
moondeck
Member
Member
Posts: 56
Joined: Sat Dec 19, 2015 12:18 pm
Libera.chat IRC: moondeck
Location: The Zone, Chernobyl

Re: A gcc patch for x86 ISRs

Post by moondeck »

Does that mean that its upstream or what? Where can i get the actual patch?
User avatar
Rusky
Member
Member
Posts: 792
Joined: Wed Jan 06, 2010 7:07 pm

Re: A gcc patch for x86 ISRs

Post by Rusky »

glauxosdever wrote:This might work now, but if this behaviour is changed in later versions, this will break very badly.
This feature is explicitly for x86 interrupts. It will not change in later versions, barring compiler bugs.
User avatar
~
Member
Member
Posts: 1228
Joined: Tue Mar 06, 2007 11:17 am
Libera.chat IRC: ArcheFire

Re: A gcc patch for x86 ISRs

Post by ~ »

glauxosdever wrote:Hi,


I don't think using the interrupt attribute is a good idea. This might work now, but if this behaviour is changed in later versions, this will break very badly. Additionally, you want fine grained control of how registers are saved and restored. Just use a separate assembly file, so you know it works.

Interrupt handlers require stuff like iret at the end and lots of pushes/pops, changes to segment registers, etc. There is no actual reason to do this in C/C++ anyway.

P.S.: I attribute this knowledge to sortie, who unfortunately does not frequent the forums anymore...


Regards,
glauxosdever
It won't break any application if those platform-dependent parts are confined to the "arch" source directory for each hardware/software platform.

They must have chosen the interupt convention call that UNIX uses.
onlyonemac
Member
Member
Posts: 1146
Joined: Sat Mar 01, 2014 2:59 pm

Re: A gcc patch for x86 ISRs

Post by onlyonemac »

Sounds interesting, but restrictive.

I just do this:

Code: Select all

#define create_isr_wrapper(irq)\
void* irq_##irq##_handler()\
{\
	volatile void*	isr_address;\
	asm goto("jmp %l[isr_end]":::"memory":isr_end);\
	asm volatile(".align 16":::"memory");\
\
	isr_start:\
	asm volatile("pushal\npushl %%ebp\nmovl %%esp, %%ebp":::"memory");\
	asm volatile("pushl %%ebx\ncall *%%eax\naddl $4, %%esp"::"a"(irq_handler), "b"((uint32_t) irq):"memory");\
	asm volatile("leave\npopal\niret":::"memory");\
	isr_end:\
\
	asm goto(".intel_syntax noprefix\nmov eax, offset %l[isr_start]\nmov [ebx], eax\n.att_syntax"::"b"(&isr_address):"eax", "memory":isr_start);\
	return (void*) isr_address;\
}
then this (outside of any function):

Code: Select all

create_isr_wrapper(0x20);
create_isr_wrapper(0x21);
create_isr_wrapper(0x22);
create_isr_wrapper(0x23);
create_isr_wrapper(0x24);
create_isr_wrapper(0x25);
create_isr_wrapper(0x26);
create_isr_wrapper(0x27);
create_isr_wrapper(0x28);
create_isr_wrapper(0x29);
create_isr_wrapper(0x2A);
create_isr_wrapper(0x2B);
create_isr_wrapper(0x2C);
create_isr_wrapper(0x2D);
create_isr_wrapper(0x2E);
create_isr_wrapper(0x2F);
then this (in my init function):

Code: Select all

	add_idt_entry(&(idt_struct->table.table[0x20]), (uint32_t) irq_0x20_handler());
	add_idt_entry(&(idt_struct->table.table[0x21]), (uint32_t) irq_0x21_handler());
	add_idt_entry(&(idt_struct->table.table[0x22]), (uint32_t) irq_0x22_handler());
	add_idt_entry(&(idt_struct->table.table[0x23]), (uint32_t) irq_0x23_handler());
	add_idt_entry(&(idt_struct->table.table[0x24]), (uint32_t) irq_0x24_handler());
	add_idt_entry(&(idt_struct->table.table[0x25]), (uint32_t) irq_0x25_handler());
	add_idt_entry(&(idt_struct->table.table[0x26]), (uint32_t) irq_0x26_handler());
	add_idt_entry(&(idt_struct->table.table[0x27]), (uint32_t) irq_0x27_handler());
	add_idt_entry(&(idt_struct->table.table[0x28]), (uint32_t) irq_0x28_handler());
	add_idt_entry(&(idt_struct->table.table[0x29]), (uint32_t) irq_0x29_handler());
	add_idt_entry(&(idt_struct->table.table[0x2A]), (uint32_t) irq_0x2A_handler());
	add_idt_entry(&(idt_struct->table.table[0x2B]), (uint32_t) irq_0x2B_handler());
	add_idt_entry(&(idt_struct->table.table[0x2C]), (uint32_t) irq_0x2C_handler());
	add_idt_entry(&(idt_struct->table.table[0x2D]), (uint32_t) irq_0x2D_handler());
	add_idt_entry(&(idt_struct->table.table[0x2E]), (uint32_t) irq_0x2E_handler());
	add_idt_entry(&(idt_struct->table.table[0x2F]), (uint32_t) irq_0x2F_handler());
then write this:

Code: Select all

void irq_handler(uint32_t irq)
{
	// handle irq

	if (irq >= 0x20 && irq < 0x30)
	{
		if (irq >= 0x28)
		{
			hardware_outb(0x00A0, 0x20);
		}

		hardware_outb(0x0020, 0x20);
	}
}
It's been a while since I wrote this code so I can't remember exactly how it works but from what I remember it creates a custom assembly function for each ISR inside a wrapper which returns the address of this function and adds them to the IDT. Each ISR is passed to a common C handler which in turn handles the ISR (possibly by passing it to another function, such as one registered by a driver) and then acknowledges the interrupt. (I should probably also disable interrupts somewhere while the IRQ is being processed.)
When you start writing an OS you do the minimum possible to get the x86 processor in a usable state, then you try to get as far away from it as possible.

Syntax checkup:
Wrong: OS's, IRQ's, zero'ing
Right: OSes, IRQs, zeroing
User avatar
sortie
Member
Member
Posts: 931
Joined: Wed Mar 21, 2012 3:01 pm
Libera.chat IRC: sortie

Re: A gcc patch for x86 ISRs

Post by sortie »

glauxosdever wrote:P.S.: I attribute this knowledge to sortie, who unfortunately does not frequent the forums anymore...
Please don't credit me like that. Don't use my name to give you legitimacy Please don't defend your opinions in #osdev by “take it up with sortie, it's his opinion”. Whenever an opinion is repeated, there is inherent loss, and you cannot represent the original opinion. It's your responsibility to truly understand why and defend it when challenged, don't invoke me.

This patch seems thought out and seems like something that could work. I'm OK with compiler support that's thought out and designed to work. I'm slightly worried it might not be ideal enough, not sure how it plays with segment registers, and if they need to be handled. It doesn't seem to save which interrupt happened either. Writing handlers like this would mean a lot of duplicated code to save and restore registers, compared to the usual method of a common stub that does this, and then calls a C handler. The design is sound though, just doesn't seem optimal.

The thing I have issue with is what onlyonemac did. He reinvented this with a lot of hacks that's not designed to work this way. It's essentially an assembler file wrapped in a C function. Changing the assembler syntax midway for extra craziness.I discourage this kind of thing because it's guaranteed to work, unlike the attribute which works together with the compiler. Note that it's possible to use asm() outside of functions, actually. You can emit arbitrary assembly that way and it doesn't have to deal with being in the middle of a function. I use that to emit system calls in libc and it works by design, it's essentially an inline assembler file.
User avatar
xenos
Member
Member
Posts: 1121
Joined: Thu Aug 11, 2005 11:00 pm
Libera.chat IRC: xenos1984
Location: Tartu, Estonia
Contact:

Re: A gcc patch for x86 ISRs

Post by xenos »

In general, I like the idea of having an "interrupt" attribute for functions used as interrupt handlers, which causes gcc to emit a suitable function prologue / epilogue, saves registers if they are used by the handler, and so on. This makes writing interrupt handlers a bit easier, but only if one has understood how x86 interrupts work, what handlers should do and what this gcc attribute does. So I would call it a nice feature for advanced users.

But there are also some drawbacks. Less advanced users, who don't know the quirks of interrupt handling well, might just use this attribute without fully understanding or even seeing the need to learn interrupt handling, and then run into problems if gcc is not doing what they expect. So at the very least this feature must be documented very well so that it doesn't become such kind of trap.

Another drawback is that if registers are saved in the background, there is no way to access or modify their contents. This makes this attribute unusable for writing system calls, where you want to get the original registers contents, take some action depending on their value, and return some other values to the calling code.
Programmers' Hardware Database // GitHub user: xenos1984; OS project: NOS
glauxosdever
Member
Member
Posts: 501
Joined: Wed Jun 17, 2015 9:40 am
Libera.chat IRC: glauxosdever
Location: Athens, Greece

Re: A gcc patch for x86 ISRs

Post by glauxosdever »

Hi,

sortie wrote:Please don't credit me like that. Don't use my name to give you legitimacy Please don't defend your opinions in #osdev by “take it up with sortie, it's his opinion”. Whenever an opinion is repeated, there is inherent loss, and you cannot represent the original opinion. It's your responsibility to truly understand why and defend it when challenged, don't invoke me.
No. I did not credit you to give myself legitimacy. I thought it would be unfair to be egoist and simply state this opinion, while it was borrowed from you. However, you are right that I should not invoke you when challenged and that it would be better to defend this opinion myself.

I stated this opinion here because I thought it would be helpful to the thread. I did not state it just to show I am experienced, or that I have good opinions.

Anyway, welcome back to the forums! Hope you stay! :)


Regards,
glauxosdever
Post Reply