flat assembler
Message board for the users of flat assembler.

Index > MenuetOS > Bugs in clock drivers

Author
Thread Post new topic Reply to topic
Mihasik



Joined: 08 Jan 2005
Posts: 3
Location: Stavropol, Russia
Mihasik
There's some bugs in kernel, causes getting wrong time and date, when clock registers are
updating. That's why there's no testing bit 7 in register 0x0a before reading all other
registers (if bit 7 is set, clock reading is forbidden). I've fixed this bug, new procedures
sys_clock and sys_date (see servetable) are below. This bug is not seen in clocks, but can
cause errors in such applications as screensaver or task planner.
So IMHO, it's neccesary to fix the Menuet kernel (2 Ville and Mike).

sys_clock:
call test_clock
xor al,al ; seconds
out 0x70,al
in al,0x71
movzx ecx,al
mov al,2 ; minutes
shl ecx,16
out 0x70,al
in al,0x71
movzx edx,al
mov al,4 ; hours
shl edx,8
out 0x70,al
in al,0x71
add ecx,edx
movzx edx,al
add ecx,edx
sti
mov [esp+36],ecx
ret
sys_date:
call test_clock
mov al,6 ; day of week
out 0x70,al
in al,0x71
mov ch,al
mov al,7 ; date
out 0x70,al
in al,0x71
mov cl,al
mov al,8 ; month
shl ecx,16
out 0x70,al
in al,0x71
mov ch,al
mov al,9 ; year
out 0x70,al
in al,0x71
mov cl,al
sti
mov [esp+36],ecx
ret
test_clock:
cli
mov al,0x0a
out 0x70,al
in al,0x71
bt ax,7
jnc end_test_clock
sti
mov esi,1
call delay_ms
jmp test_clock
end_test_clock:
ret
Post 08 Jan 2005, 15:52
View user's profile Send private message Reply with quote
Pj



Joined: 29 Dec 2004
Posts: 12
Pj
I could be wrong about this, but I believe that flag indicates when the time is being updated, not when it is unsafe to read. There's a very fine distinction there. If you only check to see if the flag is clear, then it may set itself the instant after you read it, and go on to update the time values, returning to you a bad time value.

So the safe thing to do would be to wait until the flag is set, then wait until it is clear, this way you know you have a full second before it will be set again. The bad thing about this is that it makes it take an average of half a second to read the clock.

It's possible the real time clock designers though ahead and made the clock set that bit a milisecond or so before the clock changes the values, but I wouldn't count on it.

I notice that if you type "hwclock; hwclock; hwclock" in linux, there's a one second delay between each clock reading. So I assume the hwclock program does it the way I describe.

Anyway, I know you're probably not responsible for the way this code looks, but a blank line now and then does wonders for readability. I also don't know what the deal was with the choice of instructions and in particular their ordering. Perhaps someone was trying to optimize the code for dual pipelines or something.

Optimize inner loops for speed, optimize everything else for maintainability.

Code:
sys_clock:
  ; This code reads the time from the CMOS clock.
  ; It stores the final result in [esp+36].
  ; Don't you wish [esp+36] were something like [process_eax]?
  ; I know in NASM you can %define process_eax esp+36, I assume FASM can do something similar.
  ; Anyway, the data format is 0x00HHMMSS...
  ; That's the wrong format, but it's nice to have the time value increase in size through the day.

  cli

  call clock_wait_until_safe

  ; read hours
  mov eax, 4
  out 0x70, al
  in al, 0x71
  mov ecx, eax
  shl ecx, 16

  ; read minutes
  mov al, 2
  out 0x70, al
  in al, 0x71
  mov ch, al

  ; read seconds
  mov al, 0 ; Boo me for not using xor!
  out 0x70, al
  in al, 0x71
  mov cl, al

  sti

  mov [process_eax], ecx
ret

sys_date:
  ; This function reads the date...
  ; It stores in [process_eax] the value 0xYYYYMMDD
  ; That's the wrong value, mind you, but it makes so much more sense,
  ; same reason I mucked with the time value.

  cli

  call clock_wait_until_safe

  ; read century
  mov eax, 50
  out 0x70, al
  in al, 0x71
  mov ch, al

  ; read year
  mov eax, 9
  out 0x70, al
  in al, 0x71
  mov cl, al

  shl ecx, 16

  ; read month
  mov eax, 8
  out 0x70, al
  in al, 0x71
  mov ch, al

  ; read day
  mov eax, 7
  out 0x70, al
  in al, 0x71
  mov cl, al

  sti

  mov [process_eax], ecx
ret

clock_wait_until_safe:
  ; This function delays until it is safe to read or write the CMOS clock.
  ; This is unnecessary for the rest of the CMOS RAM, btw.

  wait_until_set:
    mov al, 10
    out 0x70, al
    in al, 0x71
    bt eax, 7
  jnc wait_until_set

  wait_until_clear:
    mov al, 10
    out 0x70, al
    in al, 0x71
    bt eax, 7
  jc wait_until_clear

ret
    

There were lots of cli/sti instructions in there. Whatever they were trying to do, they obviously wern't doing it right. The ones I included aren't necessary if interrupts are already disabled during system calls.
Post 08 Jan 2005, 21:46
View user's profile Send private message Visit poster's website Reply with quote
Mihasik



Joined: 08 Jan 2005
Posts: 3
Location: Stavropol, Russia
Mihasik
Yes, you're right, your solution is better, and it already uses.
See here:
http://www.merlyn.demon.co.uk/programs/int_test.pas
About cli and sti. IMHO, it's neccesarry to use it when registers
are reading, it can help avoid task switching by timer interrupt
and reading wrong info when the task switches once again. But when
we are waiting for setting/reseting bit 7, the IF must be set.
That's why the delay of waiting may be too long - about 1 second
and such delay with reseted IF may cause 'pseudo-hanging' all the
system because the tasks can't switch.
(IF is an interrupt flag)
Post 09 Jan 2005, 18:24
View user's profile Send private message Reply with quote
Pj



Joined: 29 Dec 2004
Posts: 12
Pj
I guess it wouldn't be a good idea to leave that IF flag clear. I have no idea how the hwclock program in linux does it. I can set that thing up to run continuously with the command "while true; do hwclock; done" and it never misses a second no matter how much system load I create.

I wonder for how long that flag bit in the CMOS ram stays set. I wrote my code assuming it is only set for an instant, but if, for instance, it's on for half a second, then off for half a second, you could easily sleep for a quarter of a second between each time you check the flag. Then when you see it switch from set to clear, you know you have at least a quarter of a second to read the time, which is pleanty of time.

I suppose someone (not me) could write a test program and find out how long that bit stays set, then we would know how often we have to check it in order to avoid missing when it is set.
Post 09 Jan 2005, 18:45
View user's profile Send private message Visit poster's website Reply with quote
Mihasik



Joined: 08 Jan 2005
Posts: 3
Location: Stavropol, Russia
Mihasik
IMHO, it's the best solution, the quickest and the smallest.
sys_clock:
call tst_clock
xor ax,ax ; seconds
cli
out 0x70,al
in al,0x71
shl eax,8
mov al,2 ; minutes
out 0x70,al
in al,0x71
shl eax,8
mov al,4 ; hours
out 0x70,al
in al,0x71
sti
mov [esp+36],eax
ret
sys_date:
call tst_clock
mov al,6 ; day of week
cli
out 0x70,al
in al,0x71
shl eax,8
mov al,7 ; date
out 0x70,al
in al,0x71
shl eax,8
mov al,8 ; month
out 0x70,al
in al,0x71
shl eax,8
mov al,9 ; year
out 0x70,al
in al,0x71
sti
mov [esp+36],eax
ret
tst_clock:
call tst_uip
jnc tst_clock ;waiting for a 1
tst_uip0:
call tst_uip
jc tst_uip0 ;waiting for a 0
ret
tst_uip:
mov al,0x0a
cli
out 0x70,al
in al,0x71
sti
bt ax,7
ret
Post 10 Jan 2005, 21:21
View user's profile Send private message Reply with quote
Pj



Joined: 29 Dec 2004
Posts: 12
Pj
> IMHO, it's the best solution, the quickest and the smallest.

Hmm... Let me see what I can do to mine...
Code:

sys_clock: 
  ; This code reads the time from the CMOS clock. 
  ; It stores the final result in [esp+36], whatever that is.
  ; Anyway, the data format is 0x00SSMMHH... 
  ; That's a silly format, but it's the right one! 

  ; Of course, I wonder if I really should be writing code that
  ; I have no intention of ever testing to see if it works...

  ; Especially since I don't know if MenuetOS allows other processes access
  ; to the RTC, or if it tasks switches during kernel calls, or how long that
  ; there bit stays set in the RTC.  

  ; Oh well.  I'll just code and hope it works...

  call clock_wait_until_safe 

  ; read seconds
  mov eax, 0 
  out 0x70, al 
  in al, 0x71 
  mov edx, eax 

  shl edx, 16 

  ; read minutes 
  mov eax, 2 
  out 0x70, al 
  in al, 0x71 
  mov dh, al 

  ; read hours 
  mov eax, 4
  out 0x70, al 
  in al, 0x71 
  mov dl, al 

  mov [esp+36], edx 
ret 

sys_date: 
  ; This function reads the date... 
  ; It stores in [esp+36] the value 0x00YYDDMM 
  ; That's a silly date format, but it's the right one! 

  call clock_wait_until_safe 

  ; read year 
  mov eax, 9 
  out 0x70, al 
  in al, 0x71 
  mov edx, eax 

  shl edx, 16 

  ; read day 
  mov eax, 7 
  out 0x70, al 
  in al, 0x71 
  mov dh, al 

  ; read month 
  mov eax, 8 
  out 0x70, al 
  in al, 0x71 
  mov dl, al 

  mov [esp+36], edx 
ret 

clock_wait_until_safe: 
  ; This function delays until it is safe to read or write the CMOS clock. 

  mov eax, 10 
  out 0x70, al 

  wait_until_set: 
    in eax, 0x71 
    bt eax, 7 
  jnc wait_until_set 

  wait_until_clear: 
    in eax, 0x71 
    bt eax, 7 
  jc wait_until_clear 

ret

    
Anyway, using eax always makes the fastest and smallest code, except in the case of "mov ax, x" where x > 127. I think. Someone might correct me if I'm wrong.

I see no reason to make the code harder to read by using eax for two purposes while other registers remain unused.

The date isn't supposed to have the day of week in it.

Also, my complete lack of cli/sti instructions works only if Menuet does not allow anything else access to the RTC and never executes this function reentrently, such as switching tasks while in the middle of this function, then allowing this function to be called by the new task. Not knowing how Menuet works, I just assume it won't do either.

Anyway, that's in all likelyhood not the quickest or smallest, or even correctest, but due to it's readability, I think it has a shot at being the best.
Post 10 Jan 2005, 23:02
View user's profile Send private message Visit poster's website Reply with quote
Display posts from previous:
Post new topic Reply to topic

Jump to:  


< Last Thread | Next Thread >
Forum Rules:
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You can attach files in this forum
You can download files in this forum


Copyright © 1999-2020, Tomasz Grysztar. Also on GitHub, YouTube, Twitter.

Website powered by rwasa.