Weird PIT issues

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

I have this code:

Code:

#include "pit.h"
#include "pic.h"
#include "../lib/common/port.h"
#include "../lib/typeout.h"

void timer_interrupt(){

    pitd_tick++;
}

uint32 pitd_init(uint32 clock_frequency){

    uint32 divisor = 1193182 / clock_frequency;
    p_write8(0x43, 0x36);
    pitd_tick = 0;

    uint8 l = (uint8)(divisor & 0xFF);
    uint8 h = (uint8)(divisor >> 8);
    p_write8(0x40, l);
    p_write8(0x40, h);
    picd_register_interrupt_handler(0, timer_interrupt);
}

void pitd_wait(uint32 ticks){
    uint32 ticks_now = pitd_tick;
    uint32 time_passed = 0;
    while (time_passed <= ticks){
        time_passed = pitd_tick - ticks_now;
        //screen_printf("is\0", time_passed, "\n\0");
    }
    return;
}

And I'm getting this weird issue where after I call the pitd_wait() function, it seems like the emulator just skips right over the while loop and doesn't wait at all. However, if I print the value of the time_passed variable (by uncommenting the screen_printf() line) it does actually wait the set the amount of time. Any advice here would be much appreciated as I'm at a loss.

Edit: It seems like any function call fixes the issue, so now I'm wondering if this is the compiler doing something funky. Is it possible that for whatever reason the compiler is throwing out my loop?

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

Can you show how you declared `pitd_tick`? I will gather you didn't mark it as volatile so the compiler probably optimized the entire loop away. If you don't mark it volatile then the compiler will just assume its value never changed.

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

Code:

uint32 pitd_tick;

I also tried marking it as volatile just now, but the same issues occur.

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

Can you compile the file with this code using the `-S` option to output the assembly code and post it here? Either that or objdump your kernel and show us the generated instructions for the function `void pitd_wait(uint32 ticks)`. Anything that can show us the generated code for that function will do.

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

Certainly, here's the output of doing an objdump -d,

Code:

drivers/pit.o:     file format elf64-x86-64


Disassembly of section .text:

0000000000000000 <timer_interrupt>:
   0:   f3 0f 1e fa             endbr64 
   4:   55                      push   %rbp
   5:   48 89 e5                mov    %rsp,%rbp
   8:   8b 05 00 00 00 00       mov    0x0(%rip),%eax        # e <timer_interrupt+0xe>
   e:   83 c0 01                add    $0x1,%eax
  11:   89 05 00 00 00 00       mov    %eax,0x0(%rip)        # 17 <timer_interrupt+0x17>
  17:   90                      nop
  18:   5d                      pop    %rbp
  19:   c3                      retq   

000000000000001a <pitd_init>:
  1a:   f3 0f 1e fa             endbr64 
  1e:   55                      push   %rbp
  1f:   48 89 e5                mov    %rsp,%rbp
  22:   48 83 ec 20             sub    $0x20,%rsp
  26:   89 7d ec                mov    %edi,-0x14(%rbp)
  29:   b8 de 34 12 00          mov    $0x1234de,%eax
  2e:   ba 00 00 00 00          mov    $0x0,%edx
  33:   f7 75 ec                divl   -0x14(%rbp)
  36:   89 45 fc                mov    %eax,-0x4(%rbp)
  39:   be 36 00 00 00          mov    $0x36,%esi
  3e:   bf 43 00 00 00          mov    $0x43,%edi
  43:   e8 00 00 00 00          callq  48 <pitd_init+0x2e>
  48:   c7 05 00 00 00 00 00    movl   $0x0,0x0(%rip)        # 52 <pitd_init+0x38>
  4f:   00 00 00 
  52:   8b 45 fc                mov    -0x4(%rbp),%eax
  55:   88 45 fb                mov    %al,-0x5(%rbp)
  58:   8b 45 fc                mov    -0x4(%rbp),%eax
  5b:   c1 e8 08                shr    $0x8,%eax
  5e:   88 45 fa                mov    %al,-0x6(%rbp)
  61:   0f b6 45 fb             movzbl -0x5(%rbp),%eax
  65:   89 c6                   mov    %eax,%esi
  67:   bf 40 00 00 00          mov    $0x40,%edi
  6c:   e8 00 00 00 00          callq  71 <pitd_init+0x57>
  71:   0f b6 45 fa             movzbl -0x6(%rbp),%eax
  75:   89 c6                   mov    %eax,%esi
  77:   bf 40 00 00 00          mov    $0x40,%edi
  7c:   e8 00 00 00 00          callq  81 <pitd_init+0x67>
  81:   be 00 00 00 00          mov    $0x0,%esi
  86:   bf 00 00 00 00          mov    $0x0,%edi
  8b:   e8 00 00 00 00          callq  90 <pitd_init+0x76>
  90:   90                      nop
  91:   c9                      leaveq 
  92:   c3                      retq   

0000000000000093 <pitd_wait>:
  93:   f3 0f 1e fa             endbr64 
  97:   55                      push   %rbp
  98:   48 89 e5                mov    %rsp,%rbp
  9b:   89 7d ec                mov    %edi,-0x14(%rbp)
  9e:   8b 05 00 00 00 00       mov    0x0(%rip),%eax        # a4 <pitd_wait+0x11>
  a4:   89 45 f8                mov    %eax,-0x8(%rbp)
  a7:   c7 45 fc 00 00 00 00    movl   $0x0,-0x4(%rbp)
  ae:   eb 0c                   jmp    bc <pitd_wait+0x29>
  b0:   8b 05 00 00 00 00       mov    0x0(%rip),%eax        # b6 <pitd_wait+0x23>
  b6:   2b 45 f8                sub    -0x8(%rbp),%eax
  b9:   89 45 fc                mov    %eax,-0x4(%rbp)
  bc:   8b 45 fc                mov    -0x4(%rbp),%eax
  bf:   3b 45 ec                cmp    -0x14(%rbp),%eax
  c2:   76 ec                   jbe    b0 <pitd_wait+0x1d>
  c4:   90                      nop
  c5:   5d                      pop    %rbp
  c6:   c3                      retq   

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

When you called `pitd_init` what was the `clock_frequency` value you passed to the function? when you are calling `pitd_wait` what value for `ticks` did you pass into it?

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

I'm passing 1000 for clock_frequency.

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

What kind of wait value are you passing to pitd_wait that doesn't seem to delay?

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

No matter what value I pass, it doesn't wait. I've been testing with passing in the time in milliseconds. At the moment, I'm testing with the value of 5000 ticks, which should be around 5 seconds.

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

For completeness can you show the code for `p_write8`. Thanks. Do you have a Github project I can look at, or some way to build your code here so I can look at it?

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

https://github.com/AsherBearce/AbstractOs

Here you go! I just pushed so you're looking at the same version of the project that I have

MichaelPetch · **Joined:** Fri Aug 26, 2016 1:41 pm **Posts:** 692

You don't save and restore the registers. Since you are still doing all this in kernel mode this should get you started although you only really need to save/restore the volatile registers per the AMD64 System V ABI. This doesn't handle `swapgs`; 16 byte stack alignment; or anything but it should avoid your registers being corrupted while sitting in the `pitd_wait` function.

Code:

irq_asm:
    push rax
    push rbx
    push rcx
    push rdx
    push rbp
    push rdi
    push rsi
    push r8
    push r9
    push r10
    push r11
    push r12
    push r13
    push r14
    push r15

    cld
    call master_IRQ
    pop r15
    pop r14
    pop r13
    pop r12
    pop r11
    pop r10
    pop r9
    pop r8
    pop rsi
    pop rdi
    pop rbp
    pop rdx
    pop rcx
    pop rbx
    pop rax

    iretq

The OSDev wiki I believe has more information on doing this properly and other things to consider.

You also take the slow approach to determining which IRQ fired. Your interrupt handler queries the PICs to figure that out. If you created an entry point for each IRQ in irq.asm then pass the interrupt number to `master_irq` as the first parameter it would be a better option.

You should also compile your kernel code with `-mno-red-zone` and consider disabling SIMD instructions with something like `-mno-mmx -mno-sse -mno-sse2`

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

Thank you, that fixed the problem. However, I don't plan on disabling the SSE registers, as I will be using floating point instructions in the future. Is there a way to save the SSE registers onto the stack and then restore them?

Octocontrabass · **Joined:** Mon Mar 25, 2013 7:01 pm **Posts:** 5137

Yes. Subtract an appropriate amount from RSP to create a buffer where you can save the registers, then use FXSAVE/XSAVE/XSAVEC/XSAVEOPT/XSAVES to store them in that buffer. Afterwards, use FXRSTOR/XRSTOR/XRSTORS to restore them and add an appropriate amount to RSP to free the buffer.

It's also possible to save and restore the SSE registers one at a time, but I don't think there's any benefit to doing things that way.

Don't forget that your kernel's floating-point code will depend on things like the x87 FPU control and status words and MXCSR. It may be a good idea to "restore" a clean FPU state for your kernel code.

Improperly saving/restoring AVX registers can cause a performance penalty.

abstractmath · **Joined:** Mon Sep 07, 2020 5:50 pm **Posts:** 46

Got it, thanks for the advice guys! It's very much appreciated!

OSDev.org

Weird PIT issues

Who is online