Page 1 of 1

A gcc patch for x86 ISRs

Posted: Fri Jun 24, 2016 4:03 pm
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.

Re: A gcc patch for x86 ISRs

Posted: Sat Jun 25, 2016 2:25 am
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.

Re: A gcc patch for x86 ISRs

Posted: Sat Jun 25, 2016 3:55 am
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

Re: A gcc patch for x86 ISRs

Posted: Mon Jun 27, 2016 12:01 pm
by moondeck
Does that mean that its upstream or what? Where can i get the actual patch?

Re: A gcc patch for x86 ISRs

Posted: Mon Jun 27, 2016 1:03 pm
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.

Re: A gcc patch for x86 ISRs

Posted: Mon Jun 27, 2016 2:24 pm
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.

Re: A gcc patch for x86 ISRs

Posted: Mon Jun 27, 2016 2:47 pm
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.)

Re: A gcc patch for x86 ISRs

Posted: Mon Jun 27, 2016 3:19 pm
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.

Re: A gcc patch for x86 ISRs

Posted: Tue Jun 28, 2016 4:08 am
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.

Re: A gcc patch for x86 ISRs

Posted: Tue Jun 28, 2016 5:49 am
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