OSDev.org

The Place to Start for Operating System Developers
http://f.osdev.org/

Issue with put pixel connected with keyboard irq handler.

http://f.osdev.org/viewtopic.php?t=31564

Page 1 of 1

Issue with put pixel connected with keyboard irq handler.

Posted: Sun Apr 23, 2017 4:54 am
by viruss33
Hello. I've had an issue with put pixel. When I put more than some amount of pixel on the screen (I believe it was around 15 letters that when printed, didn't bug the keyboard, but 16 letters was buggin it) my keyboard handler was not called and I saw in Bochs logs that keyboard internal buffer was full. I have fixed it by moving the put pixel calls after doing all initialization routines (like isr install, init timer and init keyboard). Altho it's fixed I'm trying to understand what was the cause for this issue. So the question is: why the put pixel (or print string which uses put pixel) has to be called after initializing irq handlers. Here's some code:
kernel.c start method (this is the version that was bugging the keyboard).

Code: Select all

print_string("11111111111111111111111111111111111111111111111111111111111111111111111111111111111111");
isr_install();
asm volatile("sti");
init_timer(50);
init_keyboard();
put_pixel.c

Code: Select all

#include "draw_pixel.h"
#include "../cpu/types.h"
#include "../libc/hex_to_string.h"

u32* put_pixel(u32 x_pos, u32 y_pos, u32 color){
	u32 *video = (u32*)(best_video_mode.framebuffer+
			y_pos*best_video_mode.bytes_per_line+(x_pos*(best_video_mode.bpp/8)));
	*video=color;
	return video;
}

void draw_horizontal_line (u32 x_pos, u32 y_pos, u32 color, int length){
	u32* video = put_pixel(x_pos, y_pos, color);
	int i=1;
	while (i<length){
		video[i]=color;
		i++;
	}
}

void draw_vertical_line (u32 x_pos, u32 y_pos, u32 color, int length){
	u32* video = put_pixel(x_pos, y_pos, color);
	int i=0;
	while (i<length*best_video_mode.bytes_per_line/4){
		video[i]=color;
		i+=best_video_mode.bytes_per_line/4;
	}
}

draw_string.c

Code: Select all

#include "../cpu/types.h"
#include "draw_pixel.h"
#include "font_encoding.h"

unsigned int cursor_position_x = 0;
unsigned int cursor_position_y = 0;



void draw_string(char* a, u32 color){
	unsigned int i=0;
	unsigned int j=0;

	unsigned int k=0;
	while (a[k]!=0){
		if (a[k]=='\n'){
			cursor_position_y +=8;
			cursor_position_x = 0;
			break;
		}
		for (i=0; i<8; i++){
			int row = font8x8_basic[(int)a[k]][i];
			for (j=0; j<8; j++){
				u16 bit = (row & (1 << j));
				if (bit!=0){
					put_pixel((u32)(cursor_position_x + j), (u32)(cursor_position_y + i), color);
				}

			}
		}
		cursor_position_x+=8;
		if (cursor_position_x >= 1200){
			cursor_position_y += 8;
			cursor_position_x = 0;
		}
		k++;
	}
}

void print_string(char *a){
	draw_string(a, 0x00FFFFFF);
}

void println(char *a){
	print_string(a);
	print_string("\n");
}



isr.c

Code: Select all

#include "isr.h"
#include "idt.h"
#include "types.h"
#include "../libc/strings.h"
#include "port_read_write.h"
#include "../graphics/draw_pixel.h"
#include "../graphics/draw_string.h"
#include "../libc/hex_to_string.h"

#define PIC_MASTER_COMMAND 0x20
#define PIC_SLAVE_COMMAND 0xA0
#define PIC_MASTER_DATA (PIC_MASTER_COMMAND +1)
#define PIC_SLAVE_DATA (PIC_SLAVE_COMMAND +1)

#define END_OF_INTERRUPT 0x20
#define INITIALIZATION_AND_CONNECTION 0x11
#define MASTER_VECTOR_OFFSET 0x20
#define SLAVE_VECTOR_OFFSET 0x28
#define SLAVE_TO_MASTER_IRQ 0x04
#define ARCHITECTURE_80_86 0x01
#define PIC_MASK 0x0

isr_t interrupt_handlers[256];

/* To print the message which defines every exception */
char *exception_messages[] = {
    "Division By Zero",
    "Debug",
    "Non Maskable Interrupt",
    "Breakpoint",
    "Into Detected Overflow",
    "Out of Bounds",
    "Invalid Opcode",
    "No Coprocessor",

    "Double Fault",
    "Coprocessor Segment Overrun",
    "Bad TSS",
    "Segment Not Present",
    "Stack Fault",
    "General Protection Fault",
    "Page Fault",
    "Unknown Interrupt",

    "Coprocessor Fault",
    "Alignment Check",
    "Machine Check",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",

    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved",
    "Reserved"
};

/* Can't do this with a loop because we need the address
 * of the function names */

void installISRs(){
	set_idt_gate(0, (u32)isr0);
	set_idt_gate(1, (u32)isr1);
	set_idt_gate(2, (u32)isr2);
	set_idt_gate(3, (u32)isr3);
	set_idt_gate(4, (u32)isr4);
	set_idt_gate(5, (u32)isr5);
	set_idt_gate(6, (u32)isr6);
	set_idt_gate(7, (u32)isr7);
	set_idt_gate(8, (u32)isr8);
	set_idt_gate(9, (u32)isr9);
	set_idt_gate(10, (u32)isr10);
	set_idt_gate(11, (u32)isr11);
	set_idt_gate(12, (u32)isr12);
	set_idt_gate(13, (u32)isr13);
	set_idt_gate(14, (u32)isr14);
	set_idt_gate(15, (u32)isr15);
	set_idt_gate(16, (u32)isr16);
	set_idt_gate(17, (u32)isr17);
	set_idt_gate(18, (u32)isr18);
	set_idt_gate(19, (u32)isr19);
	set_idt_gate(20, (u32)isr20);
	set_idt_gate(21, (u32)isr21);
	set_idt_gate(22, (u32)isr22);
	set_idt_gate(23, (u32)isr23);
	set_idt_gate(24, (u32)isr24);
	set_idt_gate(25, (u32)isr25);
	set_idt_gate(26, (u32)isr26);
	set_idt_gate(27, (u32)isr27);
	set_idt_gate(28, (u32)isr28);
	set_idt_gate(29, (u32)isr29);
	set_idt_gate(30, (u32)isr30);
	set_idt_gate(31, (u32)isr31);
}

void remapThePIC(){
	port_byte_out(PIC_MASTER_COMMAND, INITIALIZATION_AND_CONNECTION);
	port_byte_out(PIC_SLAVE_COMMAND, INITIALIZATION_AND_CONNECTION);
	port_byte_out(PIC_MASTER_DATA, MASTER_VECTOR_OFFSET);
	port_byte_out(PIC_SLAVE_DATA, SLAVE_VECTOR_OFFSET);
	port_byte_out(PIC_MASTER_DATA, SLAVE_TO_MASTER_IRQ);
	port_byte_out(PIC_SLAVE_DATA, 0x02);
	port_byte_out(PIC_MASTER_DATA, ARCHITECTURE_80_86);
	port_byte_out(PIC_SLAVE_DATA, ARCHITECTURE_80_86);
	port_byte_out(PIC_MASTER_DATA, PIC_MASK);
	port_byte_out(PIC_SLAVE_DATA, PIC_MASK);
}

void installIRQs(){
	set_idt_gate(32, (u32)irq0);
	set_idt_gate(33, (u32)irq1);
	set_idt_gate(34, (u32)irq2);
	set_idt_gate(35, (u32)irq3);
	set_idt_gate(36, (u32)irq4);
	set_idt_gate(37, (u32)irq5);
	set_idt_gate(38, (u32)irq6);
	set_idt_gate(39, (u32)irq7);
	set_idt_gate(40, (u32)irq8);
	set_idt_gate(41, (u32)irq9);
	set_idt_gate(42, (u32)irq10);
	set_idt_gate(43, (u32)irq11);
	set_idt_gate(44, (u32)irq12);
	set_idt_gate(45, (u32)irq13);
	set_idt_gate(46, (u32)irq14);
	set_idt_gate(47, (u32)irq15);
}

void isr_install() {
	installISRs();
	remapThePIC();
	installIRQs();
	set_idt(); // Load with ASM
}

void isr_handler(registers_t* r) {
//    print("received interrupt: ");
    char s[3];
    int_to_ascii(r->int_no, s);
//    print(s);
//    println(exception_messages[r->int_no]);
}

void register_interrupt_handler(u8 n, isr_t handler) {
    interrupt_handlers[n] = handler;
}

void irq_handler(registers_t r) {
    /* Tell the PICs that we handled the IRQ, so that he can accept
     * more IRQs */

    if (r.int_no >= 40) port_byte_out(PIC_SLAVE_COMMAND, END_OF_INTERRUPT); /* slave */
    port_byte_out(PIC_MASTER_COMMAND, END_OF_INTERRUPT); /* master */

    /* Handle the interrupt in a more modular way */
    if (interrupt_handlers[r.int_no] != 0) {

        isr_t handler = interrupt_handlers[r.int_no];
        handler(r);
    }
}
keyboard.c

Code: Select all

#include "keyboard.h"
#include "../cpu/isr.h"
#include "../cpu/port_read_write.h"
#include "../graphics/draw_string.h"
#include "../graphics/draw_pixel.h"
#include "../libc/function.h"
#include "../libc/strings.h"
#include "../cpu/port_read_write.h"
#include "../kernel/kernel.h"

#define BACKSPACE 0x0E
#define TAB 0x0F
#define ENTER 0x1C
#define SHIFT_PRESS 0x2A
#define SHIFT_RELEASE 0xAA
#define CAPSLOCK_PRESS 0x3A


#define SC_MAX 57

static char key_buffer[256];

const char *sc_name[] = { "ERROR", "Esc", "1", "2", "3", "4", "5", "6",
    "7", "8", "9", "0", "-", "=", "Backspace", "Tab", "Q", "W", "E",
        "R", "T", "Y", "U", "I", "O", "P", "[", "]", "Enter", "Lctrl",
        "A", "S", "D", "F", "G", "H", "J", "K", "L", ";", "'", "`",
        "LShift", "\\", "Z", "X", "C", "V", "B", "N", "M", ",", ".",
        "/", "RShift", "Keypad *", "LAlt", "Spacebar","Capslock"};

const char sc_ascii_capital[] = { '?', '?', '!', '@', '#', '$', '%', '^',
    '&', '*', '(', ')', '_', '+', '?', '?', 'Q', 'W', 'E', 'R', 'T', 'Y',
        'U', 'I', 'O', 'P', '{', '}', '?', '?', 'A', 'S', 'D', 'F', 'G',
        'H', 'J', 'K', 'L', ':', '"', '`', '?', '|', 'Z', 'X', 'C', 'V',
        'B', 'N', 'M', '<', '>', '?', '?', '?', '?', ' ', 'Q'};

const char sc_ascii_low[] = { '?', '?', '1', '2', '3', '4', '5', '6',
    '7', '8', '9', '0', '-', '=', '?', '?', 'q', 'w', 'e', 'r', 't', 'y',
        'u', 'i', 'o', 'p', '[', ']', '?', '?', 'a', 's', 'd', 'f', 'g',
        'h', 'j', 'k', 'l', ';', '\'', '`', '?', '\\', 'z', 'x', 'c', 'v',
        'b', 'n', 'm', ',', '.', '/', '?', '?', '?', ' ', '?'};

static int shift_on = 0; // 0 is low, 1 is high
static int capslock_on = 0;

boolean should_use_capital(){
	return !shift_on != !capslock_on;
}

boolean is_letter(u8 scancode){
	return (scancode <=0X19 && scancode>=0X10)
			|| (scancode <=0X26 && scancode>=0x1E) || (scancode<=0X32 && scancode >=0x2C);
}

static void keyboard_callback(registers_t regs) {
	/* The PIC leaves us the scancode in port 0x60 */
	    u8 scancode = port_byte_in(0x60);

	    if (scancode == SHIFT_PRESS){
			shift_on=1-shift_on;
		}
		else if (scancode == SHIFT_RELEASE){
			shift_on=1-shift_on;
		}
		else if (scancode == CAPSLOCK_PRESS){
			capslock_on = 1 - capslock_on;
		}

	    if (scancode > SC_MAX) return;
	    if (scancode == BACKSPACE) {
//	        backspace(key_buffer);
//	        print_backspace();
	    }
	    else if (scancode == ENTER) {
	    	println("");
	        user_input(key_buffer); /* kernel-controlled function */
	        key_buffer[0] = '\0';
	    }
	    else if (scancode == TAB){
//	    	print_tab();
	    }
	    else if (scancode == SHIFT_PRESS){
	    	//do not display shift key TODO as well as alt caps esc etc.
	    }

	    else {
	    	char letter;
	    	if (capslock_on && !shift_on){
	    		if (is_letter(scancode)){
	    			letter = sc_ascii_capital[(int)scancode];
	    		}
	    		else{
	    			letter = sc_ascii_low[(int)scancode];
	    		}
	    	}
	    	else if (shift_on && !capslock_on){
	    		letter = sc_ascii_capital[(int)scancode];
	    	}
	    	else {
	    		letter = sc_ascii_low[(int)scancode];
	    	}
	    	if (strlen(key_buffer)>255){
	    		int i;
	    		for (i=0; i<256; i++){
	    			key_buffer[i]='\0';
	    		}
	    	}
	        char str[2] = {letter, '\0'};
	        append(key_buffer, letter);
	        print_string(str);
	    }
	    UNUSED(regs);
}



void init_keyboard() {
   register_interrupt_handler(IRQ1, keyboard_callback);
}
timer.c

Code: Select all

#include "timer.h"
#include "isr.h"
#include "types.h"
#include "port_read_write.h"
#include "../libc/function.h"
#include "../libc/strings.h"

#define COMMAND_CHANNEL_0_LO_AND_HI_BYTE_SQUARE_WAVE_BINARY 0x36
#define PIT_COMMAND_PORT 0x43
#define PIT_CHANNEL_0_DATA_PORT 0x40
#define PIT_OSCILLATOR_FREQUENCY_HZ 1193180

u32 tick = 0;

static void timer_callback (registers_t regs){
	UNUSED(regs);
	tick++;

	if (tick%100==0){
		char tick_ascii [256];
		int_to_ascii(tick/100, tick_ascii);
//		print("Minelo: ");
//		print	(tick_ascii);
//		println(" sekund");
	}

}

void init_timer(u32 freq){
	register_interrupt_handler(IRQ0, timer_callback);

	u32 divisor = PIT_OSCILLATOR_FREQUENCY_HZ / freq;
	u8 low = (u8) (divisor & 0xFF);
	u8 high = (u8) ( (divisor>>8) & 0xFF);

	port_byte_out (PIT_COMMAND_PORT, COMMAND_CHANNEL_0_LO_AND_HI_BYTE_SQUARE_WAVE_BINARY);
	port_byte_out (PIT_CHANNEL_0_DATA_PORT, low);
	port_byte_out (PIT_CHANNEL_0_DATA_PORT, high);
}
interrupt.asm (this one is the most suspicious for me as I followed Carl Fenollosa's tutorial. https://github.com/cfenollosa/os-tutorial
When I applied the fixes from 23 lesson I still had the issues)

Code: Select all

[extern isr_handler]
[extern irq_handler]
; Defined in isr.c


; Common ISR code
isr_common_stub:
    ; 1. Save CPU state
	pusha ; Pushes edi,esi,ebp,esp,ebx,edx,ecx,eax
	mov ax, ds ; Lower 16-bits of eax = ds.
	push eax ; save the data segment descriptor
	mov ax, 0x10  ; kernel data segment descriptor
	mov ds, ax
	mov es, ax
	mov fs, ax
	mov gs, ax

	push esp
	cld
    ; 2. Call C handler
	call isr_handler

    ; 3. Restore state
	pop eax
	pop eax
	mov ds, ax
	mov es, ax
	mov fs, ax
	mov gs, ax
	popa
	add esp, 8 ; Cleans up the pushed error code and pushed ISR number
	sti
	iret ; pops 5 things at once: CS, EIP, EFLAGS, SS, and ESP

; Common IRQ code. Identical to ISR code except for the 'call'
; and the 'pop ebx'
irq_common_stub:
    pusha
    mov ax, ds
    push eax
    mov ax, 0x10
    mov ds, ax
    mov es, ax
    mov fs, ax
    mov gs, ax

    call irq_handler ; Different than the ISR code
    pop ebx  ; Different than the ISR code
    mov ds, bx
    mov es, bx
    mov fs, bx
    mov gs, bx
    popa
    add esp, 8
    sti
    iret

; We don't get information about which interrupt was caller
; when the handler is run, so we will need to have a different handler
; for every interrupt.
; Furthermore, some interrupts push an error code onto the stack but others
; don't, so we will push a dummy error code for those which don't, so that
; we have a consistent stack for all of them.

; First make the ISRs global
global isr0
global isr1
global isr2
global isr3
global isr4
global isr5
global isr6
global isr7
global isr8
global isr9
global isr10
global isr11
global isr12
global isr13
global isr14
global isr15
global isr16
global isr17
global isr18
global isr19
global isr20
global isr21
global isr22
global isr23
global isr24
global isr25
global isr26
global isr27
global isr28
global isr29
global isr30
global isr31

global irq0
global irq1
global irq2
global irq3
global irq4
global irq5
global irq6
global irq7
global irq8
global irq9
global irq10
global irq11
global irq12
global irq13
global irq14
global irq15

; 0: Divide By Zero Exception
isr0:
    cli
    push byte 0
    push byte 0
    jmp isr_common_stub

; 1: Debug Exception
isr1:
    cli
    push byte 0
    push byte 1
    jmp isr_common_stub

; 2: Non Maskable Interrupt Exception
isr2:
    cli
    push byte 0
    push byte 2
    jmp isr_common_stub

; 3: Int 3 Exception
isr3:
    cli
    push byte 0
    push byte 3
    jmp isr_common_stub

; 4: INTO Exception
isr4:
    cli
    push byte 0
    push byte 4
    jmp isr_common_stub

; 5: Out of Bounds Exception
isr5:
    cli
    push byte 0
    push byte 5
    jmp isr_common_stub

; 6: Invalid Opcode Exception
isr6:
    cli
    push byte 0
    push byte 6
    jmp isr_common_stub

; 7: Coprocessor Not Available Exception
isr7:
    cli
    push byte 0
    push byte 7
    jmp isr_common_stub

; 8: Double Fault Exception (With Error Code!)
isr8:
    cli
    push byte 8
    jmp isr_common_stub

; 9: Coprocessor Segment Overrun Exception
isr9:
    cli
    push byte 0
    push byte 9
    jmp isr_common_stub

; 10: Bad TSS Exception (With Error Code!)
isr10:
    cli
    push byte 10
    jmp isr_common_stub

; 11: Segment Not Present Exception (With Error Code!)
isr11:
    cli
    push byte 11
    jmp isr_common_stub

; 12: Stack Fault Exception (With Error Code!)
isr12:
    cli
    push byte 12
    jmp isr_common_stub

; 13: General Protection Fault Exception (With Error Code!)
isr13:
    cli
    push byte 13
    jmp isr_common_stub

; 14: Page Fault Exception (With Error Code!)
isr14:
    cli
    push byte 14
    jmp isr_common_stub

; 15: Reserved Exception
isr15:
    cli
    push byte 0
    push byte 15
    jmp isr_common_stub

; 16: Floating Point Exception
isr16:
    cli
    push byte 0
    push byte 16
    jmp isr_common_stub

; 17: Alignment Check Exception
isr17:
    cli
    push byte 0
    push byte 17
    jmp isr_common_stub

; 18: Machine Check Exception
isr18:
    cli
    push byte 0
    push byte 18
    jmp isr_common_stub

; 19: Reserved
isr19:
    cli
    push byte 0
    push byte 19
    jmp isr_common_stub

; 20: Reserved
isr20:
    cli
    push byte 0
    push byte 20
    jmp isr_common_stub

; 21: Reserved
isr21:
    cli
    push byte 0
    push byte 21
    jmp isr_common_stub

; 22: Reserved
isr22:
    cli
    push byte 0
    push byte 22
    jmp isr_common_stub

; 23: Reserved
isr23:
    cli
    push byte 0
    push byte 23
    jmp isr_common_stub

; 24: Reserved
isr24:
    cli
    push byte 0
    push byte 24
    jmp isr_common_stub

; 25: Reserved
isr25:
    cli
    push byte 0
    push byte 25
    jmp isr_common_stub

; 26: Reserved
isr26:
    cli
    push byte 0
    push byte 26
    jmp isr_common_stub

; 27: Reserved
isr27:
    cli
    push byte 0
    push byte 27
    jmp isr_common_stub

; 28: Reserved
isr28:
    cli
    push byte 0
    push byte 28
    jmp isr_common_stub

; 29: Reserved
isr29:
    cli
    push byte 0
    push byte 29
    jmp isr_common_stub

; 30: Reserved
isr30:
    cli
    push byte 0
    push byte 30
    jmp isr_common_stub

; 31: Reserved
isr31:
    cli
    push byte 0
    push byte 31
    jmp isr_common_stub

; IRQ handlers
irq0:
	cli
	push byte 0
	push byte 32
	jmp irq_common_stub

irq1:
	cli
	push byte 1
	push byte 33
	jmp irq_common_stub

irq2:
	cli
	push byte 2
	push byte 34
	jmp irq_common_stub

irq3:
	cli
	push byte 3
	push byte 35
	jmp irq_common_stub

irq4:
	cli
	push byte 4
	push byte 36
	jmp irq_common_stub

irq5:
	cli
	push byte 5
	push byte 37
	jmp irq_common_stub

irq6:
	cli
	push byte 6
	push byte 38
	jmp irq_common_stub

irq7:
	cli
	push byte 7
	push byte 39
	jmp irq_common_stub

irq8:
	cli
	push byte 8
	push byte 40
	jmp irq_common_stub

irq9:
	cli
	push byte 9
	push byte 41
	jmp irq_common_stub

irq10:
	cli
	push byte 10
	push byte 42
	jmp irq_common_stub

irq11:
	cli
	push byte 11
	push byte 43
	jmp irq_common_stub

irq12:
	cli
	push byte 12
	push byte 44
	jmp irq_common_stub

irq13:
	cli
	push byte 13
	push byte 45
	jmp irq_common_stub

irq14:
	cli
	push byte 14
	push byte 46
	jmp irq_common_stub

irq15:
	cli
	push byte 15
	push byte 47
	jmp irq_common_stub

Re: Issue with put pixel connected with keyboard irq handler

Posted: Sun Apr 23, 2017 7:03 am
by sleephacker
All you do to initialise the keyboard is set up an IRQ handler...
Any data that arrived before you set up the IRQ will still be stuck after setting up the IRQ, because the PS/2 controller is waiting for you to read it and you are waiting for the controller to send an IRQ, which is why you should initialise the PS/2 controller and keyboard before using them.

Re: Issue with put pixel connected with keyboard irq handler

Posted: Sun Apr 23, 2017 6:42 pm
by Octocontrabass
viruss33 wrote:interrupt.asm (this one is the most suspicious for me as I followed Carl Fenollosa's tutorial. https://github.com/cfenollosa/os-tutorial
When I applied the fixes from 23 lesson I still had the issues)
Lesson 23 about fixing bugs from James Molloy's tutorial? Why doesn't it teach you to do things the right way from the beginning?

Although, speaking of fixes, you missed the fix for irq_common_stub and irq_handler(). You're still passing a registers_t struct instead of a pointer to a struct.

(Why do you have those two functions anyway? They're just copies of isr_common_stub and isr_handler(). Wouldn't it be easier to have a single set of functions for exceptions and IRQs?)

Re: Issue with put pixel connected with keyboard irq handler

Posted: Sun Apr 23, 2017 10:17 pm
by Brendan
Hi,
Octocontrabass wrote:
viruss33 wrote:interrupt.asm (this one is the most suspicious for me as I followed Carl Fenollosa's tutorial. https://github.com/cfenollosa/os-tutorial
When I applied the fixes from 23 lesson I still had the issues)
Lesson 23 about fixing bugs from James Molloy's tutorial? Why doesn't it teach you to do things the right way from the beginning?

Although, speaking of fixes, you missed the fix for irq_common_stub and irq_handler(). You're still passing a registers_t struct instead of a pointer to a struct./quote]
Probably also doing "CLI" at the start if interrupts handlers and "STI" at the end, which is completely pointless (for "interrupt gate", the CPU disables interrupts for you and IRET always restores EFLAGS). There's also an IRQ handler for "PIC IRQ2" (which can't/doesn't exist because it's used for the connection from slave PIC to master PIC).

I'd also recommend having special "IRQ handling stubs" for both IRQ7 and IRQ15 which detect if the IRQ was spurious before calling the generic IRQ handler, that handles spurious IRQs themselves without calling the generic IRQ handler.
Octocontrabass wrote:(Why do you have those two functions anyway? They're just copies of isr_common_stub and isr_handler(). Wouldn't it be easier to have a single set of functions for exceptions and IRQs?)
Keeping exceptions and IRQs separate is better (in a "separation of concerns" way). Eventually (hopefully) when someone reaches the point of caring about IO APICs (and maybe MSI) you end up with wanting to setup exceptions early (when the IDT is created) because they're always needed; and wanting to setup IRQs "less early" and "more dynamically" (e.g. after examining things like ACPI tables, and with some "if(PIC) { ... } else if(IO_APIC) { ... } else if(IO_APIC_AND_DIRECTED_IO) { .... }").

Also, as far as I'm concerned, using a common interrupt handler for exceptions is bad/lazy/stupid thing to do. Exception handlers are not similar to each other so you end up with a "switch(exception) {case 0: case 1: case 2: ...." mess (including unpredictable branch) just to get back to having different code for different exception handlers (which is what CPU gave you in the first place). Much better is to have a general purpose "user-space crashed" function (which might also be used by kernel API, etc and isn't used by exception handlers alone) and a general purpose "kernel crashed" function (which might also be used by kernel itself and isn't used by exception handlers alone); where all of the exceptions contain "exception specific code" which might call these generic crash handlers if/when appropriate.

Don't forget that for a lot of exceptions you need to do a bunch of work to determine if it's a crash or not. Page fault handler does a bunch of virtual memory manager tricks. Invalid opcode handler tries to emulate "not supported by CPU" instructions. General protection fault handler may do some tricks and may emulate some restricted instructions (even if you don't support virtual8086). The "device not available" exception is part of task switching (the "lazy FPU/MMX state saving" mechanism). All of these kinds of things should be taken care of before any generic code is used.

There are also exceptions like the debug exception and breakpoint exception which never indicate a crash (and are tied in with the kernel's support for debuggers).

Finally there's exceptions like NMI, machine check and double fault; where it's "very different" because you can't rely on the CPU having a sane state and in some cases can't even assume that the rest of the kernel is usable; and where you might be using task gates.


Cheers,

Brendan

Re: Issue with put pixel connected with keyboard irq handler

Posted: Mon Apr 24, 2017 12:01 am
by viruss33
Thanks for the replies guys.
As for the fixes from the tutorial, I reverted them while searching for the solution. Now I'm gonna apply them and clean my code.

Re: Issue with put pixel connected with keyboard irq handler

Posted: Mon Apr 24, 2017 3:48 am
by dozniak
JFYI i have a copy of jamesm-tutorials from googlecode where I fixed some things as well: https://github.com/berkus/jamesm-tutorials

maybe you'll find something useful there (at least the memory management and interrupts should be all working in that version).
All times are UTC-06:00
Page 1 of 1

Powered by phpBB® Forum Software © phpBB Limited