what is wrong in this assembly function to make idt entries?

i was writing a simple bootloader in assembly and testing it on qemu. i needed an idt and wrote a function to make entries. idk what is wrong here- please check-

makeidt:
;privilage level in cl
;index to a gdt entry in ax
;handler function pointer in edx
;put 0x01 in ch to make this interrupt active
;the table entry would be given in eax:edx
mov bx, dx ; the first part
shl ebx, 16 
mov bl, cl 
shl ax, 3 ; else, bx would be overwriten
or bx, ax ; selector in gdt
mov [answerh], ebx ; high part made
shr edx, 16
mov ebx, 0xe
shl ebx, 3
or bl , cl
shl ebx, 1
or bl, ch
shl ebx, 16
mov bx, dx
mov eax, [answerh]
ret


intcode:
call myfunct
iret


answerh dd 0

idtdesc:
idtlen dw 100h
dd idt
idt:

i called this function like this-

mov edx, intcode
mov ax, 2
mov ch, 1
mov cl, 3
call makeidt
mov [idt], eax
mov [idt+4], edx
lidt [idtdesc]
mov ebx, string
sti
int 0
cli

myfunct is working fine when called from somewhere else.. but the machine restarts again and again after encountering this block why?? btw i am in 32 bit protected mode.

Solution

For the makeidt: routine; I think you've got "endianness" (or byte or word order?) all mixed up. E.g. "lowest 16 bits of offset" should end up in the lowest 16 bits of edx (which is where it was in the input parameter to begin with) but you're treating it as "highest 16 bits of offset".

What you want would be more like (untested, NASM syntax):

makeidt:

    ;Put "offset" in the right places

    mov ebx,edx          ;Put highest 16 bits of offset into highest 16 bits of ebx
    and edx,0x0000FFFF   ;Mask lowest 16 bits of offset
    and ebx,0xFFFF0000   ;Mask highest 16 bits of offset

    ;Put "code segment" in the right place (and combine with "low 16 bits of offset")

    shl eax,3+16         ;eax = convert "GDT index" into a code segment ("<< 3") then and shift code segment into the right place ("<< 16")
    or eax,edx           ;eax = first dword of result

    ;Build "type and attributes"

    shl cl,6             ;cl = DPL in highest 2 bits (to bring them next to "present" bit in CH)
    shl cx,7             ;cx = DPL in bits 13 to 14, present bit in bit 15

    or ch,0x0E           ;Set "descriptor type = interrupt gate" (trap gates not supported?)

    ;Combine "type and attributes" with highest 16 bits of offset

    and ecx,0x0000FFFF   ;Make sure highest 16 bits of ECX are zeros
    lea edx,[ecx+ebx]    ;Merge (using addition as "bitwise or" so the result can be moved to a different register for free)

    ret

The IDT's limit (idtlen dw 100h) is probably wrong. It's supposed to be "total bytes - 1", so in protected mode (where an entry is 8 bytes) for 256 entries you want 256*8 - 1 or 0x07FF. Note: if you only want the first 32 IDT entries (which are reserved for exceptions), then a limit of 0x00FF would be right but most people don't want that (they want space for IRQs, etc).

Notes:

it's faster and more flexible to let the assembler do most of this (e.g. mov ecx,(1 << 15) | (3 << 14) and mov eax,(2 << 3) << 16 in the caller so you don't need to do so much in the makeidt: routine); and if you want you could use a macro to make it even faster (no function call/return overhead, cleaner separation without shifts in code that uses it).
"DPL=3" means that unprivileged code can use the interrupt via. an int n instruction. That is almost never what you want (other than using something like int 0x80 for kernel API, maybe). Exceptions and IRQs should use "DPL=0" to disallow direct use by unprivileged code (while still allowing exceptions and IRQs to interrupt unprivileged code - for the purpose of protection checks, the "interrupt source" is assumed to be CPL=0 when the CPU or hardware is generating the interrupt even when the code being interrupted is CPL=3).